[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7kpf7gnftumdwdowutryjz73nkap4vp3cfisrigt6def4f36vg@ijlj2eqwzsip>
Date: Mon, 17 Nov 2025 14:27:03 +0800
From: Xu Yang <xu.yang_2@....com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"shawnguo@...nel.org" <shawnguo@...nel.org>, "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kernel@...gutronix.de" <kernel@...gutronix.de>, "festevam@...il.com" <festevam@...il.com>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"imx@...ts.linux.dev" <imx@...ts.linux.dev>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, "jun.li@....com" <jun.li@....com>
Subject: Re: [PATCH 2/2] usb: dwc3: imx8mp: disable auto suspend for host role
On Thu, Nov 13, 2025 at 11:05:43PM +0000, Thinh Nguyen wrote:
> On Wed, Nov 05, 2025, Xu Yang wrote:
> > Do dwc3 core auto suspend enable for device and disable for host
> > , this can make sure dwc3 core device auto suspend setting is
> > correct all the time, the background of disable dwc3 core device
> > auto suspend is to make its parent device suspend immediately
> > (so wakeup enable can be enabled) after xhci-plat device suspended,
> > for device mode, we keep the dwc3 core device auto suspend is to
> > give some wait for gadget to be enumerated.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@....com>
> > ---
> > drivers/usb/dwc3/dwc3-imx8mp.c | 31 +++++++++++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/dwc3-imx8mp.c b/drivers/usb/dwc3/dwc3-imx8mp.c
> > index bce6af82f54c..80a431cb6fed 100644
> > --- a/drivers/usb/dwc3/dwc3-imx8mp.c
> > +++ b/drivers/usb/dwc3/dwc3-imx8mp.c
> > @@ -158,11 +158,31 @@ static irqreturn_t dwc3_imx8mp_interrupt(int irq, void *_dwc3_imx)
> > return IRQ_HANDLED;
> > }
> >
> > +static void dwc3_imx_pre_set_role(struct dwc3 *dwc, enum usb_role role)
> > +{
> > + if (role == USB_ROLE_HOST)
> > + /*
> > + * For xhci host, we need disable dwc core auto
> > + * suspend, because during this auto suspend delay(5s),
> > + * xhci host RUN_STOP is cleared and wakeup is not
> > + * enabled, if device is inserted, xhci host can't
> > + * response the connection.
> > + */
> > + pm_runtime_dont_use_autosuspend(dwc->dev);
> > + else
> > + pm_runtime_use_autosuspend(dwc->dev);
>
> Would this override the user setting everytime there's a role switch?
>From what I know, the user can't control whether to enable or disable
autosuspend feature. So there should be no overriding problem. When
user change autosuspend_delay_ms, the delay setting will be kept
everytime there's a role switch.
>
> > +}
> > +
> > +struct dwc3_glue_ops dwc3_imx_glue_ops = {
> > + .pre_set_role = dwc3_imx_pre_set_role,
> > +};
> > +
> > static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > {
> > struct device *dev = &pdev->dev;
> > struct device_node *node = dev->of_node;
> > struct dwc3_imx8mp *dwc3_imx;
> > + struct dwc3 *dwc3;
> > struct resource *res;
> > int err, irq;
> >
> > @@ -240,6 +260,17 @@ static int dwc3_imx8mp_probe(struct platform_device *pdev)
> > goto depopulate;
> > }
> >
> > + dwc3 = platform_get_drvdata(dwc3_imx->dwc3);
>
> It's confusing how dwc3_imx->dwc3 is also named dwc3...
I will rename it later.
>
> > + if (!dwc3) {
> > + err = dev_err_probe(dev, -EPROBE_DEFER, "failed to get dwc3 platform data\n");
> > + goto depopulate;
> > + }
> > +
> > + dwc3->glue_ops = &dwc3_imx_glue_ops;
>
> If you want to use glue_ops, please use the new flatten model. I
> don't want dwc3 to be initialized again after of_platform_populate().
I understand the new flatten model is a more suitable way. Considering that
many i.MX devices are using this legacy unflatten model, do you mind allow
this change firstly and we will switch to the new flatten model in the future?
Thanks,
Xu Yang
>
> BR,
> Thinh
>
> > +
> > + if (dwc3->dr_mode == USB_DR_MODE_HOST)
> > + pm_runtime_dont_use_autosuspend(dwc3->dev);
> > +
> > err = devm_request_threaded_irq(dev, irq, NULL, dwc3_imx8mp_interrupt,
> > IRQF_ONESHOT, dev_name(dev), dwc3_imx);
> > if (err) {
> > --
> > 2.34.1
> >
Powered by blists - more mailing lists