Re: [PATCH] usb: chipidea: Fix ULPI on imx51

From: Fabio Estevam <festevam@gmail.com>
To: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	Peter Chen <peter.chen@nxp.com>,
	USB list <linux-usb@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Chris Healy <cphealy@gmail.com>,
	Lucas Stach <l.stach@pengutronix.de>,
	Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Subject: Re: [PATCH] usb: chipidea: Fix ULPI on imx51
Date: Sun, 24 Jun 2018 20:51:20 -0300	[thread overview]
Message-ID: <CAOMZO5DAbooRgA+sHJa5EmEGy-CHW3FPydcOFtK5AqpdJzD7MQ@mail.gmail.com> (raw)
In-Reply-To: <CAHQ1cqHpNvZGAfFKrp-OiZevsha5CBwJsw8GT+uhgNSfEUejkw@mail.gmail.com>

Hi Andrey,

On Sun, Jun 24, 2018 at 7:40 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:

> It's definitely the bootloader, or more specifically whether or not it
> initialized/used USB before booting the kernel. Some interesting
> highlights:
>
>  - On your "good" 4.17.2 based image, if you interrupt U-Boot, do "usb
> start" (optionally "usb stop") and then "boot", you'll get the hang
> that I was trying to fix with my patch.
>
>  - Things are exact opposite with 4.18-rc1 and doing the above would
> _prevent_ the hang and the image would boot fine.
>
>  - Disabling USB in Barebox based boot "stack" gets it to behave the
> same way as U-boot from your image (hanging when booting 4.18-rc1)
>
> Digging more into code it seems that the reason for 4.18-rc1's
> behavior is the fact that it's missing a call hw_phymode_configure()
> after usb_phy_init() and, AFAICT, the only reason it is not happening
> is because default image is being built without
> CONFIG_USB_CHIPIDEA_ULPI and CONFIG_USB_ULPI_BUS. Enabling those two
> options on 4.18-rc1, seem to fix the problem on both your U-Boot based
> image and my "special" Barebox setup.
>
> So AFAICT, this patch still fixes a valid issue (my use-case was net
> booting via USB-Ethernet dongle), but an additional patch enabling
> those two configuration options might be needed. Thoughts?

Yes, I can confirm that your suggested change avoids the kernel hang:

diff --git a/arch/arm/configs/imx_v6_v7_defconfig
b/arch/arm/configs/imx_v6_v7_defconfig
index d14de86..2da678c 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -300,6 +300,7 @@ CONFIG_USB_STORAGE=y
 CONFIG_USB_CHIPIDEA=y
 CONFIG_USB_CHIPIDEA_UDC=y
 CONFIG_USB_CHIPIDEA_HOST=y
+CONFIG_USB_CHIPIDEA_ULPI=y
 CONFIG_USB_SERIAL=m
 CONFIG_USB_SERIAL_GENERIC=y
 CONFIG_USB_SERIAL_FTDI_SIO=m
@@ -336,6 +337,7 @@ CONFIG_USB_GADGETFS=m
 CONFIG_USB_FUNCTIONFS=m
 CONFIG_USB_MASS_STORAGE=m
 CONFIG_USB_G_SERIAL=m
+CONFIG_USB_ULPI_BUS=y
 CONFIG_MMC=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_PLTFM=y

> Also, I don't have any i.MX53 HW, so I wasn't able to test the effects
> of enabling those two options there.

The imx53 board I have access to does not use ULPI.

Maybe Sebastian on Cc may help testing this on his imx53 board as he
is the author of:
be9cae2479f48 ("usb: chipidea: imx: Fix ULPI on imx53")

I am wondering if we need to force some kind of USB reset to put the
USB controller into a known state independently of what the bootloader
has done previously.

Thanks
