[PATCH 1/3] gpio: pca953x: only use single read/write for No AI mode

[PATCH 1/3] gpio: pca953x: only use single read/write for No AI mode

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

For the device use NO AI mode(not support auto address increment),
only use the single read/write when config the regmap.

We meet issue on PCA9557PW on i.MX8QXP/DXL evk board, this device
do not support AI mode, but when do the regmap sync, regmap will
sync 3 byte data to register 1, logically this means write first
data to register 1, write second data to register 2, write third data
to register 3. But this device do not support AI mode, finally, these
three data write only into register 1 one by one. the reault is the
value of register 1 alway equal to the latest data, here is the third
data, no operation happened on register 2 and register 3. This is
not what we expect.

Fixes: 49427232764d ("gpio: pca953x: Perform basic regmap conversion")
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/gpio/gpio-pca953x.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 08bc52c3cdcb..7209f69a8e8d 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -351,6 +351,9 @@ static const struct regmap_config pca953x_i2c_regmap = {
 	.reg_bits = 8,
 	.val_bits = 8,
 
+	.use_single_read = true,
+	.use_single_write = true,
+
 	.readable_reg = pca953x_readable_register,
 	.writeable_reg = pca953x_writeable_register,
 	.volatile_reg = pca953x_volatile_register,
-- 
2.25.1

[PATCH 2/3] gpio: pca953x: use the correct range when do regmap sync

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

regmap will sync a range of registers, here use the correct range
to make sure the sync do not touch other unexpected registers.

Find on pca9557pw on imx8qxp/dxl evk board, this device support
8 pin, so only need one register(8 bits) to cover all the 8 pins's
property setting. But when sync the output, we find it actually
update two registers, output register and the following register.

Fixes: b76574300504 ("gpio: pca953x: Restore registers after suspend/resume cycle")
Fixes: ec82d1eba346 ("gpio: pca953x: Zap ad-hoc reg_output cache")
Fixes: 0f25fda840a9 ("gpio: pca953x: Zap ad-hoc reg_direction cache")
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/gpio/gpio-pca953x.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 7209f69a8e8d..18888ec24d04 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -912,12 +912,12 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert)
 	int ret;
 
 	ret = regcache_sync_region(chip->regmap, chip->regs->output,
-				   chip->regs->output + NBANK(chip));
+				   chip->regs->output + NBANK(chip) - 1);
 	if (ret)
 		goto out;
 
 	ret = regcache_sync_region(chip->regmap, chip->regs->direction,
-				   chip->regs->direction + NBANK(chip));
+				   chip->regs->direction + NBANK(chip) - 1);
 	if (ret)
 		goto out;
 
@@ -1130,14 +1130,14 @@ static int pca953x_regcache_sync(struct device *dev)
 	 * sync these registers first and only then sync the rest.
 	 */
 	regaddr = pca953x_recalc_addr(chip, chip->regs->direction, 0);
-	ret = regcache_sync_region(chip->regmap, regaddr, regaddr + NBANK(chip));
+	ret = regcache_sync_region(chip->regmap, regaddr, regaddr + NBANK(chip) - 1);
 	if (ret) {
 		dev_err(dev, "Failed to sync GPIO dir registers: %d\n", ret);
 		return ret;
 	}
 
 	regaddr = pca953x_recalc_addr(chip, chip->regs->output, 0);
-	ret = regcache_sync_region(chip->regmap, regaddr, regaddr + NBANK(chip));
+	ret = regcache_sync_region(chip->regmap, regaddr, regaddr + NBANK(chip) - 1);
 	if (ret) {
 		dev_err(dev, "Failed to sync GPIO out registers: %d\n", ret);
 		return ret;
@@ -1147,7 +1147,7 @@ static int pca953x_regcache_sync(struct device *dev)
 	if (chip->driver_data & PCA_PCAL) {
 		regaddr = pca953x_recalc_addr(chip, PCAL953X_IN_LATCH, 0);
 		ret = regcache_sync_region(chip->regmap, regaddr,
-					   regaddr + NBANK(chip));
+					   regaddr + NBANK(chip) - 1);
 		if (ret) {
 			dev_err(dev, "Failed to sync INT latch registers: %d\n",
 				ret);
@@ -1156,7 +1156,7 @@ static int pca953x_regcache_sync(struct device *dev)
 
 		regaddr = pca953x_recalc_addr(chip, PCAL953X_INT_MASK, 0);
 		ret = regcache_sync_region(chip->regmap, regaddr,
-					   regaddr + NBANK(chip));
+					   regaddr + NBANK(chip) - 1);
 		if (ret) {
 			dev_err(dev, "Failed to sync INT mask registers: %d\n",
 				ret);
-- 
2.25.1

[PATCH 3/3] gpio: pca953x: use the correct register address when regcache sync during init

[haibo.chen]

From: Haibo Chen <haibo.chen@nxp.com>

For regcache_sync_region, need to use pca953x_recalc_addr() to get
the real register address.

Fixes: ec82d1eba346 ("gpio: pca953x: Zap ad-hoc reg_output cache")
Fixes: 0f25fda840a9 ("gpio: pca953x: Zap ad-hoc reg_direction cache")
Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 drivers/gpio/gpio-pca953x.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 18888ec24d04..1747b6a9d5bf 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -910,14 +910,17 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert)
 {
 	DECLARE_BITMAP(val, MAX_LINE);
 	int ret;
+	u8 regaddr;
 
-	ret = regcache_sync_region(chip->regmap, chip->regs->output,
-				   chip->regs->output + NBANK(chip) - 1);
+	regaddr = pca953x_recalc_addr(chip, chip->regs->output, 0);
+	ret = regcache_sync_region(chip->regmap, regaddr,
+				   regaddr + NBANK(chip) - 1);
 	if (ret)
 		goto out;
 
-	ret = regcache_sync_region(chip->regmap, chip->regs->direction,
-				   chip->regs->direction + NBANK(chip) - 1);
+	regaddr = pca953x_recalc_addr(chip, chip->regs->direction, 0);
+	ret = regcache_sync_region(chip->regmap, regaddr,
+				   regaddr + NBANK(chip) - 1);
 	if (ret)
 		goto out;
 
-- 
2.25.1

Re: [PATCH 3/3] gpio: pca953x: use the correct register address when regcache sync during init

[Andy Shevchenko]

On Mon, Jul 18, 2022 at 10:56 AM <haibo.chen@nxp.com> wrote:
>
> From: Haibo Chen <haibo.chen@nxp.com>
>
> For regcache_sync_region, need to use pca953x_recalc_addr() to get

we need

> the real register address.

A couple of comments, here, otherwise you can add
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
to _all_three_ patches.

> Fixes: ec82d1eba346 ("gpio: pca953x: Zap ad-hoc reg_output cache")
> Fixes: 0f25fda840a9 ("gpio: pca953x: Zap ad-hoc reg_direction cache")
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  drivers/gpio/gpio-pca953x.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 18888ec24d04..1747b6a9d5bf 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -910,14 +910,17 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert)
>  {
>         DECLARE_BITMAP(val, MAX_LINE);
>         int ret;
> +       u8 regaddr;

Please, keep it on "longest line first" order.

> -       ret = regcache_sync_region(chip->regmap, chip->regs->output,
> -                                  chip->regs->output + NBANK(chip) - 1);
> +       regaddr = pca953x_recalc_addr(chip, chip->regs->output, 0);
> +       ret = regcache_sync_region(chip->regmap, regaddr,
> +                                  regaddr + NBANK(chip) - 1);
>         if (ret)
>                 goto out;
>
> -       ret = regcache_sync_region(chip->regmap, chip->regs->direction,
> -                                  chip->regs->direction + NBANK(chip) - 1);
> +       regaddr = pca953x_recalc_addr(chip, chip->regs->direction, 0);
> +       ret = regcache_sync_region(chip->regmap, regaddr,
> +                                  regaddr + NBANK(chip) - 1);
>         if (ret)
>                 goto out;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] gpio: pca953x: use the correct register address when regcache sync during init

[Bartosz Golaszewski]

On Mon, Jul 18, 2022 at 2:00 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Jul 18, 2022 at 10:56 AM <haibo.chen@nxp.com> wrote:
> >
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > For regcache_sync_region, need to use pca953x_recalc_addr() to get
>
> we need
>
> > the real register address.
>
> A couple of comments, here, otherwise you can add
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> to _all_three_ patches.
>
> > Fixes: ec82d1eba346 ("gpio: pca953x: Zap ad-hoc reg_output cache")
> > Fixes: 0f25fda840a9 ("gpio: pca953x: Zap ad-hoc reg_direction cache")
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  drivers/gpio/gpio-pca953x.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> > index 18888ec24d04..1747b6a9d5bf 100644
> > --- a/drivers/gpio/gpio-pca953x.c
> > +++ b/drivers/gpio/gpio-pca953x.c
> > @@ -910,14 +910,17 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert)
> >  {
> >         DECLARE_BITMAP(val, MAX_LINE);
> >         int ret;
> > +       u8 regaddr;
>
> Please, keep it on "longest line first" order.
>
> > -       ret = regcache_sync_region(chip->regmap, chip->regs->output,
> > -                                  chip->regs->output + NBANK(chip) - 1);
> > +       regaddr = pca953x_recalc_addr(chip, chip->regs->output, 0);
> > +       ret = regcache_sync_region(chip->regmap, regaddr,
> > +                                  regaddr + NBANK(chip) - 1);
> >         if (ret)
> >                 goto out;
> >
> > -       ret = regcache_sync_region(chip->regmap, chip->regs->direction,
> > -                                  chip->regs->direction + NBANK(chip) - 1);
> > +       regaddr = pca953x_recalc_addr(chip, chip->regs->direction, 0);
> > +       ret = regcache_sync_region(chip->regmap, regaddr,
> > +                                  regaddr + NBANK(chip) - 1);
> >         if (ret)
> >                 goto out;
>
> --
> With Best Regards,
> Andy Shevchenko

I fixed those up locally and applied all three.

Bart