On Montag, 23. August 2021 12:54:35 CEST Sugar Zhang wrote:
From: Xing Zheng zhengxing@rock-chips.com
If there is only one lrck (tx or rx) by hardware, we need to use 'rockchip,clk-trcm' to specify which lrck can be used.
Change-Id: I3bf8d87a6bc8c45e183040012d87d8be21a4c133 Signed-off-by: Xing Zheng zhengxing@rock-chips.com Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com
sound/soc/rockchip/rockchip_i2s.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 6ccb62e..b9d9c88 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -54,6 +54,7 @@ struct rk_i2s_dev { bool is_master_mode; const struct rk_i2s_pins *pins; unsigned int bclk_ratio;
- unsigned int clk_trcm;
};
/* tx/rx ctrl lock */ @@ -321,7 +322,6 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct rk_i2s_dev *i2s = to_info(dai);
- struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); unsigned int val = 0; unsigned int mclk_rate, bclk_rate, div_bclk, div_lrck;
@@ -421,13 +421,6 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream, regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDL_MASK, I2S_DMACR_RDL(16));
- val = I2S_CKR_TRCM_TXRX;
- if (dai->driver->symmetric_rate && rtd->dai_link->symmetric_rate)
val = I2S_CKR_TRCM_TXONLY;- regmap_update_bits(i2s->regmap, I2S_CKR,
I2S_CKR_TRCM_MASK,val);}
@@ -531,7 +524,6 @@ static struct snd_soc_dai_driver rockchip_i2s_dai = { SNDRV_PCM_FMTBIT_S32_LE), }, .ops = &rockchip_i2s_dai_ops,
- .symmetric_rate = 1,
};
static const struct snd_soc_component_driver rockchip_i2s_component = { @@ -750,6 +742,18 @@ static int rockchip_i2s_probe(struct platform_device *pdev) else if (of_property_read_bool(node, "rockchip,capture-only")) soc_dai->playback.channels_min = 0;
- i2s->clk_trcm = I2S_CKR_TRCM_TXRX;
- if (!of_property_read_u32(node, "rockchip,clk-trcm", &val)) {
if (val >= 0 && val <= 2) {i2s->clk_trcm = val << I2S_CKR_TRCM_SHIFT;if (i2s->clk_trcm)soc_dai->symmetric_rate = 1;}- }
- regmap_update_bits(i2s->regmap, I2S_CKR,
I2S_CKR_TRCM_MASK, i2s->clk_trcm);- ret = devm_snd_soc_register_component(&pdev->dev, &rockchip_i2s_component, soc_dai, 1);
Hello,
I recommend doing the same thing with clk-trcm that I'm going to do in v3 of my i2s-tdm driver, as per Robin Murphy's suggestion:
Have tx-only and rx-only be two boolean properties. I named them rockchip,trcm-sync-tx-only and rockchip,trcm-sync-rx-only.
I also recommend only shifting the value when writing it to registers, and storing it in its unshifted state for debug reasons.
My probe function looks like this:
i2s_tdm->clk_trcm = TRCM_TXRX; if (of_property_read_bool(node, "rockchip,trcm-sync-tx-only")) i2s_tdm->clk_trcm = TRCM_TX; if (of_property_read_bool(node, "rockchip,trcm-sync-rx-only")) { if (i2s_tdm->clk_trcm) { dev_err(i2s_tdm->dev, "invalid trcm-sync configuration\n"); return -EINVAL; } i2s_tdm->clk_trcm = TRCM_RX; } if (i2s_tdm->clk_trcm != TRCM_TXRX) i2s_tdm_dai.symmetric_rate = 1;
When writing clk_trcm to a register, I then just do:
regmap_update_bits(i2s_tdm->regmap, I2S_CKR, I2S_CKR_TRCM_MASK, i2s_tdm->clk_trcm << I2S_CKR_TRCM_SHIFT);
This way if I need to add an error message or debug print somewhere, then clk_trcm is still either 0, 1 or 2.
In general, we should look into supporting both i2s and i2s-tdm controllers in the same driver if possible. This way we don't need to duplicate work like this. Do you think this is feasible to do? When I looked at the register maps I saw that the bits I2S/TDM uses were reserved in the I2S version of the controller, so I think it should work.
Regards, Nicolas Frattaroli