Re: [RFC PATCH] soundwire: use driver callbacks directly with proper locking

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>>> @@ -846,12 +847,18 @@ static int sdw_slave_clk_stop_callback(struct sdw_slave *slave,
>>>  				       enum sdw_clk_stop_mode mode,
>>>  				       enum sdw_clk_stop_type type)
>>>  {
>>> -	int ret;
>>> +	struct device *dev = &slave->dev;
>>> +	struct sdw_driver *drv;
>>>  
>>> -	if (slave->ops && slave->ops->clk_stop) {
>>> -		ret = slave->ops->clk_stop(slave, mode, type);
>>> -		if (ret < 0)
>>> -			return ret;
>>> +	/*
>>> +	 * this function can only be called from a pm_runtime
>>> +	 * sequence where the device is already locked
>>> +	 */
>>
>> If this is guaranteed..
>>
>>> +
>>> +	if (dev->driver) {
>>
>> do we need to check this? Did you find a case where this was not valid
>> while device is locked, maybe do this while holding the lock (kind of
>> moot to process the calls if driver is gone)
> 
> Humm, good feedback. I will re-check for cases where the driver is 'blacklisted' and also cases there there's no power management supported.

I rechecked all this and it turns out I was mistaken. This function is part of a pm_runtime sequence indeed, but at the parent bus/manager device level. I confused levels and adding a deplock_assert showed very quickly that the peripheral device was never locked.

Thanks for pushing back on this!

In all other cases, I think it's valid and safe to take the lock and test dev->driver. It can happen that there is no driver enabled in the build, or that the driver is 'blacklisted', and in theory the user could muck with sysfs to trigger a peripheral driver binding sequence that would happen smack while the bus is suspended or resume.

I'll do more validation and send an update next week.

-Pierre