On Sat, Jul 22, 2023 at 09:24:19PM +0800, Yuanjun Gong wrote: > in qcom_swrm_probe() and swrm_runtime_resume(), the return value > of function clk_prepare_enable() should be checked, since it may > fail. > > Signed-off-by: Yuanjun Gong <ruc_gongyuanjun@xxxxxxx> > --- > drivers/soundwire/qcom.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c > index 7970fdb27ba0..cf1e13a35023 100644 > --- a/drivers/soundwire/qcom.c > +++ b/drivers/soundwire/qcom.c > @@ -1555,7 +1555,9 @@ static int qcom_swrm_probe(struct platform_device *pdev) > goto err_init; > } > > - clk_prepare_enable(ctrl->hclk); > + ret = clk_prepare_enable(ctrl->hclk); > + if (ret) > + goto err_init; FWIW, this is correct. > > ctrl->dev = dev; > dev_set_drvdata(&pdev->dev, ctrl); > @@ -1673,7 +1675,9 @@ static int __maybe_unused swrm_runtime_resume(struct device *dev) > disable_irq_nosync(ctrl->wake_irq); > } > > - clk_prepare_enable(ctrl->hclk); > + ret = clk_prepare_enable(ctrl->hclk); > + if (ret) > + return ret; On the lines above this the wakeup IRQ is disabled, if you return here without re-enabling that, this device will no longer be able to wake the system up. The significance of the ordering here (and the irq handler also disabling of the irq) is a little bit unclear to me, so I'd like to hear what Vinod and Srinivas has to say, before you send another version. Regards, Bjorn
