linux-kernel - Re: [PATCH v4 1/2] usb: ulpi: defer ulpi_register on ulpi_read

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Tue, 22 Nov 2022 01:47:51 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Ferry Toth <ftoth@...londelft.nl>
CC:     "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Sean Anderson <sean.anderson@...o.com>,
        Liu Shixin <liushixin2@...wei.com>,
        Ferry Toth <fntoth@...il.com>,
        Andrey Smirnov <andrew.smirnov@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v4 1/2] usb: ulpi: defer ulpi_register on ulpi_read_id
 timeout

On Sun, Nov 20, 2022, Ferry Toth wrote:
> Since commit 0f0101719138 ("usb: dwc3: Don't switch OTG -> peripheral if extcon is present")
> Dual Role support on Intel Merrifield platform broke due to rearranging
> the call to dwc3_get_extcon().
> 
> It appears to be caused by ulpi_read_id() on the first test write failing
> with -ETIMEDOUT. Currently ulpi_read_id() expects to discover the phy via
> DT when the test write fails and returns 0 in that case, even if DT does not
> provide the phy. As a result usb probe completes without phy.
> 
> On Intel Merrifield the issue is reproducible but difficult to find the
> root cause. Using an ordinary kernel and rootfs with buildroot ulpi_read_id()

ordinary = mainline?

> succeeds. As soon as adding ftrace / bootconfig to find out why,
> ulpi_read_id() fails and we can't analyze the flow. Using another rootfs
> ulpi_read_id() fails even without adding ftrace. Appearantly the issue is

This info fits more for the dwc3 patch, and can we use another word for
"apparently" when we still don't know the real cause? Maybe "we
suspect?"

Thanks,
Thinh

> some kind of timing / race, but merely retrying ulpi_read_id() does not
> resolve the issue.
> 
> This patch makes ulpi_read_id() return -ETIMEDOUT to its user if the first
> test write fails. The user should then handle it appropriately. A follow up
> patch will make dwc3_core_init() set -EPROBE_DEFER in this case and bail out.
> 
> Fixes: ef6a7bcfb01c ("usb: ulpi: Support device discovery via DT")
> Cc: stable@...r.kernel.org
> 
> Signed-off-by: Ferry Toth <ftoth@...londelft.nl>
> ---
>  drivers/usb/common/ulpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index d7c8461976ce..60e8174686a1 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -207,7 +207,7 @@ static int ulpi_read_id(struct ulpi *ulpi)
>  	/* Test the interface */
>  	ret = ulpi_write(ulpi, ULPI_SCRATCH, 0xaa);
>  	if (ret < 0)
> -		goto err;
> +		return ret;
>  
>  	ret = ulpi_read(ulpi, ULPI_SCRATCH);
>  	if (ret < 0)
> -- 
> 2.37.2
>