* [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
@ 2020-05-22 8:17 Hui Wang
2020-05-22 9:16 ` Mukunda, Vijendar
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Hui Wang @ 2020-05-22 8:17 UTC (permalink / raw)
To: alsa-devel, broonie, Vijendar.Mukunda
If the mach driver's probe is called ahead of codec driver's probe,
the kernel will print -517 error message although the audio still
works finally:
acp_pdm_mach acp_pdm_mach.0: snd_soc_register_card(acp) failed: -517
we could workaround this issue by moving the registration of mach
platform_dev to plat driver's probe.
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
sound/soc/amd/renoir/acp3x-pdm-dma.c | 13 +++++++++++++
sound/soc/amd/renoir/rn-pci-acp3x.c | 3 ---
sound/soc/amd/renoir/rn_acp3x.h | 3 ++-
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/sound/soc/amd/renoir/acp3x-pdm-dma.c b/sound/soc/amd/renoir/acp3x-pdm-dma.c
index 623dfd3ea705..46055c244998 100644
--- a/sound/soc/amd/renoir/acp3x-pdm-dma.c
+++ b/sound/soc/amd/renoir/acp3x-pdm-dma.c
@@ -402,6 +402,7 @@ static int acp_pdm_audio_probe(struct platform_device *pdev)
struct pdm_dev_data *adata;
unsigned int irqflags;
int status;
+ struct platform_device_info pdevinfo = {0};
if (!pdev->dev.platform_data) {
dev_err(&pdev->dev, "platform_data not retrieved\n");
@@ -448,6 +449,16 @@ static int acp_pdm_audio_probe(struct platform_device *pdev)
dev_err(&pdev->dev, "ACP PDM IRQ request failed\n");
return -ENODEV;
}
+
+ pdevinfo.name = "acp_pdm_mach";
+ pdevinfo.id = 0;
+ pdevinfo.parent = &pdev->dev;
+ adata->m_pdev = platform_device_register_full(&pdevinfo);
+ if (IS_ERR(adata->m_pdev)) {
+ dev_err(&pdev->dev, "cannot register %s device\n",
+ pdevinfo.name);
+ return PTR_ERR(adata->m_pdev);
+ }
pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
pm_runtime_use_autosuspend(&pdev->dev);
pm_runtime_enable(&pdev->dev);
@@ -457,7 +468,9 @@ static int acp_pdm_audio_probe(struct platform_device *pdev)
static int acp_pdm_audio_remove(struct platform_device *pdev)
{
+ struct pdm_dev_data *adata = dev_get_drvdata(&pdev->dev);
pm_runtime_disable(&pdev->dev);
+ platform_device_unregister(adata->m_pdev);
return 0;
}
diff --git a/sound/soc/amd/renoir/rn-pci-acp3x.c b/sound/soc/amd/renoir/rn-pci-acp3x.c
index 859ed67b93cf..f5f450cbd249 100644
--- a/sound/soc/amd/renoir/rn-pci-acp3x.c
+++ b/sound/soc/amd/renoir/rn-pci-acp3x.c
@@ -230,9 +230,6 @@ static int snd_rn_acp_probe(struct pci_dev *pci,
pdevinfo[1].name = "dmic-codec";
pdevinfo[1].id = 0;
pdevinfo[1].parent = &pci->dev;
- pdevinfo[2].name = "acp_pdm_mach";
- pdevinfo[2].id = 0;
- pdevinfo[2].parent = &pci->dev;
for (index = 0; index < ACP_DEVS; index++) {
adata->pdev[index] =
platform_device_register_full(&pdevinfo[index]);
diff --git a/sound/soc/amd/renoir/rn_acp3x.h b/sound/soc/amd/renoir/rn_acp3x.h
index 75228e306e0b..232eda4db055 100644
--- a/sound/soc/amd/renoir/rn_acp3x.h
+++ b/sound/soc/amd/renoir/rn_acp3x.h
@@ -7,7 +7,7 @@
#include "rn_chip_offset_byte.h"
-#define ACP_DEVS 3
+#define ACP_DEVS 2
#define ACP_PHY_BASE_ADDRESS 0x1240000
#define ACP_REG_START 0x1240000
#define ACP_REG_END 0x1250200
@@ -59,6 +59,7 @@ struct pdm_dev_data {
u32 pdm_irq;
void __iomem *acp_base;
struct snd_pcm_substream *capture_stream;
+ struct platform_device *m_pdev;
};
struct pdm_stream_instance {
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* RE: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 8:17 [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err Hui Wang
@ 2020-05-22 9:16 ` Mukunda, Vijendar
2020-05-22 11:08 ` Mark Brown
2020-05-22 19:10 ` kbuild test robot
2 siblings, 0 replies; 14+ messages in thread
From: Mukunda, Vijendar @ 2020-05-22 9:16 UTC (permalink / raw)
To: Hui Wang, alsa-devel, broonie
> -----Original Message-----
> From: Hui Wang <hui.wang@canonical.com>
> Sent: Friday, May 22, 2020 1:48 PM
> To: alsa-devel@alsa-project.org; broonie@kernel.org; Mukunda, Vijendar
> <Vijendar.Mukunda@amd.com>
> Subject: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid
> -517 err
>
> If the mach driver's probe is called ahead of codec driver's probe,
> the kernel will print -517 error message although the audio still
> works finally:
> acp_pdm_mach acp_pdm_mach.0: snd_soc_register_card(acp) failed: -517
>
> we could workaround this issue by moving the registration of mach
> platform_dev to plat driver's probe.
ACP PCI driver creates the platform devices for DMA driver,
Machine driver & DMIC generic driver.
During machine driver probe, Sound card registration fails if
it is unable to find Codec component and platform component
throwing an error as EPROBE_DEFER.(-517)
It will try few more iterations to bind the DAI links.
I guess this is an expected behavior only.
Sound card registration is successful after several attempts.
This is the reason audio is working without any issues.
Below is the sample log.
acp_pdm_mach acp_pdm_mach.0: dmic-hifi <-> acp_rn_pdm_dma.0 mapping ok
Not really sure do we still need to fix the sound card registration fail issue?
We have seen similar issue in Raven I2S driver also.
Ideally ACP PCI device should be parent device for platform devices needed by
DMA driver, generic dmic driver and machine driver.
In one way, we are forcing machine driver gets probed only after
DMA driver and codec driver to avoid sound card registration failure
Correct me , If my understanding is wrong.
>
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
> sound/soc/amd/renoir/acp3x-pdm-dma.c | 13 +++++++++++++
> sound/soc/amd/renoir/rn-pci-acp3x.c | 3 ---
> sound/soc/amd/renoir/rn_acp3x.h | 3 ++-
> 3 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/amd/renoir/acp3x-pdm-dma.c
> b/sound/soc/amd/renoir/acp3x-pdm-dma.c
> index 623dfd3ea705..46055c244998 100644
> --- a/sound/soc/amd/renoir/acp3x-pdm-dma.c
> +++ b/sound/soc/amd/renoir/acp3x-pdm-dma.c
> @@ -402,6 +402,7 @@ static int acp_pdm_audio_probe(struct
> platform_device *pdev)
> struct pdm_dev_data *adata;
> unsigned int irqflags;
> int status;
> + struct platform_device_info pdevinfo = {0};
>
> if (!pdev->dev.platform_data) {
> dev_err(&pdev->dev, "platform_data not retrieved\n");
> @@ -448,6 +449,16 @@ static int acp_pdm_audio_probe(struct
> platform_device *pdev)
> dev_err(&pdev->dev, "ACP PDM IRQ request failed\n");
> return -ENODEV;
> }
> +
> + pdevinfo.name = "acp_pdm_mach";
> + pdevinfo.id = 0;
> + pdevinfo.parent = &pdev->dev;
> + adata->m_pdev = platform_device_register_full(&pdevinfo);
> + if (IS_ERR(adata->m_pdev)) {
> + dev_err(&pdev->dev, "cannot register %s device\n",
> + pdevinfo.name);
> + return PTR_ERR(adata->m_pdev);
> + }
> pm_runtime_set_autosuspend_delay(&pdev->dev,
> ACP_SUSPEND_DELAY_MS);
> pm_runtime_use_autosuspend(&pdev->dev);
> pm_runtime_enable(&pdev->dev);
> @@ -457,7 +468,9 @@ static int acp_pdm_audio_probe(struct
> platform_device *pdev)
>
> static int acp_pdm_audio_remove(struct platform_device *pdev)
> {
> + struct pdm_dev_data *adata = dev_get_drvdata(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> + platform_device_unregister(adata->m_pdev);
> return 0;
> }
>
> diff --git a/sound/soc/amd/renoir/rn-pci-acp3x.c b/sound/soc/amd/renoir/rn-
> pci-acp3x.c
> index 859ed67b93cf..f5f450cbd249 100644
> --- a/sound/soc/amd/renoir/rn-pci-acp3x.c
> +++ b/sound/soc/amd/renoir/rn-pci-acp3x.c
> @@ -230,9 +230,6 @@ static int snd_rn_acp_probe(struct pci_dev *pci,
> pdevinfo[1].name = "dmic-codec";
> pdevinfo[1].id = 0;
> pdevinfo[1].parent = &pci->dev;
> - pdevinfo[2].name = "acp_pdm_mach";
> - pdevinfo[2].id = 0;
> - pdevinfo[2].parent = &pci->dev;
> for (index = 0; index < ACP_DEVS; index++) {
> adata->pdev[index] =
>
> platform_device_register_full(&pdevinfo[index]);
> diff --git a/sound/soc/amd/renoir/rn_acp3x.h
> b/sound/soc/amd/renoir/rn_acp3x.h
> index 75228e306e0b..232eda4db055 100644
> --- a/sound/soc/amd/renoir/rn_acp3x.h
> +++ b/sound/soc/amd/renoir/rn_acp3x.h
> @@ -7,7 +7,7 @@
>
> #include "rn_chip_offset_byte.h"
>
> -#define ACP_DEVS 3
> +#define ACP_DEVS 2
> #define ACP_PHY_BASE_ADDRESS 0x1240000
> #define ACP_REG_START 0x1240000
> #define ACP_REG_END 0x1250200
> @@ -59,6 +59,7 @@ struct pdm_dev_data {
> u32 pdm_irq;
> void __iomem *acp_base;
> struct snd_pcm_substream *capture_stream;
> + struct platform_device *m_pdev;
> };
>
> struct pdm_stream_instance {
> --
> 2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 8:17 [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err Hui Wang
2020-05-22 9:16 ` Mukunda, Vijendar
@ 2020-05-22 11:08 ` Mark Brown
2020-05-22 11:13 ` Mukunda, Vijendar
2020-05-22 19:10 ` kbuild test robot
2 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2020-05-22 11:08 UTC (permalink / raw)
To: Hui Wang; +Cc: alsa-devel, Vijendar.Mukunda
[-- Attachment #1: Type: text/plain, Size: 460 bytes --]
On Fri, May 22, 2020 at 04:17:38PM +0800, Hui Wang wrote:
> If the mach driver's probe is called ahead of codec driver's probe,
> the kernel will print -517 error message although the audio still
> works finally:
> acp_pdm_mach acp_pdm_mach.0: snd_soc_register_card(acp) failed: -517
This is just a bodge which will be at best unrelaible, it might work
around your problem right now on your specific system but it may well
introduce issues on someone else's.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 11:08 ` Mark Brown
@ 2020-05-22 11:13 ` Mukunda, Vijendar
2020-05-22 11:22 ` Mark Brown
0 siblings, 1 reply; 14+ messages in thread
From: Mukunda, Vijendar @ 2020-05-22 11:13 UTC (permalink / raw)
To: Mark Brown, Hui Wang; +Cc: alsa-devel
[AMD Official Use Only - Internal Distribution Only]
> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, May 22, 2020 4:38 PM
> To: Hui Wang <hui.wang@canonical.com>
> Cc: alsa-devel@alsa-project.org; Mukunda, Vijendar
> <Vijendar.Mukunda@amd.com>
> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to
> avoid -517 err
>
> On Fri, May 22, 2020 at 04:17:38PM +0800, Hui Wang wrote:
> > If the mach driver's probe is called ahead of codec driver's probe,
> > the kernel will print -517 error message although the audio still
> > works finally:
> > acp_pdm_mach acp_pdm_mach.0: snd_soc_register_card(acp) failed: -517
>
> This is just a bodge which will be at best unrelaible, it might work
> around your problem right now on your specific system but it may well
> introduce issues on someone else's.
Does sound card registration failure at boot time due to modules loading order can be considered as bug ?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 11:13 ` Mukunda, Vijendar
@ 2020-05-22 11:22 ` Mark Brown
2020-05-22 11:28 ` Mukunda, Vijendar
0 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2020-05-22 11:22 UTC (permalink / raw)
To: Mukunda, Vijendar; +Cc: Hui Wang, alsa-devel
[-- Attachment #1: Type: text/plain, Size: 285 bytes --]
On Fri, May 22, 2020 at 11:13:43AM +0000, Mukunda, Vijendar wrote:
> Does sound card registration failure at boot time due to modules loading order can be considered as bug ?
No, this is totally normal. If it failed to bind ever then that'd be a
problem but this is just deferring.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 11:22 ` Mark Brown
@ 2020-05-22 11:28 ` Mukunda, Vijendar
2020-05-22 12:59 ` Hui Wang
0 siblings, 1 reply; 14+ messages in thread
From: Mukunda, Vijendar @ 2020-05-22 11:28 UTC (permalink / raw)
To: Mark Brown; +Cc: Hui Wang, alsa-devel
> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, May 22, 2020 4:52 PM
> To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>
> Cc: Hui Wang <hui.wang@canonical.com>; alsa-devel@alsa-project.org
> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to
> avoid -517 err
>
> On Fri, May 22, 2020 at 11:13:43AM +0000, Mukunda, Vijendar wrote:
>
> > Does sound card registration failure at boot time due to modules loading
> order can be considered as bug ?
>
> No, this is totally normal. If it failed to bind ever then that'd be a
> problem but this is just deferring.
In that case, we don't need this work around I guess.
At maximum during boot time, we may see few sound card registration failure logs,
Which is normal.
With this change, our concept of ACP parent device which will create platform devices
for DMA driver, Generic DMIC driver and Machine driver will be changed.
We implemented same design for Raven I2S driver as well which got productized.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 11:28 ` Mukunda, Vijendar
@ 2020-05-22 12:59 ` Hui Wang
2020-05-22 13:57 ` Hui Wang
0 siblings, 1 reply; 14+ messages in thread
From: Hui Wang @ 2020-05-22 12:59 UTC (permalink / raw)
To: Mukunda, Vijendar, Mark Brown; +Cc: alsa-devel
On 2020/5/22 下午7:28, Mukunda, Vijendar wrote:
>
>> -----Original Message-----
>> From: Mark Brown <broonie@kernel.org>
>> Sent: Friday, May 22, 2020 4:52 PM
>> To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>
>> Cc: Hui Wang <hui.wang@canonical.com>; alsa-devel@alsa-project.org
>> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to
>> avoid -517 err
>>
>> On Fri, May 22, 2020 at 11:13:43AM +0000, Mukunda, Vijendar wrote:
>>
>>> Does sound card registration failure at boot time due to modules loading
>> order can be considered as bug ?
>>
>> No, this is totally normal. If it failed to bind ever then that'd be a
>> problem but this is just deferring.
> In that case, we don't need this work around I guess.
> At maximum during boot time, we may see few sound card registration failure logs,
> Which is normal.
>
> With this change, our concept of ACP parent device which will create platform devices
> for DMA driver, Generic DMIC driver and Machine driver will be changed.
> We implemented same design for Raven I2S driver as well which got productized.
>
OK, got it. It doesn't affect the sound function anyway.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 12:59 ` Hui Wang
@ 2020-05-22 13:57 ` Hui Wang
2020-05-22 14:30 ` Mukunda, Vijendar
0 siblings, 1 reply; 14+ messages in thread
From: Hui Wang @ 2020-05-22 13:57 UTC (permalink / raw)
To: Mukunda, Vijendar, Mark Brown; +Cc: alsa-devel
On 2020/5/22 下午8:59, Hui Wang wrote:
>
> On 2020/5/22 下午7:28, Mukunda, Vijendar wrote:
>>
>>> -----Original Message-----
>>> From: Mark Brown <broonie@kernel.org>
>>> Sent: Friday, May 22, 2020 4:52 PM
>>> To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>
>>> Cc: Hui Wang <hui.wang@canonical.com>; alsa-devel@alsa-project.org
>>> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach
>>> platform_dev to
>>> avoid -517 err
>>>
>>> On Fri, May 22, 2020 at 11:13:43AM +0000, Mukunda, Vijendar wrote:
>>>
>>>> Does sound card registration failure at boot time due to modules
>>>> loading
>>> order can be considered as bug ?
>>>
>>> No, this is totally normal. If it failed to bind ever then that'd be a
>>> problem but this is just deferring.
>> In that case, we don't need this work around I guess.
>> At maximum during boot time, we may see few sound card registration
>> failure logs,
>> Which is normal.
>>
>> With this change, our concept of ACP parent device which will create
>> platform devices
>> for DMA driver, Generic DMIC driver and Machine driver will be changed.
>> We implemented same design for Raven I2S driver as well which got
>> productized.
>>
> OK, got it. It doesn't affect the sound function anyway.
BTW, so far, I have not seen the -517 error from ubuntu kernel dmesg on
PC or laptop. Maybe this is the 1st driver to print this error in the
dmesg on PC or laptop, so I guess it probably confuses the users or our QA.
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 13:57 ` Hui Wang
@ 2020-05-22 14:30 ` Mukunda, Vijendar
2020-05-22 15:14 ` Mukunda, Vijendar
0 siblings, 1 reply; 14+ messages in thread
From: Mukunda, Vijendar @ 2020-05-22 14:30 UTC (permalink / raw)
To: Hui Wang, Mark Brown; +Cc: alsa-devel
[AMD Official Use Only - Internal Distribution Only]
> -----Original Message-----
> From: Hui Wang <hui.wang@canonical.com>
> Sent: Friday, May 22, 2020 7:27 PM
> To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>; Mark Brown
> <broonie@kernel.org>
> Cc: alsa-devel@alsa-project.org
> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to
> avoid -517 err
>
>
> On 2020/5/22 下午8:59, Hui Wang wrote:
> >
> > On 2020/5/22 下午7:28, Mukunda, Vijendar wrote:
> >>
> >>> -----Original Message-----
> >>> From: Mark Brown <broonie@kernel.org>
> >>> Sent: Friday, May 22, 2020 4:52 PM
> >>> To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>
> >>> Cc: Hui Wang <hui.wang@canonical.com>; alsa-devel@alsa-project.org
> >>> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach
> >>> platform_dev to
> >>> avoid -517 err
> >>>
> >>> On Fri, May 22, 2020 at 11:13:43AM +0000, Mukunda, Vijendar wrote:
> >>>
> >>>> Does sound card registration failure at boot time due to modules
> >>>> loading
> >>> order can be considered as bug ?
> >>>
> >>> No, this is totally normal. If it failed to bind ever then that'd be a
> >>> problem but this is just deferring.
> >> In that case, we don't need this work around I guess.
> >> At maximum during boot time, we may see few sound card registration
> >> failure logs,
> >> Which is normal.
> >>
> >> With this change, our concept of ACP parent device which will create
> >> platform devices
> >> for DMA driver, Generic DMIC driver and Machine driver will be changed.
> >> We implemented same design for Raven I2S driver as well which got
> >> productized.
> >>
> > OK, got it. It doesn't affect the sound function anyway.
>
> BTW, so far, I have not seen the -517 error from ubuntu kernel dmesg on
> PC or laptop. Maybe this is the 1st driver to print this error in the
> dmesg on PC or laptop, so I guess it probably confuses the users or our QA.
>
We have similar design for I2S endpoint which got productized on other OS
variant.
Most of the PC and Laptop comes up with Legacy HD Audio Driver solution.
This is ASoC based driver.
As this failure message only appears during boot time, Its okay to go ahead.
End user should really worry if sound card is not created even after system
boots up.
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 14:30 ` Mukunda, Vijendar
@ 2020-05-22 15:14 ` Mukunda, Vijendar
2020-05-22 15:32 ` Mark Brown
0 siblings, 1 reply; 14+ messages in thread
From: Mukunda, Vijendar @ 2020-05-22 15:14 UTC (permalink / raw)
To: Hui Wang, Mark Brown; +Cc: alsa-devel
> -----Original Message-----
> From: Mukunda, Vijendar
> Sent: Friday, May 22, 2020 8:01 PM
> To: 'Hui Wang' <hui.wang@canonical.com>; Mark Brown
> <broonie@kernel.org>
> Cc: alsa-devel@alsa-project.org
> Subject: RE: [PATCH v2] ASoC: amd: put off registering mach platform_dev to
> avoid -517 err
>
> [AMD Official Use Only - Internal Distribution Only]
>
>
>
> > -----Original Message-----
> > From: Hui Wang <hui.wang@canonical.com>
> > Sent: Friday, May 22, 2020 7:27 PM
> > To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>; Mark Brown
> > <broonie@kernel.org>
> > Cc: alsa-devel@alsa-project.org
> > Subject: Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to
> > avoid -517 err
> >
> >
> > On 2020/5/22 下午8:59, Hui Wang wrote:
> > >
> > > On 2020/5/22 下午7:28, Mukunda, Vijendar wrote:
> > >>
> > >>> -----Original Message-----
> > >>> From: Mark Brown <broonie@kernel.org>
> > >>> Sent: Friday, May 22, 2020 4:52 PM
> > >>> To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>
> > >>> Cc: Hui Wang <hui.wang@canonical.com>; alsa-devel@alsa-project.org
> > >>> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach
> > >>> platform_dev to
> > >>> avoid -517 err
> > >>>
> > >>> On Fri, May 22, 2020 at 11:13:43AM +0000, Mukunda, Vijendar wrote:
> > >>>
> > >>>> Does sound card registration failure at boot time due to modules
> > >>>> loading
> > >>> order can be considered as bug ?
> > >>>
> > >>> No, this is totally normal. If it failed to bind ever then that'd be a
> > >>> problem but this is just deferring.
> > >> In that case, we don't need this work around I guess.
> > >> At maximum during boot time, we may see few sound card registration
> > >> failure logs,
> > >> Which is normal.
> > >>
> > >> With this change, our concept of ACP parent device which will create
> > >> platform devices
> > >> for DMA driver, Generic DMIC driver and Machine driver will be changed.
> > >> We implemented same design for Raven I2S driver as well which got
> > >> productized.
> > >>
> > > OK, got it. It doesn't affect the sound function anyway.
> >
> > BTW, so far, I have not seen the -517 error from ubuntu kernel dmesg on
> > PC or laptop. Maybe this is the 1st driver to print this error in the
> > dmesg on PC or laptop, so I guess it probably confuses the users or our QA.
> >
>
> We have similar design for I2S endpoint which got productized on other OS
> variant.
> Most of the PC and Laptop comes up with Legacy HD Audio Driver solution.
> This is ASoC based driver.
> As this failure message only appears during boot time, Its okay to go ahead.
> End user should really worry if sound card is not created even after system
> boots up.
I have seen sample implementation of deferred probe in one of the machine
driver code using late_initcall() API.
Not sure how this api really works which will resolve the modules loading sequence
issue.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 15:14 ` Mukunda, Vijendar
@ 2020-05-22 15:32 ` Mark Brown
2020-05-23 0:11 ` Mukunda, Vijendar
0 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2020-05-22 15:32 UTC (permalink / raw)
To: Mukunda, Vijendar; +Cc: Hui Wang, alsa-devel
[-- Attachment #1: Type: text/plain, Size: 693 bytes --]
On Fri, May 22, 2020 at 03:14:21PM +0000, Mukunda, Vijendar wrote:
> I have seen sample implementation of deferred probe in one of the machine
> driver code using late_initcall() API.
> Not sure how this api really works which will resolve the modules loading sequence
> issue.
What deferred probe does is keep a list of devices that failed to bind
with a deferred probe error code then every time a device does manage to
bind it retries all those failed devices in case the new device provides
whatever was missing from one of the others. It's a bit brute force and
ignorance but it does sort things out in the end if all the drivers are
actually there and just loaded in the wrong order.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 15:32 ` Mark Brown
@ 2020-05-23 0:11 ` Mukunda, Vijendar
2020-05-25 3:32 ` Hui Wang
0 siblings, 1 reply; 14+ messages in thread
From: Mukunda, Vijendar @ 2020-05-23 0:11 UTC (permalink / raw)
To: Mark Brown; +Cc: Hui Wang, alsa-devel
> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, May 22, 2020 9:02 PM
> To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>
> Cc: Hui Wang <hui.wang@canonical.com>; alsa-devel@alsa-project.org
> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to
> avoid -517 err
>
> On Fri, May 22, 2020 at 03:14:21PM +0000, Mukunda, Vijendar wrote:
>
> > I have seen sample implementation of deferred probe in one of the machine
> > driver code using late_initcall() API.
> > Not sure how this api really works which will resolve the modules loading
> sequence
> > issue.
>
> What deferred probe does is keep a list of devices that failed to bind
> with a deferred probe error code then every time a device does manage to
> bind it retries all those failed devices in case the new device provides
> whatever was missing from one of the others. It's a bit brute force and
> ignorance but it does sort things out in the end if all the drivers are
> actually there and just loaded in the wrong order.
We see one more problem with this patch.
We are going to add support for I2S endpoint on another
platform based on Renoir.
Platform device creation logic going to be expanded in ACP PCI
driver probe call.
Because ACP Is Parent device , Ideally platform devices creation logic
should be handled in ACP PCI driver.
Based on Audio configuration, During ACP PCI driver probe call,
platform devices will be created.
In case of I2S endpoint support, machine driver gets probed via acpi dev id match.
If we go ahead with this patch, We have to expand the work around logic
by adding extra check in PDM DMA driver which doesn't seems to be good.
Currently I observed only two times sound card registration failure in dmesg
during boot time.
For the sake of avoiding few card registration failure logs observed in dmesg,
I don't think at this stage, we really need to go ahead with this patch.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-23 0:11 ` Mukunda, Vijendar
@ 2020-05-25 3:32 ` Hui Wang
0 siblings, 0 replies; 14+ messages in thread
From: Hui Wang @ 2020-05-25 3:32 UTC (permalink / raw)
To: Mukunda, Vijendar, Mark Brown; +Cc: alsa-devel
On 2020/5/23 上午8:11, Mukunda, Vijendar wrote:
>
>> -----Original Message-----
>> From: Mark Brown <broonie@kernel.org>
>> Sent: Friday, May 22, 2020 9:02 PM
>> To: Mukunda, Vijendar <Vijendar.Mukunda@amd.com>
>> Cc: Hui Wang <hui.wang@canonical.com>; alsa-devel@alsa-project.org
>> Subject: Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to
>> avoid -517 err
>>
>> On Fri, May 22, 2020 at 03:14:21PM +0000, Mukunda, Vijendar wrote:
>>
>>> I have seen sample implementation of deferred probe in one of the machine
>>> driver code using late_initcall() API.
>>> Not sure how this api really works which will resolve the modules loading
>> sequence
>>> issue.
>> What deferred probe does is keep a list of devices that failed to bind
>> with a deferred probe error code then every time a device does manage to
>> bind it retries all those failed devices in case the new device provides
>> whatever was missing from one of the others. It's a bit brute force and
>> ignorance but it does sort things out in the end if all the drivers are
>> actually there and just loaded in the wrong order.
> We see one more problem with this patch.
> We are going to add support for I2S endpoint on another
> platform based on Renoir.
>
> Platform device creation logic going to be expanded in ACP PCI
> driver probe call.
> Because ACP Is Parent device , Ideally platform devices creation logic
> should be handled in ACP PCI driver.
>
> Based on Audio configuration, During ACP PCI driver probe call,
> platform devices will be created.
>
> In case of I2S endpoint support, machine driver gets probed via acpi dev id match.
> If we go ahead with this patch, We have to expand the work around logic
> by adding extra check in PDM DMA driver which doesn't seems to be good.
>
> Currently I observed only two times sound card registration failure in dmesg
> during boot time.
> For the sake of avoiding few card registration failure logs observed in dmesg,
> I don't think at this stage, we really need to go ahead with this patch.
OK, got it, thanks.
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err
2020-05-22 8:17 [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err Hui Wang
2020-05-22 9:16 ` Mukunda, Vijendar
2020-05-22 11:08 ` Mark Brown
@ 2020-05-22 19:10 ` kbuild test robot
2 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2020-05-22 19:10 UTC (permalink / raw)
To: Hui Wang, alsa-devel, broonie, Vijendar.Mukunda; +Cc: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3727 bytes --]
Hi Hui,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on asoc/for-next]
[also build test WARNING on next-20200522]
[cannot apply to v5.7-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Hui-Wang/ASoC-amd-put-off-registering-mach-platform_dev-to-avoid-517-err/20200522-162223
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-193-gb8fad4bc-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=x86_64 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> sound/soc/amd/renoir/acp3x-pdm-dma.c:405:49: sparse: sparse: Using plain integer as NULL pointer
vim +405 sound/soc/amd/renoir/acp3x-pdm-dma.c
398
399 static int acp_pdm_audio_probe(struct platform_device *pdev)
400 {
401 struct resource *res;
402 struct pdm_dev_data *adata;
403 unsigned int irqflags;
404 int status;
> 405 struct platform_device_info pdevinfo = {0};
406
407 if (!pdev->dev.platform_data) {
408 dev_err(&pdev->dev, "platform_data not retrieved\n");
409 return -ENODEV;
410 }
411 irqflags = *((unsigned int *)(pdev->dev.platform_data));
412
413 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
414 if (!res) {
415 dev_err(&pdev->dev, "IORESOURCE_MEM FAILED\n");
416 return -ENODEV;
417 }
418
419 adata = devm_kzalloc(&pdev->dev, sizeof(*adata), GFP_KERNEL);
420 if (!adata)
421 return -ENOMEM;
422
423 adata->acp_base = devm_ioremap(&pdev->dev, res->start,
424 resource_size(res));
425 if (!adata->acp_base)
426 return -ENOMEM;
427
428 res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
429 if (!res) {
430 dev_err(&pdev->dev, "IORESOURCE_IRQ FAILED\n");
431 return -ENODEV;
432 }
433
434 adata->pdm_irq = res->start;
435 adata->capture_stream = NULL;
436
437 dev_set_drvdata(&pdev->dev, adata);
438 status = devm_snd_soc_register_component(&pdev->dev,
439 &acp_pdm_component,
440 &acp_pdm_dai_driver, 1);
441 if (status) {
442 dev_err(&pdev->dev, "Fail to register acp pdm dai\n");
443
444 return -ENODEV;
445 }
446 status = devm_request_irq(&pdev->dev, adata->pdm_irq, pdm_irq_handler,
447 irqflags, "ACP_PDM_IRQ", adata);
448 if (status) {
449 dev_err(&pdev->dev, "ACP PDM IRQ request failed\n");
450 return -ENODEV;
451 }
452
453 pdevinfo.name = "acp_pdm_mach";
454 pdevinfo.id = 0;
455 pdevinfo.parent = &pdev->dev;
456 adata->m_pdev = platform_device_register_full(&pdevinfo);
457 if (IS_ERR(adata->m_pdev)) {
458 dev_err(&pdev->dev, "cannot register %s device\n",
459 pdevinfo.name);
460 return PTR_ERR(adata->m_pdev);
461 }
462 pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
463 pm_runtime_use_autosuspend(&pdev->dev);
464 pm_runtime_enable(&pdev->dev);
465 pm_runtime_allow(&pdev->dev);
466 return 0;
467 }
468
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 74068 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-05-25 3:33 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22 8:17 [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err Hui Wang
2020-05-22 9:16 ` Mukunda, Vijendar
2020-05-22 11:08 ` Mark Brown
2020-05-22 11:13 ` Mukunda, Vijendar
2020-05-22 11:22 ` Mark Brown
2020-05-22 11:28 ` Mukunda, Vijendar
2020-05-22 12:59 ` Hui Wang
2020-05-22 13:57 ` Hui Wang
2020-05-22 14:30 ` Mukunda, Vijendar
2020-05-22 15:14 ` Mukunda, Vijendar
2020-05-22 15:32 ` Mark Brown
2020-05-23 0:11 ` Mukunda, Vijendar
2020-05-25 3:32 ` Hui Wang
2020-05-22 19:10 ` kbuild test robot
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).