* [PATCH 0/2] *** Add support for TI TCA6418 GPIO chip ***
@ 2025-06-17 20:44 Maria Garcia
2025-06-17 20:44 ` [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418 Maria Garcia
2025-06-17 20:44 ` [PATCH 2/2] gpio: pca953x: Add support for " Maria Garcia
0 siblings, 2 replies; 8+ messages in thread
From: Maria Garcia @ 2025-06-17 20:44 UTC (permalink / raw)
To: linux-gpio, devicetree, linux-kernel
Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maria Garcia, Maria Garcia
This series introduces support for the Texas Instruments TCA6418
GPIO expander chip to the gpio-pca953x driver. It also includes the
necessary device tree binding definition.
Signed-off-by: Maria Garcia <mariagarcia7293@gmail.com>
---
Maria Garcia (2):
dt-bindings: gpio: pca95xx: add TI TCA6418
gpio: pca953x: Add support for TI TCA6418
.../bindings/gpio/gpio-pca95xx.yaml | 1 +
drivers/gpio/gpio-pca953x.c | 110 ++++++++++++++++--
2 files changed, 99 insertions(+), 12 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418
2025-06-17 20:44 [PATCH 0/2] *** Add support for TI TCA6418 GPIO chip *** Maria Garcia
@ 2025-06-17 20:44 ` Maria Garcia
2025-06-18 15:58 ` Conor Dooley
2025-06-17 20:44 ` [PATCH 2/2] gpio: pca953x: Add support for " Maria Garcia
1 sibling, 1 reply; 8+ messages in thread
From: Maria Garcia @ 2025-06-17 20:44 UTC (permalink / raw)
To: linux-gpio, devicetree, linux-kernel
Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maria Garcia, Maria Garcia
The TCA6418E is a 18-channel I2C I/O expander with integrated ESD
protection.
Signed-off-by: Maria Garcia <mariagarcia7293@gmail.com>
---
Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml b/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
index 4d3f52f8d1b8..12134c737ad8 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
@@ -68,6 +68,7 @@ properties:
- ti,pca9536
- ti,tca6408
- ti,tca6416
+ - ti,tca6418
- ti,tca6424
- ti,tca9535
- ti,tca9538
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] gpio: pca953x: Add support for TI TCA6418
2025-06-17 20:44 [PATCH 0/2] *** Add support for TI TCA6418 GPIO chip *** Maria Garcia
2025-06-17 20:44 ` [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418 Maria Garcia
@ 2025-06-17 20:44 ` Maria Garcia
2025-06-25 11:53 ` Bartosz Golaszewski
1 sibling, 1 reply; 8+ messages in thread
From: Maria Garcia @ 2025-06-17 20:44 UTC (permalink / raw)
To: linux-gpio, devicetree, linux-kernel
Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maria Garcia, Maria Garcia
The TI TCA6418 is a 18-channel I2C I/O expander. It is slightly
different to other models from the same family, such as TCA6416,
but has enough in common with them to make it work with just a
few tweaks, which are explained in the code's documentation.
Signed-off-by: Maria Garcia <mariagarcia7293@gmail.com>
---
drivers/gpio/gpio-pca953x.c | 110 ++++++++++++++++++++++++++++++++----
1 file changed, 98 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index b852e4997629..9fe28d7ba667 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -38,6 +38,10 @@
#define PCA953X_INVERT 0x02
#define PCA953X_DIRECTION 0x03
+#define TCA6418_INPUT 0x14
+#define TCA6418_OUTPUT 0x17
+#define TCA6418_DIRECTION 0x23
+
#define REG_ADDR_MASK GENMASK(5, 0)
#define REG_ADDR_EXT BIT(6)
#define REG_ADDR_AI BIT(7)
@@ -76,7 +80,8 @@
#define PCA953X_TYPE BIT(12)
#define PCA957X_TYPE BIT(13)
#define PCAL653X_TYPE BIT(14)
-#define PCA_TYPE_MASK GENMASK(15, 12)
+#define TCA6418_TYPE BIT(16)
+#define PCA_TYPE_MASK GENMASK(16, 12)
#define PCA_CHIP_TYPE(x) ((x) & PCA_TYPE_MASK)
@@ -115,6 +120,7 @@ static const struct i2c_device_id pca953x_id[] = {
{ "pca6107", 8 | PCA953X_TYPE | PCA_INT, },
{ "tca6408", 8 | PCA953X_TYPE | PCA_INT, },
{ "tca6416", 16 | PCA953X_TYPE | PCA_INT, },
+ { "tca6418", 18 | TCA6418_TYPE | PCA_INT, },
{ "tca6424", 24 | PCA953X_TYPE | PCA_INT, },
{ "tca9538", 8 | PCA953X_TYPE | PCA_INT, },
{ "tca9539", 16 | PCA953X_TYPE | PCA_INT, },
@@ -204,6 +210,13 @@ static const struct pca953x_reg_config pca957x_regs = {
.invert = PCA957X_INVRT,
};
+static const struct pca953x_reg_config tca6418_regs = {
+ .direction = TCA6418_DIRECTION,
+ .output = TCA6418_OUTPUT,
+ .input = TCA6418_INPUT,
+ .invert = 0xFF, /* Does not apply */
+};
+
struct pca953x_chip {
unsigned gpio_start;
struct mutex i2c_lock;
@@ -237,6 +250,21 @@ static int pca953x_bank_shift(struct pca953x_chip *chip)
return fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
}
+/* Helper function to get the correct bit mask for a given offset and chip type.
+ * The TCA6418's input, output, and direction banks have a peculiar bit order:
+ * the first byte uses reversed bit order, while the second byte uses standard order.
+ */
+static inline u8 pca953x_get_bit_mask(struct pca953x_chip *chip, unsigned int offset)
+{
+ unsigned int bit_pos_in_bank = offset % BANK_SZ;
+ int msb = BANK_SZ - 1;
+
+ if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE && offset <= msb)
+ return BIT(msb - bit_pos_in_bank);
+ else
+ return BIT(bit_pos_in_bank);
+}
+
#define PCA953x_BANK_INPUT BIT(0)
#define PCA953x_BANK_OUTPUT BIT(1)
#define PCA953x_BANK_POLARITY BIT(2)
@@ -353,6 +381,23 @@ static bool pcal6534_check_register(struct pca953x_chip *chip, unsigned int reg,
return true;
}
+/* TCA6418 breaks the PCA953x register order rule */
+static bool tca6418_check_register(struct pca953x_chip *chip, unsigned int reg,
+ u32 access_type_mask)
+{
+ /* Valid Input Registers - BIT(0) for readable access */
+ if (reg >= TCA6418_INPUT && reg < (TCA6418_INPUT + NBANK(chip)))
+ return (access_type_mask & BIT(0));
+ /* Valid Output Registers - BIT(1) for writeable access */
+ if (reg >= TCA6418_OUTPUT && reg < (TCA6418_OUTPUT + NBANK(chip)))
+ return (access_type_mask & (BIT(0) | BIT(1)));
+ /* Valid Direction Registers - BIT(2) for volatile access */
+ if (reg >= TCA6418_DIRECTION && reg < (TCA6418_DIRECTION + NBANK(chip)))
+ return (access_type_mask & (BIT(0) | BIT(1)));
+
+ return false;
+}
+
static bool pca953x_readable_register(struct device *dev, unsigned int reg)
{
struct pca953x_chip *chip = dev_get_drvdata(dev);
@@ -362,6 +407,9 @@ static bool pca953x_readable_register(struct device *dev, unsigned int reg)
bank = PCA957x_BANK_INPUT | PCA957x_BANK_OUTPUT |
PCA957x_BANK_POLARITY | PCA957x_BANK_CONFIG |
PCA957x_BANK_BUSHOLD;
+ } else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE) {
+ /* BIT(0) to indicate read access */
+ return tca6418_check_register(chip, reg, BIT(0));
} else {
bank = PCA953x_BANK_INPUT | PCA953x_BANK_OUTPUT |
PCA953x_BANK_POLARITY | PCA953x_BANK_CONFIG;
@@ -384,6 +432,9 @@ static bool pca953x_writeable_register(struct device *dev, unsigned int reg)
if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
bank = PCA957x_BANK_OUTPUT | PCA957x_BANK_POLARITY |
PCA957x_BANK_CONFIG | PCA957x_BANK_BUSHOLD;
+ } else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE) {
+ /* BIT(1) for write access */
+ return tca6418_check_register(chip, reg, BIT(1));
} else {
bank = PCA953x_BANK_OUTPUT | PCA953x_BANK_POLARITY |
PCA953x_BANK_CONFIG;
@@ -403,6 +454,9 @@ static bool pca953x_volatile_register(struct device *dev, unsigned int reg)
if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE)
bank = PCA957x_BANK_INPUT;
+ else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
+ /* BIT(2) for volatile access */
+ return tca6418_check_register(chip, reg, BIT(2));
else
bank = PCA953x_BANK_INPUT;
@@ -489,6 +543,15 @@ static u8 pcal6534_recalc_addr(struct pca953x_chip *chip, int reg, int off)
return pinctrl + addr + (off / BANK_SZ);
}
+static u8 tca6418_recalc_addr(struct pca953x_chip *chip, int reg_base, int offset)
+{
+ /* reg_base will be TCA6418_INPUT, TCA6418_OUTPUT, or TCA6418_DIRECTION
+ * offset is the global GPIO line offset (0-17)
+ * BANK_SZ is 8 for TCA6418 (8 bits per register bank)
+ */
+ return reg_base + (offset / BANK_SZ);
+}
+
static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long *val)
{
u8 regaddr = chip->recalc_addr(chip, reg, 0);
@@ -529,11 +592,14 @@ static int pca953x_gpio_direction_input(struct gpio_chip *gc, unsigned off)
{
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 dirreg = chip->recalc_addr(chip, chip->regs->direction, off);
- u8 bit = BIT(off % BANK_SZ);
+ u8 bit = pca953x_get_bit_mask(chip, off);
guard(mutex)(&chip->i2c_lock);
- return regmap_write_bits(chip->regmap, dirreg, bit, bit);
+ if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
+ return regmap_write_bits(chip->regmap, dirreg, bit, 0);
+ else
+ return regmap_write_bits(chip->regmap, dirreg, bit, bit);
}
static int pca953x_gpio_direction_output(struct gpio_chip *gc,
@@ -542,7 +608,7 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 dirreg = chip->recalc_addr(chip, chip->regs->direction, off);
u8 outreg = chip->recalc_addr(chip, chip->regs->output, off);
- u8 bit = BIT(off % BANK_SZ);
+ u8 bit = pca953x_get_bit_mask(chip, off);
int ret;
guard(mutex)(&chip->i2c_lock);
@@ -552,15 +618,20 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
if (ret)
return ret;
- /* then direction */
- return regmap_write_bits(chip->regmap, dirreg, bit, 0);
+ /* then direction
+ * (in/out logic is inverted on TCA6418)
+ */
+ if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
+ return regmap_write_bits(chip->regmap, dirreg, bit, bit);
+ else
+ return regmap_write_bits(chip->regmap, dirreg, bit, 0);
}
static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
{
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 inreg = chip->recalc_addr(chip, chip->regs->input, off);
- u8 bit = BIT(off % BANK_SZ);
+ u8 bit = pca953x_get_bit_mask(chip, off);
u32 reg_val;
int ret;
@@ -577,7 +648,7 @@ static int pca953x_gpio_set_value(struct gpio_chip *gc, unsigned int off,
{
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 outreg = chip->recalc_addr(chip, chip->regs->output, off);
- u8 bit = BIT(off % BANK_SZ);
+ u8 bit = pca953x_get_bit_mask(chip, off);
guard(mutex)(&chip->i2c_lock);
@@ -588,7 +659,7 @@ static int pca953x_gpio_get_direction(struct gpio_chip *gc, unsigned off)
{
struct pca953x_chip *chip = gpiochip_get_data(gc);
u8 dirreg = chip->recalc_addr(chip, chip->regs->direction, off);
- u8 bit = BIT(off % BANK_SZ);
+ u8 bit = pca953x_get_bit_mask(chip, off);
u32 reg_val;
int ret;
@@ -597,10 +668,17 @@ static int pca953x_gpio_get_direction(struct gpio_chip *gc, unsigned off)
if (ret < 0)
return ret;
- if (reg_val & bit)
+ /* (in/out logic is inverted on TCA6418) */
+ if (reg_val & bit) {
+ if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
+ return GPIO_LINE_DIRECTION_OUT;
+ else
+ return GPIO_LINE_DIRECTION_IN;
+ }
+ if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
return GPIO_LINE_DIRECTION_IN;
-
- return GPIO_LINE_DIRECTION_OUT;
+ else
+ return GPIO_LINE_DIRECTION_OUT;
}
static int pca953x_gpio_get_multiple(struct gpio_chip *gc,
@@ -1120,6 +1198,11 @@ static int pca953x_probe(struct i2c_client *client)
if (PCA_CHIP_TYPE(chip->driver_data) == PCAL653X_TYPE) {
chip->recalc_addr = pcal6534_recalc_addr;
chip->check_reg = pcal6534_check_register;
+ } else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE) {
+ chip->recalc_addr = tca6418_recalc_addr;
+ /* We don't assign chip->check_reg = tca6418_check_register directly here.
+ * Instead, the wrappers handle the dispatch based on PCA_CHIP_TYPE.
+ */
} else {
chip->recalc_addr = pca953x_recalc_addr;
chip->check_reg = pca953x_check_register;
@@ -1157,6 +1240,8 @@ static int pca953x_probe(struct i2c_client *client)
if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
chip->regs = &pca957x_regs;
ret = device_pca957x_init(chip);
+ } else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE) {
+ chip->regs = &tca6418_regs;
} else {
chip->regs = &pca953x_regs;
ret = device_pca95xx_init(chip);
@@ -1325,6 +1410,7 @@ static const struct of_device_id pca953x_dt_ids[] = {
{ .compatible = "ti,pca9536", .data = OF_953X( 4, 0), },
{ .compatible = "ti,tca6408", .data = OF_953X( 8, PCA_INT), },
{ .compatible = "ti,tca6416", .data = OF_953X(16, PCA_INT), },
+ { .compatible = "ti,tca6418", .data = (void *)(18 | TCA6418_TYPE | PCA_INT), },
{ .compatible = "ti,tca6424", .data = OF_953X(24, PCA_INT), },
{ .compatible = "ti,tca9535", .data = OF_953X(16, PCA_INT), },
{ .compatible = "ti,tca9538", .data = OF_953X( 8, PCA_INT), },
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418
2025-06-17 20:44 ` [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418 Maria Garcia
@ 2025-06-18 15:58 ` Conor Dooley
0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2025-06-18 15:58 UTC (permalink / raw)
To: Maria Garcia
Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Maria Garcia
[-- Attachment #1: Type: text/plain, Size: 54 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] gpio: pca953x: Add support for TI TCA6418
2025-06-17 20:44 ` [PATCH 2/2] gpio: pca953x: Add support for " Maria Garcia
@ 2025-06-25 11:53 ` Bartosz Golaszewski
2025-07-03 13:07 ` Bartosz Golaszewski
0 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2025-06-25 11:53 UTC (permalink / raw)
To: Maria Garcia
Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maria Garcia
On Tue, Jun 17, 2025 at 10:44 PM Maria Garcia <mariagarcia7293@gmail.com> wrote:
>
> The TI TCA6418 is a 18-channel I2C I/O expander. It is slightly
> different to other models from the same family, such as TCA6416,
> but has enough in common with them to make it work with just a
> few tweaks, which are explained in the code's documentation.
>
> Signed-off-by: Maria Garcia <mariagarcia7293@gmail.com>
> ---
Thanks, looks good overall. Just a few nits below.
>
> +/* Helper function to get the correct bit mask for a given offset and chip type.
> + * The TCA6418's input, output, and direction banks have a peculiar bit order:
> + * the first byte uses reversed bit order, while the second byte uses standard order.
> + */
> +static inline u8 pca953x_get_bit_mask(struct pca953x_chip *chip, unsigned int offset)
> +{
> + unsigned int bit_pos_in_bank = offset % BANK_SZ;
> + int msb = BANK_SZ - 1;
> +
> + if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE && offset <= msb)
> + return BIT(msb - bit_pos_in_bank);
> + else
> + return BIT(bit_pos_in_bank);
No need for else.
> static bool pca953x_readable_register(struct device *dev, unsigned int reg)
> {
> struct pca953x_chip *chip = dev_get_drvdata(dev);
> @@ -362,6 +407,9 @@ static bool pca953x_readable_register(struct device *dev, unsigned int reg)
> bank = PCA957x_BANK_INPUT | PCA957x_BANK_OUTPUT |
> PCA957x_BANK_POLARITY | PCA957x_BANK_CONFIG |
> PCA957x_BANK_BUSHOLD;
> + } else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE) {
> + /* BIT(0) to indicate read access */
> + return tca6418_check_register(chip, reg, BIT(0));
> } else {
> bank = PCA953x_BANK_INPUT | PCA953x_BANK_OUTPUT |
> PCA953x_BANK_POLARITY | PCA953x_BANK_CONFIG;
> @@ -384,6 +432,9 @@ static bool pca953x_writeable_register(struct device *dev, unsigned int reg)
> if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
> bank = PCA957x_BANK_OUTPUT | PCA957x_BANK_POLARITY |
> PCA957x_BANK_CONFIG | PCA957x_BANK_BUSHOLD;
> + } else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE) {
> + /* BIT(1) for write access */
> + return tca6418_check_register(chip, reg, BIT(1));
> } else {
> bank = PCA953x_BANK_OUTPUT | PCA953x_BANK_POLARITY |
> PCA953x_BANK_CONFIG;
Can you convert these to a switch statement for better readability? I
have a slight preference for cases over ifelserry.
> @@ -403,6 +454,9 @@ static bool pca953x_volatile_register(struct device *dev, unsigned int reg)
>
> if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE)
> bank = PCA957x_BANK_INPUT;
> + else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
> + /* BIT(2) for volatile access */
> + return tca6418_check_register(chip, reg, BIT(2));
> else
> bank = PCA953x_BANK_INPUT;
>
> @@ -489,6 +543,15 @@ static u8 pcal6534_recalc_addr(struct pca953x_chip *chip, int reg, int off)
> return pinctrl + addr + (off / BANK_SZ);
> }
>
> +static u8 tca6418_recalc_addr(struct pca953x_chip *chip, int reg_base, int offset)
> +{
> + /* reg_base will be TCA6418_INPUT, TCA6418_OUTPUT, or TCA6418_DIRECTION
> + * offset is the global GPIO line offset (0-17)
> + * BANK_SZ is 8 for TCA6418 (8 bits per register bank)
> + */
Please use regular kernel comments, not the networking style.
> + return reg_base + (offset / BANK_SZ);
> +}
> +
> static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long *val)
> {
> u8 regaddr = chip->recalc_addr(chip, reg, 0);
> @@ -529,11 +592,14 @@ static int pca953x_gpio_direction_input(struct gpio_chip *gc, unsigned off)
> {
> struct pca953x_chip *chip = gpiochip_get_data(gc);
> u8 dirreg = chip->recalc_addr(chip, chip->regs->direction, off);
> - u8 bit = BIT(off % BANK_SZ);
> + u8 bit = pca953x_get_bit_mask(chip, off);
>
> guard(mutex)(&chip->i2c_lock);
>
> - return regmap_write_bits(chip->regmap, dirreg, bit, bit);
> + if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
> + return regmap_write_bits(chip->regmap, dirreg, bit, 0);
> + else
> + return regmap_write_bits(chip->regmap, dirreg, bit, bit);
> }
>
No need for else.
> static int pca953x_gpio_direction_output(struct gpio_chip *gc,
> @@ -542,7 +608,7 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
> struct pca953x_chip *chip = gpiochip_get_data(gc);
> u8 dirreg = chip->recalc_addr(chip, chip->regs->direction, off);
> u8 outreg = chip->recalc_addr(chip, chip->regs->output, off);
> - u8 bit = BIT(off % BANK_SZ);
> + u8 bit = pca953x_get_bit_mask(chip, off);
> int ret;
>
> guard(mutex)(&chip->i2c_lock);
> @@ -552,15 +618,20 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
> if (ret)
> return ret;
>
> - /* then direction */
> - return regmap_write_bits(chip->regmap, dirreg, bit, 0);
> + /* then direction
> + * (in/out logic is inverted on TCA6418)
> + */
> + if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
> + return regmap_write_bits(chip->regmap, dirreg, bit, bit);
> + else
> + return regmap_write_bits(chip->regmap, dirreg, bit, 0);
> }
Same here.
>
> static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
> {
> struct pca953x_chip *chip = gpiochip_get_data(gc);
> u8 inreg = chip->recalc_addr(chip, chip->regs->input, off);
> - u8 bit = BIT(off % BANK_SZ);
> + u8 bit = pca953x_get_bit_mask(chip, off);
> u32 reg_val;
> int ret;
>
> @@ -577,7 +648,7 @@ static int pca953x_gpio_set_value(struct gpio_chip *gc, unsigned int off,
> {
> struct pca953x_chip *chip = gpiochip_get_data(gc);
> u8 outreg = chip->recalc_addr(chip, chip->regs->output, off);
> - u8 bit = BIT(off % BANK_SZ);
> + u8 bit = pca953x_get_bit_mask(chip, off);
>
> guard(mutex)(&chip->i2c_lock);
>
> @@ -588,7 +659,7 @@ static int pca953x_gpio_get_direction(struct gpio_chip *gc, unsigned off)
> {
> struct pca953x_chip *chip = gpiochip_get_data(gc);
> u8 dirreg = chip->recalc_addr(chip, chip->regs->direction, off);
> - u8 bit = BIT(off % BANK_SZ);
> + u8 bit = pca953x_get_bit_mask(chip, off);
> u32 reg_val;
> int ret;
>
> @@ -597,10 +668,17 @@ static int pca953x_gpio_get_direction(struct gpio_chip *gc, unsigned off)
> if (ret < 0)
> return ret;
>
> - if (reg_val & bit)
> + /* (in/out logic is inverted on TCA6418) */
> + if (reg_val & bit) {
> + if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
> + return GPIO_LINE_DIRECTION_OUT;
> + else
> + return GPIO_LINE_DIRECTION_IN;
Same here.
> + }
> + if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE)
> return GPIO_LINE_DIRECTION_IN;
> -
> - return GPIO_LINE_DIRECTION_OUT;
> + else
> + return GPIO_LINE_DIRECTION_OUT;
And here.
> }
>
> static int pca953x_gpio_get_multiple(struct gpio_chip *gc,
> @@ -1120,6 +1198,11 @@ static int pca953x_probe(struct i2c_client *client)
> if (PCA_CHIP_TYPE(chip->driver_data) == PCAL653X_TYPE) {
> chip->recalc_addr = pcal6534_recalc_addr;
> chip->check_reg = pcal6534_check_register;
> + } else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE) {
> + chip->recalc_addr = tca6418_recalc_addr;
> + /* We don't assign chip->check_reg = tca6418_check_register directly here.
> + * Instead, the wrappers handle the dispatch based on PCA_CHIP_TYPE.
> + */
> } else {
> chip->recalc_addr = pca953x_recalc_addr;
> chip->check_reg = pca953x_check_register;
> @@ -1157,6 +1240,8 @@ static int pca953x_probe(struct i2c_client *client)
> if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
> chip->regs = &pca957x_regs;
> ret = device_pca957x_init(chip);
> + } else if (PCA_CHIP_TYPE(chip->driver_data) == TCA6418_TYPE) {
> + chip->regs = &tca6418_regs;
> } else {
Same thing about if-elses.
> chip->regs = &pca953x_regs;
> ret = device_pca95xx_init(chip);
> @@ -1325,6 +1410,7 @@ static const struct of_device_id pca953x_dt_ids[] = {
> { .compatible = "ti,pca9536", .data = OF_953X( 4, 0), },
> { .compatible = "ti,tca6408", .data = OF_953X( 8, PCA_INT), },
> { .compatible = "ti,tca6416", .data = OF_953X(16, PCA_INT), },
> + { .compatible = "ti,tca6418", .data = (void *)(18 | TCA6418_TYPE | PCA_INT), },
> { .compatible = "ti,tca6424", .data = OF_953X(24, PCA_INT), },
> { .compatible = "ti,tca9535", .data = OF_953X(16, PCA_INT), },
> { .compatible = "ti,tca9538", .data = OF_953X( 8, PCA_INT), },
> --
> 2.43.0
>
Bartosz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418
2025-07-01 10:05 [PATCH 0/2] *** Add support for TI TCA6418 GPIO chip *** Maria Garcia
@ 2025-07-01 10:05 ` Maria Garcia
2025-07-01 16:08 ` Conor Dooley
0 siblings, 1 reply; 8+ messages in thread
From: Maria Garcia @ 2025-07-01 10:05 UTC (permalink / raw)
To: linux-gpio, devicetree, linux-kernel
Cc: Linus Walleij, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maria Garcia, Maria Garcia,
Conor Dooley
The TCA6418E is a 18-channel I2C I/O expander with integrated ESD
protection.
Signed-off-by: Maria Garcia <mariagarcia7293@gmail.com>
Acked-by: Conor Dooley <conor@kernel.org>
---
Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml b/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
index 4d3f52f8d1b8..12134c737ad8 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
+++ b/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
@@ -68,6 +68,7 @@ properties:
- ti,pca9536
- ti,tca6408
- ti,tca6416
+ - ti,tca6418
- ti,tca6424
- ti,tca9535
- ti,tca9538
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418
2025-07-01 10:05 ` [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418 Maria Garcia
@ 2025-07-01 16:08 ` Conor Dooley
0 siblings, 0 replies; 8+ messages in thread
From: Conor Dooley @ 2025-07-01 16:08 UTC (permalink / raw)
To: Maria Garcia
Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij,
Bartosz Golaszewski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Maria Garcia
[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]
On Tue, Jul 01, 2025 at 12:05:36PM +0200, Maria Garcia wrote:
> The TCA6418E is a 18-channel I2C I/O expander with integrated ESD
> protection.
>
> Signed-off-by: Maria Garcia <mariagarcia7293@gmail.com>
> Acked-by: Conor Dooley <conor@kernel.org>
This tag is fake, I never provide tags using that email address.
Please do not make up tags, instead copy them exactly as they are given
to you:
https://lore.kernel.org/all/20250618-concierge-fencing-e62c1e884ce9@spud/
Additionally, you forgot to mark this patch as v2.
Cheers,
Conor.
> ---
> Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml b/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
> index 4d3f52f8d1b8..12134c737ad8 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pca95xx.yaml
> @@ -68,6 +68,7 @@ properties:
> - ti,pca9536
> - ti,tca6408
> - ti,tca6416
> + - ti,tca6418
> - ti,tca6424
> - ti,tca9535
> - ti,tca9538
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] gpio: pca953x: Add support for TI TCA6418
2025-06-25 11:53 ` Bartosz Golaszewski
@ 2025-07-03 13:07 ` Bartosz Golaszewski
0 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2025-07-03 13:07 UTC (permalink / raw)
To: Maria Garcia
Cc: linux-gpio, devicetree, linux-kernel, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maria Garcia
On Wed, Jun 25, 2025 at 1:53 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> >
> > +static u8 tca6418_recalc_addr(struct pca953x_chip *chip, int reg_base, int offset)
> > +{
> > + /* reg_base will be TCA6418_INPUT, TCA6418_OUTPUT, or TCA6418_DIRECTION
> > + * offset is the global GPIO line offset (0-17)
> > + * BANK_SZ is 8 for TCA6418 (8 bits per register bank)
> > + */
>
> Please use regular kernel comments, not the networking style.
>
I asked for this under version 1. It's still not fixed in version 3.
Bart
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-07-03 13:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-17 20:44 [PATCH 0/2] *** Add support for TI TCA6418 GPIO chip *** Maria Garcia
2025-06-17 20:44 ` [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418 Maria Garcia
2025-06-18 15:58 ` Conor Dooley
2025-06-17 20:44 ` [PATCH 2/2] gpio: pca953x: Add support for " Maria Garcia
2025-06-25 11:53 ` Bartosz Golaszewski
2025-07-03 13:07 ` Bartosz Golaszewski
-- strict thread matches above, loose matches on Subject: below --
2025-07-01 10:05 [PATCH 0/2] *** Add support for TI TCA6418 GPIO chip *** Maria Garcia
2025-07-01 10:05 ` [PATCH 1/2] dt-bindings: gpio: pca95xx: add TI TCA6418 Maria Garcia
2025-07-01 16:08 ` Conor Dooley
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).