[PATCH 1/8] ASoC: amd: acp: add a null check for chip_pdev structure

[PATCH 1/8] ASoC: amd: acp: add a null check for chip_pdev structure

[Vijendar Mukunda]

When acp platform device creation is skipped, chip->chip_pdev value will
remain NULL. Add NULL check for chip->chip_pdev structure in
snd_acp_resume() function to avoid null pointer dereference.

Fixes: 088a40980efb ("ASoC: amd: acp: add pm ops support for acp pci driver")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-pci.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c
index ad320b29e87d..aa3e72d13451 100644
--- a/sound/soc/amd/acp/acp-pci.c
+++ b/sound/soc/amd/acp/acp-pci.c
@@ -199,10 +199,12 @@ static int __maybe_unused snd_acp_resume(struct device *dev)
 	ret = acp_init(chip);
 	if (ret)
 		dev_err(dev, "ACP init failed\n");
-	child = chip->chip_pdev->dev;
-	adata = dev_get_drvdata(&child);
-	if (adata)
-		acp_enable_interrupts(adata);
+	if (chip->chip_pdev) {
+		child = chip->chip_pdev->dev;
+		adata = dev_get_drvdata(&child);
+		if (adata)
+			acp_enable_interrupts(adata);
+	}
 	return ret;
 }
 
-- 
2.34.1

[PATCH 2/8] ASoC: amd: acp: remove i2s configuration check in acp_i2s_probe()

[Vijendar Mukunda]

ACP supports different pin configurations for I2S IO. Checking ACP pin
configuration value against specific value breaks the functionality for
other I2S pin configurations. This check is no longer required in i2s dai
driver probe call as i2s configuration check will be verified during acp
platform device creation sequence.
Remove i2s_mode check in acp_i2s_probe() function.

Fixes: b24484c18b10 ("ASoC: amd: acp: ACP code generic to support newer platforms")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-i2s.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
index 60cbc881be6e..ef12f97ddc69 100644
--- a/sound/soc/amd/acp/acp-i2s.c
+++ b/sound/soc/amd/acp/acp-i2s.c
@@ -588,20 +588,12 @@ static int acp_i2s_probe(struct snd_soc_dai *dai)
 {
 	struct device *dev = dai->component->dev;
 	struct acp_dev_data *adata = dev_get_drvdata(dev);
-	struct acp_resource *rsrc = adata->rsrc;
-	unsigned int val;
 
 	if (!adata->acp_base) {
 		dev_err(dev, "I2S base is NULL\n");
 		return -EINVAL;
 	}
 
-	val = readl(adata->acp_base + rsrc->i2s_pin_cfg_offset);
-	if (val != rsrc->i2s_mode) {
-		dev_err(dev, "I2S Mode not supported val %x\n", val);
-		return -EINVAL;
-	}
-
 	return 0;
 }
 
-- 
2.34.1

[PATCH 3/8] ASoC: amd: acp: move chip->flag variable assignment

[Vijendar Mukunda]

chip->flag variable assignment will be skipped when acp platform device
creation is skipped. In this case chip>flag value will not be set.
chip->flag variable should be assigned along with other structure
variables for 'chip' structure. Move chip->flag variable assignment
prior to acp platform device creation.

Fixes: 3a94c8ad0aae ("ASoC: amd: acp: add code for scanning acp pdm controller")
Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c
index aa3e72d13451..777b5a78d8a9 100644
--- a/sound/soc/amd/acp/acp-pci.c
+++ b/sound/soc/amd/acp/acp-pci.c
@@ -100,6 +100,7 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
 		ret = -EINVAL;
 		goto release_regions;
 	}
+	chip->flag = flag;
 	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");
@@ -139,7 +140,6 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
 		}
 	}
 
-	chip->flag = flag;
 	memset(&pdevinfo, 0, sizeof(pdevinfo));
 
 	pdevinfo.name = chip->name;
-- 
2.34.1

[PATCH 4/8] ASoC: amd: acp: remove acp_i2s_probe function

[Vijendar Mukunda]

In acp_i2s_probe(), acp_base null check is verified.
As already acp_base null check will be verified in acp platform
driver probe sequence, additional NULL check in acp_i2s_probe() is not
needed. Remove acp_i2s_probe() function.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-i2s.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
index ef12f97ddc69..7da414bc3b96 100644
--- a/sound/soc/amd/acp/acp-i2s.c
+++ b/sound/soc/amd/acp/acp-i2s.c
@@ -584,21 +584,7 @@ static int acp_i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_d
 	return 0;
 }
 
-static int acp_i2s_probe(struct snd_soc_dai *dai)
-{
-	struct device *dev = dai->component->dev;
-	struct acp_dev_data *adata = dev_get_drvdata(dev);
-
-	if (!adata->acp_base) {
-		dev_err(dev, "I2S base is NULL\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 const struct snd_soc_dai_ops asoc_acp_cpu_dai_ops = {
-	.probe		= acp_i2s_probe,
 	.startup	= acp_i2s_startup,
 	.hw_params	= acp_i2s_hwparams,
 	.prepare	= acp_i2s_prepare,
-- 
2.34.1

[PATCH 5/8] ASoC: amd: acp: remove unused variables from acp_resource structure

[Vijendar Mukunda]

Remove unused variables i2s_pin_cfg_offset and i2s_mode from acp_resource
structure entries.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-rembrandt.c | 2 --
 sound/soc/amd/acp/acp-renoir.c    | 2 --
 sound/soc/amd/acp/acp63.c         | 2 --
 sound/soc/amd/acp/acp70.c         | 2 --
 sound/soc/amd/acp/amd.h           | 2 --
 5 files changed, 10 deletions(-)

diff --git a/sound/soc/amd/acp/acp-rembrandt.c b/sound/soc/amd/acp/acp-rembrandt.c
index 158f819f8da4..953a793de9a3 100644
--- a/sound/soc/amd/acp/acp-rembrandt.c
+++ b/sound/soc/amd/acp/acp-rembrandt.c
@@ -39,8 +39,6 @@ static struct acp_resource rsrc = {
 	.irqp_used = 1,
 	.soc_mclk = true,
 	.irq_reg_offset = 0x1a00,
-	.i2s_pin_cfg_offset = 0x1440,
-	.i2s_mode = 0x0a,
 	.scratch_reg_offset = 0x12800,
 	.sram_pte_offset = 0x03802800,
 };
diff --git a/sound/soc/amd/acp/acp-renoir.c b/sound/soc/amd/acp/acp-renoir.c
index b0e181c9a733..db835ed7c208 100644
--- a/sound/soc/amd/acp/acp-renoir.c
+++ b/sound/soc/amd/acp/acp-renoir.c
@@ -32,8 +32,6 @@ static struct acp_resource rsrc = {
 	.no_of_ctrls = 1,
 	.irqp_used = 0,
 	.irq_reg_offset = 0x1800,
-	.i2s_pin_cfg_offset = 0x1400,
-	.i2s_mode = 0x04,
 	.scratch_reg_offset = 0x12800,
 	.sram_pte_offset = 0x02052800,
 };
diff --git a/sound/soc/amd/acp/acp63.c b/sound/soc/amd/acp/acp63.c
index 4d342441a650..f223311b6740 100644
--- a/sound/soc/amd/acp/acp63.c
+++ b/sound/soc/amd/acp/acp63.c
@@ -55,8 +55,6 @@ static struct acp_resource rsrc = {
 	.irqp_used = 1,
 	.soc_mclk = true,
 	.irq_reg_offset = 0x1a00,
-	.i2s_pin_cfg_offset = 0x1440,
-	.i2s_mode = 0x0a,
 	.scratch_reg_offset = 0x12800,
 	.sram_pte_offset = 0x03802800,
 };
diff --git a/sound/soc/amd/acp/acp70.c b/sound/soc/amd/acp/acp70.c
index 0d7cdd4017e5..a2cbdcca4313 100644
--- a/sound/soc/amd/acp/acp70.c
+++ b/sound/soc/amd/acp/acp70.c
@@ -31,8 +31,6 @@ static struct acp_resource rsrc = {
 	.irqp_used = 1,
 	.soc_mclk = true,
 	.irq_reg_offset = 0x1a00,
-	.i2s_pin_cfg_offset = 0x1440,
-	.i2s_mode = 0x0a,
 	.scratch_reg_offset = 0x12800,
 	.sram_pte_offset = 0x03802800,
 };
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index d75b4eb34de8..cbf9ca26f3ee 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -162,8 +162,6 @@ struct acp_resource {
 	int irqp_used;
 	bool soc_mclk;
 	u32 irq_reg_offset;
-	u32 i2s_pin_cfg_offset;
-	int i2s_mode;
 	u64 scratch_reg_offset;
 	u64 sram_pte_offset;
 };
-- 
2.34.1

[PATCH 6/8] ASoC: amd: acp: modify conditional check for programming i2s mclk

[Vijendar Mukunda]

ACP provides different IO configurations(ACP PDM, I2S and SoundWire).
I2S mclk should be programmed only when I2S configuration is selected and
I2S controller is programmed as clock master.
Modify the conditional check for programming i2s mclk.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-rembrandt.c | 5 +++--
 sound/soc/amd/acp/acp63.c         | 5 +++--
 sound/soc/amd/acp/amd.h           | 1 +
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/sound/soc/amd/acp/acp-rembrandt.c b/sound/soc/amd/acp/acp-rembrandt.c
index 953a793de9a3..e19981c7d65a 100644
--- a/sound/soc/amd/acp/acp-rembrandt.c
+++ b/sound/soc/amd/acp/acp-rembrandt.c
@@ -229,12 +229,13 @@ static int rembrandt_audio_probe(struct platform_device *pdev)
 	adata->rsrc = &rsrc;
 	adata->platform = REMBRANDT;
 	adata->flag = chip->flag;
+	adata->is_i2s_config = chip->is_i2s_config;
 	adata->machines = snd_soc_acpi_amd_rmb_acp_machines;
 	acp_machine_select(adata);
 
 	dev_set_drvdata(dev, adata);
 
-	if (chip->flag != FLAG_AMD_LEGACY_ONLY_DMIC) {
+	if (chip->is_i2s_config && rsrc.soc_mclk) {
 		ret = acp6x_master_clock_generate(dev);
 		if (ret)
 			return ret;
@@ -267,7 +268,7 @@ static int __maybe_unused rmb_pcm_resume(struct device *dev)
 	snd_pcm_uframes_t buf_in_frames;
 	u64 buf_size;
 
-	if (adata->flag != FLAG_AMD_LEGACY_ONLY_DMIC)
+	if (adata->is_i2s_config && adata->rsrc->soc_mclk)
 		acp6x_master_clock_generate(dev);
 
 	spin_lock(&adata->acp_lock);
diff --git a/sound/soc/amd/acp/acp63.c b/sound/soc/amd/acp/acp63.c
index f223311b6740..f340920b3289 100644
--- a/sound/soc/amd/acp/acp63.c
+++ b/sound/soc/amd/acp/acp63.c
@@ -239,11 +239,12 @@ static int acp63_audio_probe(struct platform_device *pdev)
 	adata->rsrc = &rsrc;
 	adata->platform = ACP63;
 	adata->flag = chip->flag;
+	adata->is_i2s_config = chip->is_i2s_config;
 	adata->machines = snd_soc_acpi_amd_acp63_acp_machines;
 	acp_machine_select(adata);
 	dev_set_drvdata(dev, adata);
 
-	if (chip->flag != FLAG_AMD_LEGACY_ONLY_DMIC) {
+	if (chip->is_i2s_config && rsrc.soc_mclk) {
 		ret = acp63_i2s_master_clock_generate(adata);
 		if (ret)
 			return ret;
@@ -276,7 +277,7 @@ static int __maybe_unused acp63_pcm_resume(struct device *dev)
 	snd_pcm_uframes_t buf_in_frames;
 	u64 buf_size;
 
-	if (adata->flag != FLAG_AMD_LEGACY_ONLY_DMIC)
+	if (adata->is_i2s_config && adata->rsrc->soc_mclk)
 		acp63_i2s_master_clock_generate(adata);
 
 	spin_lock(&adata->acp_lock);
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index cbf9ca26f3ee..87a4813783f9 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -173,6 +173,7 @@ struct acp_dev_data {
 	unsigned int i2s_irq;
 
 	bool tdm_mode;
+	bool is_i2s_config;
 	/* SOC specific dais */
 	struct snd_soc_dai_driver *dai_driver;
 	int num_dai;
-- 
2.34.1

[PATCH 7/8] ASoC: amd: acp: move i2s clock generation sequence

[Vijendar Mukunda]

I2S clock generation registers should be programmed before starting the I2S
dma when I2S controller is programmed as clock master. Move i2s clock
generation register programming sequence prior to i2s dma start.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-i2s.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
index 7da414bc3b96..88985e9d318b 100644
--- a/sound/soc/amd/acp/acp-i2s.c
+++ b/sound/soc/amd/acp/acp-i2s.c
@@ -369,12 +369,12 @@ static int acp_i2s_trigger(struct snd_pcm_substream *substream, int cmd, struct
 		}
 		writel(period_bytes, adata->acp_base + water_val);
 		writel(buf_size, adata->acp_base + buf_reg);
+		if (rsrc->soc_mclk)
+			acp_set_i2s_clk(adata, dai->driver->id);
 		val = readl(adata->acp_base + reg_val);
 		val = val | BIT(0);
 		writel(val, adata->acp_base + reg_val);
 		writel(1, adata->acp_base + ier_val);
-		if (rsrc->soc_mclk)
-			acp_set_i2s_clk(adata, dai->driver->id);
 		return 0;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
-- 
2.34.1

[PATCH 8/8] ASoC: amd: acp: add pcm constraints for buffer size and period size

[Vijendar Mukunda]

ACP common dma driver has a buffer size and period size restriction
which should be 64 byte aligned. Add pcm constraints for the same.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp/acp-platform.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/sound/soc/amd/acp/acp-platform.c b/sound/soc/amd/acp/acp-platform.c
index aaac8aa744cb..4f409cd09c11 100644
--- a/sound/soc/amd/acp/acp-platform.c
+++ b/sound/soc/amd/acp/acp-platform.c
@@ -197,6 +197,20 @@ static int acp_dma_open(struct snd_soc_component *component, struct snd_pcm_subs
 	else
 		runtime->hw = acp_pcm_hardware_capture;
 
+	ret = snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_PERIOD_BYTES, DMA_SIZE);
+	if (ret) {
+		dev_err(component->dev, "set hw constraint HW_PARAM_PERIOD_BYTES failed\n");
+		kfree(stream);
+		return ret;
+	}
+
+	ret = snd_pcm_hw_constraint_step(runtime, 0, SNDRV_PCM_HW_PARAM_BUFFER_BYTES, DMA_SIZE);
+	if (ret) {
+		dev_err(component->dev, "set hw constraint HW_PARAM_BUFFER_BYTES failed\n");
+		kfree(stream);
+		return ret;
+	}
+
 	ret = snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
 	if (ret < 0) {
 		dev_err(component->dev, "set integer constraint failed\n");
-- 
2.34.1

Re: (subset) [PATCH 1/8] ASoC: amd: acp: add a null check for chip_pdev structure

[Mark Brown]

On Mon, 17 Jun 2024 12:58:34 +0530, Vijendar Mukunda wrote:
> When acp platform device creation is skipped, chip->chip_pdev value will
> remain NULL. Add NULL check for chip->chip_pdev structure in
> snd_acp_resume() function to avoid null pointer dereference.
> 
> 

Applied to

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

Thanks!

[1/8] ASoC: amd: acp: add a null check for chip_pdev structure
      commit: 98d919dfee1cc402ca29d45da642852d7c9a2301
[2/8] ASoC: amd: acp: remove i2s configuration check in acp_i2s_probe()
      commit: 70fa3900c3ed92158628710e81d274e5cb52f92b
[3/8] ASoC: amd: acp: move chip->flag variable assignment
      commit: 379bcd2c9197bf2c429434e8a01cea0ee1852316

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

Re: (subset) [PATCH 1/8] ASoC: amd: acp: add a null check for chip_pdev structure

[Mark Brown]

On Mon, 17 Jun 2024 12:58:34 +0530, Vijendar Mukunda wrote:
> When acp platform device creation is skipped, chip->chip_pdev value will
> remain NULL. Add NULL check for chip->chip_pdev structure in
> snd_acp_resume() function to avoid null pointer dereference.
> 
> 

Applied to

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

Thanks!

[4/8] ASoC: amd: acp: remove acp_i2s_probe function
      commit: 75a08ec8c3a6aef914235c409a99046a3d29d1d4
[5/8] ASoC: amd: acp: remove unused variables from acp_resource structure
      commit: 50f1670145392398f4f2ffe9abe4599c86d11ec2
[6/8] ASoC: amd: acp: modify conditional check for programming i2s mclk
      commit: 5b162f60e7e051624e187e45b0fdc481c3573f17
[7/8] ASoC: amd: acp: move i2s clock generation sequence
      commit: d85695b01cbb2455a2f70528bb9e53f2463a39cf
[8/8] ASoC: amd: acp: add pcm constraints for buffer size and period size
      commit: 8978e1f7bc26655e0373c4a6b31e17fcdd497329

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