[PATCH v2 0/3] ASoC: loongson: Fix configuration and clock error handling

[PATCH v2 0/3] ASoC: loongson: Fix configuration and clock error handling

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Hi all,

This series fixes I2S configuration updates and improves clock error 
handling in the Loongson ASoC driver.

Link v1: 
https://lore.kernel.org/linux-sound/20261022084040.31181-1-phucduc.bui@gmail.com/

Changes in v2:
 - Use FIELD_PREP() for register fields as suggested by Binbin.
 - Apply the same approach to the other registers in hw_params().

Compile-tested only.

Best regards,
Phuc

bui duc phuc (3):
  ASoC: loongson: Fix IISConfig0 update in hw_params for rev 1
  ASoC: loongson: Use FIELD_PREP() for register fields
  ASoC: loongson: Handle clock enable errors in set_fmt

 sound/soc/loongson/loongson_i2s.c | 58 +++++++++++++++++++++----------
 sound/soc/loongson/loongson_i2s.h | 11 ++++++
 2 files changed, 51 insertions(+), 18 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] ASoC: loongson: Fix IISConfig0 update in hw_params for rev 1

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

On revision 1 controllers, loongson_i2s_hw_params() reads LS_I2S_CFG
and ORs the new LR_LEN, BCLK_RATIO and RES_DEPTH values into it
without clearing the old ones.

The first stream after boot is configured correctly, but any later
stream with a different rate or sample width ends up with a mix of
old and new bits, so it may run with the wrong rate or sample width.

Use regmap_update_bits() to replace LR_LEN, BCLK_RATIO and the
RES_DEPTH field for the current direction while preserving the
other direction's RES_DEPTH field.

Fixes: d84881e06836 ("ASoC: Add support for Loongson I2S controller")
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

Changes in v2:
 - Use FIELD_PREP() for register fields as suggested by Binbin.

 sound/soc/loongson/loongson_i2s.c | 21 +++++++++++++--------
 sound/soc/loongson/loongson_i2s.h |  6 ++++++
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/sound/soc/loongson/loongson_i2s.c b/sound/soc/loongson/loongson_i2s.c
index da0d029c028a..aebea177e7e0 100644
--- a/sound/soc/loongson/loongson_i2s.c
+++ b/sound/soc/loongson/loongson_i2s.c
@@ -6,6 +6,7 @@
 // Author: Yingkun Meng <mengyingkun@loongson.cn>
 //
 
+#include <linux/bitfield.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
@@ -69,6 +70,7 @@ static int loongson_i2s_hw_params(struct snd_pcm_substream *substream,
 	u32 fs = params_rate(params);
 	u32 bclk_ratio, mclk_ratio;
 	u32 mclk_ratio_frac;
+	u32 mask;
 	u32 val = 0;
 
 	switch (i2s->rev_id) {
@@ -92,14 +94,17 @@ static int loongson_i2s_hw_params(struct snd_pcm_substream *substream,
 		mclk_ratio_frac = DIV_ROUND_CLOSEST_ULL(((u64)clk_rate << 16),
 						    sysclk) - (mclk_ratio << 16);
 
-		regmap_read(i2s->regmap, LS_I2S_CFG, &val);
-		val |= (bits << 24);
-		val |= (bclk_ratio << 8);
-		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
-			val |= (bits << 16);
-		else
-			val |= bits;
-		regmap_write(i2s->regmap, LS_I2S_CFG, val);
+		mask = LS_I2S_CFG_LR_LEN_MASK | LS_I2S_CFG_BCLK_RATIO_MASK;
+		val = FIELD_PREP(LS_I2S_CFG_LR_LEN_MASK, bits) |
+		      FIELD_PREP(LS_I2S_CFG_BCLK_RATIO_MASK, bclk_ratio);
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			mask |= LS_I2S_CFG_TX_RES_DEPTH_MASK;
+			val |= FIELD_PREP(LS_I2S_CFG_TX_RES_DEPTH_MASK, bits);
+		} else {
+			mask |= LS_I2S_CFG_RX_RES_DEPTH_MASK;
+			val |= FIELD_PREP(LS_I2S_CFG_RX_RES_DEPTH_MASK, bits);
+		}
+		regmap_update_bits(i2s->regmap, LS_I2S_CFG, mask, val);
 
 		val = (mclk_ratio_frac << 16) | mclk_ratio;
 		regmap_write(i2s->regmap, LS_I2S_CFG1, val);
diff --git a/sound/soc/loongson/loongson_i2s.h b/sound/soc/loongson/loongson_i2s.h
index 8b4603c876c5..457d90e3d5b6 100644
--- a/sound/soc/loongson/loongson_i2s.h
+++ b/sound/soc/loongson/loongson_i2s.h
@@ -40,6 +40,12 @@
 #define I2S_CTRL_RX_INT_EN	BIT(1)  /* RX interrupt enable */
 #define I2S_CTRL_TX_INT_EN	BIT(0)  /* TX interrupt enable */
 
+/* Loongson I2S Config Register */
+#define LS_I2S_CFG_LR_LEN_MASK		GENMASK(31, 24)
+#define LS_I2S_CFG_TX_RES_DEPTH_MASK	GENMASK(23, 16)
+#define LS_I2S_CFG_BCLK_RATIO_MASK	GENMASK(15, 8)
+#define LS_I2S_CFG_RX_RES_DEPTH_MASK	GENMASK(7, 0)		/* rev 1 */
+
 #define LS_I2S_DRVNAME		"loongson-i2s"
 
 struct loongson_idma_data {
-- 
2.43.0

[PATCH v2 2/3] ASoC: loongson: Use FIELD_PREP() for register fields

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

Use FIELD_PREP() to set the bit fields in the I2S configuration
registers instead of open-coded shifts.

This makes the register field assignments clearer and consistent
between revision 0 and revision 1.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/loongson/loongson_i2s.c | 11 ++++++-----
 sound/soc/loongson/loongson_i2s.h |  5 +++++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/sound/soc/loongson/loongson_i2s.c b/sound/soc/loongson/loongson_i2s.c
index aebea177e7e0..0d5278203024 100644
--- a/sound/soc/loongson/loongson_i2s.c
+++ b/sound/soc/loongson/loongson_i2s.c
@@ -80,10 +80,10 @@ static int loongson_i2s_hw_params(struct snd_pcm_substream *substream,
 		mclk_ratio = DIV_ROUND_CLOSEST(clk_rate, (sysclk * 2)) - 1;
 
 		/* According to 2k1000LA user manual, set bits == depth */
-		val |= (bits << 24);
-		val |= (bits << 16);
-		val |= (bclk_ratio << 8);
-		val |= mclk_ratio;
+		val = FIELD_PREP(LS_I2S_CFG_LR_LEN_MASK, bits) |
+		      FIELD_PREP(LS_I2S_CFG_TX_RES_DEPTH_MASK, bits) |
+		      FIELD_PREP(LS_I2S_CFG_BCLK_RATIO_MASK, bclk_ratio) |
+		      FIELD_PREP(LS_I2S_CFG_MCLK_RATIO_MASK, mclk_ratio);
 		regmap_write(i2s->regmap, LS_I2S_CFG, val);
 
 		break;
@@ -106,7 +106,8 @@ static int loongson_i2s_hw_params(struct snd_pcm_substream *substream,
 		}
 		regmap_update_bits(i2s->regmap, LS_I2S_CFG, mask, val);
 
-		val = (mclk_ratio_frac << 16) | mclk_ratio;
+		val = FIELD_PREP(LS_I2S_CFG1_MCLK_RATIO_FRAC_MASK, mclk_ratio_frac) |
+		      FIELD_PREP(LS_I2S_CFG1_MCLK_RATIO_MASK, mclk_ratio);
 		regmap_write(i2s->regmap, LS_I2S_CFG1, val);
 
 		break;
diff --git a/sound/soc/loongson/loongson_i2s.h b/sound/soc/loongson/loongson_i2s.h
index 457d90e3d5b6..0c280e9b7505 100644
--- a/sound/soc/loongson/loongson_i2s.h
+++ b/sound/soc/loongson/loongson_i2s.h
@@ -45,6 +45,11 @@
 #define LS_I2S_CFG_TX_RES_DEPTH_MASK	GENMASK(23, 16)
 #define LS_I2S_CFG_BCLK_RATIO_MASK	GENMASK(15, 8)
 #define LS_I2S_CFG_RX_RES_DEPTH_MASK	GENMASK(7, 0)		/* rev 1 */
+#define LS_I2S_CFG_MCLK_RATIO_MASK	GENMASK(7, 0)		/* rev 0 */
+
+/* Loongson I2S Config 1 Register (rev 1) */
+#define LS_I2S_CFG1_MCLK_RATIO_FRAC_MASK	GENMASK(31, 16)
+#define LS_I2S_CFG1_MCLK_RATIO_MASK		GENMASK(15, 0)
 
 #define LS_I2S_DRVNAME		"loongson-i2s"
 
-- 
2.43.0

[PATCH v2 3/3] ASoC: loongson: Handle clock enable errors in set_fmt

[phucduc.bui]

From: bui duc phuc <phucduc.bui@gmail.com>

loongson_i2s_set_fmt() treats all clock enable errors as timeouts,
warns, and returns success. This hides errors other than -ETIMEDOUT.

Preserve the existing behaviour for -ETIMEDOUT, but propagate other
errors. Also clear MASTER, MCLK_EN and MSB on the error path to
clean up the controller state, including when the clock provider
format is invalid.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/loongson/loongson_i2s.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/sound/soc/loongson/loongson_i2s.c b/sound/soc/loongson/loongson_i2s.c
index 0d5278203024..438bbae0888f 100644
--- a/sound/soc/loongson/loongson_i2s.c
+++ b/sound/soc/loongson/loongson_i2s.c
@@ -185,34 +185,50 @@ static int loongson_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
 		regmap_update_bits(i2s->regmap, LS_I2S_CTRL, I2S_CTRL_MASTER,
 				   I2S_CTRL_MASTER);
 		ret = loongson_i2s_enable_bclk(i2s);
-		if (ret < 0)
+		if (ret == -ETIMEDOUT)
 			dev_warn(dai->dev, "wait BCLK ready timeout\n");
+		else if (ret)
+			goto err;
 		break;
 	case SND_SOC_DAIFMT_BC_FP:
 		/* Enable MCLK */
 		ret = loongson_i2s_enable_mclk(i2s);
-		if (ret < 0)
+		if (ret == -ETIMEDOUT)
 			dev_warn(dai->dev, "wait MCLK ready timeout\n");
+		else if (ret)
+			goto err;
 		break;
 	case SND_SOC_DAIFMT_BP_FP:
 		/* Enable MCLK */
 		ret = loongson_i2s_enable_mclk(i2s);
-		if (ret < 0)
+		if (ret == -ETIMEDOUT)
 			dev_warn(dai->dev, "wait MCLK ready timeout\n");
+		else if (ret)
+			goto err;
 
 		/* Enable master mode */
 		regmap_update_bits(i2s->regmap, LS_I2S_CTRL, I2S_CTRL_MASTER,
 				   I2S_CTRL_MASTER);
 
 		ret = loongson_i2s_enable_bclk(i2s);
-		if (ret < 0)
+		if (ret == -ETIMEDOUT)
 			dev_warn(dai->dev, "wait BCLK ready timeout\n");
+		else if (ret)
+			goto err;
 		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	return 0;
+
+err:
+	regmap_update_bits(i2s->regmap, LS_I2S_CTRL, I2S_CTRL_MASTER, 0);
+	regmap_update_bits(i2s->regmap, LS_I2S_CTRL, I2S_CTRL_MCLK_EN, 0);
+	regmap_update_bits(i2s->regmap, LS_I2S_CTRL, I2S_CTRL_MSB, 0);
+
+	return ret;
 }
 
 static int loongson_i2s_dai_probe(struct snd_soc_dai *cpu_dai)
-- 
2.43.0