linux-kernel - Re: [PATCH] ASoC: rockchip: i2s-tdm: Fix trcm mode by setting clock on right mclk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 5 Jun 2024 10:13:23 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Alibek Omarov <a1ba.omarov@...il.com>
Cc: Nicolas Frattaroli <frattaroli.nicolas@...il.com>, Liam Girdwood
 <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Jaroslav Kysela
 <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, Heiko Stuebner
 <heiko@...ech.de>, linux-rockchip@...ts.infradead.org,
 linux-sound@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ASoC: rockchip: i2s-tdm: Fix trcm mode by setting clock
 on right mclk

Hello Alibek,

On Tue,  4 Jun 2024 21:47:52 +0300
Alibek Omarov <a1ba.omarov@...il.com> wrote:

> When TRCM mode is enabled, I2S RX and TX clocks are synchronized through
> selected clock source. Without this fix BCLK and LRCK might get parented
> to an uninitialized MCLK and the DAI will receive data at wrong pace.
> 
> However, unlike in original i2s-tdm driver, there is no need to manually
> synchronize mclk_rx and mclk_tx, as only one gets used anyway.
> 
> Tested on a board with RK3568 SoC and Silergy SY24145S codec with enabled and
> disabled TRCM mode.
> 
> Fixes: 9e2ab4b18ebd ("ASoC: rockchip: i2s-tdm: Fix inaccurate sampling rates")
> Signed-off-by: Alibek Omarov <a1ba.omarov@...il.com>
> ---
>  sound/soc/rockchip/rockchip_i2s_tdm.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
> index 9fa020ef7eab..ee517d7b5b7b 100644
> --- a/sound/soc/rockchip/rockchip_i2s_tdm.c
> +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
> @@ -655,8 +655,17 @@ static int rockchip_i2s_tdm_hw_params(struct snd_pcm_substream *substream,
>  	int err;
>  
>  	if (i2s_tdm->is_master_mode) {
> -		struct clk *mclk = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
> -			i2s_tdm->mclk_tx : i2s_tdm->mclk_rx;
> +		struct clk *mclk;
> +
> +		if (i2s_tdm->clk_trcm == TRCM_TX) {
> +			mclk = i2s_tdm->mclk_tx;
> +		} else if (i2s_tdm->clk_trcm == TRCM_RX) {
> +			mclk = i2s_tdm->mclk_rx;
> +		} else if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +			mclk = i2s_tdm->mclk_tx;
> +		} else {
> +			mclk = i2s_tdm->mclk_rx;
> +		}

I cannot test right now, but it definitely looks correct.

Reviewed-by: Luca Ceresoli <luca.ceresoli@...tlin.com>

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com