[PATCH] ASoC: amd: acp: Fix incorrect retrival of acp_chip_info
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;
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;
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;
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.
participants (3)
-
Cezary Rojewski -
Prasad, Prasad -
Venkata Prasad Potturu