Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [alsa-devel] [PATCH v8 2/6] ASoC: amd: Refactoring of DAI from
 DMA driver
To:     vishnu <vravulap@amd.com>,
        Ravulapati Vishnu vardhan rao 
        <Vishnuvardhanrao.Ravulapati@amd.com>
Cc:     "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." 
        <alsa-devel@alsa-project.org>,
        Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
        "Gustavo A. R. Silva" <gustavo@embeddedor.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        YueHaibing <yuehaibing@huawei.com>,
        Takashi Iwai <tiwai@suse.com>,
        open list <linux-kernel@vger.kernel.org>,
        Mark Brown <broonie@kernel.org>, djkurtz@google.com,
        Vijendar Mukunda <Vijendar.Mukunda@amd.com>,
        Alexander.Deucher@amd.com, Akshu.Agrawal@amd.com
References: <1574155967-1315-1-git-send-email-Vishnuvardhanrao.Ravulapati@amd.com>
 <1574155967-1315-3-git-send-email-Vishnuvardhanrao.Ravulapati@amd.com>
 <0c3d3545-b0ee-4bb3-558a-045633a30e46@linux.intel.com>
 <991a1c7a-6f34-caab-132d-5687b1f1bfa0@amd.com>
From:   Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Message-ID: <9005946f-69b6-1cc6-5a1a-b894d826b960@linux.intel.com>
Date:   Wed, 20 Nov 2019 08:48:57 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.2
MIME-Version: 1.0
In-Reply-To: <991a1c7a-6f34-caab-132d-5687b1f1bfa0@amd.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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