All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ettore Chimenti" <ettore.chimenti@linaro.org>
Cc: linux-usb@vger.kernel.org
Subject: Re: [PATCH] usb: musb: musb_dsps: Wait for controller reset
Date: Wed, 14 Oct 2026 17:59:06 +0000	[thread overview]
Message-ID: <sashiko-outbox-171639@kernel.org> (raw)
In-Reply-To: <20261014-patch-ektor5-dm8168-usb-v1-1-9670414271d6@linaro.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] usb: musb: dsps: PHY resource leak on reset timeout error path
--

commit dd6538e5ce8a20102c268b6b495d88356ce426bd
Author: Andrew Goodbody <andrew.goodbody@linaro.org>

usb: musb: musb_dsps: Wait for controller reset

This commit introduces a polling delay after soft resetting the USB controller
to ensure it completes before setting the controller mode. It uses a timeout of
5000us with read_poll_timeout to verify the controller reaches a sane state.

> diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> index e3935f18dd56c..d909c45c253d4 100644
> --- a/drivers/usb/musb/musb_dsps.c
> +++ b/drivers/usb/musb/musb_dsps.c

[ ... ]

> @@ -466,7 +468,13 @@ static int dsps_musb_init(struct musb *musb)
>  	timer_setup(&musb->dev_timer, otg_timer, 0);
>  
>  	/* Reset the musb */
> -	musb_writel(reg_base, wrp->control, (1 << wrp->reset));
> +	musb_writel(reg_base, wrp->control, BIT(wrp->reset));
> +	read_poll_timeout(musb_readl, val, !(val & BIT(wrp->reset)), 2, 5000,
> +			  false, reg_base, wrp->control);
> +	if (val & BIT(wrp->reset))
> +		return -ENODEV;

[Severity: Medium]
If the controller reset times out here, does this leak the previously
initialized PHY?

Looking earlier in dsps_musb_init(), the PHY is initialized and powered on:

        ret = phy_init(musb->phy);
        if (ret < 0)
                return ret;
        ret = phy_power_on(musb->phy);
        if (ret) {
                phy_exit(musb->phy);
                return ret;
        }

If the hardware reset does not complete within 5000us, this new polling loop
times out and the function returns -ENODEV directly. This skips calling
phy_power_off() and phy_exit() to tear the PHY down.

Should we be adding cleanup for musb->phy on this error path?

> +
> +	dsps_musb_set_mode(musb, musb->port_mode);
>  
>  	musb->isr = dsps_interrupt;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014-patch-ektor5-dm8168-usb-v1-1-9670414271d6@linaro.org?part=1

      reply	other threads:[~2026-10-14 17:59 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 17:47 [PATCH] usb: musb: musb_dsps: Wait for controller reset Ettore Chimenti
2026-10-14 17:59 ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-171639@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ettore.chimenti@linaro.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.