Messages in this thread | | | From | "Lad, Prabhakar" <> | Date | Tue, 27 Jul 2021 09:54:20 +0100 | Subject | Re: [PATCH v3 2/3] iio: adc: Add driver for Renesas RZ/G2L A/D converter |
| |
Hi Philipp,
On Tue, Jul 27, 2021 at 9:13 AM Philipp Zabel <p.zabel@pengutronix.de> wrote: > > 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. > Agreed, will do it the right way!
Cheers, Prabhakar
| |