RE: [PATCH] ASoC: rsnd: adg: make rsnd_adg_clk_control() idempotent

[John Madieu]

Hi Morimoto-san,

Thank you for the Ack!

> -----Original Message-----
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> Sent: mardi 16 juin 2026 04:43
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH] ASoC: rsnd: adg: make rsnd_adg_clk_control()
> idempotent
> 
[...]

> 
> OK
> So, I have no objection about the patch.
> 
> Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> 
> But, now rsnd_adg_clk_control() is very complicated enough, and you will
> add new flag into it. Will be more complicated.
> 
> Maybe it is time to separate it into enable/disable() ?
> 
> 	- #define rsnd_adg_clk_enable(priv)	...
> 	- #define rsnd_adg_clk_disable(priv)	...
> 	- int rsnd_adg_clk_control(...);
> 	+ #define rsnd_adg_clk_enable(priv)
> 	+ #define rsnd_adg_clk_disable(priv)

Agreed, that reads much better. Splitting the two directions also lets
the new flag handling fall out naturally: the enable side sets the flag
on success and the disable side just early-returns when it is already
disabled, so the "set clk_enabled = true before the rollback" trick from
this patch goes away. The shared teardown moves into a small
__rsnd_adg_clk_disable() helper used both by the public disable and as
the enable() error rollback.
 
I will resend as a v2 series:
 
  1/2 the idempotency fix, unchanged and carrying your Ack, kept first
      and minimal so it stays easy to backport through the Fixes: tag
  2/2 the split into rsnd_adg_clk_enable() / rsnd_adg_clk_disable() on
      top, as a pure cleanup with no functional change
 
Regards,
John