* [PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params
@ 2026-10-23 10:28 phucduc.bui
2026-10-27 6:31 ` Gaddipati, Naveen Kumar
0 siblings, 1 reply; 2+ messages in thread
From: phucduc.bui @ 2026-10-23 10:28 UTC (permalink / raw)
To: Vincenzo Frascino, Mark Brown, Michal Simek
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Maruthi Srinivas Bayyavarapu, linux-sound, linux-arm-kernel,
linux-kernel, bui duc phuc
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params
2026-10-23 10:28 [PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params phucduc.bui
@ 2026-10-27 6:31 ` Gaddipati, Naveen Kumar
0 siblings, 0 replies; 2+ messages in thread
From: Gaddipati, Naveen Kumar @ 2026-10-27 6:31 UTC (permalink / raw)
To: phucduc.bui, Vincenzo Frascino, Mark Brown, Michal Simek
Cc: Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Maruthi Srinivas Bayyavarapu, linux-sound, linux-arm-kernel,
linux-kernel
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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-27 6:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-23 10:28 [PATCH] ASoC: xilinx: formatter_pcm: Fix read-modify-write of AUD_CTRL in hw_params phucduc.bui
2026-10-27 6:31 ` Gaddipati, Naveen Kumar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®