[PATCH] imx{5,6}: reset otg port after usb-serial boot

[PATCH] imx{5,6}: reset otg port after usb-serial boot

[Michael Grzeschik]

The ROM loader leaves the otg port in peripheral mode which confuses the
host controller. After the handover to barebox we ensure to bring in the
port into the otg state by resetting it.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
This was tested with mx53 and mx6 for now. I suppose we should
adopt this to the mx2 and mx3 models.

 arch/arm/mach-imx/imx50.c | 3 +++
 arch/arm/mach-imx/imx51.c | 3 +++
 arch/arm/mach-imx/imx53.c | 3 +++
 arch/arm/mach-imx/imx6.c  | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/arch/arm/mach-imx/imx50.c b/arch/arm/mach-imx/imx50.c
index f7cbc9d4ba..ac326cf3c0 100644
--- a/arch/arm/mach-imx/imx50.c
+++ b/arch/arm/mach-imx/imx50.c
@@ -92,6 +92,9 @@ void imx50_init_lowlevel_early(unsigned int cpufreq_mhz)
 
 	imx5_init_lowlevel();
 
+	/* reset otg: in case we started via usb-serial */
+	writel((readl(0x53F80140) | 2), 0x53F80140);
+
 	/*
 	 * AIPS setup - Only setup MPROTx registers.
 	 * The PACR default values are good.
diff --git a/arch/arm/mach-imx/imx51.c b/arch/arm/mach-imx/imx51.c
index ec8cdd868b..91f6df1cfa 100644
--- a/arch/arm/mach-imx/imx51.c
+++ b/arch/arm/mach-imx/imx51.c
@@ -142,6 +142,9 @@ void imx51_init_lowlevel(unsigned int cpufreq_mhz)
 
 	imx5_init_lowlevel();
 
+	/* reset otg: in case we started via usb-serial */
+	writel((readl(0x73F80140) | 2), 0x73F80140);
+
 	/* disable write combine for TO 2 and lower revs */
 	if (rev < IMX_CHIP_REV_3_0) {
 		__asm__ __volatile__("mrc 15, 1, %0, c9, c0, 1":"=r"(r));
diff --git a/arch/arm/mach-imx/imx53.c b/arch/arm/mach-imx/imx53.c
index b22929f749..99e942ed52 100644
--- a/arch/arm/mach-imx/imx53.c
+++ b/arch/arm/mach-imx/imx53.c
@@ -90,6 +90,9 @@ void imx53_init_lowlevel_early(unsigned int cpufreq_mhz)
 
 	imx5_init_lowlevel();
 
+	/* reset otg: in case we started via usb-serial */
+	writel((readl(0x53F80140) | 2), 0x53F80140);
+
 	/*
 	 * AIPS setup - Only setup MPROTx registers.
 	 * The PACR default values are good.
diff --git a/arch/arm/mach-imx/imx6.c b/arch/arm/mach-imx/imx6.c
index 3d95c9e374..8af051c34a 100644
--- a/arch/arm/mach-imx/imx6.c
+++ b/arch/arm/mach-imx/imx6.c
@@ -193,6 +193,9 @@ int imx6_init(void)
 
 	imx6_init_lowlevel();
 
+	/* reset otg: in case we started via usb-serial */
+	writel((readl(0x21840140) | 2), 0x21840140);
+
 	imx6_boot_save_loc();
 
 	mx6_silicon_revision = imx6_cpu_revision();
-- 
2.18.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Sam Ravnborg]

Hi Michael.

On Fri, Aug 24, 2018 at 05:17:39PM +0200, Michael Grzeschik wrote:
> The ROM loader leaves the otg port in peripheral mode which confuses the
> host controller. After the handover to barebox we ensure to bring in the
> port into the otg state by resetting it.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
> This was tested with mx53 and mx6 for now. I suppose we should
> adopt this to the mx2 and mx3 models.
> 
>  arch/arm/mach-imx/imx50.c | 3 +++
>  arch/arm/mach-imx/imx51.c | 3 +++
>  arch/arm/mach-imx/imx53.c | 3 +++
>  arch/arm/mach-imx/imx6.c  | 3 +++
>  4 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/imx50.c b/arch/arm/mach-imx/imx50.c
> index f7cbc9d4ba..ac326cf3c0 100644
> --- a/arch/arm/mach-imx/imx50.c
> +++ b/arch/arm/mach-imx/imx50.c
> @@ -92,6 +92,9 @@ void imx50_init_lowlevel_early(unsigned int cpufreq_mhz)
>  
>  	imx5_init_lowlevel();
>  
> +	/* reset otg: in case we started via usb-serial */
> +	writel((readl(0x53F80140) | 2), 0x53F80140);
> +

This looks like constants you just pulled out of the air.
Can you hide it behind a descriptive name?


>  	/*
>  	 * AIPS setup - Only setup MPROTx registers.
>  	 * The PACR default values are good.
> diff --git a/arch/arm/mach-imx/imx51.c b/arch/arm/mach-imx/imx51.c
> index ec8cdd868b..91f6df1cfa 100644
> --- a/arch/arm/mach-imx/imx51.c
> +++ b/arch/arm/mach-imx/imx51.c
> @@ -142,6 +142,9 @@ void imx51_init_lowlevel(unsigned int cpufreq_mhz)
>  
>  	imx5_init_lowlevel();
>  
> +	/* reset otg: in case we started via usb-serial */
> +	writel((readl(0x73F80140) | 2), 0x73F80140);
Likewise.

> +
>  	/* disable write combine for TO 2 and lower revs */
>  	if (rev < IMX_CHIP_REV_3_0) {
>  		__asm__ __volatile__("mrc 15, 1, %0, c9, c0, 1":"=r"(r));
> diff --git a/arch/arm/mach-imx/imx53.c b/arch/arm/mach-imx/imx53.c
> index b22929f749..99e942ed52 100644
> --- a/arch/arm/mach-imx/imx53.c
> +++ b/arch/arm/mach-imx/imx53.c
> @@ -90,6 +90,9 @@ void imx53_init_lowlevel_early(unsigned int cpufreq_mhz)
>  
>  	imx5_init_lowlevel();
>  
> +	/* reset otg: in case we started via usb-serial */
> +	writel((readl(0x53F80140) | 2), 0x53F80140);
And then you did not have to hardcode the same value here again.

> +
>  	/*
>  	 * AIPS setup - Only setup MPROTx registers.
>  	 * The PACR default values are good.
> diff --git a/arch/arm/mach-imx/imx6.c b/arch/arm/mach-imx/imx6.c
> index 3d95c9e374..8af051c34a 100644
> --- a/arch/arm/mach-imx/imx6.c
> +++ b/arch/arm/mach-imx/imx6.c
> @@ -193,6 +193,9 @@ int imx6_init(void)
>  
>  	imx6_init_lowlevel();
>  
> +	/* reset otg: in case we started via usb-serial */
> +	writel((readl(0x21840140) | 2), 0x21840140);
Likewise.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Sascha Hauer]

On Fri, Aug 24, 2018 at 05:17:39PM +0200, Michael Grzeschik wrote:
> The ROM loader leaves the otg port in peripheral mode which confuses the
> host controller. After the handover to barebox we ensure to bring in the
> port into the otg state by resetting it.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
> This was tested with mx53 and mx6 for now. I suppose we should
> adopt this to the mx2 and mx3 models.

Looks like a variant of the patch I posted here:
https://www.spinics.net/lists/u-boot-v2/msg34153.html
I have given the reason this can't be applied directly here:
https://www.spinics.net/lists/u-boot-v2/msg34196.html

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Michael Grzeschik]

[-- Attachment #1.1: Type: text/plain, Size: 1299 bytes --]

On Fri, Aug 24, 2018 at 07:04:07PM +0200, Sascha Hauer wrote:
> On Fri, Aug 24, 2018 at 05:17:39PM +0200, Michael Grzeschik wrote:
> > The ROM loader leaves the otg port in peripheral mode which confuses the
> > host controller. After the handover to barebox we ensure to bring in the
> > port into the otg state by resetting it.
> > 
> > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > ---
> > This was tested with mx53 and mx6 for now. I suppose we should
> > adopt this to the mx2 and mx3 models.
> 
> Looks like a variant of the patch I posted here:
> https://www.spinics.net/lists/u-boot-v2/msg34153.html
> I have given the reason this can't be applied directly here:
> https://www.spinics.net/lists/u-boot-v2/msg34196.html

Just toggling RS was not enough in my case.

I get that it is not safe to toggle bits if the clock is not
there. We could check for the bootsource. Is it possible to
ensure that we are started by imx-usb-loader?

mgr

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Fabio Estevam]

Hi Michael,

On Fri, Aug 24, 2018 at 4:55 PM, Michael Grzeschik <mgr@pengutronix.de> wrote:

> I get that it is not safe to toggle bits if the clock is not
> there. We could check for the bootsource. Is it possible to
> ensure that we are started by imx-usb-loader?

It is possible to determine if we have started via USB download mode.

Check the is_boot_from_usb() implementation in U-Boot for a reference.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Michael Grzeschik]

[-- Attachment #1.1: Type: text/plain, Size: 1135 bytes --]

On Fri, Aug 24, 2018 at 05:18:02PM -0300, Fabio Estevam wrote:
> Hi Michael,
> 
> On Fri, Aug 24, 2018 at 4:55 PM, Michael Grzeschik <mgr@pengutronix.de> wrote:
> 
> > I get that it is not safe to toggle bits if the clock is not
> > there. We could check for the bootsource. Is it possible to
> > ensure that we are started by imx-usb-loader?
> 
> It is possible to determine if we have started via USB download mode.
> 
> Check the is_boot_from_usb() implementation in U-Boot for a reference.

Right, So it seems to be safe to reach for the phys registers on mx6.

On mx5 and similar we have a nop phy and the only way to
communicate with it is through usbmisc, which is also in the
register space that leads to a hang if one clock is missing.

I will search for another solutions on those SoCs.

mgr

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Sascha Hauer]

On Mon, Aug 27, 2018 at 09:47:41AM +0200, Michael Grzeschik wrote:
> On Fri, Aug 24, 2018 at 05:18:02PM -0300, Fabio Estevam wrote:
> > Hi Michael,
> > 
> > On Fri, Aug 24, 2018 at 4:55 PM, Michael Grzeschik <mgr@pengutronix.de> wrote:
> > 
> > > I get that it is not safe to toggle bits if the clock is not
> > > there. We could check for the bootsource. Is it possible to
> > > ensure that we are started by imx-usb-loader?
> > 
> > It is possible to determine if we have started via USB download mode.
> > 
> > Check the is_boot_from_usb() implementation in U-Boot for a reference.
> 
> Right, So it seems to be safe to reach for the phys registers on mx6.
> 
> On mx5 and similar we have a nop phy and the only way to
> communicate with it is through usbmisc, which is also in the
> register space that leads to a hang if one clock is missing.
> 
> I will search for another solutions on those SoCs.

It's not necessary to ask for the bootsource. You just have to enable
the clocks necessary to access the USB registers. Or the other way
round: When the clocks are not enabled already, you could assume we have
not booted from USB.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Michael Grzeschik]

[-- Attachment #1.1: Type: text/plain, Size: 1758 bytes --]

On Mon, Aug 27, 2018 at 09:52:24AM +0200, Sascha Hauer wrote:
> On Mon, Aug 27, 2018 at 09:47:41AM +0200, Michael Grzeschik wrote:
> > On Fri, Aug 24, 2018 at 05:18:02PM -0300, Fabio Estevam wrote:
> > > Hi Michael,
> > > 
> > > On Fri, Aug 24, 2018 at 4:55 PM, Michael Grzeschik <mgr@pengutronix.de> wrote:
> > > 
> > > > I get that it is not safe to toggle bits if the clock is not
> > > > there. We could check for the bootsource. Is it possible to
> > > > ensure that we are started by imx-usb-loader?
> > > 
> > > It is possible to determine if we have started via USB download mode.
> > > 
> > > Check the is_boot_from_usb() implementation in U-Boot for a reference.
> > 
> > Right, So it seems to be safe to reach for the phys registers on mx6.
> > 
> > On mx5 and similar we have a nop phy and the only way to
> > communicate with it is through usbmisc, which is also in the
> > register space that leads to a hang if one clock is missing.
> > 
> > I will search for another solutions on those SoCs.
> 
> It's not necessary to ask for the bootsource. You just have to enable
> the clocks necessary to access the USB registers. Or the other way
> round: When the clocks are not enabled already, you could assume we have
> not booted from USB.

Right, I think enabling the clocks is a little bit more safe
as we otherwise would need to ensure that nobody slips in
a DCD that already ungates all clocks.

mgr

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Michael Grzeschik]

[-- Attachment #1.1: Type: text/plain, Size: 1982 bytes --]

On Mon, Aug 27, 2018 at 10:13:22AM +0200, Michael Grzeschik wrote:
> On Mon, Aug 27, 2018 at 09:52:24AM +0200, Sascha Hauer wrote:
> > On Mon, Aug 27, 2018 at 09:47:41AM +0200, Michael Grzeschik wrote:
> > > On Fri, Aug 24, 2018 at 05:18:02PM -0300, Fabio Estevam wrote:
> > > > Hi Michael,
> > > > 
> > > > On Fri, Aug 24, 2018 at 4:55 PM, Michael Grzeschik <mgr@pengutronix.de> wrote:
> > > > 
> > > > > I get that it is not safe to toggle bits if the clock is not
> > > > > there. We could check for the bootsource. Is it possible to
> > > > > ensure that we are started by imx-usb-loader?
> > > > 
> > > > It is possible to determine if we have started via USB download mode.
> > > > 
> > > > Check the is_boot_from_usb() implementation in U-Boot for a reference.
> > > 
> > > Right, So it seems to be safe to reach for the phys registers on mx6.
> > > 
> > > On mx5 and similar we have a nop phy and the only way to
> > > communicate with it is through usbmisc, which is also in the
> > > register space that leads to a hang if one clock is missing.
> > > 
> > > I will search for another solutions on those SoCs.
> > 
> > It's not necessary to ask for the bootsource. You just have to enable
> > the clocks necessary to access the USB registers. Or the other way
> > round: When the clocks are not enabled already, you could assume we have
> > not booted from USB.
> 
> Right, I think enabling the clocks is a little bit more safe
> as we otherwise would need to ensure that nobody slips in
> a DCD that already ungates all clocks.

Forget that! Even in that case it is no problem to
reset the OTG core! :D ... more coffee

mgr

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] imx{5,6}: reset otg port after usb-serial boot

[Sascha Hauer]

On Mon, Aug 27, 2018 at 10:13:22AM +0200, Michael Grzeschik wrote:
> On Mon, Aug 27, 2018 at 09:52:24AM +0200, Sascha Hauer wrote:
> > On Mon, Aug 27, 2018 at 09:47:41AM +0200, Michael Grzeschik wrote:
> > > On Fri, Aug 24, 2018 at 05:18:02PM -0300, Fabio Estevam wrote:
> > > > Hi Michael,
> > > > 
> > > > On Fri, Aug 24, 2018 at 4:55 PM, Michael Grzeschik <mgr@pengutronix.de> wrote:
> > > > 
> > > > > I get that it is not safe to toggle bits if the clock is not
> > > > > there. We could check for the bootsource. Is it possible to
> > > > > ensure that we are started by imx-usb-loader?
> > > > 
> > > > It is possible to determine if we have started via USB download mode.
> > > > 
> > > > Check the is_boot_from_usb() implementation in U-Boot for a reference.
> > > 
> > > Right, So it seems to be safe to reach for the phys registers on mx6.
> > > 
> > > On mx5 and similar we have a nop phy and the only way to
> > > communicate with it is through usbmisc, which is also in the
> > > register space that leads to a hang if one clock is missing.
> > > 
> > > I will search for another solutions on those SoCs.
> > 
> > It's not necessary to ask for the bootsource. You just have to enable
> > the clocks necessary to access the USB registers. Or the other way
> > round: When the clocks are not enabled already, you could assume we have
> > not booted from USB.
> 
> Right, I think enabling the clocks is a little bit more safe
> as we otherwise would need to ensure that nobody slips in
> a DCD that already ungates all clocks.

What could happen is that somebody *dis*abled the USB clocks in DCD. In
that case though the USB on the host will already be confused and we
already have lost - no need to do anything in that case.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox