Re: [PATCH v4 1/2] usb: ulpi: defer ulpi_register on ulpi_read_id timeout

[Thinh Nguyen]

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@xxxxxxxxxxxxxxx
> 
> Signed-off-by: Ferry Toth <ftoth@xxxxxxxxxxxxxx>
> ---
>  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
>