RE: [PATCH v2] ASoC: amd: put off registering mach platform_dev to avoid -517 err

["Mukunda, Vijendar" <Vijendar.Mukunda@amd.com>]

> -----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