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
