[PATCH 1/5] ASoC: amd: acp: Modify local variables name to generic
Change local variables name to be generic in irq handler.
Signed-off-by: Venkata Prasad Potturu venkataprasad.potturu@amd.com Reviewed-by: Vijendar Mukunda Vijendar.Mukunda@amd.com --- sound/soc/amd/acp/acp-platform.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/sound/soc/amd/acp/acp-platform.c b/sound/soc/amd/acp/acp-platform.c index 327e17736dbd..b1ca52274375 100644 --- a/sound/soc/amd/acp/acp-platform.c +++ b/sound/soc/amd/acp/acp-platform.c @@ -94,19 +94,19 @@ static irqreturn_t i2s_irq_handler(int irq, void *data) struct acp_resource *rsrc = adata->rsrc; struct acp_stream *stream; u16 i2s_flag = 0; - u32 val, val1, i; + u32 ext_intr_stat, ext_intr_stat1, i;
if (!adata) return IRQ_NONE;
if (adata->rsrc->no_of_ctrls == 2) - val1 = readl(ACP_EXTERNAL_INTR_STAT(adata, (rsrc->irqp_used - 1))); + ext_intr_stat = readl(ACP_EXTERNAL_INTR_STAT(adata, (rsrc->irqp_used - 1)));
- val = readl(ACP_EXTERNAL_INTR_STAT(adata, rsrc->irqp_used)); + ext_intr_stat = readl(ACP_EXTERNAL_INTR_STAT(adata, rsrc->irqp_used));
for (i = 0; i < ACP_MAX_STREAM; i++) { stream = adata->stream[i]; - if (stream && (val & stream->irq_bit)) { + if (stream && (ext_intr_stat & stream->irq_bit)) { writel(stream->irq_bit, ACP_EXTERNAL_INTR_STAT(adata, rsrc->irqp_used)); snd_pcm_period_elapsed(stream->substream); @@ -114,7 +114,7 @@ static irqreturn_t i2s_irq_handler(int irq, void *data) break; } if (adata->rsrc->no_of_ctrls == 2) { - if (stream && (val1 & stream->irq_bit)) { + if (stream && (ext_intr_stat1 & stream->irq_bit)) { writel(stream->irq_bit, ACP_EXTERNAL_INTR_STAT(adata, (rsrc->irqp_used - 1))); snd_pcm_period_elapsed(stream->substream);
Remove mmap callback as ASoC AMD drivers just call the standard mmap handler.
Signed-off-by: Venkata Prasad Potturu venkataprasad.potturu@amd.com Reviewed-by: Vijendar Mukunda Vijendar.Mukunda@amd.com --- sound/soc/amd/acp/acp-platform.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/sound/soc/amd/acp/acp-platform.c b/sound/soc/amd/acp/acp-platform.c index b1ca52274375..10730d33c3b0 100644 --- a/sound/soc/amd/acp/acp-platform.c +++ b/sound/soc/amd/acp/acp-platform.c @@ -258,13 +258,6 @@ static int acp_dma_new(struct snd_soc_component *component, return 0; }
-static int acp_dma_mmap(struct snd_soc_component *component, - struct snd_pcm_substream *substream, - struct vm_area_struct *vma) -{ - return snd_pcm_lib_default_mmap(substream, vma); -} - static int acp_dma_close(struct snd_soc_component *component, struct snd_pcm_substream *substream) { @@ -288,7 +281,6 @@ static const struct snd_soc_component_driver acp_pcm_component = { .close = acp_dma_close, .hw_params = acp_dma_hw_params, .pointer = acp_dma_pointer, - .mmap = acp_dma_mmap, .pcm_construct = acp_dma_new, .legacy_dai_naming = 1, };
From: Ajit Kumar Pandey AjitKumar.Pandey@amd.com
We are currently allocating acp_stream during pcm_open and saving it in static array corresponds to array index calculated based on cpu dai->driver id. This approach will fail if we have single dai linked to multiple pcm device as we will have same dai->driver id or array index for multiple pcm open. Initialize new linked list stream_list to store opened pcm stream info dynamically.
Signed-off-by: Ajit Kumar Pandey AjitKumar.Pandey@amd.com Signed-off-by: Venkata Prasad Potturu venkataprasad.potturu@amd.com Reviewed-by: Vijendar Mukunda Vijendar.Mukunda@amd.com --- sound/soc/amd/acp/acp-platform.c | 30 +++++++++++------------------- sound/soc/amd/acp/amd.h | 3 ++- 2 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/sound/soc/amd/acp/acp-platform.c b/sound/soc/amd/acp/acp-platform.c index 10730d33c3b0..97198db7000e 100644 --- a/sound/soc/amd/acp/acp-platform.c +++ b/sound/soc/amd/acp/acp-platform.c @@ -104,7 +104,7 @@ static irqreturn_t i2s_irq_handler(int irq, void *data)
ext_intr_stat = readl(ACP_EXTERNAL_INTR_STAT(adata, rsrc->irqp_used));
- for (i = 0; i < ACP_MAX_STREAM; i++) { + list_for_each_entry(stream, &adata->stream_list, list) { stream = adata->stream[i]; if (stream && (ext_intr_stat & stream->irq_bit)) { writel(stream->irq_bit, @@ -146,9 +146,8 @@ static void config_pte_for_stream(struct acp_dev_data *adata, struct acp_stream writel(0x01, adata->acp_base + ACPAXI2AXI_ATU_CTRL); }
-static void config_acp_dma(struct acp_dev_data *adata, int cpu_id, int size) +static void config_acp_dma(struct acp_dev_data *adata, struct acp_stream *stream, int size) { - struct acp_stream *stream = adata->stream[cpu_id]; struct snd_pcm_substream *substream = stream->substream; struct acp_resource *rsrc = adata->rsrc; dma_addr_t addr = substream->dma_buffer.addr; @@ -174,13 +173,10 @@ static void config_acp_dma(struct acp_dev_data *adata, int cpu_id, int size)
static int acp_dma_open(struct snd_soc_component *component, struct snd_pcm_substream *substream) { - struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream); - struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0); struct snd_pcm_runtime *runtime = substream->runtime; struct device *dev = component->dev; struct acp_dev_data *adata = dev_get_drvdata(dev); struct acp_stream *stream; - int stream_id = cpu_dai->driver->id * 2 + substream->stream; int ret;
stream = kzalloc(sizeof(*stream), GFP_KERNEL); @@ -188,7 +184,8 @@ static int acp_dma_open(struct snd_soc_component *component, struct snd_pcm_subs return -ENOMEM;
stream->substream = substream; - adata->stream[stream_id] = stream; + + list_add_tail(&stream->list, &adata->stream_list);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) runtime->hw = acp_pcm_hardware_playback; @@ -212,16 +209,13 @@ static int acp_dma_hw_params(struct snd_soc_component *component, struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { - struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream); struct acp_dev_data *adata = snd_soc_component_get_drvdata(component); - struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0); struct acp_stream *stream = substream->runtime->private_data; - int stream_id = cpu_dai->driver->id * 2 + substream->stream; u64 size = params_buffer_bytes(params);
/* Configure ACP DMA block with params */ config_pte_for_stream(adata, stream); - config_acp_dma(adata, stream_id, size); + config_acp_dma(adata, stream, size);
return 0; } @@ -261,16 +255,11 @@ static int acp_dma_new(struct snd_soc_component *component, static int acp_dma_close(struct snd_soc_component *component, struct snd_pcm_substream *substream) { - struct snd_soc_pcm_runtime *soc_runtime = asoc_substream_to_rtd(substream); - struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(soc_runtime, 0); - struct device *dev = component->dev; - struct acp_dev_data *adata = dev_get_drvdata(dev); - struct acp_stream *stream; - int stream_id = cpu_dai->driver->id * 2 + substream->stream; + struct acp_stream *stream = substream->runtime->private_data;
- stream = adata->stream[stream_id]; + /* Remove entry from list */ + list_del(&stream->list); kfree(stream); - adata->stream[stream_id] = NULL;
return 0; } @@ -305,6 +294,9 @@ int acp_platform_register(struct device *dev) dev_err(dev, "Fail to register acp i2s component\n"); return status; } + + INIT_LIST_HEAD(&adata->stream_list); + return 0; } EXPORT_SYMBOL_NS_GPL(acp_platform_register, SND_SOC_ACP_COMMON); diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h index af9603724a68..148a9ab6206d 100644 --- a/sound/soc/amd/acp/amd.h +++ b/sound/soc/amd/acp/amd.h @@ -91,6 +91,7 @@ struct acp_chip_info { };
struct acp_stream { + struct list_head list; struct snd_pcm_substream *substream; int irq_bit; int dai_id; @@ -123,7 +124,7 @@ struct acp_dev_data { struct snd_soc_dai_driver *dai_driver; int num_dai;
- struct acp_stream *stream[ACP_MAX_STREAM]; + struct list_head stream_list;
struct snd_soc_acpi_mach *machines; struct platform_device *mach_dev;
On Thu, Jul 21, 2022 at 11:50:35AM +0530, Venkata Prasad Potturu wrote:
From: Ajit Kumar Pandey AjitKumar.Pandey@amd.com
We are currently allocating acp_stream during pcm_open and saving it in static array corresponds to array index calculated based on cpu dai->driver id. This approach will fail if we have single dai linked to multiple pcm device as we will have same dai->driver id or array index for multiple pcm open. Initialize new linked list stream_list to store opened pcm stream info dynamically.
This breaks an x86 allmodconfig build:
/build/stage/linux/sound/soc/amd/acp/acp-platform.c: In function ‘i2s_irq_handler’: /build/stage/linux/sound/soc/amd/acp/acp-platform.c:108:31: error: ‘struct acp_dev_data’ has no member named ‘stream’ 108 | stream = adata->stream[i]; | ^~ make[5]: *** [/build/stage/linux/scripts/Makefile.build:249: sound/soc/amd/acp/acp-platform.o] Error 1
[AMD Official Use Only - General]
On 7/22/2022 3:53 AM, Mark Brown wrote: Thanks for you time Mark.
On Thu, Jul 21, 2022 at 11:50:35AM +0530, Venkata Prasad Potturu wrote:
From: Ajit Kumar Pandey AjitKumar.Pandey@amd.commailto:AjitKumar.Pandey@amd.com
We are currently allocating acp_stream during pcm_open and saving
it in static array corresponds to array index calculated based on
cpu dai->driver id. This approach will fail if we have single dai
linked to multiple pcm device as we will have same dai->driver id
or array index for multiple pcm open. Initialize new linked list
stream_list to store opened pcm stream info dynamically.
This breaks an x86 allmodconfig build:
/build/stage/linux/sound/soc/amd/acp/acp-platform.c: In function 'i2s_irq_handler':
/build/stage/linux/sound/soc/amd/acp/acp-platform.c:108:31: error: 'struct acp_dev_data' has no member named 'stream'
108 | stream = adata->stream[i];
| ^~
make[5]: *** [/build/stage/linux/scripts/Makefile.build:249: sound/soc/amd/acp/acp-platform.o] Error 1 Okay, will fix and send in next version.
Change platform specific constant resource structure variable to generic name.
Signed-off-by: Venkata Prasad Potturu venkataprasad.potturu@amd.com Reviewed-by: Vijendar Mukunda Vijendar.Mukunda@amd.com --- sound/soc/amd/acp/acp-pci.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c index c03bcd31fc95..34d75603eea1 100644 --- a/sound/soc/amd/acp/acp-pci.c +++ b/sound/soc/amd/acp/acp-pci.c @@ -29,7 +29,7 @@ static struct platform_device *dmic_dev; static struct platform_device *pdev;
-static const struct resource acp3x_res[] = { +static const struct resource acp_res[] = { { .start = 0, .end = ACP3x_REG_END - ACP3x_REG_START, @@ -75,16 +75,15 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
pci_set_master(pci);
+ res_acp = acp_res; + num_res = ARRAY_SIZE(acp_res); + switch (pci->revision) { case 0x01: - res_acp = acp3x_res; - num_res = ARRAY_SIZE(acp3x_res); chip->name = "acp_asoc_renoir"; chip->acp_rev = ACP3X_DEV; break; case 0x6f: - res_acp = acp3x_res; - num_res = ARRAY_SIZE(acp3x_res); chip->name = "acp_asoc_rembrandt"; chip->acp_rev = ACP6X_DEV; break;
Add error handling in acp pci driver probe function.
Signed-off-by: Venkata Prasad Potturu venkataprasad.potturu@amd.com Reviewed-by: Vijendar Mukunda Vijendar.Mukunda@amd.com --- sound/soc/amd/acp/acp-pci.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c index 34d75603eea1..2c8e960cc9a6 100644 --- a/sound/soc/amd/acp/acp-pci.c +++ b/sound/soc/amd/acp/acp-pci.c @@ -70,7 +70,8 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id ret = pci_request_regions(pci, "AMD ACP3x audio"); if (ret < 0) { dev_err(&pci->dev, "pci_request_regions failed\n"); - return -ENOMEM; + ret = -ENOMEM; + goto disable_pci; }
pci_set_master(pci); @@ -89,22 +90,29 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id break; default: dev_err(dev, "Unsupported device revision:0x%x\n", pci->revision); - return -EINVAL; + ret = -EINVAL; + goto release_regions; }
dmic_dev = platform_device_register_data(dev, "dmic-codec", PLATFORM_DEVID_NONE, NULL, 0); if (IS_ERR(dmic_dev)) { dev_err(dev, "failed to create DMIC device\n"); - return PTR_ERR(dmic_dev); + ret = PTR_ERR(dmic_dev); + goto release_regions; }
addr = pci_resource_start(pci, 0); chip->base = devm_ioremap(&pci->dev, addr, pci_resource_len(pci, 0)); + if (!chip->base) { + ret = -ENOMEM; + goto release_regions; + }
res = devm_kzalloc(&pci->dev, sizeof(struct resource) * num_res, GFP_KERNEL); if (!res) { platform_device_unregister(dmic_dev); - return -ENOMEM; + ret = -ENOMEM; + goto release_regions; }
for (i = 0; i < num_res; i++, res_acp++) { @@ -133,8 +141,16 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id dev_err(&pci->dev, "cannot register %s device\n", pdevinfo.name); platform_device_unregister(dmic_dev); ret = PTR_ERR(pdev); + goto release_regions; }
+ return ret; + +release_regions: + pci_release_regions(pci); +disable_pci: + pci_disable_device(pci); + return ret; };
On Thu, 21 Jul 2022 11:50:33 +0530, Venkata Prasad Potturu wrote:
Change local variables name to be generic in irq handler.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/5] ASoC: amd: acp: Modify local variables name to generic commit: 93f53881473cbf6a364be36ccbb99568e04ffe59 [2/5] ASoC: amd: acp: Drop superfluous mmap callback commit: afde6727a9b66ff96e20d74ac392f3efdae1ceaf [3/5] ASoC: amd: acp: Initialize list to store acp_stream during pcm_open (no commit info) [4/5] ASoC: amd: acp: Modify const resource struct variable to generic commit: 96b008a1c2e9f455d982e9cfb7117a3a0fc8f550 [5/5] ASoC: amd: acp: Add error handling cases commit: c49f5e74a11e3909c424cada0f5d52345084933f
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
participants (3)
-
Mark Brown -
Prasad, Prasad -
Venkata Prasad Potturu