diff mbox series

gpio: pca953x: Add mutex_lock for regcache sync in PM

Message ID 1661942255-13177-1-git-send-email-haibo.chen@nxp.com
State New
Headers show
Series gpio: pca953x: Add mutex_lock for regcache sync in PM | expand

Commit Message

Bough Chen Aug. 31, 2022, 10:37 a.m. UTC
From: Haibo Chen <haibo.chen@nxp.com>

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@nxp.com>
---
 drivers/gpio/gpio-pca953x.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Bartosz Golaszewski Aug. 31, 2022, 11:41 a.m. UTC | #1
On Wed, Aug 31, 2022 at 12:55 PM <haibo.chen@nxp.com> wrote:
>
> From: Haibo Chen <haibo.chen@nxp.com>
>
> 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@nxp.com>
> ---
>  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
diff mbox series

Patch

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;