ASoC: rockchip_i2s_tdm calibration clocking problem

[Pavel Hofman]

Hi,

I am hitting a clock issue with rockchip_i2s_tdm.c + simple-audio-card 
(+ RK3308).

At boot the mclk clk_i2s0_8ch_tx is (somehow) initialized to some 
(unimportant?) value 50176000 Hz. Note that this frequency is not 
multiple of either 48kHz or 44.1kHz.

Method asoc_simple_parse_clk() reads this value and sets it to 
simple_dai->sysclk.

Subsequently at asoc_simple_dai_init this "random" initial value is 
stored to i2s_tdm->mclk_tx_freq:

    17.839330]  rockchip_i2s_tdm_set_sysclk+0x50/0xbc 
[snd_soc_rockchip_i2s_tdm]
[   17.839367]  snd_soc_dai_set_sysclk+0x38/0xb8 [snd_soc_core]
[   17.839596]  asoc_simple_init_dai+0x94/0xc0 [snd_soc_simple_card_utils]
[   17.839640]  asoc_simple_dai_init+0x130/0x230 [snd_soc_simple_card_utils]
[   17.839672]  snd_soc_link_init+0x28/0x90 [snd_soc_core]
[   17.839843]  snd_soc_bind_card+0x60c/0xbb4 [snd_soc_core]


When starting playback, called by rockchip_i2s_tdm_hw_params(), 
rockchip_i2s_tdm_calibrate_mclk() correctly switches parent of 
mclk_parent to correct root pll clock mclk_root0/1 for the given 
samplerate and correctly configures mclk_parent frequency.

https://github.com/torvalds/linux/blob/master/sound/soc/rockchip/rockchip_i2s_tdm.c#L862-L864

But right after that, the next line of rockchip_i2s_tdm_hw_params() 
calls rockchip_i2s_tdm_set_mclk()

https://github.com/torvalds/linux/blob/master/sound/soc/rockchip/rockchip_i2s_tdm.c#L866C9-L866C34

This method calls clk_set_rate(i2s_tdm->mclk_tx, i2s_tdm->mclk_tx_freq), 
which resets the clock and its parental chain to the original incorrect 
value stored in i2s_tdm->mclk_tx_freq from the dai initialization.

https://github.com/torvalds/linux/blob/master/sound/soc/rockchip/rockchip_i2s_tdm.c#L693

As a result, no matter what sample rate is being played, the i2s mclk 
clock always ends up configured incorrectly.


DTS I2S sets all clocks, therefore the clk calibration in 
rockchip_i2s_tdm.c should be (and is) used:

	i2s_8ch_0: i2s at ff300000 {
		compatible = "rockchip,rk3308-i2s-tdm";
		reg = <0x0 0xff300000 0x0 0x1000>;
		interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
		clocks = <&cru SCLK_I2S0_8CH_TX>, <&cru SCLK_I2S0_8CH_RX>, <&cru 
HCLK_I2S0_8CH>,
			 <&cru SCLK_I2S0_8CH_TX_SRC>,
			 <&cru SCLK_I2S0_8CH_RX_SRC>,
			 <&cru PLL_VPLL0>,
			 <&cru PLL_VPLL1>;
		clock-names = "mclk_tx", "mclk_rx", "hclk",
			      "mclk_tx_src", "mclk_rx_src",
			      "mclk_root0", "mclk_root1";
		.........


It seems to me that the calibration code should also rewrite the 
initially incorrect i2s_tdm->mclk_tx_freq and i2s_tdm->mclk_rx_freq with 
correct values corresponding to the momentary hw_params rate, or maybe 
rockchip_i2s_tdm_set_mclk() should not be called if 
rockchip_i2s_tdm_calibrate_mclk() is called a line above (i.e. putting 
the call to rockchip_i2s_tdm_set_mclk() into "else" branch).

Thank you very much for help.

With regards,

Pavel.