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

[Kuninori Morimoto]

Hi John

> rsnd_adg_clk_control() is asymmetric on the disable path: the clkin
> clocks are guarded by clkin_rate[], but the "adg" clock is disabled
> unconditionally. If an enable attempt fails (for example a clkin
> failing to turn on during resume), the error path correctly rolls
> everything back, but rsnd_resume() ignores the return value, so the
> following system suspend calls rsnd_adg_clk_disable() again and
> underflows the "adg" clock enable count:
> 
>   adg_0_clks1 already disabled
>   WARNING: drivers/clk/clk.c:1188 clk_core_disable+0xa4/0xac
>   Call trace:
>    clk_core_disable+0xa4/0xac (P)
>    clk_disable+0x30/0x4c
>    rsnd_adg_clk_control+0x9c/0x2cc
>    rsnd_suspend+0x20/0x74
>    device_suspend+0x140/0x3ec
>    dpm_suspend+0x168/0x270
> 
> Track the enable state explicitly and bail out of redundant
> enable/disable calls, mirroring what is already done for the per-SSI
> clock prepare state. A failed enable leaves the state as disabled, so
> the next suspend becomes a no-op and the next resume retries cleanly.
> 
> Fixes: 47899d53f86f ("ASoC: rsnd: adg: Add per-SSI ADG and SSIF supply clock management")
> Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> ---
>  sound/soc/renesas/rcar/adg.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/renesas/rcar/adg.c b/sound/soc/renesas/rcar/adg.c
> index 5479cefb6dbe..53efd1be5139 100644
> --- a/sound/soc/renesas/rcar/adg.c
> +++ b/sound/soc/renesas/rcar/adg.c
> @@ -45,6 +45,7 @@ struct rsnd_adg {
>  	struct rsnd_mod mod;
>  	int clkin_rate[CLKINMAX];
>  	bool ssi_clk_prepared;
> +	bool clk_enabled;

Can we use clk_is_enabled_when_prepared() instead ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto