linux-kernel - Re: [PATCH] usb: chipidea: Fix ULPI on imx51

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 25 Jun 2018 14:18:13 +0200
From:   Sebastian Reichel <sebastian.reichel@...labora.co.uk>
To:     Fabio Estevam <festevam@...il.com>
Cc:     Andrey Smirnov <andrew.smirnov@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Nikita Yushchenko <nikita.yoush@...entembedded.com>,
        Peter Chen <peter.chen@....com>,
        USB list <linux-usb@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        Fabio Estevam <fabio.estevam@....com>,
        Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>
Subject: Re: [PATCH] usb: chipidea: Fix ULPI on imx51

Hi,

On Sun, Jun 24, 2018 at 08:51:20PM -0300, Fabio Estevam wrote:
> Hi Andrey,
> 
> On Sun, Jun 24, 2018 at 7:40 PM, Andrey Smirnov
> <andrew.smirnov@...il.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 still have access to the imx53 based PPD from General Electric.
But I don't understand what patch you want me to test. FWIW the
images I use(d) for testing already had CONFIG_USB_CHIPIDEA_ULPI
and CONFIG_USB_ULPI_BUS enabled.

> 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.

Makes sense to me.

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)