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