Re: [PATCH AUTOSEL 5.15 10/19] usb: cdns3: Put the cdns set active part outside the spin lock

[wangxiaolei]

On 9/11/23 6:00 PM, Pavel Machek wrote:
> Hi!
>
> > From: Xiaolei Wang <xiaolei.wang@xxxxxxxxxxxxx>
> >
> > [ Upstream commit 2319b9c87fe243327285f2fefd7374ffd75a65fc ]
> >
> > The device may be scheduled during the resume process,
> > so this cannot appear in atomic operations. Since
> > pm_runtime_set_active will resume suppliers, put set
> > active outside the spin lock, which is only used to
> > protect the struct cdns data structure, otherwise the
> > kernel will report the following warning:
> There's something wrong with this patch: cdns_set_active returns
> either void or int depending on config. That can't be intentional.

Thanks for the reminder, I will send a new patch to fix this problem

thanks

xiaolei

> Best regards,
> 								Pavel
>
> > +++ b/drivers/usb/cdns3/core.c
> > @@ -556,15 +555,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
> ...
> > +
> > +void cdns_set_active(struct cdns *cdns, u8 set_active)
> > +{
> > +	struct device *dev = cdns->dev;
> > +
> >   	if (set_active) {
> >   		pm_runtime_disable(dev);
> >   		pm_runtime_set_active(dev);
> >   		pm_runtime_enable(dev);
> >   	}
> >   
> > -	return 0;
> > +	return;
> >   }
> > +++ b/drivers/usb/cdns3/core.h
> > @@ -125,10 +125,13 @@ int cdns_init(struct cdns *cdns);
> >   int cdns_remove(struct cdns *cdns);
> >   
> >   #ifdef CONFIG_PM_SLEEP
> ...
> >   int cdns_suspend(struct cdns *cdns);
> > +void cdns_set_active(struct cdns *cdns, u8 set_active);
> >   #else /* CONFIG_PM_SLEEP */
> ...
> > +static inline int cdns_set_active(struct cdns *cdns, u8 set_active)
> >   { return 0; }
> >   static inline int cdns_suspend(struct cdns *cdns)
> >   { return 0; }