LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver
@ 2018-05-16 17:01 H. Nikolaus Schaller
  2018-05-16 17:01 ` [PATCH v6 1/3] gpio: pca953x: set the PCA_PCAL flag also when matching by DT H. Nikolaus Schaller
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2018-05-16 17:01 UTC (permalink / raw)
  To: galak, andy.shevchenko, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Linus Walleij, Alexandre Courbot
  Cc: devicetree, linux-gpio, linux-kernel, letux-kernel, kernel,
	H. Nikolaus Schaller

V6:
* added proper attribution to the formula used for fixing the
  pcal6524 register address (changes commit message only)
* add back missing first patch from V2 that defines the
  PCA_LATCH_INT constant
* removed patches already merged

2018-04-28 18:33:42: V5:
* fix wrong split up between patches 1/7 and 2/7.

2018-04-26 19:35:07: V4:
* introduced PCA_LATCH_INT constant to make of_table more
  readable (suggested by Andy Shevchenko)
* converted all register constants to hex in a separate
  patch (suggested by Andy Shevchenko)
* separated additional pcal953x and pcal6524 register
  definitions into separate patches (suggested by Andy Shevchenko)
* made special pcal6524 address adjustment more readable
  (suggested by Andy Shevchenko)
* moved gpio-controller and interrupt-controller to the
  "required" section (reviewed by Rob Herring)

2018-04-10 18:07:07: V3:
* add Reported-by: and Reviewed-by:
* fix wording for bindings description and example
* convert all register offsets to hex
* omit the LEVEL-IRQ RFC/hack commit

2018-04-04 21:00:27: V2:
* added PCA_PCAL flags if matched through of-table
* fix address calculation for extended PCAL6524 registers
* hack to map LEVEL_LOW to EDGE_FALLING to be able to
  test in combination with ts3a227e driver
* improve description of bindings for optional vcc-supply
  and interrupt-controller;

2018-03-10 09:32:53: no initial description

H. Nikolaus Schaller (3):
  gpio: pca953x: set the PCA_PCAL flag also when matching by DT
  gpio: pca953x: define masks for addressing common and extended
    registers
  gpio: pca953x: fix address calculation for pcal6524

 drivers/gpio/gpio-pca953x.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

-- 
2.12.2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v6 1/3] gpio: pca953x: set the PCA_PCAL flag also when matching by DT
  2018-05-16 17:01 [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver H. Nikolaus Schaller
@ 2018-05-16 17:01 ` H. Nikolaus Schaller
  2018-05-16 17:01 ` [PATCH v6 2/3] gpio: pca953x: define masks for addressing common and extended registers H. Nikolaus Schaller
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2018-05-16 17:01 UTC (permalink / raw)
  To: galak, andy.shevchenko, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Linus Walleij, Alexandre Courbot
  Cc: devicetree, linux-gpio, linux-kernel, letux-kernel, kernel,
	H. Nikolaus Schaller

The of_device_table is missing the PCA_PCAL flag so the
pcal6524 would be operated in tca6424 compatibility mode which
does not handle the new interrupt mask registers.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpio/gpio-pca953x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 7d37692d672e..2b667166e855 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -58,6 +58,7 @@
 #define PCA_GPIO_MASK		0x00FF
 #define PCA_INT			0x0100
 #define PCA_PCAL		0x0200
+#define PCA_LATCH_INT (PCA_PCAL | PCA_INT)
 #define PCA953X_TYPE		0x1000
 #define PCA957X_TYPE		0x2000
 #define PCA_TYPE_MASK		0xF000
@@ -946,8 +947,8 @@ static const struct of_device_id pca953x_dt_ids[] = {
 	{ .compatible = "nxp,pca9575", .data = OF_957X(16, PCA_INT), },
 	{ .compatible = "nxp,pca9698", .data = OF_953X(40, 0), },
 
-	{ .compatible = "nxp,pcal6524", .data = OF_953X(24, PCA_INT), },
-	{ .compatible = "nxp,pcal9555a", .data = OF_953X(16, PCA_INT), },
+	{ .compatible = "nxp,pcal6524", .data = OF_953X(24, PCA_LATCH_INT), },
+	{ .compatible = "nxp,pcal9555a", .data = OF_953X(16, PCA_LATCH_INT), },
 
 	{ .compatible = "maxim,max7310", .data = OF_953X( 8, 0), },
 	{ .compatible = "maxim,max7312", .data = OF_953X(16, PCA_INT), },
-- 
2.12.2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v6 2/3] gpio: pca953x: define masks for addressing common and extended registers
  2018-05-16 17:01 [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver H. Nikolaus Schaller
  2018-05-16 17:01 ` [PATCH v6 1/3] gpio: pca953x: set the PCA_PCAL flag also when matching by DT H. Nikolaus Schaller
@ 2018-05-16 17:01 ` H. Nikolaus Schaller
  2018-05-16 19:50   ` Andy Shevchenko
  2018-05-16 17:01 ` [PATCH v6 3/3] gpio: pca953x: fix address calculation for pcal6524 H. Nikolaus Schaller
  2018-05-16 19:53 ` [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver Andy Shevchenko
  3 siblings, 1 reply; 7+ messages in thread
From: H. Nikolaus Schaller @ 2018-05-16 17:01 UTC (permalink / raw)
  To: galak, andy.shevchenko, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Linus Walleij, Alexandre Courbot
  Cc: devicetree, linux-gpio, linux-kernel, letux-kernel, kernel,
	H. Nikolaus Schaller

These mask bits are to be used to map the extended register
addreseses (which are defined for an unsupported 8-bit pcal chip)
to 16 and 24 bit chips (pcal6524).

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpio/gpio-pca953x.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 2b667166e855..c0eb679e60d4 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -56,6 +56,10 @@
 #define PCAL6524_DEBOUNCE	0x2d
 
 #define PCA_GPIO_MASK		0x00FF
+
+#define PCAL_GPIO_MASK		GENMASK(4, 0)
+#define PCAL_PINCTRL_MASK	(~PCAL_GPIO_MASK)
+
 #define PCA_INT			0x0100
 #define PCA_PCAL		0x0200
 #define PCA_LATCH_INT (PCA_PCAL | PCA_INT)
-- 
2.12.2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v6 3/3] gpio: pca953x: fix address calculation for pcal6524
  2018-05-16 17:01 [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver H. Nikolaus Schaller
  2018-05-16 17:01 ` [PATCH v6 1/3] gpio: pca953x: set the PCA_PCAL flag also when matching by DT H. Nikolaus Schaller
  2018-05-16 17:01 ` [PATCH v6 2/3] gpio: pca953x: define masks for addressing common and extended registers H. Nikolaus Schaller
@ 2018-05-16 17:01 ` H. Nikolaus Schaller
  2018-05-16 19:53 ` [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver Andy Shevchenko
  3 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2018-05-16 17:01 UTC (permalink / raw)
  To: galak, andy.shevchenko, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Linus Walleij, Alexandre Courbot
  Cc: devicetree, linux-gpio, linux-kernel, letux-kernel, kernel,
	H. Nikolaus Schaller

The register constants are so far defined in a way that they fit
for the pcal9555a when shifted by the number of banks, i.e. are
multiplied by 2 in the accessor function.

Now, the pcal6524 has 3 banks which means the relative offset
is multiplied by 4 for the standard registers.

Simply applying the bit shift to the extended registers gives
a wrong result, since the base offset is already included in
the offset.

Therefore, we have to add code to the 24 bit accessor functions
that adjusts the register number for these exended registers.

The formula finally used was developed and proposed by
Andy Shevchenko <andy.shevchenko@gmail.com>.

Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/gpio/gpio-pca953x.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index c0eb679e60d4..8c63664d0e74 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -222,9 +222,11 @@ static int pca957x_write_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
 static int pca953x_write_regs_24(struct pca953x_chip *chip, int reg, u8 *val)
 {
 	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+	int addr = (reg & PCAL_GPIO_MASK) << bank_shift;
+	int pinctrl = (reg & PCAL_PINCTRL_MASK) << 1;
 
 	return i2c_smbus_write_i2c_block_data(chip->client,
-					      (reg << bank_shift) | REG_ADDR_AI,
+					      pinctrl | addr | REG_ADDR_AI,
 					      NBANK(chip), val);
 }
 
@@ -264,9 +266,11 @@ static int pca953x_read_regs_16(struct pca953x_chip *chip, int reg, u8 *val)
 static int pca953x_read_regs_24(struct pca953x_chip *chip, int reg, u8 *val)
 {
 	int bank_shift = fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
+	int addr = (reg & PCAL_GPIO_MASK) << bank_shift;
+	int pinctrl = (reg & PCAL_PINCTRL_MASK) << 1;
 
 	return i2c_smbus_read_i2c_block_data(chip->client,
-					     (reg << bank_shift) | REG_ADDR_AI,
+					     pinctrl | addr | REG_ADDR_AI,
 					     NBANK(chip), val);
 }
 
-- 
2.12.2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 2/3] gpio: pca953x: define masks for addressing common and extended registers
  2018-05-16 17:01 ` [PATCH v6 2/3] gpio: pca953x: define masks for addressing common and extended registers H. Nikolaus Schaller
@ 2018-05-16 19:50   ` Andy Shevchenko
  2018-05-16 19:57     ` H. Nikolaus Schaller
  0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2018-05-16 19:50 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Kumar Gala, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Linus Walleij, Alexandre Courbot, devicetree,
	open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
	Discussions about the Letux Kernel, kernel

On Wed, May 16, 2018 at 8:01 PM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> These mask bits are to be used to map the extended register
> addreseses (which are defined for an unsupported 8-bit pcal chip)
> to 16 and 24 bit chips (pcal6524).
>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/gpio/gpio-pca953x.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 2b667166e855..c0eb679e60d4 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -56,6 +56,10 @@
>  #define PCAL6524_DEBOUNCE      0x2d
>
>  #define PCA_GPIO_MASK          0x00FF
> +
> +#define PCAL_GPIO_MASK         GENMASK(4, 0)
> +#define PCAL_PINCTRL_MASK      (~PCAL_GPIO_MASK)
> +

I give second thought about it, and think
either plain values, or second converted to its own explicit GENMASK
would be better.

(most confusing part to me is unknowness of the side of PINCTRL part
in the mask)


>  #define PCA_INT                        0x0100
>  #define PCA_PCAL               0x0200
>  #define PCA_LATCH_INT (PCA_PCAL | PCA_INT)
> --
> 2.12.2
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver
  2018-05-16 17:01 [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver H. Nikolaus Schaller
                   ` (2 preceding siblings ...)
  2018-05-16 17:01 ` [PATCH v6 3/3] gpio: pca953x: fix address calculation for pcal6524 H. Nikolaus Schaller
@ 2018-05-16 19:53 ` Andy Shevchenko
  3 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2018-05-16 19:53 UTC (permalink / raw)
  To: H. Nikolaus Schaller
  Cc: Kumar Gala, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Linus Walleij, Alexandre Courbot, devicetree,
	open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
	Discussions about the Letux Kernel, kernel

On Wed, May 16, 2018 at 8:01 PM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> V6:
> * added proper attribution to the formula used for fixing the
>   pcal6524 register address (changes commit message only)
> * add back missing first patch from V2 that defines the
>   PCA_LATCH_INT constant
> * removed patches already merged
>

Thanks for an update.
I think we still need to address the constant representation in patch 2.

For the rest take my
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> 2018-04-28 18:33:42: V5:
> * fix wrong split up between patches 1/7 and 2/7.
>
> 2018-04-26 19:35:07: V4:
> * introduced PCA_LATCH_INT constant to make of_table more
>   readable (suggested by Andy Shevchenko)
> * converted all register constants to hex in a separate
>   patch (suggested by Andy Shevchenko)
> * separated additional pcal953x and pcal6524 register
>   definitions into separate patches (suggested by Andy Shevchenko)
> * made special pcal6524 address adjustment more readable
>   (suggested by Andy Shevchenko)
> * moved gpio-controller and interrupt-controller to the
>   "required" section (reviewed by Rob Herring)
>
> 2018-04-10 18:07:07: V3:
> * add Reported-by: and Reviewed-by:
> * fix wording for bindings description and example
> * convert all register offsets to hex
> * omit the LEVEL-IRQ RFC/hack commit
>
> 2018-04-04 21:00:27: V2:
> * added PCA_PCAL flags if matched through of-table
> * fix address calculation for extended PCAL6524 registers
> * hack to map LEVEL_LOW to EDGE_FALLING to be able to
>   test in combination with ts3a227e driver
> * improve description of bindings for optional vcc-supply
>   and interrupt-controller;
>
> 2018-03-10 09:32:53: no initial description
>
> H. Nikolaus Schaller (3):
>   gpio: pca953x: set the PCA_PCAL flag also when matching by DT
>   gpio: pca953x: define masks for addressing common and extended
>     registers
>   gpio: pca953x: fix address calculation for pcal6524
>
>  drivers/gpio/gpio-pca953x.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> --
> 2.12.2
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v6 2/3] gpio: pca953x: define masks for addressing common and extended registers
  2018-05-16 19:50   ` Andy Shevchenko
@ 2018-05-16 19:57     ` H. Nikolaus Schaller
  0 siblings, 0 replies; 7+ messages in thread
From: H. Nikolaus Schaller @ 2018-05-16 19:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Kumar Gala, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Linus Walleij, Alexandre Courbot, devicetree,
	open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
	Discussions about the Letux Kernel, kernel


> Am 16.05.2018 um 21:50 schrieb Andy Shevchenko <andy.shevchenko@gmail.com>:
> 
> On Wed, May 16, 2018 at 8:01 PM, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> These mask bits are to be used to map the extended register
>> addreseses (which are defined for an unsupported 8-bit pcal chip)
>> to 16 and 24 bit chips (pcal6524).
>> 
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/gpio/gpio-pca953x.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
>> index 2b667166e855..c0eb679e60d4 100644
>> --- a/drivers/gpio/gpio-pca953x.c
>> +++ b/drivers/gpio/gpio-pca953x.c
>> @@ -56,6 +56,10 @@
>> #define PCAL6524_DEBOUNCE      0x2d
>> 
>> #define PCA_GPIO_MASK          0x00FF
>> +
>> +#define PCAL_GPIO_MASK         GENMASK(4, 0)
>> +#define PCAL_PINCTRL_MASK      (~PCAL_GPIO_MASK)
>> +
> 
> I give second thought about it, and think
> either plain values, or second converted to its own explicit GENMASK
> would be better.
> 
> (most confusing part to me is unknowness of the side of PINCTRL part
> in the mask)

I see.

Then, I'd also prefer plain values.

If ok, I can send a v7 tomorrow.

BR,
Nikolaus

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-05-16 19:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-16 17:01 [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver H. Nikolaus Schaller
2018-05-16 17:01 ` [PATCH v6 1/3] gpio: pca953x: set the PCA_PCAL flag also when matching by DT H. Nikolaus Schaller
2018-05-16 17:01 ` [PATCH v6 2/3] gpio: pca953x: define masks for addressing common and extended registers H. Nikolaus Schaller
2018-05-16 19:50   ` Andy Shevchenko
2018-05-16 19:57     ` H. Nikolaus Schaller
2018-05-16 17:01 ` [PATCH v6 3/3] gpio: pca953x: fix address calculation for pcal6524 H. Nikolaus Schaller
2018-05-16 19:53 ` [PATCH v6 0/3] pcal6524 extensions and fixes for pca953x driver Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).