linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ASoC: amd: added error checks in dma driver
@ 2017-12-04 15:16 Vijendar Mukunda
  2017-12-04 17:54 ` Mark Brown
  2017-12-04 18:49 ` Applied "ASoC: amd: added error checks in dma driver" to the asoc tree Mark Brown
  0 siblings, 2 replies; 12+ messages in thread
From: Vijendar Mukunda @ 2017-12-04 15:16 UTC (permalink / raw)
  To: broonie, alsa-devel
  Cc: tiwai, lgirdwood, perex, Alexander.Deucher, linux-kernel,
	Vijendar Mukunda

added additional error checks in acp dma driver
v2: printed error codes for acp init & acp deinit
failure cases.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp-pcm-dma.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index fb09578..29be517 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -848,6 +848,9 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
+	if (!rtd)
+		return -EINVAL;
+
 	buffersize = frames_to_bytes(runtime, runtime->buffer_size);
 	bytescount = acp_get_byte_count(rtd->acp_mmio, substream->stream);
 
@@ -873,6 +876,8 @@ static int acp_dma_prepare(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
+	if (!rtd)
+		return -EINVAL;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		config_acp_dma_channel(rtd->acp_mmio, SYSRAM_TO_ACP_CH_NUM,
 					PLAYBACK_START_DMA_DESCR_CH12,
@@ -1082,7 +1087,11 @@ static int acp_audio_probe(struct platform_device *pdev)
 	dev_set_drvdata(&pdev->dev, audio_drv_data);
 
 	/* Initialize the ACP */
-	acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
+	status = acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
+	if (status) {
+		dev_err(&pdev->dev, "ACP Init failed status:%d\n", status);
+		return status;
+	}
 
 	status = snd_soc_register_platform(&pdev->dev, &acp_asoc_platform);
 	if (status != 0) {
@@ -1099,9 +1108,12 @@ static int acp_audio_probe(struct platform_device *pdev)
 
 static int acp_audio_remove(struct platform_device *pdev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(&pdev->dev);
 
-	acp_deinit(adata->acp_mmio);
+	status = acp_deinit(adata->acp_mmio);
+	if (status)
+		dev_err(&pdev->dev, "ACP Deinit failed status:%d\n", status);
 	snd_soc_unregister_platform(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
@@ -1111,9 +1123,14 @@ static int acp_audio_remove(struct platform_device *pdev)
 static int acp_pcm_resume(struct device *dev)
 {
 	u16 bank;
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_init(adata->acp_mmio, adata->asic_type);
+	status = acp_init(adata->acp_mmio, adata->asic_type);
+	if (status) {
+		dev_err(dev, "ACP Init failed status:%d\n", status);
+		return status;
+	}
 
 	if (adata->play_stream && adata->play_stream->runtime) {
 		/* For Stoney, Memory gating is disabled,i.e SRAM Banks
@@ -1145,18 +1162,26 @@ static int acp_pcm_resume(struct device *dev)
 
 static int acp_pcm_runtime_suspend(struct device *dev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_deinit(adata->acp_mmio);
+	status = acp_deinit(adata->acp_mmio);
+	if (status)
+		dev_err(dev, "ACP Deinit failed status:%d\n", status);
 	acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
 
 static int acp_pcm_runtime_resume(struct device *dev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_init(adata->acp_mmio, adata->asic_type);
+	status = acp_init(adata->acp_mmio, adata->asic_type);
+	if (status) {
+		dev_err(dev, "ACP Init failed status:%d\n", status);
+		return status;
+	}
 	acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] ASoC: amd: added error checks in dma driver
  2017-12-04 15:16 [PATCH] ASoC: amd: added error checks in dma driver Vijendar Mukunda
@ 2017-12-04 17:54 ` Mark Brown
  2017-12-04 18:49 ` Applied "ASoC: amd: added error checks in dma driver" to the asoc tree Mark Brown
  1 sibling, 0 replies; 12+ messages in thread
From: Mark Brown @ 2017-12-04 17:54 UTC (permalink / raw)
  To: Vijendar Mukunda
  Cc: alsa-devel, tiwai, lgirdwood, perex, Alexander.Deucher, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

On Mon, Dec 04, 2017 at 08:46:24PM +0530, Vijendar Mukunda wrote:
> added additional error checks in acp dma driver
> v2: printed error codes for acp init & acp deinit
> failure cases.

Don't include noise like inter-version changelogs in commit messages,
add them after the --- if they're important as covered in
SubmittingPatches.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Applied "ASoC: amd: added error checks in dma driver" to the asoc tree
  2017-12-04 15:16 [PATCH] ASoC: amd: added error checks in dma driver Vijendar Mukunda
  2017-12-04 17:54 ` Mark Brown
@ 2017-12-04 18:49 ` Mark Brown
  1 sibling, 0 replies; 12+ messages in thread
From: Mark Brown @ 2017-12-04 18:49 UTC (permalink / raw)
  To: Mukunda, Vijendar
  Cc: Vijendar Mukunda, Mark Brown, broonie, alsa-devel, tiwai,
	lgirdwood, linux-kernel, Vijendar Mukunda, Alexander.Deucher,
	alsa-devel

The patch

   ASoC: amd: added error checks in dma driver

has been applied to the asoc tree at

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

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

>From 7afa535eb107d587b22ffbbbaaeb4a0b87b94496 Mon Sep 17 00:00:00 2001
From: "Mukunda, Vijendar" <Vijendar.Mukunda@amd.com>
Date: Mon, 4 Dec 2017 20:46:24 +0530
Subject: [PATCH] ASoC: amd: added error checks in dma driver

added additional error checks in acp dma driver
v2: printed error codes for acp init & acp deinit
failure cases.

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/amd/acp-pcm-dma.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index b5e41df6bb3a..c33a512283a4 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -850,6 +850,9 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
+	if (!rtd)
+		return -EINVAL;
+
 	buffersize = frames_to_bytes(runtime, runtime->buffer_size);
 	bytescount = acp_get_byte_count(rtd->acp_mmio, substream->stream);
 
@@ -875,6 +878,8 @@ static int acp_dma_prepare(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
+	if (!rtd)
+		return -EINVAL;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		config_acp_dma_channel(rtd->acp_mmio, SYSRAM_TO_ACP_CH_NUM,
 					PLAYBACK_START_DMA_DESCR_CH12,
@@ -1091,7 +1096,11 @@ static int acp_audio_probe(struct platform_device *pdev)
 	dev_set_drvdata(&pdev->dev, audio_drv_data);
 
 	/* Initialize the ACP */
-	acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
+	status = acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
+	if (status) {
+		dev_err(&pdev->dev, "ACP Init failed status:%d\n", status);
+		return status;
+	}
 
 	status = snd_soc_register_platform(&pdev->dev, &acp_asoc_platform);
 	if (status != 0) {
@@ -1108,9 +1117,12 @@ static int acp_audio_probe(struct platform_device *pdev)
 
 static int acp_audio_remove(struct platform_device *pdev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(&pdev->dev);
 
-	acp_deinit(adata->acp_mmio);
+	status = acp_deinit(adata->acp_mmio);
+	if (status)
+		dev_err(&pdev->dev, "ACP Deinit failed status:%d\n", status);
 	snd_soc_unregister_platform(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
@@ -1120,9 +1132,14 @@ static int acp_audio_remove(struct platform_device *pdev)
 static int acp_pcm_resume(struct device *dev)
 {
 	u16 bank;
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_init(adata->acp_mmio, adata->asic_type);
+	status = acp_init(adata->acp_mmio, adata->asic_type);
+	if (status) {
+		dev_err(dev, "ACP Init failed status:%d\n", status);
+		return status;
+	}
 
 	if (adata->play_stream && adata->play_stream->runtime) {
 		/* For Stoney, Memory gating is disabled,i.e SRAM Banks
@@ -1154,18 +1171,26 @@ static int acp_pcm_resume(struct device *dev)
 
 static int acp_pcm_runtime_suspend(struct device *dev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_deinit(adata->acp_mmio);
+	status = acp_deinit(adata->acp_mmio);
+	if (status)
+		dev_err(dev, "ACP Deinit failed status:%d\n", status);
 	acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
 
 static int acp_pcm_runtime_resume(struct device *dev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_init(adata->acp_mmio, adata->asic_type);
+	status = acp_init(adata->acp_mmio, adata->asic_type);
+	if (status) {
+		dev_err(dev, "ACP Init failed status:%d\n", status);
+		return status;
+	}
 	acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
-- 
2.15.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] ASoC: amd: added error checks in dma driver
  2017-11-28 11:52 ` Mark Brown
@ 2017-11-30  5:29   ` Mukunda,Vijendar
  0 siblings, 0 replies; 12+ messages in thread
From: Mukunda,Vijendar @ 2017-11-30  5:29 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, tiwai, lgirdwood, perex, Alexander.Deucher, linux-kernel



On Tuesday 28 November 2017 05:22 PM, Mark Brown wrote:
> On Tue, Nov 28, 2017 at 10:13:44AM +0530, Vijendar Mukunda wrote:
>
>> -	acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
>> +	status = acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
>> +	if (status) {
>> +		dev_err(&pdev->dev, "ACP Init failed\n");
>> +		return status;
>> +	}
>>   
> Better to print the error code to help people see what went wrong.
>
>>   static int acp_audio_remove(struct platform_device *pdev)
>>   {
>> +	int status;
>>   	struct audio_drv_data *adata = dev_get_drvdata(&pdev->dev);
>>   
>> -	acp_deinit(adata->acp_mmio);
>> +	status = acp_deinit(adata->acp_mmio);
>> +	if (status) {
>> +		dev_err(&pdev->dev, "ACP Deinit failed\n");
>> +		return status;
>> +	}
>>   	snd_soc_unregister_platform(&pdev->dev);
> Remove operations can't meaningfully fail, better to just log the error
> and carry on.
    Will prepare a patch based on your review comments and post it as V2 version.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ASoC: amd: added error checks in dma driver
  2017-11-28  4:43 [PATCH] ASoC: amd: added error checks in dma driver Vijendar Mukunda
@ 2017-11-28 11:52 ` Mark Brown
  2017-11-30  5:29   ` Mukunda,Vijendar
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2017-11-28 11:52 UTC (permalink / raw)
  To: Vijendar Mukunda
  Cc: alsa-devel, tiwai, lgirdwood, perex, Alexander.Deucher, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 851 bytes --]

On Tue, Nov 28, 2017 at 10:13:44AM +0530, Vijendar Mukunda wrote:

> -	acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
> +	status = acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
> +	if (status) {
> +		dev_err(&pdev->dev, "ACP Init failed\n");
> +		return status;
> +	}
>  

Better to print the error code to help people see what went wrong.

>  static int acp_audio_remove(struct platform_device *pdev)
>  {
> +	int status;
>  	struct audio_drv_data *adata = dev_get_drvdata(&pdev->dev);
>  
> -	acp_deinit(adata->acp_mmio);
> +	status = acp_deinit(adata->acp_mmio);
> +	if (status) {
> +		dev_err(&pdev->dev, "ACP Deinit failed\n");
> +		return status;
> +	}
>  	snd_soc_unregister_platform(&pdev->dev);

Remove operations can't meaningfully fail, better to just log the error
and carry on.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ASoC: amd: added error checks in dma driver
@ 2017-11-28  4:43 Vijendar Mukunda
  2017-11-28 11:52 ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Vijendar Mukunda @ 2017-11-28  4:43 UTC (permalink / raw)
  To: broonie, alsa-devel
  Cc: tiwai, lgirdwood, perex, Alexander.Deucher, linux-kernel,
	Vijendar Mukunda

added additional error checks in acp dma driver

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
---
 sound/soc/amd/acp-pcm-dma.c | 39 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index fb09578..71ab5b5 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -848,6 +848,9 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
+	if (!rtd)
+		return -EINVAL;
+
 	buffersize = frames_to_bytes(runtime, runtime->buffer_size);
 	bytescount = acp_get_byte_count(rtd->acp_mmio, substream->stream);
 
@@ -873,6 +876,8 @@ static int acp_dma_prepare(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
+	if (!rtd)
+		return -EINVAL;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		config_acp_dma_channel(rtd->acp_mmio, SYSRAM_TO_ACP_CH_NUM,
 					PLAYBACK_START_DMA_DESCR_CH12,
@@ -1082,7 +1087,11 @@ static int acp_audio_probe(struct platform_device *pdev)
 	dev_set_drvdata(&pdev->dev, audio_drv_data);
 
 	/* Initialize the ACP */
-	acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
+	status = acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
+	if (status) {
+		dev_err(&pdev->dev, "ACP Init failed\n");
+		return status;
+	}
 
 	status = snd_soc_register_platform(&pdev->dev, &acp_asoc_platform);
 	if (status != 0) {
@@ -1099,9 +1108,14 @@ static int acp_audio_probe(struct platform_device *pdev)
 
 static int acp_audio_remove(struct platform_device *pdev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(&pdev->dev);
 
-	acp_deinit(adata->acp_mmio);
+	status = acp_deinit(adata->acp_mmio);
+	if (status) {
+		dev_err(&pdev->dev, "ACP Deinit failed\n");
+		return status;
+	}
 	snd_soc_unregister_platform(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
@@ -1111,9 +1125,14 @@ static int acp_audio_remove(struct platform_device *pdev)
 static int acp_pcm_resume(struct device *dev)
 {
 	u16 bank;
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_init(adata->acp_mmio, adata->asic_type);
+	status = acp_init(adata->acp_mmio, adata->asic_type);
+	if (status) {
+		dev_err(dev, "ACP Init failed\n");
+		return status;
+	}
 
 	if (adata->play_stream && adata->play_stream->runtime) {
 		/* For Stoney, Memory gating is disabled,i.e SRAM Banks
@@ -1145,18 +1164,28 @@ static int acp_pcm_resume(struct device *dev)
 
 static int acp_pcm_runtime_suspend(struct device *dev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_deinit(adata->acp_mmio);
+	status = acp_deinit(adata->acp_mmio);
+	if (status) {
+		dev_err(dev, "ACP Deinit failed\n");
+		return status;
+	}
 	acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
 
 static int acp_pcm_runtime_resume(struct device *dev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_init(adata->acp_mmio, adata->asic_type);
+	status = acp_init(adata->acp_mmio, adata->asic_type);
+	if (status) {
+		dev_err(dev, "ACP Init failed\n");
+		return status;
+	}
 	acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] ASoC: amd: added error checks in dma driver
  2017-11-24  8:11       ` Guenter Roeck
@ 2017-11-24 14:19         ` Mukunda,Vijendar
  0 siblings, 0 replies; 12+ messages in thread
From: Mukunda,Vijendar @ 2017-11-24 14:19 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Brown, alsa-devel, Takashi Iwai, Liam Girdwood, perex,
	Alex Deucher, Akshu.Agrawal, Guenter Roeck, linux-kernel,
	Dominik Behr, Daniel Kurtz, Guenter Roeck



On Friday 24 November 2017 01:41 PM, Guenter Roeck wrote:
> On Fri, Nov 24, 2017 at 3:07 AM, Mukunda,Vijendar
> <vijendar.mukunda@amd.com> wrote:
>>
>>
>> On Thursday 23 November 2017 10:59 PM, Mark Brown wrote:
>>> On Thu, Nov 23, 2017 at 08:59:43AM -0800, Guenter Roeck wrote:
>>>> On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda
>>>> <Vijendar.Mukunda@amd.com> wrote:
>>>>> added error checks in acp dma driver
>>>>> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
>>>>> Signed-off-by: Akshu Agrawal <Akshu.Agrawal@amd.com>
>>>>> Signed-off-by: Guenter Roeck <groeck@chromium.org>
>>>> This is inappropriate.
>>> Specifically: if Guenter wasn't involved in writing or forwarding the
>>> patch he shouldn't have a signoff in there, and if you're the one
>>> sending the mail you should be the last person in the chain of signoffs.
>>> Please see SubmittingPatches for details of what a signoff means and why
>>> they're important.
>>
>>    This patch was implemented on top of changes implemented by Guenter.
>>    There is a separate thread - RE: [PATCH] ASoC: amd: Add error checking
>>    to probe function in which Guenter posted changes.
> That was my patch. This is yours.
>
> Guenter
     Got it , Let your patch go as it is. Will submit a fresh patch for additional
     error checks in acp dma driver.
>
>>        Got it, apologies will post changes as v2 version.
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ASoC: amd: added error checks in dma driver
  2017-11-23 17:29   ` Mark Brown
@ 2017-11-24 11:07     ` Mukunda,Vijendar
  2017-11-24  8:11       ` Guenter Roeck
  0 siblings, 1 reply; 12+ messages in thread
From: Mukunda,Vijendar @ 2017-11-24 11:07 UTC (permalink / raw)
  To: Mark Brown, Guenter Roeck
  Cc: alsa-devel, Takashi Iwai, Liam Girdwood, perex, Alex Deucher,
	Akshu.Agrawal, Guenter Roeck, linux-kernel, Dominik Behr,
	Daniel Kurtz, Guenter Roeck




On Thursday 23 November 2017 10:59 PM, Mark Brown wrote:
> On Thu, Nov 23, 2017 at 08:59:43AM -0800, Guenter Roeck wrote:
>> On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda
>> <Vijendar.Mukunda@amd.com> wrote:
>>> added error checks in acp dma driver
>>> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
>>> Signed-off-by: Akshu Agrawal <Akshu.Agrawal@amd.com>
>>> Signed-off-by: Guenter Roeck <groeck@chromium.org>
>> This is inappropriate.
> Specifically: if Guenter wasn't involved in writing or forwarding the
> patch he shouldn't have a signoff in there, and if you're the one
> sending the mail you should be the last person in the chain of signoffs.
> Please see SubmittingPatches for details of what a signoff means and why
> they're important.

   This patch was implemented on top of changes implemented by Guenter.
   There is a separate thread - RE: [PATCH] ASoC: amd: Add error checking
   to probe function in which Guenter posted changes.
     
   Got it, apologies will post changes as v2 version.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ASoC: amd: added error checks in dma driver
  2017-11-24 11:07     ` Mukunda,Vijendar
@ 2017-11-24  8:11       ` Guenter Roeck
  2017-11-24 14:19         ` Mukunda,Vijendar
  0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2017-11-24  8:11 UTC (permalink / raw)
  To: Mukunda,Vijendar
  Cc: Mark Brown, alsa-devel, Takashi Iwai, Liam Girdwood, perex,
	Alex Deucher, Akshu.Agrawal, Guenter Roeck, linux-kernel,
	Dominik Behr, Daniel Kurtz, Guenter Roeck

On Fri, Nov 24, 2017 at 3:07 AM, Mukunda,Vijendar
<vijendar.mukunda@amd.com> wrote:
>
>
>
> On Thursday 23 November 2017 10:59 PM, Mark Brown wrote:
>>
>> On Thu, Nov 23, 2017 at 08:59:43AM -0800, Guenter Roeck wrote:
>>>
>>> On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda
>>> <Vijendar.Mukunda@amd.com> wrote:
>>>>
>>>> added error checks in acp dma driver
>>>> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
>>>> Signed-off-by: Akshu Agrawal <Akshu.Agrawal@amd.com>
>>>> Signed-off-by: Guenter Roeck <groeck@chromium.org>
>>>
>>> This is inappropriate.
>>
>> Specifically: if Guenter wasn't involved in writing or forwarding the
>> patch he shouldn't have a signoff in there, and if you're the one
>> sending the mail you should be the last person in the chain of signoffs.
>> Please see SubmittingPatches for details of what a signoff means and why
>> they're important.
>
>
>   This patch was implemented on top of changes implemented by Guenter.
>   There is a separate thread - RE: [PATCH] ASoC: amd: Add error checking
>   to probe function in which Guenter posted changes.

That was my patch. This is yours.

Guenter

>       Got it, apologies will post changes as v2 version.
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ASoC: amd: added error checks in dma driver
  2017-11-23 16:59 ` Guenter Roeck
@ 2017-11-23 17:29   ` Mark Brown
  2017-11-24 11:07     ` Mukunda,Vijendar
  0 siblings, 1 reply; 12+ messages in thread
From: Mark Brown @ 2017-11-23 17:29 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Vijendar Mukunda, alsa-devel, Takashi Iwai, Liam Girdwood, perex,
	Alex Deucher, Akshu.Agrawal, Guenter Roeck, linux-kernel,
	Dominik Behr, Daniel Kurtz, Guenter Roeck

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

On Thu, Nov 23, 2017 at 08:59:43AM -0800, Guenter Roeck wrote:
> On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda
> <Vijendar.Mukunda@amd.com> wrote:
> > added error checks in acp dma driver

> > Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
> > Signed-off-by: Akshu Agrawal <Akshu.Agrawal@amd.com>
> > Signed-off-by: Guenter Roeck <groeck@chromium.org>

> This is inappropriate.

Specifically: if Guenter wasn't involved in writing or forwarding the
patch he shouldn't have a signoff in there, and if you're the one
sending the mail you should be the last person in the chain of signoffs.
Please see SubmittingPatches for details of what a signoff means and why
they're important.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ASoC: amd: added error checks in dma driver
  2017-11-23 16:30 Vijendar Mukunda
@ 2017-11-23 16:59 ` Guenter Roeck
  2017-11-23 17:29   ` Mark Brown
  0 siblings, 1 reply; 12+ messages in thread
From: Guenter Roeck @ 2017-11-23 16:59 UTC (permalink / raw)
  To: Vijendar Mukunda
  Cc: Mark Brown, alsa-devel, Takashi Iwai, Liam Girdwood, perex,
	Alex Deucher, Akshu.Agrawal, Guenter Roeck, linux-kernel,
	Dominik Behr, Daniel Kurtz, Guenter Roeck

On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda
<Vijendar.Mukunda@amd.com> wrote:
> added error checks in acp dma driver
>
> Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
> Signed-off-by: Akshu Agrawal <Akshu.Agrawal@amd.com>
> Signed-off-by: Guenter Roeck <groeck@chromium.org>

This is inappropriate.

Guenter

> ---
>  sound/soc/amd/acp-pcm-dma.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
> index 17d76fa..804e659 100644
> --- a/sound/soc/amd/acp-pcm-dma.c
> +++ b/sound/soc/amd/acp-pcm-dma.c
> @@ -848,6 +848,9 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
>         struct snd_pcm_runtime *runtime = substream->runtime;
>         struct audio_substream_data *rtd = runtime->private_data;
>
> +       if (!rtd)
> +               return -EINVAL;
> +
>         buffersize = frames_to_bytes(runtime, runtime->buffer_size);
>         bytescount = acp_get_byte_count(rtd->acp_mmio, substream->stream);
>
> @@ -873,6 +876,8 @@ static int acp_dma_prepare(struct snd_pcm_substream *substream)
>         struct snd_pcm_runtime *runtime = substream->runtime;
>         struct audio_substream_data *rtd = runtime->private_data;
>
> +       if (!rtd)
> +               return -EINVAL;
>         if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
>                 config_acp_dma_channel(rtd->acp_mmio, SYSRAM_TO_ACP_CH_NUM,
>                                         PLAYBACK_START_DMA_DESCR_CH12,
> @@ -1066,6 +1071,10 @@ static int acp_audio_probe(struct platform_device *pdev)
>         struct resource *res;
>         const u32 *pdata = pdev->dev.platform_data;
>
> +       if (!pdata) {
> +               dev_err(&pdev->dev, "Missing platform data\n");
> +               return -ENODEV;
> +       }
>         audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(struct audio_drv_data),
>                                         GFP_KERNEL);
>         if (audio_drv_data == NULL)
> @@ -1074,6 +1083,8 @@ static int acp_audio_probe(struct platform_device *pdev)
>         res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         audio_drv_data->acp_mmio = devm_ioremap_resource(&pdev->dev, res);
>
> +       if (IS_ERR(audio_drv_data->acp_mmio))
> +               return PTR_ERR(audio_drv_data->acp_mmio);
>         /* The following members gets populated in device 'open'
>          * function. Till then interrupts are disabled in 'acp_init'
>          * and device doesn't generate any interrupts.
> @@ -1099,7 +1110,11 @@ static int acp_audio_probe(struct platform_device *pdev)
>         dev_set_drvdata(&pdev->dev, audio_drv_data);
>
>         /* Initialize the ACP */
> -       acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
> +       status = acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
> +       if (status) {
> +               dev_err(&pdev->dev, "ACP Init failed\n");
> +               return status;
> +       }
>
>         status = snd_soc_register_platform(&pdev->dev, &acp_asoc_platform);
>         if (status != 0) {
> @@ -1116,9 +1131,14 @@ static int acp_audio_probe(struct platform_device *pdev)
>
>  static int acp_audio_remove(struct platform_device *pdev)
>  {
> +       int status;
>         struct audio_drv_data *adata = dev_get_drvdata(&pdev->dev);
>
> -       acp_deinit(adata->acp_mmio);
> +       status = acp_deinit(adata->acp_mmio);
> +       if (status) {
> +               dev_err(&pdev->dev, "ACP Deinit failed\n");
> +               return status;
> +       }
>         snd_soc_unregister_platform(&pdev->dev);
>         pm_runtime_disable(&pdev->dev);
>
> @@ -1128,9 +1148,14 @@ static int acp_audio_remove(struct platform_device *pdev)
>  static int acp_pcm_resume(struct device *dev)
>  {
>         u16 bank;
> +       int status;
>         struct audio_drv_data *adata = dev_get_drvdata(dev);
>
> -       acp_init(adata->acp_mmio, adata->asic_type);
> +       status = acp_init(adata->acp_mmio, adata->asic_type);
> +       if (status) {
> +               dev_err(dev, "ACP Init failed\n");
> +               return status;
> +       }
>
>         if (adata->play_stream && adata->play_stream->runtime) {
>                 /* For Stoney, Memory gating is disabled,i.e SRAM Banks
> @@ -1162,18 +1187,28 @@ static int acp_pcm_resume(struct device *dev)
>
>  static int acp_pcm_runtime_suspend(struct device *dev)
>  {
> +       int status;
>         struct audio_drv_data *adata = dev_get_drvdata(dev);
>
> -       acp_deinit(adata->acp_mmio);
> +       status = acp_deinit(adata->acp_mmio);
> +       if (status) {
> +               dev_err(dev, "ACP Deinit failed\n");
> +               return status;
> +       }
>         acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
>         return 0;
>  }
>
>  static int acp_pcm_runtime_resume(struct device *dev)
>  {
> +       int status;
>         struct audio_drv_data *adata = dev_get_drvdata(dev);
>
> -       acp_init(adata->acp_mmio, adata->asic_type);
> +       status = acp_init(adata->acp_mmio, adata->asic_type);
> +       if (status) {
> +               dev_err(dev, "ACP Init failed\n");
> +               return status;
> +       }
>         acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
>         return 0;
>  }
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] ASoC: amd: added error checks in dma driver
@ 2017-11-23 16:30 Vijendar Mukunda
  2017-11-23 16:59 ` Guenter Roeck
  0 siblings, 1 reply; 12+ messages in thread
From: Vijendar Mukunda @ 2017-11-23 16:30 UTC (permalink / raw)
  To: broonie, alsa-devel
  Cc: tiwai, lgirdwood, perex, Alexander.Deucher, Akshu.Agrawal, linux,
	linux-kernel, dbehr, djkurtz, Vijendar Mukunda, Guenter Roeck

added error checks in acp dma driver

Signed-off-by: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Signed-off-by: Akshu Agrawal <Akshu.Agrawal@amd.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
---
 sound/soc/amd/acp-pcm-dma.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 17d76fa..804e659 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -848,6 +848,9 @@ static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
+	if (!rtd)
+		return -EINVAL;
+
 	buffersize = frames_to_bytes(runtime, runtime->buffer_size);
 	bytescount = acp_get_byte_count(rtd->acp_mmio, substream->stream);
 
@@ -873,6 +876,8 @@ static int acp_dma_prepare(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct audio_substream_data *rtd = runtime->private_data;
 
+	if (!rtd)
+		return -EINVAL;
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		config_acp_dma_channel(rtd->acp_mmio, SYSRAM_TO_ACP_CH_NUM,
 					PLAYBACK_START_DMA_DESCR_CH12,
@@ -1066,6 +1071,10 @@ static int acp_audio_probe(struct platform_device *pdev)
 	struct resource *res;
 	const u32 *pdata = pdev->dev.platform_data;
 
+	if (!pdata) {
+		dev_err(&pdev->dev, "Missing platform data\n");
+		return -ENODEV;
+	}
 	audio_drv_data = devm_kzalloc(&pdev->dev, sizeof(struct audio_drv_data),
 					GFP_KERNEL);
 	if (audio_drv_data == NULL)
@@ -1074,6 +1083,8 @@ static int acp_audio_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	audio_drv_data->acp_mmio = devm_ioremap_resource(&pdev->dev, res);
 
+	if (IS_ERR(audio_drv_data->acp_mmio))
+		return PTR_ERR(audio_drv_data->acp_mmio);
 	/* The following members gets populated in device 'open'
 	 * function. Till then interrupts are disabled in 'acp_init'
 	 * and device doesn't generate any interrupts.
@@ -1099,7 +1110,11 @@ static int acp_audio_probe(struct platform_device *pdev)
 	dev_set_drvdata(&pdev->dev, audio_drv_data);
 
 	/* Initialize the ACP */
-	acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
+	status = acp_init(audio_drv_data->acp_mmio, audio_drv_data->asic_type);
+	if (status) {
+		dev_err(&pdev->dev, "ACP Init failed\n");
+		return status;
+	}
 
 	status = snd_soc_register_platform(&pdev->dev, &acp_asoc_platform);
 	if (status != 0) {
@@ -1116,9 +1131,14 @@ static int acp_audio_probe(struct platform_device *pdev)
 
 static int acp_audio_remove(struct platform_device *pdev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(&pdev->dev);
 
-	acp_deinit(adata->acp_mmio);
+	status = acp_deinit(adata->acp_mmio);
+	if (status) {
+		dev_err(&pdev->dev, "ACP Deinit failed\n");
+		return status;
+	}
 	snd_soc_unregister_platform(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
@@ -1128,9 +1148,14 @@ static int acp_audio_remove(struct platform_device *pdev)
 static int acp_pcm_resume(struct device *dev)
 {
 	u16 bank;
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_init(adata->acp_mmio, adata->asic_type);
+	status = acp_init(adata->acp_mmio, adata->asic_type);
+	if (status) {
+		dev_err(dev, "ACP Init failed\n");
+		return status;
+	}
 
 	if (adata->play_stream && adata->play_stream->runtime) {
 		/* For Stoney, Memory gating is disabled,i.e SRAM Banks
@@ -1162,18 +1187,28 @@ static int acp_pcm_resume(struct device *dev)
 
 static int acp_pcm_runtime_suspend(struct device *dev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_deinit(adata->acp_mmio);
+	status = acp_deinit(adata->acp_mmio);
+	if (status) {
+		dev_err(dev, "ACP Deinit failed\n");
+		return status;
+	}
 	acp_reg_write(0, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
 
 static int acp_pcm_runtime_resume(struct device *dev)
 {
+	int status;
 	struct audio_drv_data *adata = dev_get_drvdata(dev);
 
-	acp_init(adata->acp_mmio, adata->asic_type);
+	status = acp_init(adata->acp_mmio, adata->asic_type);
+	if (status) {
+		dev_err(dev, "ACP Init failed\n");
+		return status;
+	}
 	acp_reg_write(1, adata->acp_mmio, mmACP_EXTERNAL_INTR_ENB);
 	return 0;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-12-04 18:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-04 15:16 [PATCH] ASoC: amd: added error checks in dma driver Vijendar Mukunda
2017-12-04 17:54 ` Mark Brown
2017-12-04 18:49 ` Applied "ASoC: amd: added error checks in dma driver" to the asoc tree Mark Brown
  -- strict thread matches above, loose matches on Subject: below --
2017-11-28  4:43 [PATCH] ASoC: amd: added error checks in dma driver Vijendar Mukunda
2017-11-28 11:52 ` Mark Brown
2017-11-30  5:29   ` Mukunda,Vijendar
2017-11-23 16:30 Vijendar Mukunda
2017-11-23 16:59 ` Guenter Roeck
2017-11-23 17:29   ` Mark Brown
2017-11-24 11:07     ` Mukunda,Vijendar
2017-11-24  8:11       ` Guenter Roeck
2017-11-24 14:19         ` Mukunda,Vijendar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).