Hi Xavier,
Am Dienstag, 3. Januar 2023, 14:35:35 CET schrieb Xavier Roumegue (OSS):
> Hi Alexander,
>
> On 1/3/23 12:01, Alexander Stein wrote:
> > Hi,
> >
> > Am Dienstag, 3. Januar 2023, 11:55:34 CET schrieb Xavier Roumegue (OSS):
> >> From: Xavier Roumegue <xavier.roumegue@xxxxxxxxxxx>
> >>
> >> In case the IRQ allocation returns an error in dw100_probe(), the pm
> >> runtime is not disabled before to return.
> >>
> >> Add the missing unwind goto on the error handling path of the IRQ
> >> allocation request.
> >>
> >> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> >> Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> >> Signed-off-by: Xavier Roumegue <xavier.roumegue@xxxxxxxxxxx>
> >> ---
> >>
> >> drivers/media/platform/nxp/dw100/dw100.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/media/platform/nxp/dw100/dw100.c
> >> b/drivers/media/platform/nxp/dw100/dw100.c index
> >> f6d48c36f386..189d60cd5ed1
> >> 100644
> >> --- a/drivers/media/platform/nxp/dw100/dw100.c
> >> +++ b/drivers/media/platform/nxp/dw100/dw100.c
> >> @@ -1571,7 +1571,7 @@ static int dw100_probe(struct platform_device
> >> *pdev)
> >>
> >> dev_name(&pdev->dev), dw_dev);
> >>
> >> if (ret < 0) {
> >>
> >> dev_err(&pdev->dev, "Failed to request irq: %d\n", ret);
> >>
> >> - return ret;
> >> + goto err_pm;
> >>
> >> }
> >>
> >> ret = v4l2_device_register(&pdev->dev, &dw_dev->v4l2_dev);
> >
> > Doesn't it make more sense to request/allocate the IRQ (and other
> > resources) before enabling runtime PM?
>
> I would say this does as much sense as the other way around, as soon as
> something wrong happens, you have to restore things as it was prior to enter
> your routine. The most optimal function call ordering should depend on the
> failing occurrence likelihood of each individual function.
> On the probe path, I assume none of the functions are expected to fail.
> But I understand one could argue differently.
-EPROBE_DEFER teached me otherwise ;-) What I actually wanted to highlight is
that calling the devm_* functions first, reduces the cleanup path for the
following setup calls.
> So for the time being, this oneliner patch addresses the issue reported by
> the robot.
Sure, on the other hand it's less complex if you can just return in an error
path.
Best regards,
Alexander
