Sorry for the delay response. On Tue, Nov 14, 2023, Pandey, Radhey Shyam wrote: > > -----Original Message----- > > From: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> > > Sent: Saturday, November 11, 2023 5:09 AM > > To: Mehta, Piyush <piyush.mehta@xxxxxxx> > > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>; > > Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx; > > linux-kernel@xxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx> > > Subject: Re: [PATCH] usb: dwc3: xilinx: improve error handling for PM APIs > > > > On Wed, Nov 08, 2023, Piyush Mehta wrote: > > > Improve error handling for PM APIs in the dwc3_xlnx_probe function by > > > introducing devm_pm_runtime_enable and error label. Removed > > > unnecessary API pm_runtime_disable call in dwc3_xlnx_remove. > > > > > > Signed-off-by: Piyush Mehta <piyush.mehta@xxxxxxx> > > > --- > > > drivers/usb/dwc3/dwc3-xilinx.c | 10 +++++++--- > > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/usb/dwc3/dwc3-xilinx.c > > > b/drivers/usb/dwc3/dwc3-xilinx.c index 5b7e92f476de..9cf26e9a1c3d > > > 100644 > > > --- a/drivers/usb/dwc3/dwc3-xilinx.c > > > +++ b/drivers/usb/dwc3/dwc3-xilinx.c > > > @@ -294,10 +294,15 @@ static int dwc3_xlnx_probe(struct > > > platform_device *pdev) > > > > > > pm_runtime_set_active(dev); > > > pm_runtime_enable(dev); > > > + ret = devm_pm_runtime_enable(dev); > > > > You just did pm_runtime_enable() right above, why > > devm_pm_runtime_enable() again? > > This will fix in next version. > > > > > + if (ret < 0) > > > + goto err_pm_set_suspended; > > > + > > > pm_suspend_ignore_children(dev, false); > > > - pm_runtime_get_sync(dev); > > > + return pm_runtime_resume_and_get(dev); > > > > > > - return 0; > > > +err_pm_set_suspended: > > > + pm_runtime_set_suspended(dev); > > > > This doesn't look right. why set status suspended here? > > Status is set to suspended in the exit path to undo the state > set by pm_runtime_set_active(). The initial runtime PM status of > all devices is 'suspended'. > > There is a mention of in Documentation/power/runtime_pm.rst > For this reason, once pm_runtime_set_active() has been called for the device, > pm_runtime_enable() should be called for it too as soon as reasonably possible > or its run-time PM status should be changed back to 'suspended' with the help of > pm_runtime_set_suspended(). > If your intention of treating the status of devm_pm_runtime_enable() as the status of pm_runtime_enable(), then that's not correct. BR, Thinh
