There are two problems with the second parameter of rockchip_i2s_tdm_set_sysclk(): 1. The second argument to a .set_sysclk() op is a clk_id, not a stream index, so it is incorrect to compare it with SNDRV_PCM_STREAM_PLAYBACK. Technically this code works correctly anyway because SNDRV_PCM_STREAM_PLAYBACK is defined as 0, which is also the clk_id for the mclk_tx as enforced by the device tree bindings. So this is a formal error, not triggering incorrect behaviour. 2. The else branch will consider any nonzero value as "rx", while only value 1 should be allowed for the mclk_rx clock. This does trigger incorrect behaviour if passing clk_id not equal to 0 or 1. Fix problem 1 by adding a new enum for the clock indexes as enforced in device tree and replace accordingly: * stream -> clk_id * SNDRV_PCM_STREAM_PLAYBACK -> CLK_MCLK_TX (value 0) Fix problem 2 by returning error if clk_id is not 0 or 1. Also simplify and improve dev_dbg() messages. Fixes: 081068fd6414 ("ASoC: rockchip: add support for i2s-tdm controller") Signed-off-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx> --- Changed in v2: - use switch statement instead of if/then/else --- sound/soc/rockchip/rockchip_i2s_tdm.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c index 860e66ec85e8..35b36aa3b970 100644 --- a/sound/soc/rockchip/rockchip_i2s_tdm.c +++ b/sound/soc/rockchip/rockchip_i2s_tdm.c @@ -32,6 +32,9 @@ #define TRCM_TX 1 #define TRCM_RX 2 +/* Clock indexes as enforced by the DT bindings */ +enum { CLK_IDX_MCLK_TX, CLK_IDX_MCLK_RX }; + struct txrx_config { u32 addr; u32 reg; @@ -973,7 +976,7 @@ static int rockchip_i2s_tdm_trigger(struct snd_pcm_substream *substream, return 0; } -static int rockchip_i2s_tdm_set_sysclk(struct snd_soc_dai *cpu_dai, int stream, +static int rockchip_i2s_tdm_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id, unsigned int freq, int dir) { struct rk_i2s_tdm_dev *i2s_tdm = to_info(cpu_dai); @@ -982,15 +985,22 @@ static int rockchip_i2s_tdm_set_sysclk(struct snd_soc_dai *cpu_dai, int stream, if (i2s_tdm->clk_trcm) { i2s_tdm->mclk_tx_freq = freq; i2s_tdm->mclk_rx_freq = freq; + + dev_dbg(i2s_tdm->dev, "mclk freq: %u", freq); } else { - if (stream == SNDRV_PCM_STREAM_PLAYBACK) + switch (clk_id) { + case CLK_IDX_MCLK_TX: i2s_tdm->mclk_tx_freq = freq; - else + break; + case CLK_IDX_MCLK_RX: i2s_tdm->mclk_rx_freq = freq; - } + break; + default: + return -ENOTSUPP; + } - dev_dbg(i2s_tdm->dev, "The target mclk_%s freq is: %d\n", - stream ? "rx" : "tx", freq); + dev_dbg(i2s_tdm->dev, "mclk[%d] freq: %u", clk_id, freq); + } return 0; } -- 2.34.1