Re: [PATCH 1/2] i2c: designware: Lock the adapter while setting the suspended flag

[Jarkko Nikula <jarkko.nikula@linux.intel.com>]

On 2/23/22 17:15, Hans de Goede wrote:
> Hi Andy,
> 
> On 2/23/22 15:52, Andy Shevchenko wrote:
>> On Wed, Feb 23, 2022 at 02:48:38PM +0100, Hans de Goede wrote:
>>> Lock the adapter while setting the suspended flag, to ensure that other
>>> locked code always sees the change immediately, rather then possibly using
>>> a stale value.
>>>
>>> This involves splitting the suspend/resume callbacks into separate runtime
>>> and normal suspend/resume calls. This is necessary because i2c_dw_xfer()
>>> will get called by the i2c-core with the adapter locked and it in turn
>>> calls the runtime-resume callback through pm_runtime_get_sync().
>>>
>>> So the runtime versions of the suspend/resume callbacks cannot take
>>> the adapter-lock. Note this patch simply makes the runtime suspend/resume
>>> callbacks not deal with the suspended flag at all. During runtime the
>>> pm_runtime_get_sync() from i2c_dw_xfer() will always ensure that the
>>> adapter is resumed when necessary.
>>>
>>> The suspended flag check is only necessary to check proper suspend/resume
>>> ordering during normal suspend/resume which makes the pm_runtime_get_sync()
>>> call a no-op.
>>
>> Both patches look good to me and seems like a nice clean up. Not sure if the
>> first one should be backported (and hence have a Fixes tag).
>>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> Thank you for the review. As for adding a Fixes: tag, this was inspired
> by an actual bug-report, but it did not help in fixing the bug, so I left
> the fixes tag out.
> 
> If you or Wolfram still want to add a Fixes tag, then it should be:
> 
> Fixes: 275154155538  ("i2c: designware: Do not allow i2c_dw_xfer() calls while suspended")
> 
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>