[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <F31F0D1F-63E2-4ABF-9B38-10D55E773F11@global.cadence.com>
Date: Fri, 26 Jul 2019 08:33:35 +0000
From: Jan Kotas <jank@...ence.com>
To: Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com>
CC: Jan Kotas <jank@...ence.com>,
"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
"tiwai@...e.de" <tiwai@...e.de>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
LKML <linux-kernel@...r.kernel.org>,
Vinod Koul <vkoul@...nel.org>, Mark Brown <broonie@...nel.org>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
"slawomir.blauciak@...el.com" <slawomir.blauciak@...el.com>,
Sanyog Kale <sanyog.r.kale@...el.com>
Subject: Re: [alsa-devel] [RFC PATCH 17/40] soundwire: bus: use
runtime_pm_get_sync/pm when enabled
> On 26 Jul 2019, at 10:22, Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com> wrote:
>
> EXTERNAL MAIL
>
>
> Hi Jan,
>
> On Fri, Jul 26, 2019 at 07:47:04AM +0000, Jan Kotas wrote:
>> Hello,
>>
>> I while back I proposed a patch for this, but it went nowhere.
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_10887405_&d=DwIBAg&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=g7GAQENVXx_RQdyXHInPMg&m=i_0S359hFIVqNgv3fR5_MNzDOHP99trdXszZ-FMiQEE&s=ddktFZYlePh-bC7kXeoKWt4QomupzHATK4FLY4oSWKA&e=
>> Maybe something similar can be implemented?
>
> Yes, I was thinking about checkint -EACCESS too, but then I noticed this code
> in rpm_resume():
>
> else if (dev->power.disable_depth == 1 && dev->power.is_suspended
> && dev->power.runtime_status == RPM_ACTIVE)
> retval = 1;
>
> i.e. if RT-PM is disabled on the device (but only exactly once?..) and it's
> active and the device is suspended for a system suspend, the function will
> return 1. I don't understand the logic of this code, but it seems to me it
> could break the -EACCESS check?
>
Hi,
In such case ret < 0 will not be true, which I think is fine,
if I’m understanding you correctly.
Regards,
Jan
> Thanks
> Guennadi
>
>> Jan
>>
>>> On 26 Jul 2019, at 09:39, Guennadi Liakhovetski <guennadi.liakhovetski@...ux.intel.com> wrote:
>>>
>>> EXTERNAL MAIL
>>>
>>>
>>> Hi Pierre,
>>>
>>> I might be wrong but this doesn't seem right to me. (Supposedly) all RT-PM
>>> functions check for "enabled" internally. The only thing that can happen is
>>> that if RT-PM isn't enabled some of those functions will return an error.
>>> So, in those cases where the return value of RT-PM functions isn't checked,
>>> I don't think you need to do anything. Where it is checked maybe do
>>>
>>> + if (ret < 0 && pm_runtime_enabled(slave->bus->dev))
>>>
>>> Thanks
>>> Guennadi
>>>
>>> On Thu, Jul 25, 2019 at 06:40:09PM -0500, Pierre-Louis Bossart wrote:
>>>> Not all platforms support runtime_pm for now, let's use runtime_pm
>>>> only when enabled.
>>>>
>>>> Suggested-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
>>>> ---
>>>> drivers/soundwire/bus.c | 25 ++++++++++++++++---------
>>>> 1 file changed, 16 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
>>>> index 5ad4109dc72f..0a45dc5713df 100644
>>>> --- a/drivers/soundwire/bus.c
>>>> +++ b/drivers/soundwire/bus.c
>>>> @@ -332,12 +332,16 @@ int sdw_nread(struct sdw_slave *slave, u32 addr, size_t count, u8 *val)
>>>> if (ret < 0)
>>>> return ret;
>>>>
>>>> - ret = pm_runtime_get_sync(slave->bus->dev);
>>>> - if (ret < 0)
>>>> - return ret;
>>>> + if (pm_runtime_enabled(slave->bus->dev)) {
>>>> + ret = pm_runtime_get_sync(slave->bus->dev);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> + }
>>>>
>>>> ret = sdw_transfer(slave->bus, &msg);
>>>> - pm_runtime_put(slave->bus->dev);
>>>> +
>>>> + if (pm_runtime_enabled(slave->bus->dev))
>>>> + pm_runtime_put(slave->bus->dev);
>>>>
>>>> return ret;
>>>> }
>>>> @@ -359,13 +363,16 @@ int sdw_nwrite(struct sdw_slave *slave, u32 addr, size_t count, u8 *val)
>>>> slave->dev_num, SDW_MSG_FLAG_WRITE, val);
>>>> if (ret < 0)
>>>> return ret;
>>>> -
>>>> - ret = pm_runtime_get_sync(slave->bus->dev);
>>>> - if (ret < 0)
>>>> - return ret;
>>>> + if (pm_runtime_enabled(slave->bus->dev)) {
>>>> + ret = pm_runtime_get_sync(slave->bus->dev);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> + }
>>>>
>>>> ret = sdw_transfer(slave->bus, &msg);
>>>> - pm_runtime_put(slave->bus->dev);
>>>> +
>>>> + if (pm_runtime_enabled(slave->bus->dev))
>>>> + pm_runtime_put(slave->bus->dev);
>>>>
>>>> return ret;
>>>> }
>>>> --
>>>> 2.20.1
>>>>
>>>> _______________________________________________
>>>> Alsa-devel mailing list
>>>> Alsa-devel@...a-project.org
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__mailman.alsa-2Dproject.org_mailman_listinfo_alsa-2Ddevel&d=DwIBAg&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=g7GAQENVXx_RQdyXHInPMg&m=vETGQLSPeGb7K_ZsXv4Tl3VFfdXzyummTDga97ozJcg&s=LiW4SToh5U0zhnkox54oRhJ1u3vFNbBB9nmzRDuCDjI&e=
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@...a-project.org
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__mailman.alsa-2Dproject.org_mailman_listinfo_alsa-2Ddevel&d=DwIBAg&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=g7GAQENVXx_RQdyXHInPMg&m=i_0S359hFIVqNgv3fR5_MNzDOHP99trdXszZ-FMiQEE&s=RxPHxKfI3v6Fkh7qzKjq8sNi-5QMoY8XfyMDSquA38o&e=
Powered by blists - more mailing lists