diff mbox series

[04/20] bus: mhi: Cleanup the register definitions used in headers

Message ID 20211202113553.238011-5-manivannan.sadhasivam@linaro.org
State New
Headers show
Series Add initial support for MHI endpoint stack | expand

Commit Message

Manivannan Sadhasivam Dec. 2, 2021, 11:35 a.m. UTC
Cleanup includes:

1. Moving the MHI register bit definitions to common.h header (only the
   register offsets differ between host and ep not the bit definitions)
2. Using the GENMASK macro for masks
3. Removing brackets for single values
4. Using lowercase for hex values

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/bus/mhi/common.h        | 129 ++++++++++++---
 drivers/bus/mhi/host/internal.h | 282 +++++++++++---------------------
 2 files changed, 207 insertions(+), 204 deletions(-)

Comments

Alex Elder Jan. 6, 2022, 12:23 a.m. UTC | #1
On 12/2/21 5:35 AM, Manivannan Sadhasivam wrote:
> Cleanup includes:
> 
> 1. Moving the MHI register bit definitions to common.h header (only the
>     register offsets differ between host and ep not the bit definitions)

The register offsets do differ, but the group of registers for the host
differs from the group of registers for the endpoint by a fixed amount.
(MHIREGLEN = 0x0000 for host, or 0x100 for endpoint; CRCBAP_LOWER is
0x0068 for host, 0x0168 for endpoint.)

In other words, can you instead use the same symbolic offsets, but
have the endpoint add 0x0100 to them all?  It would make the fact
that they're both referencing the same basic in-memory structure
more obvious.

> 2. Using the GENMASK macro for masks
> 3. Removing brackets for single values
> 4. Using lowercase for hex values

Yay!!! For all three of the above.

More below.

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>   drivers/bus/mhi/common.h        | 129 ++++++++++++---
>   drivers/bus/mhi/host/internal.h | 282 +++++++++++---------------------
>   2 files changed, 207 insertions(+), 204 deletions(-)
> 
> diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
> index 2ea438205617..c1272d61e54e 100644
> --- a/drivers/bus/mhi/common.h
> +++ b/drivers/bus/mhi/common.h
> @@ -9,32 +9,123 @@
>   
>   #include <linux/mhi.h>
>   
> +/* MHI register bits */
> +#define MHIREGLEN_MHIREGLEN_MASK		GENMASK(31, 0)
> +#define MHIREGLEN_MHIREGLEN_SHIFT		0

Again, please eliminate all _SHIFT definitions where they define
the low bit position of a mask.

Maybe you can add some underscores for readability?

Even if you don't do that, you could add a comment here or there to
explain what certain abbreviations stand for, to make it easier to
understand.  E.g., CHDB = channel doorbell, CCA = channel context
array, BAP = base address pointer.

					-Alex


> +#define MHIVER_MHIVER_MASK			GENMASK(31, 0)
> +#define MHIVER_MHIVER_SHIFT			0
> +
> +#define MHICFG_NHWER_MASK			GENMASK(31, 24)
> +#define MHICFG_NHWER_SHIFT			24
> +#define MHICFG_NER_MASK				GENMASK(23, 16)
> +#define MHICFG_NER_SHIFT			16
> +#define MHICFG_NHWCH_MASK			GENMASK(15, 8)
> +#define MHICFG_NHWCH_SHIFT			8
> +#define MHICFG_NCH_MASK				GENMASK(7, 0)
> +#define MHICFG_NCH_SHIFT			0

. . .
Manivannan Sadhasivam Feb. 3, 2022, 11:26 a.m. UTC | #2
On Wed, Jan 05, 2022 at 06:23:52PM -0600, Alex Elder wrote:
> On 12/2/21 5:35 AM, Manivannan Sadhasivam wrote:
> > Cleanup includes:
> > 
> > 1. Moving the MHI register bit definitions to common.h header (only the
> >     register offsets differ between host and ep not the bit definitions)
> 
> The register offsets do differ, but the group of registers for the host
> differs from the group of registers for the endpoint by a fixed amount.
> (MHIREGLEN = 0x0000 for host, or 0x100 for endpoint; CRCBAP_LOWER is
> 0x0068 for host, 0x0168 for endpoint.)
> 
> In other words, can you instead use the same symbolic offsets, but
> have the endpoint add 0x0100 to them all?  It would make the fact
> that they're both referencing the same basic in-memory structure
> more obvious.
> 

Okay. I've used REG_ prefix for the register defines in common.h and used it on
host and ep internal.h.

> > 2. Using the GENMASK macro for masks
> > 3. Removing brackets for single values
> > 4. Using lowercase for hex values
> 
> Yay!!! For all three of the above.
> 
> More below.
> 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >   drivers/bus/mhi/common.h        | 129 ++++++++++++---
> >   drivers/bus/mhi/host/internal.h | 282 +++++++++++---------------------
> >   2 files changed, 207 insertions(+), 204 deletions(-)
> > 
> > diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
> > index 2ea438205617..c1272d61e54e 100644
> > --- a/drivers/bus/mhi/common.h
> > +++ b/drivers/bus/mhi/common.h
> > @@ -9,32 +9,123 @@
> >   #include <linux/mhi.h>
> > +/* MHI register bits */
> > +#define MHIREGLEN_MHIREGLEN_MASK		GENMASK(31, 0)
> > +#define MHIREGLEN_MHIREGLEN_SHIFT		0
> 
> Again, please eliminate all _SHIFT definitions where they define
> the low bit position of a mask.
> 
> Maybe you can add some underscores for readability?
> 
> Even if you don't do that, you could add a comment here or there to
> explain what certain abbreviations stand for, to make it easier to
> understand.  E.g., CHDB = channel doorbell, CCA = channel context
> array, BAP = base address pointer.
> 

I'll check on this.

Thanks,
Mani

> 					-Alex
> 
> 
> > +#define MHIVER_MHIVER_MASK			GENMASK(31, 0)
> > +#define MHIVER_MHIVER_SHIFT			0
> > +
> > +#define MHICFG_NHWER_MASK			GENMASK(31, 24)
> > +#define MHICFG_NHWER_SHIFT			24
> > +#define MHICFG_NER_MASK				GENMASK(23, 16)
> > +#define MHICFG_NER_SHIFT			16
> > +#define MHICFG_NHWCH_MASK			GENMASK(15, 8)
> > +#define MHICFG_NHWCH_SHIFT			8
> > +#define MHICFG_NCH_MASK				GENMASK(7, 0)
> > +#define MHICFG_NCH_SHIFT			0
> 
> . . .
diff mbox series

Patch

diff --git a/drivers/bus/mhi/common.h b/drivers/bus/mhi/common.h
index 2ea438205617..c1272d61e54e 100644
--- a/drivers/bus/mhi/common.h
+++ b/drivers/bus/mhi/common.h
@@ -9,32 +9,123 @@ 
 
 #include <linux/mhi.h>
 
+/* MHI register bits */
+#define MHIREGLEN_MHIREGLEN_MASK		GENMASK(31, 0)
+#define MHIREGLEN_MHIREGLEN_SHIFT		0
+
+#define MHIVER_MHIVER_MASK			GENMASK(31, 0)
+#define MHIVER_MHIVER_SHIFT			0
+
+#define MHICFG_NHWER_MASK			GENMASK(31, 24)
+#define MHICFG_NHWER_SHIFT			24
+#define MHICFG_NER_MASK				GENMASK(23, 16)
+#define MHICFG_NER_SHIFT			16
+#define MHICFG_NHWCH_MASK			GENMASK(15, 8)
+#define MHICFG_NHWCH_SHIFT			8
+#define MHICFG_NCH_MASK				GENMASK(7, 0)
+#define MHICFG_NCH_SHIFT			0
+
+#define CHDBOFF_CHDBOFF_MASK			GENMASK(31, 0)
+#define CHDBOFF_CHDBOFF_SHIFT			0
+
+#define ERDBOFF_ERDBOFF_MASK			GENMASK(31, 0)
+#define ERDBOFF_ERDBOFF_SHIFT			0
+
+#define BHIOFF_BHIOFF_MASK			GENMASK(31, 0)
+#define BHIOFF_BHIOFF_SHIFT			0
+
+#define BHIEOFF_BHIEOFF_MASK			GENMASK(31, 0)
+#define BHIEOFF_BHIEOFF_SHIFT			0
+
+#define DEBUGOFF_DEBUGOFF_MASK			GENMASK(31, 0)
+#define DEBUGOFF_DEBUGOFF_SHIFT			0
+
+#define MHICTRL_MHISTATE_MASK			GENMASK(15, 8)
+#define MHICTRL_MHISTATE_SHIFT			8
+#define MHICTRL_RESET_MASK			2
+#define MHICTRL_RESET_SHIFT			1
+
+#define MHISTATUS_MHISTATE_MASK			GENMASK(15, 8)
+#define MHISTATUS_MHISTATE_SHIFT		8
+#define MHISTATUS_SYSERR_MASK			4
+#define MHISTATUS_SYSERR_SHIFT			2
+#define MHISTATUS_READY_MASK			1
+#define MHISTATUS_READY_SHIFT			0
+
+#define CCABAP_LOWER_CCABAP_LOWER_MASK		GENMASK(31, 0)
+#define CCABAP_LOWER_CCABAP_LOWER_SHIFT		0
+
+#define CCABAP_HIGHER_CCABAP_HIGHER_MASK	GENMASK(31, 0)
+#define CCABAP_HIGHER_CCABAP_HIGHER_SHIFT	0
+
+#define ECABAP_LOWER_ECABAP_LOWER_MASK		GENMASK(31, 0)
+#define ECABAP_LOWER_ECABAP_LOWER_SHIFT		0
+
+#define ECABAP_HIGHER_ECABAP_HIGHER_MASK	GENMASK(31, 0)
+#define ECABAP_HIGHER_ECABAP_HIGHER_SHIFT	0
+
+#define CRCBAP_LOWER_CRCBAP_LOWER_MASK		GENMASK(31, 0)
+#define CRCBAP_LOWER_CRCBAP_LOWER_SHIFT		0
+
+#define CRCBAP_HIGHER_CRCBAP_HIGHER_MASK	GENMASK(31, 0)
+#define CRCBAP_HIGHER_CRCBAP_HIGHER_SHIFT	0
+
+#define CRDB_LOWER_CRDB_LOWER_MASK		GENMASK(31, 0)
+#define CRDB_LOWER_CRDB_LOWER_SHIFT		0
+
+#define CRDB_HIGHER_CRDB_HIGHER_MASK		GENMASK(31, 0)
+#define CRDB_HIGHER_CRDB_HIGHER_SHIFT		0
+
+#define MHICTRLBASE_LOWER_MHICTRLBASE_LOWER_MASK GENMASK(31, 0)
+#define MHICTRLBASE_LOWER_MHICTRLBASE_LOWER_SHIFT 0
+
+#define MHICTRLBASE_HIGHER_MHICTRLBASE_HIGHER_MASK GENMASK(31, 0)
+#define MHICTRLBASE_HIGHER_MHICTRLBASE_HIGHER_SHIFT 0
+
+#define MHICTRLLIMIT_LOWER_MHICTRLLIMIT_LOWER_MASK GENMASK(31, 0)
+#define MHICTRLLIMIT_LOWER_MHICTRLLIMIT_LOWER_SHIFT 0
+
+#define MHICTRLLIMIT_HIGHER_MHICTRLLIMIT_HIGHER_MASK GENMASK(31, 0)
+#define MHICTRLLIMIT_HIGHER_MHICTRLLIMIT_HIGHER_SHIFT 0
+
+#define MHIDATABASE_LOWER_MHIDATABASE_LOWER_MASK GENMASK(31, 0)
+#define MHIDATABASE_LOWER_MHIDATABASE_LOWER_SHIFT 0
+
+#define MHIDATABASE_HIGHER_MHIDATABASE_HIGHER_MASK GENMASK(31, 0)
+#define MHIDATABASE_HIGHER_MHIDATABASE_HIGHER_SHIFT 0
+
+#define MHIDATALIMIT_LOWER_MHIDATALIMIT_LOWER_MASK GENMASK(31, 0)
+#define MHIDATALIMIT_LOWER_MHIDATALIMIT_LOWER_SHIFT 0
+
+#define MHIDATALIMIT_HIGHER_MHIDATALIMIT_HIGHER_MASK GENMASK(31, 0)
+#define MHIDATALIMIT_HIGHER_MHIDATALIMIT_HIGHER_SHIFT 0
+
 /* Command Ring Element macros */
 /* No operation command */
-#define MHI_TRE_CMD_NOOP_PTR (0)
-#define MHI_TRE_CMD_NOOP_DWORD0 (0)
+#define MHI_TRE_CMD_NOOP_PTR 0
+#define MHI_TRE_CMD_NOOP_DWORD0 0
 #define MHI_TRE_CMD_NOOP_DWORD1 (MHI_CMD_NOP << 16)
 
 /* Channel reset command */
-#define MHI_TRE_CMD_RESET_PTR (0)
-#define MHI_TRE_CMD_RESET_DWORD0 (0)
+#define MHI_TRE_CMD_RESET_PTR 0
+#define MHI_TRE_CMD_RESET_DWORD0 0
 #define MHI_TRE_CMD_RESET_DWORD1(chid) ((chid << 24) | \
 					(MHI_CMD_RESET_CHAN << 16))
 
 /* Channel stop command */
-#define MHI_TRE_CMD_STOP_PTR (0)
-#define MHI_TRE_CMD_STOP_DWORD0 (0)
+#define MHI_TRE_CMD_STOP_PTR 0
+#define MHI_TRE_CMD_STOP_DWORD0 0
 #define MHI_TRE_CMD_STOP_DWORD1(chid) ((chid << 24) | \
 				       (MHI_CMD_STOP_CHAN << 16))
 
 /* Channel start command */
-#define MHI_TRE_CMD_START_PTR (0)
-#define MHI_TRE_CMD_START_DWORD0 (0)
+#define MHI_TRE_CMD_START_PTR 0
+#define MHI_TRE_CMD_START_DWORD0 0
 #define MHI_TRE_CMD_START_DWORD1(chid) ((chid << 24) | \
 					(MHI_CMD_START_CHAN << 16))
 
-#define MHI_TRE_GET_CMD_CHID(tre) (((tre)->dword[1] >> 24) & 0xFF)
-#define MHI_TRE_GET_CMD_TYPE(tre) (((tre)->dword[1] >> 16) & 0xFF)
+#define MHI_TRE_GET_CMD_CHID(tre) (((tre)->dword[1] >> 24) & 0xff)
+#define MHI_TRE_GET_CMD_TYPE(tre) (((tre)->dword[1] >> 16) & 0xff)
 
 /* Event descriptor macros */
 /* Transfer completion event */
@@ -42,18 +133,18 @@ 
 #define MHI_TRE_EV_DWORD0(code, len) ((code << 24) | len)
 #define MHI_TRE_EV_DWORD1(chid, type) ((chid << 24) | (type << 16))
 #define MHI_TRE_GET_EV_PTR(tre) ((tre)->ptr)
-#define MHI_TRE_GET_EV_CODE(tre) (((tre)->dword[0] >> 24) & 0xFF)
-#define MHI_TRE_GET_EV_LEN(tre) ((tre)->dword[0] & 0xFFFF)
-#define MHI_TRE_GET_EV_CHID(tre) (((tre)->dword[1] >> 24) & 0xFF)
-#define MHI_TRE_GET_EV_TYPE(tre) (((tre)->dword[1] >> 16) & 0xFF)
-#define MHI_TRE_GET_EV_STATE(tre) (((tre)->dword[0] >> 24) & 0xFF)
-#define MHI_TRE_GET_EV_EXECENV(tre) (((tre)->dword[0] >> 24) & 0xFF)
+#define MHI_TRE_GET_EV_CODE(tre) (((tre)->dword[0] >> 24) & 0xff)
+#define MHI_TRE_GET_EV_LEN(tre) ((tre)->dword[0] & 0xffff)
+#define MHI_TRE_GET_EV_CHID(tre) (((tre)->dword[1] >> 24) & 0xff)
+#define MHI_TRE_GET_EV_TYPE(tre) (((tre)->dword[1] >> 16) & 0xff)
+#define MHI_TRE_GET_EV_STATE(tre) (((tre)->dword[0] >> 24) & 0xff)
+#define MHI_TRE_GET_EV_EXECENV(tre) (((tre)->dword[0] >> 24) & 0xff)
 #define MHI_TRE_GET_EV_SEQ(tre) ((tre)->dword[0])
 #define MHI_TRE_GET_EV_TIME(tre) ((tre)->ptr)
 #define MHI_TRE_GET_EV_COOKIE(tre) lower_32_bits((tre)->ptr)
-#define MHI_TRE_GET_EV_VEID(tre) (((tre)->dword[0] >> 16) & 0xFF)
-#define MHI_TRE_GET_EV_LINKSPEED(tre) (((tre)->dword[1] >> 24) & 0xFF)
-#define MHI_TRE_GET_EV_LINKWIDTH(tre) ((tre)->dword[0] & 0xFF)
+#define MHI_TRE_GET_EV_VEID(tre) (((tre)->dword[0] >> 16) & 0xff)
+#define MHI_TRE_GET_EV_LINKSPEED(tre) (((tre)->dword[1] >> 24) & 0xff)
+#define MHI_TRE_GET_EV_LINKWIDTH(tre) ((tre)->dword[0] & 0xff)
 
 /* State change event */
 #define MHI_SC_EV_PTR 0
diff --git a/drivers/bus/mhi/host/internal.h b/drivers/bus/mhi/host/internal.h
index a324a76684d0..c882245b9133 100644
--- a/drivers/bus/mhi/host/internal.h
+++ b/drivers/bus/mhi/host/internal.h
@@ -11,197 +11,109 @@ 
 
 extern struct bus_type mhi_bus_type;
 
-#define MHIREGLEN (0x0)
-#define MHIREGLEN_MHIREGLEN_MASK (0xFFFFFFFF)
-#define MHIREGLEN_MHIREGLEN_SHIFT (0)
-
-#define MHIVER (0x8)
-#define MHIVER_MHIVER_MASK (0xFFFFFFFF)
-#define MHIVER_MHIVER_SHIFT (0)
-
-#define MHICFG (0x10)
-#define MHICFG_NHWER_MASK (0xFF000000)
-#define MHICFG_NHWER_SHIFT (24)
-#define MHICFG_NER_MASK (0xFF0000)
-#define MHICFG_NER_SHIFT (16)
-#define MHICFG_NHWCH_MASK (0xFF00)
-#define MHICFG_NHWCH_SHIFT (8)
-#define MHICFG_NCH_MASK (0xFF)
-#define MHICFG_NCH_SHIFT (0)
-
-#define CHDBOFF (0x18)
-#define CHDBOFF_CHDBOFF_MASK (0xFFFFFFFF)
-#define CHDBOFF_CHDBOFF_SHIFT (0)
-
-#define ERDBOFF (0x20)
-#define ERDBOFF_ERDBOFF_MASK (0xFFFFFFFF)
-#define ERDBOFF_ERDBOFF_SHIFT (0)
-
-#define BHIOFF (0x28)
-#define BHIOFF_BHIOFF_MASK (0xFFFFFFFF)
-#define BHIOFF_BHIOFF_SHIFT (0)
-
-#define BHIEOFF (0x2C)
-#define BHIEOFF_BHIEOFF_MASK (0xFFFFFFFF)
-#define BHIEOFF_BHIEOFF_SHIFT (0)
-
-#define DEBUGOFF (0x30)
-#define DEBUGOFF_DEBUGOFF_MASK (0xFFFFFFFF)
-#define DEBUGOFF_DEBUGOFF_SHIFT (0)
-
-#define MHICTRL (0x38)
-#define MHICTRL_MHISTATE_MASK (0x0000FF00)
-#define MHICTRL_MHISTATE_SHIFT (8)
-#define MHICTRL_RESET_MASK (0x2)
-#define MHICTRL_RESET_SHIFT (1)
-
-#define MHISTATUS (0x48)
-#define MHISTATUS_MHISTATE_MASK (0x0000FF00)
-#define MHISTATUS_MHISTATE_SHIFT (8)
-#define MHISTATUS_SYSERR_MASK (0x4)
-#define MHISTATUS_SYSERR_SHIFT (2)
-#define MHISTATUS_READY_MASK (0x1)
-#define MHISTATUS_READY_SHIFT (0)
-
-#define CCABAP_LOWER (0x58)
-#define CCABAP_LOWER_CCABAP_LOWER_MASK (0xFFFFFFFF)
-#define CCABAP_LOWER_CCABAP_LOWER_SHIFT (0)
-
-#define CCABAP_HIGHER (0x5C)
-#define CCABAP_HIGHER_CCABAP_HIGHER_MASK (0xFFFFFFFF)
-#define CCABAP_HIGHER_CCABAP_HIGHER_SHIFT (0)
-
-#define ECABAP_LOWER (0x60)
-#define ECABAP_LOWER_ECABAP_LOWER_MASK (0xFFFFFFFF)
-#define ECABAP_LOWER_ECABAP_LOWER_SHIFT (0)
-
-#define ECABAP_HIGHER (0x64)
-#define ECABAP_HIGHER_ECABAP_HIGHER_MASK (0xFFFFFFFF)
-#define ECABAP_HIGHER_ECABAP_HIGHER_SHIFT (0)
-
-#define CRCBAP_LOWER (0x68)
-#define CRCBAP_LOWER_CRCBAP_LOWER_MASK (0xFFFFFFFF)
-#define CRCBAP_LOWER_CRCBAP_LOWER_SHIFT (0)
-
-#define CRCBAP_HIGHER (0x6C)
-#define CRCBAP_HIGHER_CRCBAP_HIGHER_MASK (0xFFFFFFFF)
-#define CRCBAP_HIGHER_CRCBAP_HIGHER_SHIFT (0)
-
-#define CRDB_LOWER (0x70)
-#define CRDB_LOWER_CRDB_LOWER_MASK (0xFFFFFFFF)
-#define CRDB_LOWER_CRDB_LOWER_SHIFT (0)
-
-#define CRDB_HIGHER (0x74)
-#define CRDB_HIGHER_CRDB_HIGHER_MASK (0xFFFFFFFF)
-#define CRDB_HIGHER_CRDB_HIGHER_SHIFT (0)
-
-#define MHICTRLBASE_LOWER (0x80)
-#define MHICTRLBASE_LOWER_MHICTRLBASE_LOWER_MASK (0xFFFFFFFF)
-#define MHICTRLBASE_LOWER_MHICTRLBASE_LOWER_SHIFT (0)
-
-#define MHICTRLBASE_HIGHER (0x84)
-#define MHICTRLBASE_HIGHER_MHICTRLBASE_HIGHER_MASK (0xFFFFFFFF)
-#define MHICTRLBASE_HIGHER_MHICTRLBASE_HIGHER_SHIFT (0)
-
-#define MHICTRLLIMIT_LOWER (0x88)
-#define MHICTRLLIMIT_LOWER_MHICTRLLIMIT_LOWER_MASK (0xFFFFFFFF)
-#define MHICTRLLIMIT_LOWER_MHICTRLLIMIT_LOWER_SHIFT (0)
-
-#define MHICTRLLIMIT_HIGHER (0x8C)
-#define MHICTRLLIMIT_HIGHER_MHICTRLLIMIT_HIGHER_MASK (0xFFFFFFFF)
-#define MHICTRLLIMIT_HIGHER_MHICTRLLIMIT_HIGHER_SHIFT (0)
-
-#define MHIDATABASE_LOWER (0x98)
-#define MHIDATABASE_LOWER_MHIDATABASE_LOWER_MASK (0xFFFFFFFF)
-#define MHIDATABASE_LOWER_MHIDATABASE_LOWER_SHIFT (0)
-
-#define MHIDATABASE_HIGHER (0x9C)
-#define MHIDATABASE_HIGHER_MHIDATABASE_HIGHER_MASK (0xFFFFFFFF)
-#define MHIDATABASE_HIGHER_MHIDATABASE_HIGHER_SHIFT (0)
-
-#define MHIDATALIMIT_LOWER (0xA0)
-#define MHIDATALIMIT_LOWER_MHIDATALIMIT_LOWER_MASK (0xFFFFFFFF)
-#define MHIDATALIMIT_LOWER_MHIDATALIMIT_LOWER_SHIFT (0)
-
-#define MHIDATALIMIT_HIGHER (0xA4)
-#define MHIDATALIMIT_HIGHER_MHIDATALIMIT_HIGHER_MASK (0xFFFFFFFF)
-#define MHIDATALIMIT_HIGHER_MHIDATALIMIT_HIGHER_SHIFT (0)
+/* MHI registers */
+#define MHIREGLEN			0x0
+#define MHIVER				0x8
+#define MHICFG				0x10
+#define CHDBOFF				0x18
+#define ERDBOFF				0x20
+#define BHIOFF				0x28
+#define BHIEOFF				0x2c
+#define DEBUGOFF			0x30
+#define MHICTRL				0x38
+#define MHISTATUS			0x48
+#define CCABAP_LOWER			0x58
+#define CCABAP_HIGHER			0x5c
+#define ECABAP_LOWER			0x60
+#define ECABAP_HIGHER			0x64
+#define CRCBAP_LOWER			0x68
+#define CRCBAP_HIGHER			0x6c
+#define CRDB_LOWER			0x70
+#define CRDB_HIGHER			0x74
+#define MHICTRLBASE_LOWER		0x80
+#define MHICTRLBASE_HIGHER		0x84
+#define MHICTRLLIMIT_LOWER		0x88
+#define MHICTRLLIMIT_HIGHER		0x8c
+#define MHIDATABASE_LOWER		0x98
+#define MHIDATABASE_HIGHER		0x9c
+#define MHIDATALIMIT_LOWER		0xa0
+#define MHIDATALIMIT_HIGHER		0xa4
 
 /* Host request register */
-#define MHI_SOC_RESET_REQ_OFFSET (0xB0)
-#define MHI_SOC_RESET_REQ BIT(0)
+#define MHI_SOC_RESET_REQ_OFFSET	0xb0
+#define MHI_SOC_RESET_REQ		BIT(0)
 
 /* MHI BHI offfsets */
-#define BHI_BHIVERSION_MINOR (0x00)
-#define BHI_BHIVERSION_MAJOR (0x04)
-#define BHI_IMGADDR_LOW (0x08)
-#define BHI_IMGADDR_HIGH (0x0C)
-#define BHI_IMGSIZE (0x10)
-#define BHI_RSVD1 (0x14)
-#define BHI_IMGTXDB (0x18)
-#define BHI_TXDB_SEQNUM_BMSK (0x3FFFFFFF)
-#define BHI_TXDB_SEQNUM_SHFT (0)
-#define BHI_RSVD2 (0x1C)
-#define BHI_INTVEC (0x20)
-#define BHI_RSVD3 (0x24)
-#define BHI_EXECENV (0x28)
-#define BHI_STATUS (0x2C)
-#define BHI_ERRCODE (0x30)
-#define BHI_ERRDBG1 (0x34)
-#define BHI_ERRDBG2 (0x38)
-#define BHI_ERRDBG3 (0x3C)
-#define BHI_SERIALNU (0x40)
-#define BHI_SBLANTIROLLVER (0x44)
-#define BHI_NUMSEG (0x48)
-#define BHI_MSMHWID(n) (0x4C + (0x4 * (n)))
-#define BHI_OEMPKHASH(n) (0x64 + (0x4 * (n)))
-#define BHI_RSVD5 (0xC4)
-#define BHI_STATUS_MASK (0xC0000000)
-#define BHI_STATUS_SHIFT (30)
-#define BHI_STATUS_ERROR (3)
-#define BHI_STATUS_SUCCESS (2)
-#define BHI_STATUS_RESET (0)
+#define BHI_BHIVERSION_MINOR		0x00
+#define BHI_BHIVERSION_MAJOR		0x04
+#define BHI_IMGADDR_LOW			0x08
+#define BHI_IMGADDR_HIGH		0x0c
+#define BHI_IMGSIZE			0x10
+#define BHI_RSVD1			0x14
+#define BHI_IMGTXDB			0x18
+#define BHI_TXDB_SEQNUM_BMSK		GENMASK(29, 0)
+#define BHI_TXDB_SEQNUM_SHFT		0
+#define BHI_RSVD2			0x1c
+#define BHI_INTVEC			0x20
+#define BHI_RSVD3			0x24
+#define BHI_EXECENV			0x28
+#define BHI_STATUS			0x2c
+#define BHI_ERRCODE			0x30
+#define BHI_ERRDBG1			0x34
+#define BHI_ERRDBG2			0x38
+#define BHI_ERRDBG3			0x3c
+#define BHI_SERIALNU			0x40
+#define BHI_SBLANTIROLLVER		0x44
+#define BHI_NUMSEG			0x48
+#define BHI_MSMHWID(n)			(0x4c + (0x4 * (n)))
+#define BHI_OEMPKHASH(n)		(0x64 + (0x4 * (n)))
+#define BHI_RSVD5			0xc4
+#define BHI_STATUS_MASK			GENMASK(31, 30)
+#define BHI_STATUS_SHIFT		30
+#define BHI_STATUS_ERROR		3
+#define BHI_STATUS_SUCCESS		2
+#define BHI_STATUS_RESET		0
 
 /* MHI BHIE offsets */
-#define BHIE_MSMSOCID_OFFS (0x0000)
-#define BHIE_TXVECADDR_LOW_OFFS (0x002C)
-#define BHIE_TXVECADDR_HIGH_OFFS (0x0030)
-#define BHIE_TXVECSIZE_OFFS (0x0034)
-#define BHIE_TXVECDB_OFFS (0x003C)
-#define BHIE_TXVECDB_SEQNUM_BMSK (0x3FFFFFFF)
-#define BHIE_TXVECDB_SEQNUM_SHFT (0)
-#define BHIE_TXVECSTATUS_OFFS (0x0044)
-#define BHIE_TXVECSTATUS_SEQNUM_BMSK (0x3FFFFFFF)
-#define BHIE_TXVECSTATUS_SEQNUM_SHFT (0)
-#define BHIE_TXVECSTATUS_STATUS_BMSK (0xC0000000)
-#define BHIE_TXVECSTATUS_STATUS_SHFT (30)
-#define BHIE_TXVECSTATUS_STATUS_RESET (0x00)
-#define BHIE_TXVECSTATUS_STATUS_XFER_COMPL (0x02)
-#define BHIE_TXVECSTATUS_STATUS_ERROR (0x03)
-#define BHIE_RXVECADDR_LOW_OFFS (0x0060)
-#define BHIE_RXVECADDR_HIGH_OFFS (0x0064)
-#define BHIE_RXVECSIZE_OFFS (0x0068)
-#define BHIE_RXVECDB_OFFS (0x0070)
-#define BHIE_RXVECDB_SEQNUM_BMSK (0x3FFFFFFF)
-#define BHIE_RXVECDB_SEQNUM_SHFT (0)
-#define BHIE_RXVECSTATUS_OFFS (0x0078)
-#define BHIE_RXVECSTATUS_SEQNUM_BMSK (0x3FFFFFFF)
-#define BHIE_RXVECSTATUS_SEQNUM_SHFT (0)
-#define BHIE_RXVECSTATUS_STATUS_BMSK (0xC0000000)
-#define BHIE_RXVECSTATUS_STATUS_SHFT (30)
-#define BHIE_RXVECSTATUS_STATUS_RESET (0x00)
-#define BHIE_RXVECSTATUS_STATUS_XFER_COMPL (0x02)
-#define BHIE_RXVECSTATUS_STATUS_ERROR (0x03)
-
-#define SOC_HW_VERSION_OFFS (0x224)
-#define SOC_HW_VERSION_FAM_NUM_BMSK (0xF0000000)
-#define SOC_HW_VERSION_FAM_NUM_SHFT (28)
-#define SOC_HW_VERSION_DEV_NUM_BMSK (0x0FFF0000)
-#define SOC_HW_VERSION_DEV_NUM_SHFT (16)
-#define SOC_HW_VERSION_MAJOR_VER_BMSK (0x0000FF00)
-#define SOC_HW_VERSION_MAJOR_VER_SHFT (8)
-#define SOC_HW_VERSION_MINOR_VER_BMSK (0x000000FF)
-#define SOC_HW_VERSION_MINOR_VER_SHFT (0)
+#define BHIE_MSMSOCID_OFFS		0x0000
+#define BHIE_TXVECADDR_LOW_OFFS		0x002c
+#define BHIE_TXVECADDR_HIGH_OFFS	0x0030
+#define BHIE_TXVECSIZE_OFFS		0x0034
+#define BHIE_TXVECDB_OFFS		0x003c
+#define BHIE_TXVECDB_SEQNUM_BMSK	GENMASK(29, 0)
+#define BHIE_TXVECDB_SEQNUM_SHFT	0
+#define BHIE_TXVECSTATUS_OFFS		0x0044
+#define BHIE_TXVECSTATUS_SEQNUM_BMSK	GENMASK(29, 0)
+#define BHIE_TXVECSTATUS_SEQNUM_SHFT	0
+#define BHIE_TXVECSTATUS_STATUS_BMSK	GENMASK(31, 30)
+#define BHIE_TXVECSTATUS_STATUS_SHFT	30
+#define BHIE_TXVECSTATUS_STATUS_RESET	0x00
+#define BHIE_TXVECSTATUS_STATUS_XFER_COMPL 0x02
+#define BHIE_TXVECSTATUS_STATUS_ERROR	0x03
+#define BHIE_RXVECADDR_LOW_OFFS		0x0060
+#define BHIE_RXVECADDR_HIGH_OFFS	0x0064
+#define BHIE_RXVECSIZE_OFFS		0x0068
+#define BHIE_RXVECDB_OFFS		0x0070
+#define BHIE_RXVECDB_SEQNUM_BMSK	GENMASK(29, 0)
+#define BHIE_RXVECDB_SEQNUM_SHFT	0
+#define BHIE_RXVECSTATUS_OFFS		0x0078
+#define BHIE_RXVECSTATUS_SEQNUM_BMSK	GENMASK(29, 0)
+#define BHIE_RXVECSTATUS_SEQNUM_SHFT	0
+#define BHIE_RXVECSTATUS_STATUS_BMSK	GENMASK(31, 30)
+#define BHIE_RXVECSTATUS_STATUS_SHFT	30
+#define BHIE_RXVECSTATUS_STATUS_RESET	0x00
+#define BHIE_RXVECSTATUS_STATUS_XFER_COMPL 0x02
+#define BHIE_RXVECSTATUS_STATUS_ERROR	0x03
+
+#define SOC_HW_VERSION_OFFS		0x224
+#define SOC_HW_VERSION_FAM_NUM_BMSK	GENMASK(31, 28)
+#define SOC_HW_VERSION_FAM_NUM_SHFT	28
+#define SOC_HW_VERSION_DEV_NUM_BMSK	GENMASK(27, 16)
+#define SOC_HW_VERSION_DEV_NUM_SHFT	16
+#define SOC_HW_VERSION_MAJOR_VER_BMSK	GENMASK(15, 8)
+#define SOC_HW_VERSION_MAJOR_VER_SHFT	8
+#define SOC_HW_VERSION_MINOR_VER_BMSK	GENMASK(7, 0)
+#define SOC_HW_VERSION_MINOR_VER_SHFT	0
 
 struct mhi_ctxt {
 	struct mhi_event_ctxt *er_ctxt;