On Mon, 18 Oct 2021 at 18:29, Andy Shevchenko [off-list ref] wrote:

On Mon, Oct 18, 2021 at 7:23 PM Andy Shevchenko
[off-list ref] wrote:

quoted

On Mon, Oct 18, 2021 at 6:56 PM Emil Renner Berthing [off-list ref] wrote:

quoted

On Mon, 18 Oct 2021 at 17:48, Andy Shevchenko [off-list ref] wrote:

quoted

On Mon, Oct 18, 2021 at 6:35 PM Emil Renner Berthing [off-list ref] wrote:

quoted

On Tue, 12 Oct 2021 at 19:03, Andy Shevchenko [off-list ref] wrote:

quoted

On Tue, Oct 12, 2021 at 4:43 PM Emil Renner Berthing [off-list ref] wrote:

...

quoted

quoted

quoted

quoted

quoted

quoted

+       ret = clk_prepare_enable(clk);
+       if (ret) {

quoted

+               reset_control_deassert(rst);

Use devm_add_action_or_reset().

I don't see how that is better.

Pity. The rule of thumb is to either try to use devm_*() everywhere in
the probe, or don't use it at all. Above is the more-or-less standard
pattern where devn_add_action_or_reset() is being used in the entire
kernel.

quoted

Then I'd first need to call that and
check for errors, but just on the line below enabling the clock the
reset line is deasserted anyway, so then the action isn't needed any
longer. So that 3 lines of code for devm_add_action_or_reset +
lingering unneeded action or code to remove it again vs. just the line
above.

Then don't use devm_*() at all. What's the point?

I'm confused. So you wan't an unneeded action to linger because the
probe function temporarily asserts reset for 3 lines of code?

I;m talking about clk_prepare_enable().

Having a second look I found even problematic error paths because of
mixing devm_*() with non-devm_*() calls, which only assures me that
your ->probe() error path is broken and should be revisited.

So do you want to expand on that now or should I send v2 first?