[alsa-devel] [PATCH 3/4] soundwire: bus: check if pm runtime is enabled before calling

[Srinivas Kandagatla]

On 28/03/2019 13:55, Pierre-Louis Bossart wrote:
> On 3/28/19 9:41 AM, Srinivas Kandagatla wrote:
>> Check the device has pm runtime enabled before returning error.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla at linaro.org>
>> ---
>>   drivers/soundwire/bus.c | 16 ++++++++++------
>>   1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
>> index 1cbfedfc20ef..101562a6fb0d 100644
>> --- a/drivers/soundwire/bus.c
>> +++ b/drivers/soundwire/bus.c
>> @@ -327,9 +327,11 @@ 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);
> 
> Is this an recommended/accepted sequence in kernel circles? I did a 
> quick git grep and don't see anyone using this sort of tests.

There are lots of users in kernel with such sequences, ex:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm-smmu.c?h=v5.1-rc2#n279

> 
> 
>> +        if (ret < 0)
>> +            return ret;
>> +    }
>>       ret = sdw_transfer(slave->bus, &msg);
>>       pm_runtime_put(slave->bus->dev);
> 
> and the weird thing is that you don't test for the put() case?
> 
>> @@ -355,9 +357,11 @@ int sdw_nwrite(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);

I should add a check in this path as well.

Thanks,
srini

>>
>