Re: [PATCH v2 1/2] ASoC: cs43130: Drop only the clock reference hw_params() took

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Fri, Oct 23, 2026 at 10:29:40PM +0200, Olivier Louvignes wrote:
> cs43130_hw_params() and cs43130_dsd_hw_params() take a reference on the
> internal clock (clk_req) and cs43130_hw_free() drops one, but hw_free()
> can run twice for one hw_params(). When a stream's hw_params() fails,
> soc_pcm_hw_params() rolls back the DAIs it had set up, the codec DAIs
> included since they come before the CPU DAIs, and snd_pcm_hw_params()
> then calls hw_free() once more, which reaches every DAI of the link
> whether its hw_params() ran or not. Each failed hw_params() so left
> clk_req one lower than it should be. An application that tries a format
> the CPU DAI refuses before one it accepts then gets clk_req back to 0
> after its successful hw_params(), so the next stream skips the switch
> from the RC oscillator to the external MCLK.
> 
> Seen on a HiBy R1 (CS43131 on an Ingenic X1600 I2S controller): after
> an S32_LE attempt the I2S controller refuses, then S24_LE, SYS_CLK_CTL_1
> reads 0x06 instead of 0x04 while playing and every signal plays with
> loud white noise mixed in.
> 
> Keep the DAIs holding a reference in a bitmap: a DAI takes one at most
> once in hw_params() and hw_free() drops it only if held. A second
> hw_params() without hw_free() no longer takes a second reference either.
> 
> Fixes: 8f1e5bf9b440 ("ASoC: cs43130: Add support for CS43130 codec")
> Signed-off-by: Olivier Louvignes <olivier@mgcrea.io>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles