Re: [alsa-devel] [RFC PATCH 17/40] soundwire: bus: use runtime_pm_get_sync/pm when enabled

[Jan Kotas <jank@cadence.com>]

> On 26 Jul 2019, at 10:22, Guennadi Liakhovetski <guennadi.liakhovetski@linux.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@linux.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@linaro.org>
>>>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.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@alsa-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@alsa-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=