Hi Philipp,

On Tue, Jul 27, 2021 at 9:13 AM Philipp Zabel [off-list ref] wrote:

On Tue, 2021-07-27 at 09:02 +0100, Lad, Prabhakar wrote:
[...]

quoted

quoted

quoted

+     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