Re: [PATCH v3 2/3] iio: adc: Add driver for Renesas RZ/G2L A/D converter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2021-07-27 at 09:02 +0100, Lad, Prabhakar wrote:
[...]
> > > +     ret = devm_add_action_or_reset(&pdev->dev,
> > > +                                    rzg2l_adc_reset_assert, adc->adrstn);
> > > +     if (ret) {
> > > +             dev_err(&pdev->dev, "failed to register adrstn assert devm action, %d\n",
> > > +                     ret);
> > > +             return ret;
> > > +     }
> > 
> > This is the wrong way around. Installing devres actions should be done
> > after the thing they are supposed to revert in case of error. You should
> > move this down below the reset_control_deassert(adc->adrstn).
> > 
> Ouch my understanding was, there won't be any harm in asserting the
> reset line. Agree with will move this below
> reset_control_deassert(adc->adrstn).

You are probably right, but it's still better do it correctly. Just
imagine one of the reset lines turns out to be shared later, or somebody
else will look at this driver for inspiration.

regards
Philipp



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux