[AMD Official Use Only - Internal Distribution Only]
> -----Original Message-----
> From: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> Sent: Wednesday, May 6, 2020 3:33 AM
> To: Alex Deucher <alexdeucher@xxxxxxxxx>; alsa-devel@xxxxxxxxxxxxxxxx;
> broonie@xxxxxxxxxx; Mukunda, Vijendar <Vijendar.Mukunda@xxxxxxx>;
> tiwai@xxxxxxx
> Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>
> Subject: Re: [PATCH 05/14] ASoC: amd: add ACP3x PDM platform driver
>
>
>
>
> > +static struct snd_soc_dai_ops acp_pdm_dai_ops = {
> > + .hw_params = NULL,
> > + .trigger = NULL,
> > +};
>
> doesn't seem very useful? can you remove this pdm_dai_ops?
Will remove it .
>
> > +
> > +static struct snd_soc_dai_driver acp_pdm_dai_driver = {
> > + .capture = {
> > + .rates = SNDRV_PCM_RATE_48000,
> > + .formats = SNDRV_PCM_FMTBIT_S24_LE |
> > + SNDRV_PCM_FMTBIT_S32_LE,
> > + .channels_min = 2,
> > + .channels_max = 2,
> > + .rate_min = 48000,
> > + .rate_max = 48000,
> > + },
> > + .ops = &acp_pdm_dai_ops,
> > +};
> > +
> > +static const struct snd_soc_component_driver acp_pdm_component = {
> > + .name = DRV_NAME,
> > +};
> > +
> > +static int acp_pdm_audio_probe(struct platform_device *pdev)
> > +{
> > + struct resource *res;
> > + struct pdm_dev_data *adata;
> > + unsigned int irqflags;
> > + int status;
> > +
> > + if (!pdev->dev.platform_data) {
> > + dev_err(&pdev->dev, "platform_data not retrieved\n");
> > + return -ENODEV;
> > + }
>
> is this test needed?
Its only provides an extra check. This is a child device created by ACP device.
ACP device passes memory and IRQ resources to this child device.
>
> > + irqflags = *((unsigned int *)(pdev->dev.platform_data));
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!res) {
> > + dev_err(&pdev->dev, "IORESOURCE_MEM FAILED\n");
> > + return -ENODEV;
> > + }
> > +
> > + adata = devm_kzalloc(&pdev->dev, sizeof(*adata), GFP_KERNEL);
> > + if (!adata)
> > + return -ENOMEM;
> > +
> > + adata->acp_base = devm_ioremap(&pdev->dev, res->start,
> > + resource_size(res));
> > + if (!adata->acp_base)
> > + return -ENOMEM;
> > +
> > + adata->capture_stream = NULL;
> > +
> > + dev_set_drvdata(&pdev->dev, adata);
> > + status = devm_snd_soc_register_component(&pdev->dev,
> > + &acp_pdm_component,
> > + &acp_pdm_dai_driver, 1);
> > + if (status) {
> > + dev_err(&pdev->dev, "Fail to register acp pdm dai\n");
> > +
> > + return -ENODEV;
>
> return status;
>
> > + }
> > + return 0;
> > +}
> > +
> > +static int acp_pdm_audio_remove(struct platform_device *pdev)
> > +{
> > + return 0;
> > +}
>
> not needed?
It gets filled in future patch.
>
> > +
> > +static struct platform_driver acp_pdm_dma_driver = {
> > + .probe = acp_pdm_audio_probe,
> > + .remove = acp_pdm_audio_remove,
> > + .driver = {
> > + .name = "acp_rn_pdm_dma",
> > + },
> > +};
> > +
> > +module_platform_driver(acp_pdm_dma_driver);
