[U-Boot] [PATCH] mx25: fix the offset between the USB ports' registers

[U-Boot] [PATCH] mx25: fix the offset between the USB ports' registers

[Martin Kaiser]

From: Martin Kaiser <martin@kaiser.cx>

The USBOH module on imx25 chips contains two USB controllers, which are
called USB OTG Controller and USB Host Controller. Each one has its EHCI
root hub. The OTG Controller's EHCI registers start at offset 0, the Host
Controller's registers start at offset 0x400.

We set CONFIG_MXC_USB_PORT=0 to select the OTG Controller and 1 for the
Host Controller. Therefore, IMX_USB_PORT_OFFSET must be 0x400. Using
this setting, the Host Controller starts working on my imx25 board.

Please note that the imx25 reference manual claims that the Host
Controller's registers start at 0x200. This is not correct. The Linux
Kernel uses the correct offset 0x400 in imx25.dtsi.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 arch/arm/include/asm/arch-mx25/imx-regs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
index 1b00ed7..8d08a85 100644
--- a/arch/arm/include/asm/arch-mx25/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
@@ -360,7 +360,7 @@ struct cspi_regs {
 #define IMX_IIM_BASE		(0x53FF0000)
 #define IIM_BASE_ADDR		IMX_IIM_BASE
 #define IMX_USB_BASE		(0x53FF4000)
-#define IMX_USB_PORT_OFFSET	0x200
+#define IMX_USB_PORT_OFFSET	0x400
 #define IMX_CSI_BASE		(0x53FF8000)
 #define IMX_DRYICE_BASE		(0x53FFC000)
 
-- 
2.1.4

[U-Boot] [PATCH] mx25: fix the offset between the USB ports' registers

[Fabio Estevam]

Hi Martin,

Thanks for the patch!

On Fri, Jul 13, 2018 at 5:02 AM, Martin Kaiser <lists@kaiser.cx> wrote:
> From: Martin Kaiser <martin@kaiser.cx>
>
> The USBOH module on imx25 chips contains two USB controllers, which are
> called USB OTG Controller and USB Host Controller. Each one has its EHCI
> root hub. The OTG Controller's EHCI registers start at offset 0, the Host
> Controller's registers start at offset 0x400.
>
> We set CONFIG_MXC_USB_PORT=0 to select the OTG Controller and 1 for the
> Host Controller. Therefore, IMX_USB_PORT_OFFSET must be 0x400. Using
> this setting, the Host Controller starts working on my imx25 board.
>
> Please note that the imx25 reference manual claims that the Host
> Controller's registers start at 0x200. This is not correct. The Linux
> Kernel uses the correct offset 0x400 in imx25.dtsi.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  arch/arm/include/asm/arch-mx25/imx-regs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
> index 1b00ed7..8d08a85 100644
> --- a/arch/arm/include/asm/arch-mx25/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
> @@ -360,7 +360,7 @@ struct cspi_regs {
>  #define IMX_IIM_BASE           (0x53FF0000)
>  #define IIM_BASE_ADDR          IMX_IIM_BASE
>  #define IMX_USB_BASE           (0x53FF4000)
> -#define IMX_USB_PORT_OFFSET    0x200
> +#define IMX_USB_PORT_OFFSET    0x400

Could you also add some comments here?

Something like you wrote in the commit log would help.

This way we avoid in the future someone trying to fix the mismatch
between the Reference Manual and the code definition.

With that added:

Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>

[U-Boot] [PATCH v2] mx25: fix the offset between the USB ports' registers

[Martin Kaiser]

From: Martin Kaiser <martin@kaiser.cx>

The USBOH module on imx25 chips contains two USB controllers which are
called USB OTG Controller and USB Host Controller. Each one has its EHCI
root hub. The OTG Controller's EHCI registers start at offset 0, the Host
Controller's registers start at offset 0x400.

We set CONFIG_MXC_USB_PORT=0 to select the OTG Controller and 1 for the
Host Controller. Therefore, IMX_USB_PORT_OFFSET must be 0x400. Using
this setting, the Host Controller starts working on my imx25 board.

Please note that the imx25 reference manual claims that the Host
Controller's registers start at 0x200. This is not correct. The Linux
Kernel uses the correct offset 0x400 in imx25.dtsi.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes in v2:
 - add a comment about the incorrect info in the reference manual

 arch/arm/include/asm/arch-mx25/imx-regs.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
index 1b00ed7..28cebeb 100644
--- a/arch/arm/include/asm/arch-mx25/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
@@ -360,7 +360,10 @@ struct cspi_regs {
 #define IMX_IIM_BASE		(0x53FF0000)
 #define IIM_BASE_ADDR		IMX_IIM_BASE
 #define IMX_USB_BASE		(0x53FF4000)
-#define IMX_USB_PORT_OFFSET	0x200
+/* This is in contradiction to the imx25 reference manual, which says that
+   port 1's registers start at 0x53FF4200. The correct base address for
+   port 1 is 0x53FF4400. The kernel uses 0x53FF4400 as well. */
+#define IMX_USB_PORT_OFFSET	0x400
 #define IMX_CSI_BASE		(0x53FF8000)
 #define IMX_DRYICE_BASE		(0x53FFC000)
 
-- 
2.1.4

[U-Boot] [PATCH v2] mx25: fix the offset between the USB ports' registers

[Fabio Estevam]

Hi Martin,

On Mon, Jul 16, 2018 at 5:24 AM, Martin Kaiser <lists@kaiser.cx> wrote:

> diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
> index 1b00ed7..28cebeb 100644
> --- a/arch/arm/include/asm/arch-mx25/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
> @@ -360,7 +360,10 @@ struct cspi_regs {
>  #define IMX_IIM_BASE           (0x53FF0000)
>  #define IIM_BASE_ADDR          IMX_IIM_BASE
>  #define IMX_USB_BASE           (0x53FF4000)
> -#define IMX_USB_PORT_OFFSET    0x200
> +/* This is in contradiction to the imx25 reference manual, which says that
> +   port 1's registers start at 0x53FF4200. The correct base address for
> +   port 1 is 0x53FF4400. The kernel uses 0x53FF4400 as well. */
> +#define IMX_USB_PORT_OFFSET    0x400

Thanks for adding the comments to the code.

However the correct style for multi-lines comments are like this:

/*
 * bla bla
 * bla bla bla
 */

Thanks

[U-Boot] [PATCH v3] mx25: fix the offset between the USB ports' registers

[Martin Kaiser]

From: Martin Kaiser <martin@kaiser.cx>

The USBOH module on imx25 chips contains two USB controllers which are
called USB OTG Controller and USB Host Controller. Each one has its EHCI
root hub. The OTG Controller's EHCI registers start at offset 0, the Host
Controller's registers start at offset 0x400.

We set CONFIG_MXC_USB_PORT=0 to select the OTG Controller and 1 for the
Host Controller. Therefore, IMX_USB_PORT_OFFSET must be 0x400. Using
this setting, the Host Controller starts working on my imx25 board.

Please note that the imx25 reference manual claims that the Host
Controller's registers start at 0x200. This is not correct. The Linux
Kernel uses the correct offset 0x400 in imx25.dtsi.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 Changes in v3:
  - reformat the multi-line comment
    
 Changes in v2:
  - add a comment about the incorrect info in the reference manual

 arch/arm/include/asm/arch-mx25/imx-regs.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
index 1b00ed7..d64ca82 100644
--- a/arch/arm/include/asm/arch-mx25/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
@@ -360,7 +360,12 @@ struct cspi_regs {
 #define IMX_IIM_BASE		(0x53FF0000)
 #define IIM_BASE_ADDR		IMX_IIM_BASE
 #define IMX_USB_BASE		(0x53FF4000)
-#define IMX_USB_PORT_OFFSET	0x200
+/*
+ * This is in contradiction to the imx25 reference manual, which says that
+ * port 1's registers start at 0x53FF4200. The correct base address for
+ * port 1 is 0x53FF4400. The kernel uses 0x53FF4400 as well.
+ */
+#define IMX_USB_PORT_OFFSET	0x400
 #define IMX_CSI_BASE		(0x53FF8000)
 #define IMX_DRYICE_BASE		(0x53FFC000)
 
-- 
2.1.4

[U-Boot] [PATCH v3] mx25: fix the offset between the USB ports' registers

[Fabio Estevam]

Hi Martin,

On Mon, Jul 16, 2018 at 5:11 PM, Martin Kaiser <lists@kaiser.cx> wrote:
> From: Martin Kaiser <martin@kaiser.cx>
>
> The USBOH module on imx25 chips contains two USB controllers which are
> called USB OTG Controller and USB Host Controller. Each one has its EHCI
> root hub. The OTG Controller's EHCI registers start at offset 0, the Host
> Controller's registers start at offset 0x400.
>
> We set CONFIG_MXC_USB_PORT=0 to select the OTG Controller and 1 for the
> Host Controller. Therefore, IMX_USB_PORT_OFFSET must be 0x400. Using
> this setting, the Host Controller starts working on my imx25 board.
>
> Please note that the imx25 reference manual claims that the Host
> Controller's registers start at 0x200. This is not correct. The Linux
> Kernel uses the correct offset 0x400 in imx25.dtsi.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  Changes in v3:
>   - reformat the multi-line comment

Thanks for sending v3! All look goods now.

[U-Boot] [PATCH v3] mx25: fix the offset between the USB ports' registers

[Stefano Babic]

On 16/07/2018 22:11, Martin Kaiser wrote:
> From: Martin Kaiser <martin@kaiser.cx>
> 
> The USBOH module on imx25 chips contains two USB controllers which are
> called USB OTG Controller and USB Host Controller. Each one has its EHCI
> root hub. The OTG Controller's EHCI registers start at offset 0, the Host
> Controller's registers start at offset 0x400.
> 
> We set CONFIG_MXC_USB_PORT=0 to select the OTG Controller and 1 for the
> Host Controller. Therefore, IMX_USB_PORT_OFFSET must be 0x400. Using
> this setting, the Host Controller starts working on my imx25 board.
> 
> Please note that the imx25 reference manual claims that the Host
> Controller's registers start at 0x200. This is not correct. The Linux
> Kernel uses the correct offset 0x400 in imx25.dtsi.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  Changes in v3:
>   - reformat the multi-line comment
>     
>  Changes in v2:
>   - add a comment about the incorrect info in the reference manual
> 
>  arch/arm/include/asm/arch-mx25/imx-regs.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h
> index 1b00ed7..d64ca82 100644
> --- a/arch/arm/include/asm/arch-mx25/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
> @@ -360,7 +360,12 @@ struct cspi_regs {
>  #define IMX_IIM_BASE		(0x53FF0000)
>  #define IIM_BASE_ADDR		IMX_IIM_BASE
>  #define IMX_USB_BASE		(0x53FF4000)
> -#define IMX_USB_PORT_OFFSET	0x200
> +/*
> + * This is in contradiction to the imx25 reference manual, which says that
> + * port 1's registers start at 0x53FF4200. The correct base address for
> + * port 1 is 0x53FF4400. The kernel uses 0x53FF4400 as well.
> + */
> +#define IMX_USB_PORT_OFFSET	0x400
>  #define IMX_CSI_BASE		(0x53FF8000)
>  #define IMX_DRYICE_BASE		(0x53FFC000)
>  
> 

Applied to u-boot-imx, thanks !

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================