Re: [PATCH] ASoC: amd: added error checks in dma driver
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. signature.asc Description: PGP signature
Re: [PATCH] ASoC: amd: added error checks in dma driver
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. signature.asc Description: PGP signature
[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--- 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(>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(>dev, "ACP Init failed status:%d\n", status); + return status; + } status = snd_soc_register_platform(>dev, _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(>dev); - acp_deinit(adata->acp_mmio); + status = acp_deinit(adata->acp_mmio); + if (status) + dev_err(>dev, "ACP Deinit failed status:%d\n", status); snd_soc_unregister_platform(>dev); pm_runtime_disable(>dev); @@ -,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
[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 --- 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(>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(>dev, "ACP Init failed status:%d\n", status); + return status; + } status = snd_soc_register_platform(>dev, _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(>dev); - acp_deinit(adata->acp_mmio); + status = acp_deinit(adata->acp_mmio); + if (status) + dev_err(>dev, "ACP Deinit failed status:%d\n", status); snd_soc_unregister_platform(>dev); pm_runtime_disable(>dev); @@ -,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
Re: [PATCH] ASoC: amd: added error checks in dma driver
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(>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(>dev); - acp_deinit(adata->acp_mmio); + status = acp_deinit(adata->acp_mmio); + if (status) { + dev_err(>dev, "ACP Deinit failed\n"); + return status; + } snd_soc_unregister_platform(>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.
Re: [PATCH] ASoC: amd: added error checks in dma driver
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(>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(>dev); - acp_deinit(adata->acp_mmio); + status = acp_deinit(adata->acp_mmio); + if (status) { + dev_err(>dev, "ACP Deinit failed\n"); + return status; + } snd_soc_unregister_platform(>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.
Re: [PATCH] ASoC: amd: added error checks in dma driver
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(>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(>dev); > > - acp_deinit(adata->acp_mmio); > + status = acp_deinit(adata->acp_mmio); > + if (status) { > + dev_err(>dev, "ACP Deinit failed\n"); > + return status; > + } > snd_soc_unregister_platform(>dev); Remove operations can't meaningfully fail, better to just log the error and carry on. signature.asc Description: PGP signature
Re: [PATCH] ASoC: amd: added error checks in dma driver
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(>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(>dev); > > - acp_deinit(adata->acp_mmio); > + status = acp_deinit(adata->acp_mmio); > + if (status) { > + dev_err(>dev, "ACP Deinit failed\n"); > + return status; > + } > snd_soc_unregister_platform(>dev); Remove operations can't meaningfully fail, better to just log the error and carry on. signature.asc Description: PGP signature
[PATCH] ASoC: amd: added error checks in dma driver
added additional error checks in acp dma driver Signed-off-by: Vijendar Mukunda--- 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(>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(>dev, "ACP Init failed\n"); + return status; + } status = snd_soc_register_platform(>dev, _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(>dev); - acp_deinit(adata->acp_mmio); + status = acp_deinit(adata->acp_mmio); + if (status) { + dev_err(>dev, "ACP Deinit failed\n"); + return status; + } snd_soc_unregister_platform(>dev); pm_runtime_disable(>dev); @@ -,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
[PATCH] ASoC: amd: added error checks in dma driver
added additional error checks in acp dma driver Signed-off-by: Vijendar Mukunda --- 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(>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(>dev, "ACP Init failed\n"); + return status; + } status = snd_soc_register_platform(>dev, _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(>dev); - acp_deinit(adata->acp_mmio); + status = acp_deinit(adata->acp_mmio); + if (status) { + dev_err(>dev, "ACP Deinit failed\n"); + return status; + } snd_soc_unregister_platform(>dev); pm_runtime_disable(>dev); @@ -,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
Re: [PATCH] ASoC: amd: added error checks in dma driver
On Friday 24 November 2017 01:41 PM, Guenter Roeck wrote: On Fri, Nov 24, 2017 at 3:07 AM, Mukunda,Vijendarwrote: 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 wrote: added error checks in acp dma driver Signed-off-by: Vijendar Mukunda Signed-off-by: Akshu Agrawal Signed-off-by: Guenter Roeck 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.
Re: [PATCH] ASoC: amd: added error checks in dma driver
On Friday 24 November 2017 01:41 PM, Guenter Roeck wrote: On Fri, Nov 24, 2017 at 3:07 AM, Mukunda,Vijendar 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 wrote: added error checks in acp dma driver Signed-off-by: Vijendar Mukunda Signed-off-by: Akshu Agrawal Signed-off-by: Guenter Roeck 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.
Re: [PATCH] ASoC: amd: added error checks in dma driver
On Fri, Nov 24, 2017 at 3:07 AM, Mukunda,Vijendarwrote: > > > > 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 >>> wrote: added error checks in acp dma driver Signed-off-by: Vijendar Mukunda Signed-off-by: Akshu Agrawal Signed-off-by: Guenter Roeck >>> >>> 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. >
Re: [PATCH] ASoC: amd: added error checks in dma driver
On Fri, Nov 24, 2017 at 3:07 AM, Mukunda,Vijendar 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 >>> wrote: added error checks in acp dma driver Signed-off-by: Vijendar Mukunda Signed-off-by: Akshu Agrawal Signed-off-by: Guenter Roeck >>> >>> 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. >
Re: [PATCH] ASoC: amd: added error checks in dma driver
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 Mukundawrote: added error checks in acp dma driver Signed-off-by: Vijendar Mukunda Signed-off-by: Akshu Agrawal Signed-off-by: Guenter Roeck 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.
Re: [PATCH] ASoC: amd: added error checks in dma driver
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 wrote: added error checks in acp dma driver Signed-off-by: Vijendar Mukunda Signed-off-by: Akshu Agrawal Signed-off-by: Guenter Roeck 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.
Re: [PATCH] ASoC: amd: added error checks in dma driver
On Thu, Nov 23, 2017 at 08:59:43AM -0800, Guenter Roeck wrote: > On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda >wrote: > > added error checks in acp dma driver > > Signed-off-by: Vijendar Mukunda > > Signed-off-by: Akshu Agrawal > > Signed-off-by: Guenter Roeck > 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. signature.asc Description: PGP signature
Re: [PATCH] ASoC: amd: added error checks in dma driver
On Thu, Nov 23, 2017 at 08:59:43AM -0800, Guenter Roeck wrote: > On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda > wrote: > > added error checks in acp dma driver > > Signed-off-by: Vijendar Mukunda > > Signed-off-by: Akshu Agrawal > > Signed-off-by: Guenter Roeck > 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. signature.asc Description: PGP signature
Re: [PATCH] ASoC: amd: added error checks in dma driver
On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukundawrote: > added error checks in acp dma driver > > Signed-off-by: Vijendar Mukunda > Signed-off-by: Akshu Agrawal > Signed-off-by: Guenter Roeck 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(>dev, "Missing platform data\n"); > + return -ENODEV; > + } > audio_drv_data = devm_kzalloc(>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(>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(>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(>dev, "ACP Init failed\n"); > + return status; > + } > > status = snd_soc_register_platform(>dev, _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(>dev); > > - acp_deinit(adata->acp_mmio); > + status = acp_deinit(adata->acp_mmio); > + if (status) { > + dev_err(>dev, "ACP Deinit failed\n"); > + return status; > + } > snd_soc_unregister_platform(>dev); > pm_runtime_disable(>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); > > -
Re: [PATCH] ASoC: amd: added error checks in dma driver
On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda wrote: > added error checks in acp dma driver > > Signed-off-by: Vijendar Mukunda > Signed-off-by: Akshu Agrawal > Signed-off-by: Guenter Roeck 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(>dev, "Missing platform data\n"); > + return -ENODEV; > + } > audio_drv_data = devm_kzalloc(>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(>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(>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(>dev, "ACP Init failed\n"); > + return status; > + } > > status = snd_soc_register_platform(>dev, _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(>dev); > > - acp_deinit(adata->acp_mmio); > + status = acp_deinit(adata->acp_mmio); > + if (status) { > + dev_err(>dev, "ACP Deinit failed\n"); > + return status; > + } > snd_soc_unregister_platform(>dev); > pm_runtime_disable(>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,
[PATCH] ASoC: amd: added error checks in dma driver
added error checks in acp dma driver Signed-off-by: Vijendar MukundaSigned-off-by: Akshu Agrawal Signed-off-by: Guenter Roeck --- 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(>dev, "Missing platform data\n"); + return -ENODEV; + } audio_drv_data = devm_kzalloc(>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(>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(>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(>dev, "ACP Init failed\n"); + return status; + } status = snd_soc_register_platform(>dev, _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(>dev); - acp_deinit(adata->acp_mmio); + status = acp_deinit(adata->acp_mmio); + if (status) { + dev_err(>dev, "ACP Deinit failed\n"); + return status; + } snd_soc_unregister_platform(>dev); pm_runtime_disable(>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
[PATCH] ASoC: amd: added error checks in dma driver
added error checks in acp dma driver Signed-off-by: Vijendar Mukunda Signed-off-by: Akshu Agrawal Signed-off-by: Guenter Roeck --- 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(>dev, "Missing platform data\n"); + return -ENODEV; + } audio_drv_data = devm_kzalloc(>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(>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(>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(>dev, "ACP Init failed\n"); + return status; + } status = snd_soc_register_platform(>dev, _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(>dev); - acp_deinit(adata->acp_mmio); + status = acp_deinit(adata->acp_mmio); + if (status) { + dev_err(>dev, "ACP Deinit failed\n"); + return status; + } snd_soc_unregister_platform(>dev); pm_runtime_disable(>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