Re: [PATCH] gpio: pca953x: Add mutex_lock for regcache sync in PM

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Wed, Aug 31, 2022 at 12:55 PM <haibo.chen@xxxxxxx> wrote:
>
> From: Haibo Chen <haibo.chen@xxxxxxx>
>
> The regcache sync will set the cache_bypass = true, at that
> time, when there is regmap write operation, it will bypass
> the regmap cache, then the regcache sync will write back the
> value from cache to register, which is not as our expectation.
>
> Though regmap already use its internal lock to avoid such issue,
> but this driver force disable the regmap internal lock in its
> regmap config: disable_locking = true
>
> To avoid this issue, use the driver's own lock to do the protect
> in system PM.
>
> Fixes: b76574300504 ("gpio: pca953x: Restore registers after suspend/resume cycle")
> Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx>
> ---
>  drivers/gpio/gpio-pca953x.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 091dd573c556..cf9bf3fcaee2 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -1168,7 +1168,9 @@ static int pca953x_suspend(struct device *dev)
>  {
>         struct pca953x_chip *chip = dev_get_drvdata(dev);
>
> +       mutex_lock(&chip->i2c_lock);
>         regcache_cache_only(chip->regmap, true);
> +       mutex_unlock(&chip->i2c_lock);
>
>         if (atomic_read(&chip->wakeup_path))
>                 device_set_wakeup_path(dev);
> @@ -1191,13 +1193,17 @@ static int pca953x_resume(struct device *dev)
>                 }
>         }
>
> +       mutex_lock(&chip->i2c_lock);
>         regcache_cache_only(chip->regmap, false);
>         regcache_mark_dirty(chip->regmap);
>         ret = pca953x_regcache_sync(dev);
> -       if (ret)
> +       if (ret) {
> +               mutex_unlock(&chip->i2c_lock);
>                 return ret;
> +       }
>
>         ret = regcache_sync(chip->regmap);
> +       mutex_unlock(&chip->i2c_lock);
>         if (ret) {
>                 dev_err(dev, "Failed to restore register map: %d\n", ret);
>                 return ret;
> --
> 2.34.1
>

Good catch, applied for fixes, thanks!

Bart