[PATCH v2 00/10] ASoC: fsl_sai: Address several issues in the driver

[PATCH v2 00/10] ASoC: fsl_sai: Address several issues in the driver

[phucduc.bui]

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

Hi all,

I've continued reviewing the FSL SAI driver and referred to the
i.MX RT1180 Reference Manual when looking into its register
configurations and behavior:

https://www.nxp.com/docs/en/reference-manual/IMXRT1180RM.pdf

Link v1: 
https://lore.kernel.org/all/20261030091404.18757-1-phucduc.bui@gmail.com/
Change in v2:
 - Add five patches covering disable handling, documentation, 
   ISR error handling, BCLK ratio validation, 
   and preventing incorrect CHMOD writes to the other direction's xCR4.

Compile-tested only.

Best regards,
Phuc

bui duc phuc (10):
  ASoC: fsl_sai: Fix PM QoS cleanup on resume failure
  ASoC: fsl_sai: Use optional clock getter for MCLKs
  ASoC: fsl_sai: Remove redundant error messages
  ASoC: fsl_sai: Propagate device tree property read errors
  ASoC: fsl_sai: Drop redundant dataline mask AND
  ASoC: fsl_sai: Warn if TE/RE does not clear on disable
  ASoC: fsl_sai: Clarify fsl_sai_dir_is_synced() documentation
  ASoC: fsl_sai: Check regmap_read() return value in ISR
  ASoC: fsl_sai: Reject bclk ratio smaller than slot width
  ASoC: fsl_sai: Don't write CHMOD to the other direction's xCR4

 sound/soc/fsl/fsl_sai.c | 89 +++++++++++++++++++++++------------------
 1 file changed, 49 insertions(+), 40 deletions(-)

-- 
2.43.0

[PATCH v2 01/10] ASoC: fsl_sai: Fix PM QoS cleanup on resume failure

[phucduc.bui]

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

If regcache_sync() fails during runtime resume, the CPU latency
QoS request remains active even though the resume operation
failed.

Switch the regmap back to cache-only mode and remove the PM QoS
request before disabling the clocks, ensuring that the resources
enabled earlier in the resume path are properly cleaned up.

Fixes: 907e0cdebc1f ("ASoC: fsl_sai: Add pm qos cpu latency support")
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index d232c8f53061..042885ba415e 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1981,7 +1981,7 @@ static int fsl_sai_runtime_resume(struct device *dev)
 
 	ret = regcache_sync(sai->regmap);
 	if (ret)
-		goto disable_rx_clk;
+		goto remove_qos;
 
 	if (sai->soc_data->mclk_with_tere && sai->mclk_direction_output)
 		regmap_update_bits(sai->regmap, FSL_SAI_TCSR(ofs),
@@ -1989,7 +1989,12 @@ static int fsl_sai_runtime_resume(struct device *dev)
 
 	return 0;
 
-disable_rx_clk:
+remove_qos:
+	regcache_cache_only(sai->regmap, true);
+
+	if (sai->soc_data->flags & PMQOS_CPU_LATENCY)
+		cpu_latency_qos_remove_request(&sai->pm_qos_req);
+
 	if (sai->mclk_streams & BIT(SNDRV_PCM_STREAM_CAPTURE))
 		clk_disable_unprepare(sai->mclk_clk[sai->mclk_id[0]]);
 disable_tx_clk:
-- 
2.43.0

[PATCH v2 02/10] ASoC: fsl_sai: Use optional clock getter for MCLKs

[phucduc.bui]

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

The current code uses devm_clk_get() and sets the clock pointer
to NULL when it returns an error. However, this silently ignores
errors such as -EPROBE_DEFER, allowing the driver to continue
probing without the requested clock.

Use devm_clk_get_optional() to handle missing optional clocks
automatically, and dev_err_probe() to properly propagate other
errors, including -EPROBE_DEFER.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 042885ba415e..44a34f48a0cf 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1589,12 +1589,10 @@ static int fsl_sai_probe(struct platform_device *pdev)
 
 	for (i = 1; i < FSL_SAI_MCLK_MAX; i++) {
 		sprintf(tmp, "mclk%d", i);
-		sai->mclk_clk[i] = devm_clk_get(dev, tmp);
-		if (IS_ERR(sai->mclk_clk[i])) {
-			dev_err(dev, "failed to get mclk%d clock: %ld\n",
-					i, PTR_ERR(sai->mclk_clk[i]));
-			sai->mclk_clk[i] = NULL;
-		}
+		sai->mclk_clk[i] = devm_clk_get_optional(dev, tmp);
+		if (IS_ERR(sai->mclk_clk[i]))
+			return dev_err_probe(dev, PTR_ERR(sai->mclk_clk[i]),
+					     "failed to get mclk%d clock\n", i);
 	}
 
 	if (sai->soc_data->mclk0_is_mclk1)
-- 
2.43.0

[PATCH v2 03/10] ASoC: fsl_sai: Remove redundant error messages

[phucduc.bui]

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

The errors are either reported directly by these functions
or already reported deeper in the call chain, so additional
dev_err() and dev_warn() calls are unnecessary.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 44a34f48a0cf..5532fab80be0 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1615,10 +1615,8 @@ static int fsl_sai_probe(struct platform_device *pdev)
 
 	/* read dataline mask for rx and tx*/
 	ret = fsl_sai_read_dlcfg(sai);
-	if (ret < 0) {
-		dev_err(dev, "failed to read dlcfg %d\n", ret);
+	if (ret < 0)
 		return ret;
-	}
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -1698,9 +1696,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
 	if (ret < 0)
 		dev_warn(dev, "Error reading SAI version: %d\n", ret);
 
-	ret = fsl_sai_reset_hw(dev);
-	if (ret < 0)
-		dev_warn(dev, "Failed to reset hardware: %d\n", ret);
+	fsl_sai_reset_hw(dev);
 
 	/* Select MCLK direction */
 	if (sai->mclk_direction_output &&
@@ -1715,10 +1711,8 @@ static int fsl_sai_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(dev, irq, fsl_sai_isr, IRQF_SHARED,
 			       np->name, sai);
-	if (ret) {
-		dev_err(dev, "failed to claim irq %u\n", irq);
+	if (ret)
 		goto err_pm_get_sync;
-	}
 
 	if (of_device_is_compatible(np, "fsl,imx952-sai") &&
 	    !of_property_read_string(np, "fsl,sai-amix-mode", &str)) {
-- 
2.43.0

Re: [PATCH v2 03/10] ASoC: fsl_sai: Remove redundant error messages

[Mark Brown]

> The errors are either reported directly by these functions
> or already reported deeper in the call chain, so additional
> dev_err() and dev_warn() calls are unnecessary.

> @@ -1698,9 +1696,7 @@ static int fsl_sai_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		dev_warn(dev, "Error reading SAI version: %d\n", ret);
>  
> -	ret = fsl_sai_reset_hw(dev);
> -	if (ret < 0)
> -		dev_warn(dev, "Failed to reset hardware: %d\n", ret);
> +	fsl_sai_reset_hw(dev);

This is the only caller of fsl_sai_reset_hw(), it could just return void
now.

>  	ret = devm_request_irq(dev, irq, fsl_sai_isr, IRQF_SHARED,
>  			       np->name, sai);
> -	if (ret) {
> -		dev_err(dev, "failed to claim irq %u\n", irq);
> +	if (ret)
>  		goto err_pm_get_sync;
> -	}

This will only print an error for a subset of errors, things like -EBUSY
will now silently fail.

-- 
Mark Brown <broonie@kernel.org>

Re: [PATCH v2 03/10] ASoC: fsl_sai: Remove redundant error messages

[Bui Duc Phuc]

Hi Mark,

Thank you for your review.

> >
> > -     ret = fsl_sai_reset_hw(dev);
> > -     if (ret < 0)
> > -             dev_warn(dev, "Failed to reset hardware: %d\n", ret);
> > +     fsl_sai_reset_hw(dev);
>
> This is the only caller of fsl_sai_reset_hw(), it could just return void
> now.
>

You're right.
I'll change fsl_sai_reset_hw() to return void.
Since this series has already been applied,
I'll send this as a separate patch.


> >       ret = devm_request_irq(dev, irq, fsl_sai_isr, IRQF_SHARED,
> >                              np->name, sai);
> > -     if (ret) {
> > -             dev_err(dev, "failed to claim irq %u\n", irq);
> > +     if (ret)
> >               goto err_pm_get_sync;
> > -     }
>
> This will only print an error for a subset of errors, things like -EBUSY
> will now silently fail.
>

From what I can see in the code, devm_request_irq() ->
devm_request_threaded_irq() reports every failure via dev_err_probe()
in devm_request_result(), so -EBUSY should be printed too.

Could you please point me to the path that fails silently?
I may have missed it.

Or did you mean fsl_sai_read_dlcfg()? It does have some silent
-EINVAL returns.
I'll restore that dev_err() together with the fsl_sai_reset_hw()
change I mentioned earlier.

Best regards,
Phuc

Re: [PATCH v2 03/10] ASoC: fsl_sai: Remove redundant error messages

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

On Mon, Nov 09, 2026 at 11:08:05AM +0700, Bui Duc Phuc wrote:

> > >       ret = devm_request_irq(dev, irq, fsl_sai_isr, IRQF_SHARED,
> > >                              np->name, sai);
> > > -     if (ret) {
> > > -             dev_err(dev, "failed to claim irq %u\n", irq);
> > > +     if (ret)
> > >               goto err_pm_get_sync;
> > > -     }

> > This will only print an error for a subset of errors, things like -EBUSY
> > will now silently fail.

> From what I can see in the code, devm_request_irq() ->
> devm_request_threaded_irq() reports every failure via dev_err_probe()
> in devm_request_result(), so -EBUSY should be printed too.

Ah, that's changed since I last looked.

> Or did you mean fsl_sai_read_dlcfg()? It does have some silent
> -EINVAL returns.
> I'll restore that dev_err() together with the fsl_sai_reset_hw()
> change I mentioned earlier.

That too.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 04/10] ASoC: fsl_sai: Propagate device tree property read errors

[phucduc.bui]

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

Return the original error codes from of_property_read_u32_index()
instead of replacing them with -EINVAL.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 5532fab80be0..cb07e7d13132 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1495,15 +1495,15 @@ static int fsl_sai_read_dlcfg(struct fsl_sai *sai)
 		 */
 		ret = of_property_read_u32_index(np, propname, index++, &type);
 		if (ret)
-			return -EINVAL;
+			return ret;
 
 		ret = of_property_read_u32_index(np, propname, index++, &rx);
 		if (ret)
-			return -EINVAL;
+			return ret;
 
 		ret = of_property_read_u32_index(np, propname, index++, &tx);
 		if (ret)
-			return -EINVAL;
+			return ret;
 
 		if ((rx & ~soc_dl) || (tx & ~soc_dl)) {
 			dev_err(dev, "dataline cfg[%d] setting error, mask is 0x%x\n", i, soc_dl);
-- 
2.43.0

[PATCH v2 05/10] ASoC: fsl_sai: Drop redundant dataline mask AND

[phucduc.bui]

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

The rx/tx masks with bits outside soc_dl are already rejected just above,
so masking them with soc_dl again is no-op. Remove it.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index cb07e7d13132..8315a0d8893b 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -1510,9 +1510,6 @@ static int fsl_sai_read_dlcfg(struct fsl_sai *sai)
 			return -EINVAL;
 		}
 
-		rx = rx & soc_dl;
-		tx = tx & soc_dl;
-
 		cfg[i].type = type;
 		cfg[i].pins[0] = hweight8(rx);
 		cfg[i].mask[0] = rx;
-- 
2.43.0

[PATCH v2 06/10] ASoC: fsl_sai: Warn if TE/RE does not clear on disable

[phucduc.bui]

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

fsl_sai_config_disable() polls TE/RE with an open-coded loop and
continues silently if the bit does not clear within 1ms. Also, if
regmap_read() fails, the loop condition evaluates xcsr uninitialized.

Use regmap_read_poll_timeout_atomic() and warn on timeout.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 8315a0d8893b..f75a1d61d7b7 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -849,7 +849,8 @@ static void fsl_sai_config_disable(struct fsl_sai *sai, int dir)
 {
 	unsigned int ofs = sai->soc_data->reg_offset;
 	bool tx = dir == TX;
-	u32 xcsr, count = 100, mask;
+	u32 xcsr, mask;
+	int ret;
 
 	if (sai->soc_data->mclk_with_tere && sai->mclk_direction_output)
 		mask = FSL_SAI_CSR_TERE;
@@ -860,10 +861,15 @@ static void fsl_sai_config_disable(struct fsl_sai *sai, int dir)
 			   mask, 0);
 
 	/* TERE will remain set till the end of current frame */
-	do {
-		udelay(10);
-		regmap_read(sai->regmap, FSL_SAI_xCSR(tx, ofs), &xcsr);
-	} while (--count && xcsr & FSL_SAI_CSR_TERE);
+	ret = regmap_read_poll_timeout_atomic(sai->regmap, FSL_SAI_xCSR(tx, ofs),
+					      xcsr, !(xcsr & FSL_SAI_CSR_TERE),
+					      10, 1000);
+	if (ret == -ETIMEDOUT)
+		dev_warn(&sai->pdev->dev, "%cx not disabled at frame end\n",
+			 tx ? 'T' : 'R');
+	else if (ret)
+		dev_err(&sai->pdev->dev, "failed to read %cSCR : %d\n",
+			tx ? 'T' : 'R', ret);
 
 	regmap_update_bits(sai->regmap, FSL_SAI_xCSR(tx, ofs),
 			   FSL_SAI_CSR_FR, FSL_SAI_CSR_FR);
-- 
2.43.0

Re: [PATCH v2 06/10] ASoC: fsl_sai: Warn if TE/RE does not clear on disable

[Mark Brown]

> fsl_sai_config_disable() polls TE/RE with an open-coded loop and
> continues silently if the bit does not clear within 1ms. Also, if
> regmap_read() fails, the loop condition evaluates xcsr uninitialized.

>  	/* TERE will remain set till the end of current frame */
> -	do {
> -		udelay(10);
> -		regmap_read(sai->regmap, FSL_SAI_xCSR(tx, ofs), &xcsr);
> -	} while (--count && xcsr & FSL_SAI_CSR_TERE);
> +	ret = regmap_read_poll_timeout_atomic(sai->regmap, FSL_SAI_xCSR(tx, ofs),
> +					      xcsr, !(xcsr & FSL_SAI_CSR_TERE),
> +					      10, 1000);
> +	if (ret == -ETIMEDOUT)
> +		dev_warn(&sai->pdev->dev, "%cx not disabled at frame end\n",

There wasn't a warning before...

> +	else if (ret)
> +		dev_err(&sai->pdev->dev, "failed to read %cSCR : %d\n",

%cCSR.

-- 
Mark Brown <broonie@kernel.org>

Re: [PATCH v2 06/10] ASoC: fsl_sai: Warn if TE/RE does not clear on disable

[Bui Duc Phuc]

Hi Mark,

Thank you for your reviews.

> > +     if (ret == -ETIMEDOUT)
> > +             dev_warn(&sai->pdev->dev, "%cx not disabled at frame end\n",
>
> There wasn't a warning before...
>
> > +     else if (ret)
> > +             dev_err(&sai->pdev->dev, "failed to read %cSCR : %d\n",
>

That's correct. Since this function returns void, previous failures and
abnormal hardware conditions were silently ignored. I added a warning
for timeouts and an error message for regmap read failures to make
these issues visible and easier to diagnose.

Best regards,
Phuc

[PATCH v2 07/10] ASoC: fsl_sai: Clarify fsl_sai_dir_is_synced() documentation

[phucduc.bui]

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

The summary line of fsl_sai_dir_is_synced() reads as if @dir follows
the clocks of the opposite stream, while the function returns true
when @dir provides the clocks and the opposite stream is synchronous.

Reword the description to match the behavior and document the return
value. No functional change.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index f75a1d61d7b7..25243b6859d6 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -88,14 +88,16 @@ static const struct snd_kcontrol_new fsl_sai_timestamp_ctrls[] = {
 };
 
 /**
- * fsl_sai_dir_is_synced - Check if stream is synced by the opposite stream
- *
- * SAI supports synchronous mode using bit/frame clocks of either Transmitter's
- * or Receiver's for both streams. This function is used to check if clocks of
- * the stream's are synced by the opposite stream.
- *
+ * fsl_sai_dir_is_synced - Check if the opposite stream is synced to @dir
  * @sai: SAI context
- * @dir: stream direction
+ * @dir: stream direction, TX or RX
+ *
+ * SAI supports synchronous mode, where one direction uses the bit clock and
+ * frame sync of the other. This function checks whether @dir provides the
+ * clocks, i.e. @dir runs in asynchronous mode while the opposite direction
+ * runs in synchronous mode.
+ *
+ * Return: true if the opposite direction uses the clocks of @dir.
  */
 static inline bool fsl_sai_dir_is_synced(struct fsl_sai *sai, int dir)
 {
-- 
2.43.0

[PATCH v2 08/10] ASoC: fsl_sai: Check regmap_read() return value in ISR

[phucduc.bui]

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

The IRQ is shared, so fsl_sai_isr() may run while the SAI is runtime
suspended. The regmap is then in cache-only mode, so regmap_read()
fails and xcsr is used uninitialized.

Check the return value of regmap_read() for both TCSR and RCSR, and
skip the handling of the corresponding direction on failure.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 25243b6859d6..5c50ed7c2258 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -148,7 +148,8 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid)
 	mask = (FSL_SAI_FLAGS >> FSL_SAI_CSR_xIE_SHIFT) << FSL_SAI_CSR_xF_SHIFT;
 
 	/* Tx IRQ */
-	regmap_read(sai->regmap, FSL_SAI_TCSR(ofs), &xcsr);
+	if (regmap_read(sai->regmap, FSL_SAI_TCSR(ofs), &xcsr))
+		return IRQ_NONE;
 	flags = xcsr & mask;
 
 	if (flags)
@@ -179,7 +180,8 @@ static irqreturn_t fsl_sai_isr(int irq, void *devid)
 
 irq_rx:
 	/* Rx IRQ */
-	regmap_read(sai->regmap, FSL_SAI_RCSR(ofs), &xcsr);
+	if (regmap_read(sai->regmap, FSL_SAI_RCSR(ofs), &xcsr))
+		goto out;
 	flags = xcsr & mask;
 
 	if (flags)
-- 
2.43.0

[PATCH v2 09/10] ASoC: fsl_sai: Reject bclk ratio smaller than slot width

[phucduc.bui]

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

In hw_params(), when a bclk ratio is set, the number of slots is
computed as bclk_ratio / slot_width. If the ratio is smaller than
the slot width, slots becomes zero and the following
DIV_ROUND_UP(channels, slots) divides by zero.

Return -EINVAL in this case.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 5c50ed7c2258..c61703ddb6f8 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -632,8 +632,14 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 
 	if (sai->slots[tx])
 		slots = sai->slots[tx];
-	else if (sai->bclk_ratio)
+	else if (sai->bclk_ratio) {
+		if (sai->bclk_ratio < slot_width) {
+			dev_err(cpu_dai->dev, "bclk ratio %u < slot width %u\n",
+				sai->bclk_ratio, slot_width);
+			return -EINVAL;
+		}
 		slots = sai->bclk_ratio / slot_width;
+	}
 
 	pins = DIV_ROUND_UP(channels, slots);
 
-- 
2.43.0

[PATCH v2 10/10] ASoC: fsl_sai: Don't write CHMOD to the other direction's xCR4

[phucduc.bui]

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

When the current direction is synced to the other one, hw_params()
also writes the frame configuration to the other direction's xCR4.
The mask includes CHMOD, whose value is set by the current direction
(Tx only) but written to the other one, so it is wrong:
- TCR4[CHMOD] is cleared, while Tx needs it set (output mode).
- RCR4 bit 5 is set, while it is reserved and must be written as 0.

CHMOD is not part of the frame configuration, so drop it from the mask.

Fixes: f4c4b1bb2f5a ("ASoC: fsl_sai: Set SAI Channel Mode to Output Mode")
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
 sound/soc/fsl/fsl_sai.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index c61703ddb6f8..430e896d064e 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -720,8 +720,7 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
 
 	if (fsl_sai_dir_is_synced(sai, adir)) {
 		regmap_update_bits(sai->regmap, FSL_SAI_xCR4(!tx, ofs),
-				   FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
-				   FSL_SAI_CR4_CHMOD_MASK,
+				   FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
 				   val_cr4);
 		regmap_update_bits(sai->regmap, FSL_SAI_xCR5(!tx, ofs),
 				   FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
-- 
2.43.0

Re: [PATCH v2 00/10] ASoC: fsl_sai: Address several issues in the driver

[Shengjiu Wang]

On Thu, Nov 5, 2026 at 6:43 PM <phucduc.bui@gmail.com> wrote:
>
> From: bui duc phuc <phucduc.bui@gmail.com>
>
> Hi all,
>
> I've continued reviewing the FSL SAI driver and referred to the
> i.MX RT1180 Reference Manual when looking into its register
> configurations and behavior:
>
> https://www.nxp.com/docs/en/reference-manual/IMXRT1180RM.pdf
>
> Link v1:
> https://lore.kernel.org/all/20261030091404.18757-1-phucduc.bui@gmail.com/
> Change in v2:
>  - Add five patches covering disable handling, documentation,
>    ISR error handling, BCLK ratio validation,
>    and preventing incorrect CHMOD writes to the other direction's xCR4.
>
> Compile-tested only.
>

The series looks good to me.

Reviewed-by: Shengjiu Wang <shengjiu.wang@gmail.com>

Best regards
Shengjiu Wang

> Best regards,
> Phuc
>
> bui duc phuc (10):
>   ASoC: fsl_sai: Fix PM QoS cleanup on resume failure
>   ASoC: fsl_sai: Use optional clock getter for MCLKs
>   ASoC: fsl_sai: Remove redundant error messages
>   ASoC: fsl_sai: Propagate device tree property read errors
>   ASoC: fsl_sai: Drop redundant dataline mask AND
>   ASoC: fsl_sai: Warn if TE/RE does not clear on disable
>   ASoC: fsl_sai: Clarify fsl_sai_dir_is_synced() documentation
>   ASoC: fsl_sai: Check regmap_read() return value in ISR
>   ASoC: fsl_sai: Reject bclk ratio smaller than slot width
>   ASoC: fsl_sai: Don't write CHMOD to the other direction's xCR4
>
>  sound/soc/fsl/fsl_sai.c | 89 +++++++++++++++++++++++------------------
>  1 file changed, 49 insertions(+), 40 deletions(-)
>
> --
> 2.43.0
>

Re: [PATCH v2 00/10] ASoC: fsl_sai: Address several issues in the driver

[Bui Duc Phuc]

Hi Shengjiu Wang

>
> The series looks good to me.
>
> Reviewed-by: Shengjiu Wang <shengjiu.wang@gmail.com>
>

Thanks for reviewing the series!

I'll continue looking into other parts of the FSL audio drivers,
such as ASRC, PDM, and SPDIF, next week.

Best regards,
Phuc

Re: [PATCH v2 00/10] ASoC: fsl_sai: Address several issues in the driver

[Mark Brown]

On Thu, 05 Nov 2026 17:42:27 +0700, phucduc.bui@gmail.com wrote:
> ASoC: fsl_sai: Address several issues in the driver
> 
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> Hi all,
> 
> I've continued reviewing the FSL SAI driver and referred to the
> i.MX RT1180 Reference Manual when looking into its register
> configurations and behavior:
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.5

Thanks!

[01/10] ASoC: fsl_sai: Fix PM QoS cleanup on resume failure
        https://git.kernel.org/broonie/sound/c/f804ad135529
[02/10] ASoC: fsl_sai: Use optional clock getter for MCLKs
        https://git.kernel.org/broonie/sound/c/ee95db533522
[03/10] ASoC: fsl_sai: Remove redundant error messages
        https://git.kernel.org/broonie/sound/c/b73124cd7b91
[04/10] ASoC: fsl_sai: Propagate device tree property read errors
        https://git.kernel.org/broonie/sound/c/702a9978d580
[05/10] ASoC: fsl_sai: Drop redundant dataline mask AND
        https://git.kernel.org/broonie/sound/c/a11ee78a0b25
[06/10] ASoC: fsl_sai: Warn if TE/RE does not clear on disable
        https://git.kernel.org/broonie/sound/c/db36f0b28b11
[07/10] ASoC: fsl_sai: Clarify fsl_sai_dir_is_synced() documentation
        https://git.kernel.org/broonie/sound/c/be10761cad31
[08/10] ASoC: fsl_sai: Check regmap_read() return value in ISR
        https://git.kernel.org/broonie/sound/c/b485bfcb12a8
[09/10] ASoC: fsl_sai: Reject bclk ratio smaller than slot width
        https://git.kernel.org/broonie/sound/c/279ea69e6df5
[10/10] ASoC: fsl_sai: Don't write CHMOD to the other direction's xCR4
        https://git.kernel.org/broonie/sound/c/9580e6ecce63

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark