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? > + 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? BR, Thinh > > err_clk_put: > clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks); > @@ -315,7 +320,6 @@ static void dwc3_xlnx_remove(struct platform_device *pdev) > clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks); > priv_data->num_clocks = 0; > > - pm_runtime_disable(dev); > pm_runtime_put_noidle(dev); > pm_runtime_set_suspended(dev); > } > -- > 2.25.1 >