From 030c59df83b448fc873d1caabb9ea077ae25268e Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Thu, 25 Sep 2025 05:17:04 +0000 Subject: [PATCH 1/8] ASoC: renesas: msiof: add unique NOTE name MSIOF will have many NOTE on top of driver, give it a unique NOTE name. Signed-off-by: Kuninori Morimoto Link: https://patch.msgid.link/87ecrvyuuo.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rcar/msiof.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c index f5338bbb037c..6819471fb5b2 100644 --- a/sound/soc/renesas/rcar/msiof.c +++ b/sound/soc/renesas/rcar/msiof.c @@ -7,7 +7,7 @@ // /* - * [NOTE] + * [NOTE-CLOCK-MODE] * * This driver doesn't support Clock/Frame Provider Mode * @@ -121,7 +121,7 @@ static int msiof_hw_start(struct snd_soc_component *component, /* * see - * [NOTE] on top of this driver + * [NOTE-CLOCK-MODE] on top of this driver */ /* * see From 25226abc1affd4bf4f6dd415d475b76e7a273fa8 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Thu, 25 Sep 2025 05:17:11 +0000 Subject: [PATCH 2/8] ASoC: renesas: msiof: use reset controller MSIOF has TXRST/RXRST to reset FIFO, but it shouldn't be used during SYNC signal was asserted, because it will be cause of HW issue. When MSIOF is used as Sound driver, this driver is assuming it is used as clock consumer mode (= Codec is clock provider). This means, it can't control SYNC signal by itself. We need to use SW reset (= reset_control_xxx()) instead of TXRST/RXRST. Signed-off-by: Kuninori Morimoto Tested-by: Yusuke Goda Link: https://patch.msgid.link/87cy7fyuug.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rcar/msiof.c | 39 +++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c index 6819471fb5b2..87bec248d5c1 100644 --- a/sound/soc/renesas/rcar/msiof.c +++ b/sound/soc/renesas/rcar/msiof.c @@ -24,12 +24,25 @@ * Clock/Frame Consumer Mode. */ +/* + * [NOTE-RESET] + * + * MSIOF has TXRST/RXRST to reset FIFO, but it shouldn't be used during SYNC signal was asserted, + * because it will be cause of HW issue. + * + * When MSIOF is used as Sound driver, this driver is assuming it is used as clock consumer mode + * (= Codec is clock provider). This means, it can't control SYNC signal by itself. + * + * We need to use SW reset (= reset_control_xxx()) instead of TXRST/RXRST. + */ + #include #include #include #include #include #include +#include #include #include #include @@ -60,10 +73,13 @@ struct msiof_priv { struct device *dev; struct snd_pcm_substream *substream[SNDRV_PCM_STREAM_LAST + 1]; + struct reset_control *reset; spinlock_t lock; void __iomem *base; resource_size_t phy_addr; + int count; + /* for error */ int err_syc[SNDRV_PCM_STREAM_LAST + 1]; int err_ovf[SNDRV_PCM_STREAM_LAST + 1]; @@ -131,6 +147,16 @@ static int msiof_hw_start(struct snd_soc_component *component, * RX: Fig 109.15 */ + /* + * Use reset_control_xx() instead of TXRST/RXRST. + * see + * [NOTE-RESET] + */ + if (!priv->count) + reset_control_deassert(priv->reset); + + priv->count++; + /* reset errors */ priv->err_syc[substream->stream] = priv->err_ovf[substream->stream] = @@ -152,7 +178,6 @@ static int msiof_hw_start(struct snd_soc_component *component, val = FIELD_PREP(SIMDR2_BITLEN1, width - 1); msiof_write(priv, SITMDR2, val | FIELD_PREP(SIMDR2_GRP, 1)); msiof_write(priv, SITMDR3, val); - } /* SIRMDRx */ else { @@ -227,6 +252,11 @@ static int msiof_hw_stop(struct snd_soc_component *component, priv->err_ovf[substream->stream], priv->err_udf[substream->stream]); + priv->count--; + + if (!priv->count) + reset_control_assert(priv->reset); + return 0; } @@ -490,12 +520,19 @@ static int msiof_probe(struct platform_device *pdev) if (IS_ERR(priv->base)) return PTR_ERR(priv->base); + priv->reset = devm_reset_control_get_exclusive(dev, NULL); + if (IS_ERR(priv->reset)) + return PTR_ERR(priv->reset); + + reset_control_assert(priv->reset); + ret = devm_request_irq(dev, irq, msiof_interrupt, 0, dev_name(dev), priv); if (ret) return ret; priv->dev = dev; priv->phy_addr = res->start; + priv->count = 0; spin_lock_init(&priv->lock); platform_set_drvdata(pdev, priv); From 130947b4681c515a5e5a7961244b502de2de85ca Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Thu, 25 Sep 2025 05:17:17 +0000 Subject: [PATCH 3/8] ASoC: renesas: msiof: set SIFCTR register Because it uses DMAC, we would like to transfer data if there is any data. Set SIFCTR for it. Signed-off-by: Kuninori Morimoto Tested-by: Yusuke Goda Link: https://patch.msgid.link/87bjmzyuub.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rcar/msiof.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c index 87bec248d5c1..a239a88543ee 100644 --- a/sound/soc/renesas/rcar/msiof.c +++ b/sound/soc/renesas/rcar/msiof.c @@ -193,6 +193,12 @@ static int msiof_hw_start(struct snd_soc_component *component, msiof_write(priv, SIRMDR3, val); } + /* SIFCTR */ + if (is_play) + msiof_update(priv, SIFCTR, SIFCTR_TFWM, FIELD_PREP(SIFCTR_TFWM, SIFCTR_TFWM_1)); + else + msiof_update(priv, SIFCTR, SIFCTR_RFWM, FIELD_PREP(SIFCTR_RFWM, SIFCTR_RFWM_1)); + /* SIIER */ if (is_play) val = SIIER_TDREQE | SIIER_TDMAE | SISTR_ERR_TX; From ab77fa5533e4d1dcfdd2711b9b1e166e4ed57dab Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Thu, 25 Sep 2025 05:17:21 +0000 Subject: [PATCH 4/8] ASoC: renesas: msiof: add .symmetric_xxx on snd_soc_dai_driver MSIOF TX/RX are sharing same clock. Adds .symmetric_xxx flags. Signed-off-by: Kuninori Morimoto Tested-by: Yusuke Goda Link: https://patch.msgid.link/87a52jyuu6.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rcar/msiof.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c index a239a88543ee..d501ad9d7141 100644 --- a/sound/soc/renesas/rcar/msiof.c +++ b/sound/soc/renesas/rcar/msiof.c @@ -338,6 +338,9 @@ static struct snd_soc_dai_driver msiof_dai_driver = { .channels_max = 2, }, .ops = &msiof_dai_ops, + .symmetric_rate = 1, + .symmetric_channels = 1, + .symmetric_sample_bits = 1, }; static struct snd_pcm_hardware msiof_pcm_hardware = { From 25aa058b5c83a3c455a2a288bb3295c0b234f093 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Thu, 25 Sep 2025 05:17:27 +0000 Subject: [PATCH 5/8] ASoC: renesas: msiof: tidyup DMAC stop timing Current DMAC is stopped before HW stop, but it might be cause of sync error. Stop HW first. Signed-off-by: Kuninori Morimoto Tested-by: Yusuke Goda Link: https://patch.msgid.link/878qi3yuu0.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rcar/msiof.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c index d501ad9d7141..5b59d4bcd67e 100644 --- a/sound/soc/renesas/rcar/msiof.c +++ b/sound/soc/renesas/rcar/msiof.c @@ -238,9 +238,6 @@ static int msiof_hw_stop(struct snd_soc_component *component, val = SIIER_RDREQE | SIIER_RDMAE | SISTR_ERR_RX; msiof_update(priv, SIIER, val, 0); - /* Stop DMAC */ - snd_dmaengine_pcm_trigger(substream, cmd); - /* SICTR */ if (is_play) val = SICTR_TXE; @@ -248,6 +245,9 @@ static int msiof_hw_stop(struct snd_soc_component *component, val = SICTR_RXE; msiof_update_and_wait(priv, SICTR, val, 0, 0); + /* Stop DMAC */ + snd_dmaengine_pcm_trigger(substream, cmd); + /* indicate error status if exist */ if (priv->err_syc[substream->stream] || priv->err_ovf[substream->stream] || From dc7473e6372ee36ff232af10c910ee3a8bad6447 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Thu, 25 Sep 2025 05:17:34 +0000 Subject: [PATCH 6/8] ASoC: renesas: msiof: setup both (Playback/Capture) in the same time SITMDRn / SIRMDRn and some other registers should not be updated during working even though it was not related the target direction (for example, do TX settings during RX is working), otherwise it cause a FSERR. Setup both direction (Playback/Capture) in the same time. Signed-off-by: Kuninori Morimoto Tested-by: Yusuke Goda Link: https://patch.msgid.link/877bxnyutt.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rcar/msiof.c | 66 ++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c index 5b59d4bcd67e..df664800bf60 100644 --- a/sound/soc/renesas/rcar/msiof.c +++ b/sound/soc/renesas/rcar/msiof.c @@ -36,6 +36,16 @@ * We need to use SW reset (= reset_control_xxx()) instead of TXRST/RXRST. */ +/* + * [NOTE-BOTH-SETTING] + * + * SITMDRn / SIRMDRn and some other registers should not be updated during working even though it + * was not related the target direction (for example, do TX settings during RX is working), + * otherwise it cause a FSERR. + * + * Setup both direction (Playback/Capture) in the same time. + */ + #include #include #include @@ -165,39 +175,40 @@ static int msiof_hw_start(struct snd_soc_component *component, /* Start DMAC */ snd_dmaengine_pcm_trigger(substream, cmd); + /* + * setup both direction (Playback/Capture) in the same time. + * see + * above [NOTE-BOTH-SETTING] + */ + /* SITMDRx */ - if (is_play) { - val = SITMDR1_PCON | - FIELD_PREP(SIMDR1_SYNCMD, SIMDR1_SYNCMD_LR) | - SIMDR1_SYNCAC | SIMDR1_XXSTP; - if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY)) - val |= FIELD_PREP(SIMDR1_DTDL, 1); + val = SITMDR1_PCON | SIMDR1_SYNCAC | SIMDR1_XXSTP | + FIELD_PREP(SIMDR1_SYNCMD, SIMDR1_SYNCMD_LR); + if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY)) + val |= FIELD_PREP(SIMDR1_DTDL, 1); - msiof_write(priv, SITMDR1, val); + msiof_write(priv, SITMDR1, val); + + val = FIELD_PREP(SIMDR2_BITLEN1, width - 1); + msiof_write(priv, SITMDR2, val | FIELD_PREP(SIMDR2_GRP, 1)); + msiof_write(priv, SITMDR3, val); - val = FIELD_PREP(SIMDR2_BITLEN1, width - 1); - msiof_write(priv, SITMDR2, val | FIELD_PREP(SIMDR2_GRP, 1)); - msiof_write(priv, SITMDR3, val); - } /* SIRMDRx */ - else { - val = FIELD_PREP(SIMDR1_SYNCMD, SIMDR1_SYNCMD_LR) | - SIMDR1_SYNCAC; - if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY)) - val |= FIELD_PREP(SIMDR1_DTDL, 1); + val = SIMDR1_SYNCAC | + FIELD_PREP(SIMDR1_SYNCMD, SIMDR1_SYNCMD_LR); + if (msiof_flag_has(priv, MSIOF_FLAGS_NEED_DELAY)) + val |= FIELD_PREP(SIMDR1_DTDL, 1); - msiof_write(priv, SIRMDR1, val); + msiof_write(priv, SIRMDR1, val); - val = FIELD_PREP(SIMDR2_BITLEN1, width - 1); - msiof_write(priv, SIRMDR2, val | FIELD_PREP(SIMDR2_GRP, 1)); - msiof_write(priv, SIRMDR3, val); - } + val = FIELD_PREP(SIMDR2_BITLEN1, width - 1); + msiof_write(priv, SIRMDR2, val | FIELD_PREP(SIMDR2_GRP, 1)); + msiof_write(priv, SIRMDR3, val); /* SIFCTR */ - if (is_play) - msiof_update(priv, SIFCTR, SIFCTR_TFWM, FIELD_PREP(SIFCTR_TFWM, SIFCTR_TFWM_1)); - else - msiof_update(priv, SIFCTR, SIFCTR_RFWM, FIELD_PREP(SIFCTR_RFWM, SIFCTR_RFWM_1)); + msiof_write(priv, SIFCTR, + FIELD_PREP(SIFCTR_TFWM, SIFCTR_TFWM_1) | + FIELD_PREP(SIFCTR_RFWM, SIFCTR_RFWM_1)); /* SIIER */ if (is_play) @@ -214,10 +225,11 @@ static int msiof_hw_start(struct snd_soc_component *component, msiof_update(priv, SISTR, val, val); /* SICTR */ + val = SICTR_TEDG | SICTR_REDG; if (is_play) - val = SICTR_TXE | SICTR_TEDG; + val |= SICTR_TXE; else - val = SICTR_RXE | SICTR_REDG; + val |= SICTR_RXE; msiof_update_and_wait(priv, SICTR, val, val, val); return 0; From 8c363f61e5bcb92d5e88ca1b47be74be2683b212 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Thu, 25 Sep 2025 05:17:41 +0000 Subject: [PATCH 7/8] ASoC: renesas: msiof: Add note for The possibility of R/L opposite Capture This driver is assuming MSIOF is used as Clock/Frame Consumer Mode, and there is a case that some Codec (= Clock/Frame Provider) might output Clock/Frame before setup MSIOF. And, MSIOF will capture data without checking SYNC signal Hi/Low (= R/L). This means, if MSIOF RXE bit was set as 1 in case of SYNC signal was Hi (= R) timing, it will start capture data since next SYNC low signal (= L). Because Linux assumes sound data is lined up as R->L->R->L->..., the data R/L might be opposite. The only solution in this case is start CLK/SYNC *after* MSIOF settings, but it depends when and how Codec driver start it. Signed-off-by: Kuninori Morimoto Tested-by: Yusuke Goda Link: https://patch.msgid.link/875xd7yutm.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rcar/msiof.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c index df664800bf60..330b65b29597 100644 --- a/sound/soc/renesas/rcar/msiof.c +++ b/sound/soc/renesas/rcar/msiof.c @@ -46,6 +46,25 @@ * Setup both direction (Playback/Capture) in the same time. */ +/* + * [NOTE-R/L] + * + * The data of Captured might be R/L opposite. + * + * This driver is assuming MSIOF is used as Clock/Frame Consumer Mode, and there is a case that some + * Codec (= Clock/Frame Provider) might output Clock/Frame before setup MSIOF. It depends on Codec + * driver implementation. + * + * MSIOF will capture data without checking SYNC signal Hi/Low (= R/L). + * + * This means, if MSIOF RXE bit was set as 1 in case of SYNC signal was Hi (= R) timing, it will + * start capture data since next SYNC low singla (= L). Because Linux assumes sound data is lined + * up as R->L->R->L->..., the data R/L will be opposite. + * + * The only solution in this case is start CLK/SYNC *after* MSIOF settings, but it depends when and + * how Codec driver start it. + */ + #include #include #include From e26387e950ee4486b4ed5728b5d3c1430c33ba67 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Thu, 25 Sep 2025 05:17:47 +0000 Subject: [PATCH 8/8] ASoC: renesas: msiof: ignore 1st FSERR Renesas have tried to minimize the occurrence of FSERR errors as much as possible, but unfortunately we cannot remove them completely, because MSIOF might setup its register during CLK/SYNC are inputed. It can be happen because MSIOF is working as Clock/Frame Consumer. Ignore 1st FSERR which we can do nothing Signed-off-by: Kuninori Morimoto Tested-by: Yusuke Goda Link: https://patch.msgid.link/874isryutg.wl-kuninori.morimoto.gx@renesas.com Signed-off-by: Mark Brown --- sound/soc/renesas/rcar/msiof.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/sound/soc/renesas/rcar/msiof.c b/sound/soc/renesas/rcar/msiof.c index 330b65b29597..f2addfbac923 100644 --- a/sound/soc/renesas/rcar/msiof.c +++ b/sound/soc/renesas/rcar/msiof.c @@ -65,6 +65,16 @@ * how Codec driver start it. */ +/* + * [NOTE-FSERR] + * + * We can't remove all FSERR. + * + * Renesas have tried to minimize the occurrence of FSERR errors as much as possible, but + * unfortunately we cannot remove them completely, because MSIOF might setup its register during + * CLK/SYNC are inputed. It can be happen because MSIOF is working as Clock/Frame Consumer. + */ + #include #include #include @@ -186,8 +196,13 @@ static int msiof_hw_start(struct snd_soc_component *component, priv->count++; - /* reset errors */ - priv->err_syc[substream->stream] = + /* + * Reset errors. ignore 1st FSERR + * + * see + * [NOTE-FSERR] + */ + priv->err_syc[substream->stream] = -1; priv->err_ovf[substream->stream] = priv->err_udf[substream->stream] = 0; @@ -279,6 +294,15 @@ static int msiof_hw_stop(struct snd_soc_component *component, /* Stop DMAC */ snd_dmaengine_pcm_trigger(substream, cmd); + /* + * Ignore 1st FSERR + * + * see + * [NOTE-FSERR] + */ + if (priv->err_syc[substream->stream] < 0) + priv->err_syc[substream->stream] = 0; + /* indicate error status if exist */ if (priv->err_syc[substream->stream] || priv->err_ovf[substream->stream] ||