Re: [PATCH v2] usb: dwc3: core: Fix timeout check

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Varadarajan,

Thank you for the patch.

On mer., janv. 15, 2025 at 11:50, Varadarajan Narayanan <quic_varada@quicinc.com> wrote:

> dwc3_core_init loops 'timeout' times to check if the IP block is out
> of reset using 'while (timeout--)'. If there is some issue and
> the block doesn't come out of reset, the loop will run till
> 'timeout' becomes zero and the post decrement operator would set
> timeout to 0xffffffff. Though the IP block is not out reset, the
> subsequent if check 'if !timeout' would fail as timeout is not
> equal to zero and the function proceeds with the initialization.
>
> Use poll API instead to resolve this.
>
> Tested-By: Varadarajan Narayanan <quic_varada@quicinc.com>

s/Tested-By/Tested-by

See ./scripts/checkpatch.pl:

λ ~/work/upstream/src/u-boot/ varadarajan/dwc3-timeout ./scripts/checkpatch.pl --u-boot --git HEAD^..HEAD
WARNING: 'Tested-by:' is the preferred signature form
#16:
Tested-By: Varadarajan Narayanan <quic_varada@quicinc.com>

total: 0 errors, 1 warnings, 0 checks, 33 lines checked

> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>

I think Marek can fix-up that when he applies, but if you send a v3 to
correct this, feel free to add:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Thanks
Mattijs

> ---
> v2: * Use poll API and remove the timeout variable based loop as
>       suggested by 'Marek Vasut'
>     * Drop 'Reviewed-by: Neil Armstrong' as the patch has taken
>       a very different shape
> ---
>  drivers/usb/dwc3/core.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index a35b8c2f64..847fa1f82c 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -23,6 +23,7 @@
>  #include <linux/delay.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/err.h>
> +#include <linux/iopoll.h>
>  #include <linux/ioport.h>
>  #include <dm.h>
>  #include <generic-phy.h>
> @@ -587,7 +588,6 @@ static void dwc3_set_incr_burst_type(struct dwc3 *dwc)
>   */
>  static int dwc3_core_init(struct dwc3 *dwc)
>  {
> -	unsigned long		timeout;
>  	u32			hwparams4 = dwc->hwparams.hwparams4;
>  	u32			reg;
>  	int			ret;
> @@ -610,15 +610,11 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  	}
>  
>  	/* issue device SoftReset too */
> -	timeout = 5000;
>  	dwc3_writel(dwc->regs, DWC3_DCTL, DWC3_DCTL_CSFTRST);
> -	while (timeout--) {
> -		reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> -		if (!(reg & DWC3_DCTL_CSFTRST))
> -			break;
> -	};
> -
> -	if (!timeout) {
> +	ret = read_poll_timeout(dwc3_readl, reg,
> +				!(reg & DWC3_DCTL_CSFTRST),
> +				1, 5000, dwc->regs, DWC3_DCTL);
> +	if (ret) {
>  		dev_err(dwc->dev, "Reset Timed Out\n");
>  		ret = -ETIMEDOUT;
>  		goto err0;
> -- 
> 2.34.1