> Subject: Re: [PATCH v3 2/4] usb: uhci: Add reset control support
>
> On Fri, Sep 19, 2025 at 10:57:10AM +0800, Ryan Chen wrote:
> > Some SoCs, such as the Aspeed AST2700, require the UHCI controller to
> > be taken out of reset before it can operate. Add optional reset
> > control support to the UHCI platform driver.
> >
> > The driver now acquires an optional reset line from device tree,
> > deasserts it during probe, and asserts it again in the error path and
> > shutdown.
> >
> > Signed-off-by: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>
> > ---
> > drivers/usb/host/uhci-hcd.h | 1 +
> > drivers/usb/host/uhci-platform.c | 18 ++++++++++++++++--
> > 2 files changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
> > index 13ee2a6144b2..4326d1f3ca76 100644
> > --- a/drivers/usb/host/uhci-hcd.h
> > +++ b/drivers/usb/host/uhci-hcd.h
> > @@ -445,6 +445,7 @@ struct uhci_hcd {
> > short load[MAX_PHASE]; /* Periodic allocations */
> >
> > struct clk *clk; /* (optional) clock source */
> > + struct reset_control *rsts; /* (optional) clock reset */
> >
> > /* Reset host controller */
> > void (*reset_hc) (struct uhci_hcd *uhci);
> > diff --git a/drivers/usb/host/uhci-platform.c
> > b/drivers/usb/host/uhci-platform.c
> > index 62318291f566..f255358d6242 100644
> > --- a/drivers/usb/host/uhci-platform.c
> > +++ b/drivers/usb/host/uhci-platform.c
> > @@ -11,6 +11,7 @@
> > #include <linux/of.h>
> > #include <linux/device.h>
> > #include <linux/platform_device.h>
> > +#include <linux/reset.h>
> >
> > static int uhci_platform_init(struct usb_hcd *hcd) { @@ -132,17
> > +133,29 @@ static int uhci_hcd_platform_probe(struct platform_device
> *pdev)
> > goto err_rmr;
> > }
> >
> > + uhci->rsts = devm_reset_control_array_get_optional_shared(&pdev->dev);
> > + if (IS_ERR(uhci->rsts)) {
> > + ret = PTR_ERR(uhci->rsts);
> > + goto err_clk;
> > + }
> > + ret = reset_control_deassert(uhci->rsts);
>
> Does this work right if uhci->rsts is NULL?
>
> > + if (ret)
> > + goto err_clk;
> > +
> > ret = platform_get_irq(pdev, 0);
> > if (ret < 0)
> > - goto err_clk;
> > + goto err_reset;
> >
> > ret = usb_add_hcd(hcd, ret, IRQF_SHARED);
> > if (ret)
> > - goto err_clk;
> > + goto err_reset;
> >
> > device_wakeup_enable(hcd->self.controller);
> > return 0;
> >
> > +err_reset:
> > + if (!IS_ERR_OR_NULL(uhci->rsts))
> > + reset_control_assert(uhci->rsts);
>
> How could this code ever execute if uhci->rsts is an ERR_PTR?
>
> Also, why does this code test for NULL...
Will add with following.
if (uhci->rsts) {
ret = reset_control_deassert(uhci->rsts);
if (ret)
goto err_clk;
}
I will test it with non-resets platform.
>
> > err_clk:
> > clk_disable_unprepare(uhci->clk);
> > err_rmr:
> > @@ -156,6 +169,7 @@ static void uhci_hcd_platform_remove(struct
> platform_device *pdev)
> > struct usb_hcd *hcd = platform_get_drvdata(pdev);
> > struct uhci_hcd *uhci = hcd_to_uhci(hcd);
> >
> > + reset_control_assert(uhci->rsts);
>
> when this code doesn't?
Both reset_control_assert() and reset_control_deassert() already handle
NULL and ERR_PTR safely in the reset framework.
https://github.com/torvalds/linux/blob/master/drivers/reset/core.c#L468-L472
>
> Alan Stern
>
> > clk_disable_unprepare(uhci->clk);
> > usb_remove_hcd(hcd);
> > usb_put_hcd(hcd);
> > --
> > 2.34.1
> >
