> -----Original Message----- > From: Agrawal, Akshu > Sent: Tuesday, November 21, 2017 1:15 AM > To: Deucher, Alexander; 'Guenter Roeck'; Liam Girdwood; Mukunda, > Vijendar > Cc: Mark Brown; Jaroslav Kysela; Takashi Iwai; alsa-devel@xxxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; Dominik Behr; Daniel Kurtz > Subject: Re: [PATCH] ASoC: amd: Add error checking to probe function > > > > On 11/21/2017 10:17 AM, Deucher, Alexander wrote: > >> -----Original Message----- > >> From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter > >> Roeck > >> Sent: Monday, November 20, 2017 11:28 PM > >> To: Liam Girdwood > >> Cc: Mark Brown; Jaroslav Kysela; Takashi Iwai; alsa-devel@alsa- > project.org; > >> linux-kernel@xxxxxxxxxxxxxxx; Guenter Roeck; Deucher, Alexander; > Dominik > >> Behr; Daniel Kurtz > >> Subject: [PATCH] ASoC: amd: Add error checking to probe function > >> > >> The acp_audio_dma does not perform sufficient error checking in its > probe > >> function. This can result in crashes if a critical error path is > >> encountered. > >> > >> Fixes: 7c31335a03b6a ("ASoC: AMD: add AMD ASoC ACP 2.x DMA driver") > >> Cc: Alex Deucher <alexander.deucher@xxxxxxx> > >> Cc: Dominik Behr <dbehr@xxxxxxxxxxxx> > >> Cc: Daniel Kurtz <djkurtz@xxxxxxxxxxxx> > >> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx> > >> --- > >> I didn't add an error check to acp_init() since I was not sure if > >> its return value is ignored on purpose. > > > > Vijendar, Akshu can you comment? > > This is also the case of missing error check. > acp_init will return error if either sw reset did not happen or clock > did not get enabled. In both cases we should error out in probe. > Can you send out a patch to enable that error checking? Thanks, Alex > > > > The patch looks good to me. > > Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx> > > > >> > >> sound/soc/amd/acp-pcm-dma.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm- > >> dma.c > >> index 9f521a55d610..b5e41df6bb3a 100644 > >> --- a/sound/soc/amd/acp-pcm-dma.c > >> +++ b/sound/soc/amd/acp-pcm-dma.c > >> @@ -1051,6 +1051,11 @@ 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) > >> @@ -1058,6 +1063,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' > >> -- > >> 2.7.4 > > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
