[PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params

[PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params

[phucduc.bui]

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

xlnx_formatter_pcm_hw_params() does a read-modify-write on XLNX_AUD_CTRL
but only ORs in the new data width and active channel values without
clearing the old ones. If hw_params is called more than once on an
open stream, these fields may retain bits from the previous configuration,
resulting in an incorrect sample width or channel count.

Clear both fields before writing the new values, and use FIELD_PREP() so
the values cannot spill into neighbouring bits.

Fixes: 6f6c3c36f091 ("ASoC: xlnx: add pcm formatter platform driver")
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---

Compile-tested only.

The field masks follow the Control register layout in the Audio Formatter product guide (PG330).
https://docs.amd.com/r/en-US/pg330-audio-formatter/S2MM-Control-Register-0x10

 sound/soc/xilinx/xlnx_formatter_pcm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/sound/soc/xilinx/xlnx_formatter_pcm.c b/sound/soc/xilinx/xlnx_formatter_pcm.c
index 304220d9aaf5..781508aef885 100644
--- a/sound/soc/xilinx/xlnx_formatter_pcm.c
+++ b/sound/soc/xilinx/xlnx_formatter_pcm.c
@@ -6,6 +6,7 @@
 //
 // Author: Maruthi Srinivas Bayyavarapu <maruthis@xilinx.com>
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -55,8 +56,8 @@
 #define CFG_S2MM_XFER_SHIFT	29
 #define CFG_S2MM_PKG_MASK	BIT(28)
 
-#define AUD_CTRL_DATA_WIDTH_SHIFT	16
-#define AUD_CTRL_ACTIVE_CH_SHIFT	19
+#define AUD_CTRL_DATA_WIDTH_MASK	GENMASK(18, 16)
+#define AUD_CTRL_ACTIVE_CH_MASK		GENMASK(22, 19)
 #define PERIOD_CFG_PERIODS_SHIFT	16
 
 #define PERIODS_MIN		2
@@ -506,28 +507,29 @@ static int xlnx_formatter_pcm_hw_params(struct snd_soc_component *component,
 	writel(high, stream_data->mmio + XLNX_AUD_BUFF_ADDR_MSB);
 
 	val = readl(stream_data->mmio + XLNX_AUD_CTRL);
+	val &= ~(AUD_CTRL_DATA_WIDTH_MASK | AUD_CTRL_ACTIVE_CH_MASK);
 	bits_per_sample = params_width(params);
 	switch (bits_per_sample) {
 	case 8:
-		val |= (BIT_DEPTH_8 << AUD_CTRL_DATA_WIDTH_SHIFT);
+		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_8);
 		break;
 	case 16:
-		val |= (BIT_DEPTH_16 << AUD_CTRL_DATA_WIDTH_SHIFT);
+		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_16);
 		break;
 	case 20:
-		val |= (BIT_DEPTH_20 << AUD_CTRL_DATA_WIDTH_SHIFT);
+		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_20);
 		break;
 	case 24:
-		val |= (BIT_DEPTH_24 << AUD_CTRL_DATA_WIDTH_SHIFT);
+		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_24);
 		break;
 	case 32:
-		val |= (BIT_DEPTH_32 << AUD_CTRL_DATA_WIDTH_SHIFT);
+		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_32);
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	val |= active_ch << AUD_CTRL_ACTIVE_CH_SHIFT;
+	val |= FIELD_PREP(AUD_CTRL_ACTIVE_CH_MASK, active_ch);
 	writel(val, stream_data->mmio + XLNX_AUD_CTRL);
 
 	val = (params_periods(params) << PERIOD_CFG_PERIODS_SHIFT)
-- 
2.43.0

Re: [PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params

[Gaddipati, Naveen Kumar]

On 10/23/2026 3:58 PM, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> xlnx_formatter_pcm_hw_params() does a read-modify-write on XLNX_AUD_CTRL
> but only ORs in the new data width and active channel values without
> clearing the old ones. If hw_params is called more than once on an
> open stream, these fields may retain bits from the previous configuration,
> resulting in an incorrect sample width or channel count.
> 
> Clear both fields before writing the new values, and use FIELD_PREP() so
> the values cannot spill into neighbouring bits.
> 
> Fixes: 6f6c3c36f091 ("ASoC: xlnx: add pcm formatter platform driver")
> Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
> ---
> 
> Compile-tested only.
> 
> The field masks follow the Control register layout in the Audio Formatter product guide (PG330).
> https://docs.amd.com/r/en-US/pg330-audio-formatter/S2MM-Control-Register-0x10
> 
>   sound/soc/xilinx/xlnx_formatter_pcm.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/soc/xilinx/xlnx_formatter_pcm.c b/sound/soc/xilinx/xlnx_formatter_pcm.c
> index 304220d9aaf5..781508aef885 100644
> --- a/sound/soc/xilinx/xlnx_formatter_pcm.c
> +++ b/sound/soc/xilinx/xlnx_formatter_pcm.c
> @@ -6,6 +6,7 @@
>   //
>   // Author: Maruthi Srinivas Bayyavarapu <maruthis@xilinx.com>
>   
> +#include <linux/bitfield.h>
>   #include <linux/clk.h>
>   #include <linux/io.h>
>   #include <linux/module.h>
> @@ -55,8 +56,8 @@
>   #define CFG_S2MM_XFER_SHIFT	29
>   #define CFG_S2MM_PKG_MASK	BIT(28)
>   
> -#define AUD_CTRL_DATA_WIDTH_SHIFT	16
> -#define AUD_CTRL_ACTIVE_CH_SHIFT	19
> +#define AUD_CTRL_DATA_WIDTH_MASK	GENMASK(18, 16)
> +#define AUD_CTRL_ACTIVE_CH_MASK		GENMASK(22, 19)
>   #define PERIOD_CFG_PERIODS_SHIFT	16
>   
>   #define PERIODS_MIN		2
> @@ -506,28 +507,29 @@ static int xlnx_formatter_pcm_hw_params(struct snd_soc_component *component,
>   	writel(high, stream_data->mmio + XLNX_AUD_BUFF_ADDR_MSB);
>   
>   	val = readl(stream_data->mmio + XLNX_AUD_CTRL);
> +	val &= ~(AUD_CTRL_DATA_WIDTH_MASK | AUD_CTRL_ACTIVE_CH_MASK);
>   	bits_per_sample = params_width(params);
>   	switch (bits_per_sample) {
>   	case 8:
> -		val |= (BIT_DEPTH_8 << AUD_CTRL_DATA_WIDTH_SHIFT);
> +		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_8);
>   		break;
>   	case 16:
> -		val |= (BIT_DEPTH_16 << AUD_CTRL_DATA_WIDTH_SHIFT);
> +		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_16);
>   		break;
>   	case 20:
> -		val |= (BIT_DEPTH_20 << AUD_CTRL_DATA_WIDTH_SHIFT);
> +		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_20);
>   		break;
>   	case 24:
> -		val |= (BIT_DEPTH_24 << AUD_CTRL_DATA_WIDTH_SHIFT);
> +		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_24);
>   		break;
>   	case 32:
> -		val |= (BIT_DEPTH_32 << AUD_CTRL_DATA_WIDTH_SHIFT);
> +		val |= FIELD_PREP(AUD_CTRL_DATA_WIDTH_MASK, BIT_DEPTH_32);
>   		break;
>   	default:
>   		return -EINVAL;
>   	}
>   
> -	val |= active_ch << AUD_CTRL_ACTIVE_CH_SHIFT;
> +	val |= FIELD_PREP(AUD_CTRL_ACTIVE_CH_MASK, active_ch);
>   	writel(val, stream_data->mmio + XLNX_AUD_CTRL);
>   
>   	val = (params_periods(params) << PERIOD_CFG_PERIODS_SHIFT)

Reviewed-by: Gaddipati, Naveen <naveen.gaddipati@amd.com>

Thanks,
Naveen

Re: [PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params

[Bui Duc Phuc]

Hi Naveen,

>
> Reviewed-by: Gaddipati, Naveen <naveen.gaddipati@amd.com>
>

Thanks for reviewing!

Best regards,
Phuc

Re: [PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params

[Mark Brown]

On Fri, 23 Oct 2026 17:28:18 +0700, phucduc.bui@gmail.com wrote:
> ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params

Applied to

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

Thanks!

[1/1] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params
      https://git.kernel.org/broonie/sound/c/3b54e7392737

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