[PATCH v1 0/15] Patches to update for rockchip i2s
These patches fixup or update for rockchip i2s.
Sugar Zhang (13): ASoC: rockchip: i2s: Add support for set bclk ratio ASoC: rockchip: i2s: Fixup clk div error ASoC: rockchip: i2s: Improve dma data transfer efficiency ASoC: rockchip: i2s: Fix regmap_ops hang ASoC: rockchip: i2s: Fix concurrency between tx/rx ASoC: rockchip: i2s: Reset the controller if soft reset failed ASoC: dt-bindings: rockchip: Document reset property for i2s ASoC: rockchip: i2s: Add property to specify play/cap capability ASoC: dt-bindings: rockchip: i2s: Document property for playback/capture ASoC: rockchip: i2s: Add compatible for more SoCs ASoC: dt-bindings: rockchip: Add compatible strings for more SoCs ASoC: rockchip: i2s: Add support for frame inversion ASoC: dt-bindings: rockchip: i2s: Document property 'clk-trcm'
Xiaotan Luo (1): ASoC: rockchip: i2s: Fixup config for DAIFMT_DSP_A/B
Xing Zheng (1): ASoC: rockchip: i2s: Add support for 'rockchip,clk-trcm' property
.../devicetree/bindings/sound/rockchip-i2s.yaml | 30 ++++ sound/soc/rockchip/rockchip_i2s.c | 153 ++++++++++++++++----- sound/soc/rockchip/rockchip_i2s.h | 10 +- 3 files changed, 157 insertions(+), 36 deletions(-)
This patch adds support for set bclk ratio from machine driver.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com ---
sound/soc/rockchip/rockchip_i2s.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index c7dc350..c9d5c52 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -49,6 +49,7 @@ struct rk_i2s_dev { bool rx_start; bool is_master_mode; const struct rk_i2s_pins *pins; + unsigned int bclk_ratio; };
static int i2s_runtime_suspend(struct device *dev) @@ -278,7 +279,7 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
if (i2s->is_master_mode) { mclk_rate = clk_get_rate(i2s->mclk); - bclk_rate = 2 * 32 * params_rate(params); + bclk_rate = i2s->bclk_ratio * params_rate(params); if (bclk_rate == 0 || mclk_rate % bclk_rate) return -EINVAL;
@@ -413,6 +414,16 @@ static int rockchip_i2s_trigger(struct snd_pcm_substream *substream, return ret; }
+static int rockchip_i2s_set_bclk_ratio(struct snd_soc_dai *dai, + unsigned int ratio) +{ + struct rk_i2s_dev *i2s = to_info(dai); + + i2s->bclk_ratio = ratio; + + return 0; +} + static int rockchip_i2s_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id, unsigned int freq, int dir) { @@ -441,6 +452,7 @@ static int rockchip_i2s_dai_probe(struct snd_soc_dai *dai)
static const struct snd_soc_dai_ops rockchip_i2s_dai_ops = { .hw_params = rockchip_i2s_hw_params, + .set_bclk_ratio = rockchip_i2s_set_bclk_ratio, .set_sysclk = rockchip_i2s_set_sysclk, .set_fmt = rockchip_i2s_set_fmt, .trigger = rockchip_i2s_trigger, @@ -638,6 +650,8 @@ static int rockchip_i2s_probe(struct platform_device *pdev) i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; i2s->capture_dma_data.maxburst = 4;
+ i2s->bclk_ratio = 64; + dev_set_drvdata(&pdev->dev, i2s);
pm_runtime_enable(&pdev->dev);
MCLK maybe not precise as required because of PLL, but which still can be used and no side effect. so, using DIV_ROUND_CLOSEST instead div.
e.g.
set mclk to 11289600 Hz, but get 11289598 Hz.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com ---
sound/soc/rockchip/rockchip_i2s.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index c9d5c52..05fce2c 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -280,10 +280,10 @@ static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream, if (i2s->is_master_mode) { mclk_rate = clk_get_rate(i2s->mclk); bclk_rate = i2s->bclk_ratio * params_rate(params); - if (bclk_rate == 0 || mclk_rate % bclk_rate) + if (!bclk_rate) return -EINVAL;
- div_bclk = mclk_rate / bclk_rate; + div_bclk = DIV_ROUND_CLOSEST(mclk_rate, bclk_rate); div_lrck = bclk_rate / params_rate(params); regmap_update_bits(i2s->regmap, I2S_CKR, I2S_CKR_MDIV_MASK,
This patch changes dma data burst from 4 to 8 to improve data transfer efficiency.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com ---
sound/soc/rockchip/rockchip_i2s.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 05fce2c..2e0047d 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -644,11 +644,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
i2s->playback_dma_data.addr = res->start + I2S_TXDR; i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; - i2s->playback_dma_data.maxburst = 4; + i2s->playback_dma_data.maxburst = 8;
i2s->capture_dma_data.addr = res->start + I2S_RXDR; i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; - i2s->capture_dma_data.maxburst = 4; + i2s->capture_dma_data.maxburst = 8;
i2s->bclk_ratio = 64;
API 'set_fmt' maybe called when PD is off, in the situation, any register access will hang the system. so, enable PD before r/w register.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com ---
sound/soc/rockchip/rockchip_i2s.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 2e0047d..90877e8 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -187,7 +187,9 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, { struct rk_i2s_dev *i2s = to_info(cpu_dai); unsigned int mask = 0, val = 0; + int ret = 0;
+ pm_runtime_get_sync(cpu_dai->dev); mask = I2S_CKR_MSS_MASK; switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { case SND_SOC_DAIFMT_CBS_CFS: @@ -200,7 +202,8 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, i2s->is_master_mode = false; break; default: - return -EINVAL; + ret = -EINVAL; + goto err_pm_put; }
regmap_update_bits(i2s->regmap, I2S_CKR, mask, val); @@ -214,7 +217,8 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, val = I2S_CKR_CKP_POS; break; default: - return -EINVAL; + ret = -EINVAL; + goto err_pm_put; }
regmap_update_bits(i2s->regmap, I2S_CKR, mask, val); @@ -237,7 +241,8 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, val = I2S_TXCR_TFS_PCM | I2S_TXCR_PBM_MODE(1); break; default: - return -EINVAL; + ret = -EINVAL; + goto err_pm_put; }
regmap_update_bits(i2s->regmap, I2S_TXCR, mask, val); @@ -260,12 +265,16 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, val = I2S_RXCR_TFS_PCM | I2S_RXCR_PBM_MODE(1); break; default: - return -EINVAL; + ret = -EINVAL; + goto err_pm_put; }
regmap_update_bits(i2s->regmap, I2S_RXCR, mask, val);
- return 0; +err_pm_put: + pm_runtime_put(cpu_dai->dev); + + return ret; }
static int rockchip_i2s_hw_params(struct snd_pcm_substream *substream,
This patch adds lock to fix comcurrency between tx/rx to fix 'rockchip-i2s ff070000.i2s; fail to clear'
Considering the situation;
tx stream rx stream | | | disable enable | | reset
After this patch:
lock | tx stream | enable | unlock -------- --------- lock | rx stream | disable | reset | unlock
Change-Id: Ia156144490a61f4fa9823b1313588e44688f4bce Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com --- sound/soc/rockchip/rockchip_i2s.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index dfa0a5e..b8e35a9 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -15,6 +15,7 @@ #include <linux/clk.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> +#include <linux/spinlock.h> #include <sound/pcm_params.h> #include <sound/dmaengine_pcm.h>
@@ -52,6 +53,9 @@ struct rk_i2s_dev { unsigned int bclk_ratio; };
+/* tx/rx ctrl lock */ +static DEFINE_SPINLOCK(lock); + static int i2s_runtime_suspend(struct device *dev) { struct rk_i2s_dev *i2s = dev_get_drvdata(dev); @@ -93,6 +97,7 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) unsigned int val = 0; int retry = 10;
+ spin_lock(&lock); if (on) { regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_TDE_ENABLE, I2S_DMACR_TDE_ENABLE); @@ -133,6 +138,7 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) } } } + spin_unlock(&lock); }
static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) @@ -140,6 +146,7 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) unsigned int val = 0; int retry = 10;
+ spin_lock(&lock); if (on) { regmap_update_bits(i2s->regmap, I2S_DMACR, I2S_DMACR_RDE_ENABLE, I2S_DMACR_RDE_ENABLE); @@ -180,6 +187,7 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) } } } + spin_unlock(&lock); }
static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
This patch brings i2s back to normal by resetting i2s m/h when the soft reset failed.
Change-Id: I2fd47039b522ac89499b4a2912d5ffb7a469e75e Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com --- sound/soc/rockchip/rockchip_i2s.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index b8e35a9..a91f874d 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -15,6 +15,7 @@ #include <linux/clk.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> +#include <linux/reset.h> #include <linux/spinlock.h> #include <sound/pcm_params.h> #include <sound/dmaengine_pcm.h> @@ -40,6 +41,8 @@ struct rk_i2s_dev {
struct regmap *regmap; struct regmap *grf; + struct reset_control *reset_m; + struct reset_control *reset_h;
/* * Used to indicate the tx/rx status. @@ -92,6 +95,20 @@ static inline struct rk_i2s_dev *to_info(struct snd_soc_dai *dai) return snd_soc_dai_get_drvdata(dai); }
+static void rockchip_i2s_reset(struct rk_i2s_dev *i2s) +{ + dev_warn(i2s->dev, "Reset controller.\n"); + + reset_control_assert(i2s->reset_m); + reset_control_assert(i2s->reset_h); + udelay(1); + reset_control_deassert(i2s->reset_m); + reset_control_deassert(i2s->reset_h); + + regcache_mark_dirty(i2s->regmap); + regcache_sync(i2s->regmap); +} + static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) { unsigned int val = 0; @@ -132,7 +149,7 @@ static void rockchip_snd_txctrl(struct rk_i2s_dev *i2s, int on) regmap_read(i2s->regmap, I2S_CLR, &val); retry--; if (!retry) { - dev_warn(i2s->dev, "fail to clear\n"); + rockchip_i2s_reset(i2s); break; } } @@ -181,7 +198,7 @@ static void rockchip_snd_rxctrl(struct rk_i2s_dev *i2s, int on) regmap_read(i2s->regmap, I2S_CLR, &val); retry--; if (!retry) { - dev_warn(i2s->dev, "fail to clear\n"); + rockchip_i2s_reset(i2s); break; } } @@ -629,6 +646,14 @@ static int rockchip_i2s_probe(struct platform_device *pdev) i2s->pins = of_id->data; }
+ i2s->reset_m = devm_reset_control_get_optional(&pdev->dev, "reset-m"); + if (IS_ERR(i2s->reset_m)) + return PTR_ERR(i2s->reset_m); + + i2s->reset_h = devm_reset_control_get_optional(&pdev->dev, "reset-h"); + if (IS_ERR(i2s->reset_h)) + return PTR_ERR(i2s->reset_h); + /* try to prepare related clocks */ i2s->hclk = devm_clk_get(&pdev->dev, "i2s_hclk"); if (IS_ERR(i2s->hclk)) {
This patch documents reset property for i2s.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: Idaaaa0583ddf2254589ab465766322f210db58a7 --- Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml index 245895b..9f9cc48 100644 --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml @@ -61,6 +61,14 @@ properties: power-domains: maxItems: 1
+ reset-names: + items: + - const: reset-m + - const: reset-h + + resets: + maxItems: 2 + rockchip,capture-channels: $ref: /schemas/types.yaml#/definitions/uint32 default: 2
From: Xiaotan Luo lxt@rock-chips.com
- DSP_A: PCM delay 1 bit mode, L data MSB after FRM LRC - DSP_B: PCM no delay mode, L data MSB during FRM LRC
Change-Id: I198519c431815de3ca1fc154da78773ca705f0e0 Signed-off-by: Xiaotan Luo lxt@rock-chips.com Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com --- sound/soc/rockchip/rockchip_i2s.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index a91f874d..8a432ae 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -259,12 +259,12 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, case SND_SOC_DAIFMT_I2S: val = I2S_TXCR_IBM_NORMAL; break; - case SND_SOC_DAIFMT_DSP_A: /* PCM no delay mode */ - val = I2S_TXCR_TFS_PCM; - break; - case SND_SOC_DAIFMT_DSP_B: /* PCM delay 1 mode */ + case SND_SOC_DAIFMT_DSP_A: /* PCM delay 1 bit mode */ val = I2S_TXCR_TFS_PCM | I2S_TXCR_PBM_MODE(1); break; + case SND_SOC_DAIFMT_DSP_B: /* PCM no delay mode */ + val = I2S_TXCR_TFS_PCM; + break; default: ret = -EINVAL; goto err_pm_put; @@ -283,12 +283,12 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai, case SND_SOC_DAIFMT_I2S: val = I2S_RXCR_IBM_NORMAL; break; - case SND_SOC_DAIFMT_DSP_A: /* PCM no delay mode */ - val = I2S_RXCR_TFS_PCM; - break; - case SND_SOC_DAIFMT_DSP_B: /* PCM delay 1 mode */ + case SND_SOC_DAIFMT_DSP_A: /* PCM delay 1 bit mode */ val = I2S_RXCR_TFS_PCM | I2S_RXCR_PBM_MODE(1); break; + case SND_SOC_DAIFMT_DSP_B: /* PCM no delay mode */ + val = I2S_RXCR_TFS_PCM; + break; default: ret = -EINVAL; goto err_pm_put;
- 'rockchip,playback-only': support playback only. - 'rockchip,capture-only': support capture only.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: Ie022214de4e42bba5ba898ea39a4e39807e8d4ab --- sound/soc/rockchip/rockchip_i2s.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index 8a432ae..cedfe47 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -721,6 +721,11 @@ static int rockchip_i2s_probe(struct platform_device *pdev) soc_dai->capture.channels_max = val; }
+ if (of_property_read_bool(node, "rockchip,playback-only")) + soc_dai->capture.channels_min = 0; + else if (of_property_read_bool(node, "rockchip,capture-only")) + soc_dai->playback.channels_min = 0; + ret = devm_snd_soc_register_component(&pdev->dev, &rockchip_i2s_component, soc_dai, 1);
This patch documents property for playback-only and capture-only.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: I06a38e0e9fb8c4386cda3e5f6bc974a68180a426 --- Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml index 9f9cc48..005b6e6 100644 --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml @@ -81,6 +81,16 @@ properties: description: Max playback channels, if not set, 8 channels default.
+ rockchip,capture-only: + type: boolean + description: + Specify that the controller has capture only capability. + + rockchip,playback-only: + type: boolean + description: + Specify that the controller has playback only capability. + rockchip,grf: $ref: /schemas/types.yaml#/definitions/phandle description:
This patch adds more compatible strings for SoCs.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: I980a7c34a7f6c82a066a37e7f97fa4e69cbae583 --- sound/soc/rockchip/rockchip_i2s.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index cedfe47..a0f2778 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -613,10 +613,20 @@ static const struct rk_i2s_pins rk3399_i2s_pins = { };
static const struct of_device_id rockchip_i2s_match[] __maybe_unused = { + { .compatible = "rockchip,px30-i2s", }, + { .compatible = "rockchip,rk1808-i2s", }, + { .compatible = "rockchip,rk3036-i2s", }, { .compatible = "rockchip,rk3066-i2s", }, + { .compatible = "rockchip,rk3128-i2s", }, { .compatible = "rockchip,rk3188-i2s", }, + { .compatible = "rockchip,rk3228-i2s", }, { .compatible = "rockchip,rk3288-i2s", }, + { .compatible = "rockchip,rk3308-i2s", }, + { .compatible = "rockchip,rk3328-i2s", }, + { .compatible = "rockchip,rk3366-i2s", }, + { .compatible = "rockchip,rk3368-i2s", }, { .compatible = "rockchip,rk3399-i2s", .data = &rk3399_i2s_pins }, + { .compatible = "rockchip,rv1126-i2s", }, {}, };
This patch adds compatible strings for more SoCs.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: Id1f32a4518c01e0d50c5702a557912e165904995 --- Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml index 005b6e6..11e911a 100644 --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml @@ -20,7 +20,9 @@ properties: - items: - enum: - rockchip,px30-i2s + - rockchip,rk1808-i2s - rockchip,rk3036-i2s + - rockchip,rk3128-i2s - rockchip,rk3188-i2s - rockchip,rk3228-i2s - rockchip,rk3288-i2s @@ -29,6 +31,7 @@ properties: - rockchip,rk3366-i2s - rockchip,rk3368-i2s - rockchip,rk3399-i2s + - rockchip,rv1126-i2s - const: rockchip,rk3066-i2s
reg:
This patch adds support for frame inversion.
Change-Id: Ic77122501224cec45200ae64416745a82fb67d76 Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com --- sound/soc/rockchip/rockchip_i2s.c | 20 +++++++++++++++++--- sound/soc/rockchip/rockchip_i2s.h | 10 ++++++---- 2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c index a0f2778..6ccb62e 100644 --- a/sound/soc/rockchip/rockchip_i2s.c +++ b/sound/soc/rockchip/rockchip_i2s.c @@ -233,13 +233,27 @@ static int rockchip_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
regmap_update_bits(i2s->regmap, I2S_CKR, mask, val);
- mask = I2S_CKR_CKP_MASK; + mask = I2S_CKR_CKP_MASK | I2S_CKR_TLP_MASK | I2S_CKR_RLP_MASK; switch (fmt & SND_SOC_DAIFMT_INV_MASK) { case SND_SOC_DAIFMT_NB_NF: - val = I2S_CKR_CKP_NEG; + val = I2S_CKR_CKP_NORMAL | + I2S_CKR_TLP_NORMAL | + I2S_CKR_RLP_NORMAL; + break; + case SND_SOC_DAIFMT_NB_IF: + val = I2S_CKR_CKP_NORMAL | + I2S_CKR_TLP_INVERTED | + I2S_CKR_RLP_INVERTED; break; case SND_SOC_DAIFMT_IB_NF: - val = I2S_CKR_CKP_POS; + val = I2S_CKR_CKP_INVERTED | + I2S_CKR_TLP_NORMAL | + I2S_CKR_RLP_NORMAL; + break; + case SND_SOC_DAIFMT_IB_IF: + val = I2S_CKR_CKP_INVERTED | + I2S_CKR_TLP_INVERTED | + I2S_CKR_RLP_INVERTED; break; default: ret = -EINVAL; diff --git a/sound/soc/rockchip/rockchip_i2s.h b/sound/soc/rockchip/rockchip_i2s.h index fcaae24..251851b 100644 --- a/sound/soc/rockchip/rockchip_i2s.h +++ b/sound/soc/rockchip/rockchip_i2s.h @@ -88,15 +88,17 @@ #define I2S_CKR_MSS_SLAVE (1 << I2S_CKR_MSS_SHIFT) #define I2S_CKR_MSS_MASK (1 << I2S_CKR_MSS_SHIFT) #define I2S_CKR_CKP_SHIFT 26 -#define I2S_CKR_CKP_NEG (0 << I2S_CKR_CKP_SHIFT) -#define I2S_CKR_CKP_POS (1 << I2S_CKR_CKP_SHIFT) +#define I2S_CKR_CKP_NORMAL (0 << I2S_CKR_CKP_SHIFT) +#define I2S_CKR_CKP_INVERTED (1 << I2S_CKR_CKP_SHIFT) #define I2S_CKR_CKP_MASK (1 << I2S_CKR_CKP_SHIFT) #define I2S_CKR_RLP_SHIFT 25 #define I2S_CKR_RLP_NORMAL (0 << I2S_CKR_RLP_SHIFT) -#define I2S_CKR_RLP_OPPSITE (1 << I2S_CKR_RLP_SHIFT) +#define I2S_CKR_RLP_INVERTED (1 << I2S_CKR_RLP_SHIFT) +#define I2S_CKR_RLP_MASK (1 << I2S_CKR_RLP_SHIFT) #define I2S_CKR_TLP_SHIFT 24 #define I2S_CKR_TLP_NORMAL (0 << I2S_CKR_TLP_SHIFT) -#define I2S_CKR_TLP_OPPSITE (1 << I2S_CKR_TLP_SHIFT) +#define I2S_CKR_TLP_INVERTED (1 << I2S_CKR_TLP_SHIFT) +#define I2S_CKR_TLP_MASK (1 << I2S_CKR_TLP_SHIFT) #define I2S_CKR_MDIV_SHIFT 16 #define I2S_CKR_MDIV(x) ((x - 1) << I2S_CKR_MDIV_SHIFT) #define I2S_CKR_MDIV_MASK (0xff << I2S_CKR_MDIV_SHIFT)
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); return 0; }
@@ -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);
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
Hi Nicolas,
On 2021/8/23 19:47, Nicolas Frattaroli wrote:
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.
okay, will do in v2.
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.
It's possible, but will make the driver much more complicate(a lot of 'if...else...').
Though the registers were compatible, but the design is totally different, such as clks, reset signal, pins and core logic.
so, split into two drivers to support old controller and new one.
Regards, Nicolas Frattaroli
This patch documents property 'rockchip,clk-trcm' which is used to specify the lrck.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: I648142c57c568bbed209f2b9188b1f539a3285b2 --- Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml index 11e911a..8d9dfed 100644 --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml @@ -72,6 +72,15 @@ properties: resets: maxItems: 2
+ rockchip,clk-trcm: + $ref: /schemas/types.yaml#/definitions/uint32 + default: 0 + description: + tx and rx lrck/bclk common use. + 0: both tx_lrck/bclk and rx_lrck/bclk are used + 1: only tx_lrck/bclk is used + 2: only rx_lrck/bclk is used + rockchip,capture-channels: $ref: /schemas/types.yaml#/definitions/uint32 default: 2
On Mon, 23 Aug 2021 18:55:32 +0800, Sugar Zhang wrote:
This patch documents property 'rockchip,clk-trcm' which is used to specify the lrck.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: I648142c57c568bbed209f2b9188b1f539a3285b2
Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 9 +++++++++ 1 file changed, 9 insertions(+)
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/sound/rockchip-i2s.example.dts' Traceback (most recent call last): File "/usr/local/bin/dt-extract-example", line 45, in <module> binding = yaml.load(open(args.yamlfile, encoding='utf-8').read()) File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load return constructor.get_single_data() File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 120, in get_single_data node = self.composer.get_single_node() File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context in "<unicode string>", line 80, column 8 make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/sound/rockchip-i2s.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs.... ./Documentation/devicetree/bindings/sound/rockchip-i2s.yaml: mapping values are not allowed in this context in "<unicode string>", line 80, column 8 /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml: ignoring, error parsing file warning: no schema found in file: ./Documentation/devicetree/bindings/sound/rockchip-i2s.yaml make: *** [Makefile:1419: dt_binding_check] Error 2
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1519630
This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit.
On Mon, Aug 23, 2021 at 06:55:32PM +0800, Sugar Zhang wrote:
This patch documents property 'rockchip,clk-trcm' which is used to specify the lrck.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: I648142c57c568bbed209f2b9188b1f539a3285b2
Drop this.
Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml index 11e911a..8d9dfed 100644 --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml @@ -72,6 +72,15 @@ properties: resets: maxItems: 2
- rockchip,clk-trcm:
- $ref: /schemas/types.yaml#/definitions/uint32
- default: 0
- description:
tx and rx lrck/bclk common use.0: both tx_lrck/bclk and rx_lrck/bclk are used1: only tx_lrck/bclk is used2: only rx_lrck/bclk is used
Sounds like constraints. Make a schema.
- rockchip,capture-channels: $ref: /schemas/types.yaml#/definitions/uint32 default: 2
-- 2.7.4
On 2021/8/24 1:36, Rob Herring wrote:
On Mon, Aug 23, 2021 at 06:55:32PM +0800, Sugar Zhang wrote:
This patch documents property 'rockchip,clk-trcm' which is used to specify the lrck.
Signed-off-by: Sugar Zhang sugar.zhang@rock-chips.com Change-Id: I648142c57c568bbed209f2b9188b1f539a3285b2
Drop this.
okay, will do in v2.
Documentation/devicetree/bindings/sound/rockchip-i2s.yaml | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml index 11e911a..8d9dfed 100644 --- a/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/rockchip-i2s.yaml @@ -72,6 +72,15 @@ properties: resets: maxItems: 2
- rockchip,clk-trcm:
- $ref: /schemas/types.yaml#/definitions/uint32
- default: 0
- description:
tx and rx lrck/bclk common use.0: both tx_lrck/bclk and rx_lrck/bclk are used1: only tx_lrck/bclk is used2: only rx_lrck/bclk is usedSounds like constraints. Make a schema.
will split into two properties: rockchip,trcm-sync-tx-only, rockchip,trcm-sync-rx-only.
- rockchip,capture-channels: $ref: /schemas/types.yaml#/definitions/uint32 default: 2
-- 2.7.4
participants (3)
-
Nicolas Frattaroli -
Rob Herring -
Sugar Zhang