Re: [alsa-devel] [PATCH v8 2/6] ASoC: amd: Refactoring of DAI from DMA driver

[Pierre-Louis Bossart]

> > > +ÂÂÂ pm_runtime_set_autosuspend_delay(&pdev->dev, 5000);
> > > +ÂÂÂ pm_runtime_use_autosuspend(&pdev->dev);
> > > +ÂÂÂ pm_runtime_enable(&pdev->dev);
> >
> > question: here you want to use pm_runtime for this platform device...
> >
> > > +ÂÂÂ return 0;
> > > +}
> > > +
> > > +static int acp3x_dai_remove(struct platform_device *pdev)
> > > +{
> > > +ÂÂÂ pm_runtime_disable(&pdev->dev);
> > > +ÂÂÂ return 0;
> > > +}
> > > +static struct platform_driver acp3x_dai_driver = {
> > > +ÂÂÂ .probe = acp3x_dai_probe,
> > > +ÂÂÂ .remove = acp3x_dai_remove,
> > > +ÂÂÂ .driver = {
> > > +ÂÂÂÂÂÂÂ .name = "acp3x_i2s_playcap",
> >
> > ... but here there is no .pm structure and I don't see any 
> > suspend/resume routines for this driver...
> >
> > > +ÂÂÂ },
> > > +};
> >
> > > @@ -774,13 +586,14 @@ static struct platform_driver acp3x_dma_driver = {
> > > ÂÂÂÂÂ .probe = acp3x_audio_probe,
> > > ÂÂÂÂÂ .remove = acp3x_audio_remove,
> > > ÂÂÂÂÂ .driver = {
> > > -ÂÂÂÂÂÂÂ .name = "acp3x_rv_i2s",
> > > +ÂÂÂÂÂÂÂ .name = "acp3x_rv_i2s_dma",
> > > ÂÂÂÂÂÂÂÂÂ .pm = &acp3x_pm_ops,
> > > ÂÂÂÂÂ },
> >
> > ... but for this other platform_driver you do have a .pm structure and 
> > suspend-resume implementations.
> >
> > Wondering if this is a miss or a feature?
>
> As per our design, ACP IP specific changes like ACP power on/off will be 
> handled in ACP pci driver(parent device for DMA device and I2S 
> controller(nothing but CPU DAI))
>
> Where as In DMA driver during runtime suspend/resume interrupts will be 
> disabled and enabled.
>
> But in DAI driver there is nothing to be done in suspend and resume just 
> returning zero so we have not added PM suspend/resume here in DAI.
>
> So is it expected to add the suspend resumes with returning zero.Or if 
> pm runtime is not needed in CPU DAI shall we remove the existing PM 
> related calls in DAI.
>
> Please suggest us.

I am far from a pm_runtime expert but I'd remove the calls to

+    pm_runtime_set_autosuspend_delay(&pdev->dev, 5000);
+    pm_runtime_use_autosuspend(&pdev->dev);
+    pm_runtime_enable(&pdev->dev);

if you platform device does not provide any suspend/resume functions and 
the parent takes care of everything?

IIRC the status for the platform device would be 'unsupported' but that 
shouldn't prevent the parent from suspending/resuming.

A second opinion would be desirable here...