Hi, On 19-12-18 18:22, Lukas Wunner wrote:
On Wed, Dec 19, 2018 at 05:48:17PM +0100, Wolfram Sang wrote:+static inline void i2c_mark_adapter_suspended(struct i2c_adapter *adap) +{ + i2c_lock_bus(adap, I2C_LOCK_ROOT_ADAPTER); + set_bit(I2C_ALF_IS_SUSPENDED, &adap->locked_flags); + i2c_unlock_bus(adap, I2C_LOCK_ROOT_ADAPTER); +}This looks like a duplication of the is_suspended flag in struct dev_pm_info. Any reason why you can't use that? If so, it would be good to document the reason in the commit message.
Oh, that is a very good point and that one only gets set on system suspend and not on resume suspend, working around the problems with the i2c-designware driver. I think this might be as simple as adding: if (WARN_ON(adap->dev.parent->power.is_suspended)) return -ESHUTDOWN; To __i2c_transfer (and document the -ESHUTDOWN) combined with removing all the now no longer DIY code from various bus drivers. Regards, Hans
If the point is to constrain refusal of transfers in suspended state to certain drivers, those drivers could opt in to that functionality by setting a flag, and the i2c core could then gate refusal based on that flag and the is_suspended flag in struct dev_pm_info. Also, why is it necessary to take a lock to perform an atomic bitop? (Sorry if that's a dumb question, seems non-obvious to me.) Thanks, Lukas
