[PATCH] ASoC: amd: acp: Fix incorrect retrival of acp_chip_info

[PATCH] ASoC: amd: acp: Fix incorrect retrival of acp_chip_info

[Venkata Prasad Potturu]

Use dev_get_drvdata(dev->parent) instead of dev_get_platdata(dev)
to correctly get acp_chip_info members in acp I2S driver.
This resolves issues where some members were zero due to incorrect
data access.

Fixes: e3933683b25e ("ASoC: amd: acp: Remove redundant acp_dev_data structure")

Signed-off-by: Venkata Prasad Potturu <venkataprasad.potturu@amd.com>
---
 sound/soc/amd/acp/acp-i2s.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
index 617690362ad7..4ba0a66981ea 100644
--- a/sound/soc/amd/acp/acp-i2s.c
+++ b/sound/soc/amd/acp/acp-i2s.c
@@ -73,7 +73,7 @@ static int acp_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
 			   unsigned int fmt)
 {
 	struct device *dev = cpu_dai->component->dev;
-	struct acp_chip_info *chip = dev_get_platdata(dev);
+	struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
 	int mode;
 
 	mode = fmt & SND_SOC_DAIFMT_FORMAT_MASK;
@@ -199,7 +199,7 @@ static int acp_i2s_hwparams(struct snd_pcm_substream *substream, struct snd_pcm_
 	u32 reg_val, fmt_reg, tdm_fmt;
 	u32 lrclk_div_val, bclk_div_val;
 
-	chip = dev_get_platdata(dev);
+	chip = dev_get_drvdata(dev->parent);
 	rsrc = chip->rsrc;
 
 	/* These values are as per Hardware Spec */
@@ -386,7 +386,7 @@ static int acp_i2s_trigger(struct snd_pcm_substream *substream, int cmd, struct
 {
 	struct acp_stream *stream = substream->runtime->private_data;
 	struct device *dev = dai->component->dev;
-	struct acp_chip_info *chip = dev_get_platdata(dev);
+	struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
 	struct acp_resource *rsrc = chip->rsrc;
 	u32 val, period_bytes, reg_val, ier_val, water_val, buf_size, buf_reg;
 
@@ -516,14 +516,13 @@ static int acp_i2s_trigger(struct snd_pcm_substream *substream, int cmd, struct
 static int acp_i2s_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
 {
 	struct device *dev = dai->component->dev;
-	struct acp_chip_info *chip = dev_get_platdata(dev);
+	struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
 	struct acp_resource *rsrc = chip->rsrc;
 	struct acp_stream *stream = substream->runtime->private_data;
 	u32 reg_dma_size = 0, reg_fifo_size = 0, reg_fifo_addr = 0;
 	u32 phy_addr = 0, acp_fifo_addr = 0, ext_int_ctrl;
 	unsigned int dir = substream->stream;
 
-	chip = dev_get_platdata(dev);
 	switch (dai->driver->id) {
 	case I2S_SP_INSTANCE:
 		if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
@@ -632,7 +631,7 @@ static int acp_i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_d
 {
 	struct acp_stream *stream = substream->runtime->private_data;
 	struct device *dev = dai->component->dev;
-	struct acp_chip_info *chip = dev_get_platdata(dev);
+	struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
 	struct acp_resource *rsrc = chip->rsrc;
 	unsigned int dir = substream->stream;
 	unsigned int irq_bit = 0;
-- 
2.43.0

Re: [PATCH] ASoC: amd: acp: Fix incorrect retrival of acp_chip_info

[Cezary Rojewski]

On 2025-09-09 8:19 AM, Venkata Prasad Potturu wrote:
> Use dev_get_drvdata(dev->parent) instead of dev_get_platdata(dev)
> to correctly get acp_chip_info members in acp I2S driver.
> This resolves issues where some members were zero due to incorrect
> data access.

Hi,

'some members were zero' meaning null-ptr-deref? If so, please reword to 
make it more explicit.

Given the history of this file, mainly Commit 6d9b64156d84 ("ASoC: amd: 
acp: Fix NULL pointer deref in acp_i2s_set_tdm_slot") it's kind of 
surprising that the issue is addressed in staggered fashion. Why was 
set_tdm_slot() fixed separately?

> Fixes: e3933683b25e ("ASoC: amd: acp: Remove redundant acp_dev_data structure")
> 
> Signed-off-by: Venkata Prasad Potturu <venkataprasad.potturu@amd.com>

No newline between the tags, please.

> ---
>   sound/soc/amd/acp/acp-i2s.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
> index 617690362ad7..4ba0a66981ea 100644
> --- a/sound/soc/amd/acp/acp-i2s.c
> +++ b/sound/soc/amd/acp/acp-i2s.c
> @@ -73,7 +73,7 @@ static int acp_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
>   			   unsigned int fmt)
>   {
>   	struct device *dev = cpu_dai->component->dev;
> -	struct acp_chip_info *chip = dev_get_platdata(dev);
> +	struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
>   	int mode;
>   
>   	mode = fmt & SND_SOC_DAIFMT_FORMAT_MASK;
> @@ -199,7 +199,7 @@ static int acp_i2s_hwparams(struct snd_pcm_substream *substream, struct snd_pcm_
>   	u32 reg_val, fmt_reg, tdm_fmt;
>   	u32 lrclk_div_val, bclk_div_val;
>   
> -	chip = dev_get_platdata(dev);
> +	chip = dev_get_drvdata(dev->parent);
>   	rsrc = chip->rsrc;
>   
>   	/* These values are as per Hardware Spec */
> @@ -386,7 +386,7 @@ static int acp_i2s_trigger(struct snd_pcm_substream *substream, int cmd, struct
>   {
>   	struct acp_stream *stream = substream->runtime->private_data;
>   	struct device *dev = dai->component->dev;
> -	struct acp_chip_info *chip = dev_get_platdata(dev);
> +	struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
>   	struct acp_resource *rsrc = chip->rsrc;
>   	u32 val, period_bytes, reg_val, ier_val, water_val, buf_size, buf_reg;
>   
> @@ -516,14 +516,13 @@ static int acp_i2s_trigger(struct snd_pcm_substream *substream, int cmd, struct
>   static int acp_i2s_prepare(struct snd_pcm_substream *substream, struct snd_soc_dai *dai)
>   {
>   	struct device *dev = dai->component->dev;
> -	struct acp_chip_info *chip = dev_get_platdata(dev);
> +	struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
>   	struct acp_resource *rsrc = chip->rsrc;
>   	struct acp_stream *stream = substream->runtime->private_data;
>   	u32 reg_dma_size = 0, reg_fifo_size = 0, reg_fifo_addr = 0;
>   	u32 phy_addr = 0, acp_fifo_addr = 0, ext_int_ctrl;
>   	unsigned int dir = substream->stream;
>   
> -	chip = dev_get_platdata(dev);
>   	switch (dai->driver->id) {
>   	case I2S_SP_INSTANCE:
>   		if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
> @@ -632,7 +631,7 @@ static int acp_i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_d
>   {
>   	struct acp_stream *stream = substream->runtime->private_data;
>   	struct device *dev = dai->component->dev;
> -	struct acp_chip_info *chip = dev_get_platdata(dev);
> +	struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
>   	struct acp_resource *rsrc = chip->rsrc;
>   	unsigned int dir = substream->stream;
>   	unsigned int irq_bit = 0;

Re: [PATCH] ASoC: amd: acp: Fix incorrect retrival of acp_chip_info

[Prasad, Prasad]

On 9/10/2025 2:41 PM, Cezary Rojewski wrote:
> On 2025-09-09 8:19 AM, Venkata Prasad Potturu wrote:
>> Use dev_get_drvdata(dev->parent) instead of dev_get_platdata(dev)
>> to correctly get acp_chip_info members in acp I2S driver.
>> This resolves issues where some members were zero due to incorrect
>> data access.
>
> Hi,
>
> 'some members were zero' meaning null-ptr-deref? If so, please reword 
> to make it more explicit.
It's not a null-ptr-deref, members were not updated properly, will 
rephrase the commit description
>
> Given the history of this file, mainly Commit 6d9b64156d84 ("ASoC: 
> amd: acp: Fix NULL pointer deref in acp_i2s_set_tdm_slot") it's kind 
> of surprising that the issue is addressed in staggered fashion. Why 
> was set_tdm_slot() fixed separately?
This fix was missed earlier at the time of this commit 6d9b64156d84.
>
>> Fixes: e3933683b25e ("ASoC: amd: acp: Remove redundant acp_dev_data 
>> structure")
>>
>> Signed-off-by: Venkata Prasad Potturu <venkataprasad.potturu@amd.com>
>
> No newline between the tags, please.
Okay, will send v2 patch.
>
>> ---
>>   sound/soc/amd/acp/acp-i2s.c | 11 +++++------
>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
>> index 617690362ad7..4ba0a66981ea 100644
>> --- a/sound/soc/amd/acp/acp-i2s.c
>> +++ b/sound/soc/amd/acp/acp-i2s.c
>> @@ -73,7 +73,7 @@ static int acp_i2s_set_fmt(struct snd_soc_dai 
>> *cpu_dai,
>>                  unsigned int fmt)
>>   {
>>       struct device *dev = cpu_dai->component->dev;
>> -    struct acp_chip_info *chip = dev_get_platdata(dev);
>> +    struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
>>       int mode;
>>         mode = fmt & SND_SOC_DAIFMT_FORMAT_MASK;
>> @@ -199,7 +199,7 @@ static int acp_i2s_hwparams(struct 
>> snd_pcm_substream *substream, struct snd_pcm_
>>       u32 reg_val, fmt_reg, tdm_fmt;
>>       u32 lrclk_div_val, bclk_div_val;
>>   -    chip = dev_get_platdata(dev);
>> +    chip = dev_get_drvdata(dev->parent);
>>       rsrc = chip->rsrc;
>>         /* These values are as per Hardware Spec */
>> @@ -386,7 +386,7 @@ static int acp_i2s_trigger(struct 
>> snd_pcm_substream *substream, int cmd, struct
>>   {
>>       struct acp_stream *stream = substream->runtime->private_data;
>>       struct device *dev = dai->component->dev;
>> -    struct acp_chip_info *chip = dev_get_platdata(dev);
>> +    struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
>>       struct acp_resource *rsrc = chip->rsrc;
>>       u32 val, period_bytes, reg_val, ier_val, water_val, buf_size, 
>> buf_reg;
>>   @@ -516,14 +516,13 @@ static int acp_i2s_trigger(struct 
>> snd_pcm_substream *substream, int cmd, struct
>>   static int acp_i2s_prepare(struct snd_pcm_substream *substream, 
>> struct snd_soc_dai *dai)
>>   {
>>       struct device *dev = dai->component->dev;
>> -    struct acp_chip_info *chip = dev_get_platdata(dev);
>> +    struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
>>       struct acp_resource *rsrc = chip->rsrc;
>>       struct acp_stream *stream = substream->runtime->private_data;
>>       u32 reg_dma_size = 0, reg_fifo_size = 0, reg_fifo_addr = 0;
>>       u32 phy_addr = 0, acp_fifo_addr = 0, ext_int_ctrl;
>>       unsigned int dir = substream->stream;
>>   -    chip = dev_get_platdata(dev);
>>       switch (dai->driver->id) {
>>       case I2S_SP_INSTANCE:
>>           if (dir == SNDRV_PCM_STREAM_PLAYBACK) {
>> @@ -632,7 +631,7 @@ static int acp_i2s_startup(struct 
>> snd_pcm_substream *substream, struct snd_soc_d
>>   {
>>       struct acp_stream *stream = substream->runtime->private_data;
>>       struct device *dev = dai->component->dev;
>> -    struct acp_chip_info *chip = dev_get_platdata(dev);
>> +    struct acp_chip_info *chip = dev_get_drvdata(dev->parent);
>>       struct acp_resource *rsrc = chip->rsrc;
>>       unsigned int dir = substream->stream;
>>       unsigned int irq_bit = 0;
>

Re: [PATCH] ASoC: amd: acp: Fix incorrect retrival of acp_chip_info

[Cezary Rojewski]

On 2025-09-10 3:39 PM, Prasad, Prasad wrote:
> 
> On 9/10/2025 2:41 PM, Cezary Rojewski wrote:
>> On 2025-09-09 8:19 AM, Venkata Prasad Potturu wrote:
>>> Use dev_get_drvdata(dev->parent) instead of dev_get_platdata(dev)
>>> to correctly get acp_chip_info members in acp I2S driver.
>>> This resolves issues where some members were zero due to incorrect
>>> data access.
>>
>> Hi,
>>
>> 'some members were zero' meaning null-ptr-deref? If so, please reword
>> to make it more explicit.
> It's not a null-ptr-deref, members were not updated properly, will
> rephrase the commit description
>>
>> Given the history of this file, mainly Commit 6d9b64156d84 ("ASoC:
>> amd: acp: Fix NULL pointer deref in acp_i2s_set_tdm_slot") it's kind
>> of surprising that the issue is addressed in staggered fashion. Why
>> was set_tdm_slot() fixed separately?
> This fix was missed earlier at the time of this commit 6d9b64156d84.

Let's mention that in the commit message then. It's clear now that the 
earlier fix is insufficient and null-ptr-derefs may still occur.

>>
>>> Fixes: e3933683b25e ("ASoC: amd: acp: Remove redundant acp_dev_data
>>> structure")
>>>
>>> Signed-off-by: Venkata Prasad Potturu <venkataprasad.potturu@amd.com>
>>
>> No newline between the tags, please.
> Okay, will send v2 patch.

Re: [PATCH] ASoC: amd: acp: Fix incorrect retrival of acp_chip_info

[Mark Brown]

On Tue, 09 Sep 2025 11:49:50 +0530, Venkata Prasad Potturu wrote:
> Use dev_get_drvdata(dev->parent) instead of dev_get_platdata(dev)
> to correctly get acp_chip_info members in acp I2S driver.
> This resolves issues where some members were zero due to incorrect
> data access.
> 
> Fixes: e3933683b25e ("ASoC: amd: acp: Remove redundant acp_dev_data structure")
> 
> [...]

Applied to

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

Thanks!

[1/1] ASoC: amd: acp: Fix incorrect retrival of acp_chip_info
      commit: d7871f400cad1da376f1d7724209a1c49226c456

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