[RFC PATCH 0/2] ASoC: loongson: Fix configuration and clock error handling

[RFC PATCH 0/2] ASoC: loongson: Fix configuration and clock error handling

[phucduc.bui]

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

Hi all,

While reviewing the driver code, I noticed two potential issues and
looked into the Loongson I2S documentation to investigate them.
This RFC series proposes fixes for these issues.

I don't have the hardware to test these changes, so I'd appreciate
your review and feedback.

Compile-tested only.

Best regards,
Phuc

bui duc phuc (2):
  ASoC: loongson: Fix IISConfig0 update in hw_params for rev 1
  ASoC: loongson: Handle clock enable errors in set_fmt

 sound/soc/loongson/loongson_i2s.c | 44 +++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 11 deletions(-)

-- 
2.43.0

[RFC PATCH 1/2] 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>
---

Compile-tested only, I don't have the hardware. 
A Tested-by from someone with a 2K0300, 2K2000 board would be appreciated.

The field layout follows the Loongson-2K0300 user manual (IISConfig0) [1].
 
Not sure whether concurrent playback and capture is supported on this controller. 
If it is, note that LR_LEN and BCLK_RATIO are shared, so the stream started later 
would overwrite them when the sample width or channel count differs. 
I can follow up with symmetric_channels and symmetric_sample_bits if that is 
the right fix.

[1] https://github.com/loongson-community/docs/blob/master/2K0300/2025060909243029508.%E9%BE%99%E8%8A%AF2K0300%E5%A4%84%E7%90%86%E5%99%A8%E7%94%A8%E6%88%B7%E6%89%8B%E5%86%8C_V1.01.pdf

 sound/soc/loongson/loongson_i2s.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/sound/soc/loongson/loongson_i2s.c b/sound/soc/loongson/loongson_i2s.c
index da0d029c028a..109cb61c6ddf 100644
--- a/sound/soc/loongson/loongson_i2s.c
+++ b/sound/soc/loongson/loongson_i2s.c
@@ -69,6 +69,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 +93,19 @@ 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)
+		/* Shared fields: LR_LEN [31:24], BCLK_RATIO [15:8] */
+		mask = GENMASK(31, 24) | GENMASK(15, 8);
+		val = (bits << 24) | (bclk_ratio << 8);
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
+			/* TX_RES_DEPTH */
+			mask |= GENMASK(23, 16);
 			val |= (bits << 16);
-		else
+		} else {
+			/* RX_RES_DEPTH */
+			mask |= GENMASK(7, 0);
 			val |= bits;
-		regmap_write(i2s->regmap, LS_I2S_CFG, val);
+		}
+		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);
-- 
2.43.0

[RFC PATCH 2/2] 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 109cb61c6ddf..4823b89cab8a 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

Re: [RFC PATCH 1/2] ASoC: loongson: Fix IISConfig0 update in hw_params for rev 1

[Binbin Zhou]

Hi phucduc:

Thanks for your patch.

On Thu, Oct 22, 2026 at 4:46 PM <phucduc.bui@gmail.com> wrote:
>
> 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.

I tested this patch on a Loongson-2K0300 and Loongson-2K2000 board
(LoongsonASoC sound card).

I ran consecutive playback and capture tests alternating between
S16_LE/44100 Hz and S24_LE/48000 Hz, both in stereo. After applying
this patch, switching between different sample formats and rates, works
correctly.

>
> 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>
> ---
>
> Compile-tested only, I don't have the hardware.
> A Tested-by from someone with a 2K0300, 2K2000 board would be appreciated.
>
> The field layout follows the Loongson-2K0300 user manual (IISConfig0) [1].
>
> Not sure whether concurrent playback and capture is supported on this controller.
> If it is, note that LR_LEN and BCLK_RATIO are shared, so the stream started later
> would overwrite them when the sample width or channel count differs.
> I can follow up with symmetric_channels and symmetric_sample_bits if that is
> the right fix.
>
> [1] https://github.com/loongson-community/docs/blob/master/2K0300/2025060909243029508.%E9%BE%99%E8%8A%AF2K0300%E5%A4%84%E7%90%86%E5%99%A8%E7%94%A8%E6%88%B7%E6%89%8B%E5%86%8C_V1.01.pdf
>
>  sound/soc/loongson/loongson_i2s.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/sound/soc/loongson/loongson_i2s.c b/sound/soc/loongson/loongson_i2s.c
> index da0d029c028a..109cb61c6ddf 100644
> --- a/sound/soc/loongson/loongson_i2s.c
> +++ b/sound/soc/loongson/loongson_i2s.c
> @@ -69,6 +69,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 +93,19 @@ 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)
> +               /* Shared fields: LR_LEN [31:24], BCLK_RATIO [15:8] */
> +               mask = GENMASK(31, 24) | GENMASK(15, 8);
> +               val = (bits << 24) | (bclk_ratio << 8);
> +               if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +                       /* TX_RES_DEPTH */
> +                       mask |= GENMASK(23, 16);
>                         val |= (bits << 16);
> -               else
> +               } else {
> +                       /* RX_RES_DEPTH */
> +                       mask |= GENMASK(7, 0);

One minor suggestion: would it be possible to define these bit fields via
macros, or use FIELD_PREP()/FIELD_GET() with GENMASK(), instead of
open-coded shifts and masks? For example:

#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)

>                         val |= bits;
> -               regmap_write(i2s->regmap, LS_I2S_CFG, val);
> +               }
> +               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);
> --
> 2.43.0
>
>

-- 
Thanks.
Binbin

Re: [RFC PATCH 1/2] ASoC: loongson: Fix IISConfig0 update in hw_params for rev 1

[Bui Duc Phuc]

Hi  Binbin,

> > 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.
>
> I tested this patch on a Loongson-2K0300 and Loongson-2K2000 board
> (LoongsonASoC sound card).
>
> I ran consecutive playback and capture tests alternating between
> S16_LE/44100 Hz and S24_LE/48000 Hz, both in stereo. After applying
> this patch, switching between different sample formats and rates, works
> correctly.
>

Thanks for testing this on both boards!

>
> One minor suggestion: would it be possible to define these bit fields via
> macros, or use FIELD_PREP()/FIELD_GET() with GENMASK(), instead of
> open-coded shifts and masks? For example:
>
> #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)
>

I'll update the bit-field definitions as suggested and also apply the
same approach
to CFG1 and rev 0 to keep the code consistent in hw_params().

I'll send v2 soon.

Best regards,
Phuc