[RFC PATCH 1/3] gpio: pca953x: Replace chip type flags with a type enum

[RFC PATCH 1/3] gpio: pca953x: Replace chip type flags with a type enum

[Levente Révész]

The driver supports 8 chip types. The chip type is encoded in
driver_data so that different chip types can be handled appropriately.

Encoding the type information in separate bit flags was not too
straightforward, and it didn't make it possible to encode 8 chip
types.

Replace bit flags with a pca953x_chip_type enum. Encode this enum in
driver_data as a number. Add missing chip types based on chip functions
and register layout.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Levente Révész <levente.revesz@eilabs.com>
---
 drivers/gpio/gpio-pca953x.c | 255 ++++++++++++++++++++----------------
 1 file changed, 143 insertions(+), 112 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index e4b7f34424e4..d6099fc18b01 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -57,60 +57,78 @@
 #define PCAL6524_OUT_INDCONF	0x2c
 #define PCAL6524_DEBOUNCE	0x2d
 
-#define PCA_GPIO_MASK		GENMASK(7, 0)
-
 #define PCAL_GPIO_MASK		GENMASK(4, 0)
 #define PCAL_PINCTRL_MASK	GENMASK(6, 5)
 
-#define PCA_INT			BIT(8)
-#define PCA_PCAL		BIT(9)
-#define PCA_LATCH_INT		(PCA_PCAL | PCA_INT)
-#define PCA953X_TYPE		BIT(12)
-#define PCA957X_TYPE		BIT(13)
-#define PCAL653X_TYPE		BIT(14)
-#define PCA_TYPE_MASK		GENMASK(15, 12)
+/*
+ * driver_data
+ *
+ *   31    24 23    16 15     8 7      0
+ *   xxxxxxxx xxxxxxxx xxxx____ ________
+ *                         ^^^^ ^^^^^^^^
+ *                         |    number of gpio lines
+ *                         chip type
+ */
+
+#define PCA_GPIO_MASK		GENMASK(7, 0)
+#define PCA_TYPE_MASK		GENMASK(11, 8)
+
+enum pca953x_chip_type {
+	TYPE_PCA953X_NOINT,
+	TYPE_PCA953X,
+	TYPE_PCA950X,
+	TYPE_PCAL953X,
+	TYPE_PCAL652X,
+	TYPE_PCAL653X,
+	TYPE_PCA957X,
+	TYPE_XRA120X,
+};
+
+/* Get chip type from driver_data */
+#define PCA_GET_TYPE(x)		(((x) & PCA_TYPE_MASK) >> 8)
 
-#define PCA_CHIP_TYPE(x)	((x) & PCA_TYPE_MASK)
+/* Set chip type in driver_data */
+#define PCA_SET_TYPE(x)		((x) << 8 & PCA_TYPE_MASK)
 
 static const struct i2c_device_id pca953x_id[] = {
-	{ "pca6408", 8  | PCA953X_TYPE | PCA_INT, },
-	{ "pca6416", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "pca9505", 40 | PCA953X_TYPE | PCA_INT, },
-	{ "pca9506", 40 | PCA953X_TYPE | PCA_INT, },
-	{ "pca9534", 8  | PCA953X_TYPE | PCA_INT, },
-	{ "pca9535", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "pca9536", 4  | PCA953X_TYPE, },
-	{ "pca9537", 4  | PCA953X_TYPE | PCA_INT, },
-	{ "pca9538", 8  | PCA953X_TYPE | PCA_INT, },
-	{ "pca9539", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "pca9554", 8  | PCA953X_TYPE | PCA_INT, },
-	{ "pca9555", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "pca9556", 8  | PCA953X_TYPE, },
-	{ "pca9557", 8  | PCA953X_TYPE, },
-	{ "pca9574", 8  | PCA957X_TYPE | PCA_INT, },
-	{ "pca9575", 16 | PCA957X_TYPE | PCA_INT, },
-	{ "pca9698", 40 | PCA953X_TYPE, },
-
-	{ "pcal6408", 8 | PCA953X_TYPE | PCA_LATCH_INT, },
-	{ "pcal6416", 16 | PCA953X_TYPE | PCA_LATCH_INT, },
-	{ "pcal6524", 24 | PCA953X_TYPE | PCA_LATCH_INT, },
-	{ "pcal6534", 34 | PCAL653X_TYPE | PCA_LATCH_INT, },
-	{ "pcal9535", 16 | PCA953X_TYPE | PCA_LATCH_INT, },
-	{ "pcal9554b", 8  | PCA953X_TYPE | PCA_LATCH_INT, },
-	{ "pcal9555a", 16 | PCA953X_TYPE | PCA_LATCH_INT, },
-
-	{ "max7310", 8  | PCA953X_TYPE, },
-	{ "max7312", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "max7313", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "max7315", 8  | PCA953X_TYPE | PCA_INT, },
-	{ "max7318", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "pca6107", 8  | PCA953X_TYPE | PCA_INT, },
-	{ "tca6408", 8  | PCA953X_TYPE | PCA_INT, },
-	{ "tca6416", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "tca6424", 24 | PCA953X_TYPE | PCA_INT, },
-	{ "tca9539", 16 | PCA953X_TYPE | PCA_INT, },
-	{ "tca9554", 8  | PCA953X_TYPE | PCA_INT, },
-	{ "xra1202", 8  | PCA953X_TYPE },
+	{ "pca6408", 8 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca6416", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca9505", 40 | PCA_SET_TYPE(TYPE_PCA950X), },
+	{ "pca9506", 40 | PCA_SET_TYPE(TYPE_PCA950X), },
+	{ "pca9534", 8 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca9535", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca9536", 4 | PCA_SET_TYPE(TYPE_PCA953X_NOINT), },
+	{ "pca9537", 4 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca9538", 8 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca9539", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca9554", 8 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca9555", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca9556", 8 | PCA_SET_TYPE(TYPE_PCA953X_NOINT), },
+	{ "pca9557", 8 | PCA_SET_TYPE(TYPE_PCA953X_NOINT), },
+	{ "pca9574", 8 | PCA_SET_TYPE(TYPE_PCA957X), },
+	{ "pca9575", 16 | PCA_SET_TYPE(TYPE_PCA957X), },
+	{ "pca9698", 40 | PCA_SET_TYPE(TYPE_PCA950X), },
+
+	{ "pcal6408", 8 | PCA_SET_TYPE(TYPE_PCAL953X), },
+	{ "pcal6416", 16 | PCA_SET_TYPE(TYPE_PCAL953X), },
+	{ "pcal6524", 24 | PCA_SET_TYPE(TYPE_PCAL652X), },
+	{ "pcal6534", 34 | PCA_SET_TYPE(TYPE_PCAL653X), },
+	{ "pcal9535", 16 | PCA_SET_TYPE(TYPE_PCAL953X), },
+	{ "pcal9554b", 8 | PCA_SET_TYPE(TYPE_PCAL953X), },
+	{ "pcal9555a", 16 | PCA_SET_TYPE(TYPE_PCAL953X), },
+
+	{ "max7310", 8 | PCA_SET_TYPE(TYPE_PCA953X_NOINT), },
+	{ "max7312", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "max7313", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "max7315", 8 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "max7318", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "pca6107", 8 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "tca6408", 8 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "tca6416", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "tca6424", 24 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "tca9539", 16 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "tca9554", 8 | PCA_SET_TYPE(TYPE_PCA953X), },
+	{ "xra1202", 8 | PCA_SET_TYPE(TYPE_XRA120X) },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, pca953x_id);
@@ -162,7 +180,7 @@ static const struct dmi_system_id pca953x_dmi_acpi_irq_info[] = {
 #endif
 
 static const struct acpi_device_id pca953x_acpi_ids[] = {
-	{ "INT3491", 16 | PCA953X_TYPE | PCA_LATCH_INT, },
+	{ "INT3491", 16 | PCA_SET_TYPE(TYPE_PCAL953X), },
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids);
@@ -214,6 +232,7 @@ struct pca953x_chip {
 	unsigned long driver_data;
 	struct regulator *regulator;
 
+	enum pca953x_chip_type type;
 	const struct pca953x_reg_config *regs;
 
 	u8 (*recalc_addr)(struct pca953x_chip *chip, int reg, int off);
@@ -221,6 +240,18 @@ struct pca953x_chip {
 			  u32 checkbank);
 };
 
+static bool pca953x_has_int(struct pca953x_chip *chip)
+{
+	return chip->type != TYPE_PCA953X_NOINT;
+}
+
+static bool pca953x_is_pcal_type(struct pca953x_chip *chip)
+{
+	return chip->type == TYPE_PCAL953X ||
+	       chip->type == TYPE_PCAL652X ||
+	       chip->type == TYPE_PCAL653X;
+}
+
 static int pca953x_bank_shift(struct pca953x_chip *chip)
 {
 	return fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
@@ -280,7 +311,7 @@ static bool pca953x_check_register(struct pca953x_chip *chip, unsigned int reg,
 
 	/* Special PCAL extended register check. */
 	if (reg & REG_ADDR_EXT) {
-		if (!(chip->driver_data & PCA_PCAL))
+		if (!pca953x_is_pcal_type(chip))
 			return false;
 		bank += 8;
 	}
@@ -347,7 +378,7 @@ static bool pca953x_readable_register(struct device *dev, unsigned int reg)
 	struct pca953x_chip *chip = dev_get_drvdata(dev);
 	u32 bank;
 
-	if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
+	if (chip->type == TYPE_PCA957X) {
 		bank = PCA957x_BANK_INPUT | PCA957x_BANK_OUTPUT |
 		       PCA957x_BANK_POLARITY | PCA957x_BANK_CONFIG |
 		       PCA957x_BANK_BUSHOLD;
@@ -356,7 +387,7 @@ static bool pca953x_readable_register(struct device *dev, unsigned int reg)
 		       PCA953x_BANK_POLARITY | PCA953x_BANK_CONFIG;
 	}
 
-	if (chip->driver_data & PCA_PCAL) {
+	if (pca953x_is_pcal_type(chip)) {
 		bank |= PCAL9xxx_BANK_IN_LATCH | PCAL9xxx_BANK_PULL_EN |
 			PCAL9xxx_BANK_PULL_SEL | PCAL9xxx_BANK_IRQ_MASK |
 			PCAL9xxx_BANK_IRQ_STAT;
@@ -370,7 +401,7 @@ static bool pca953x_writeable_register(struct device *dev, unsigned int reg)
 	struct pca953x_chip *chip = dev_get_drvdata(dev);
 	u32 bank;
 
-	if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
+	if (chip->type == TYPE_PCA957X) {
 		bank = PCA957x_BANK_OUTPUT | PCA957x_BANK_POLARITY |
 			PCA957x_BANK_CONFIG | PCA957x_BANK_BUSHOLD;
 	} else {
@@ -378,7 +409,7 @@ static bool pca953x_writeable_register(struct device *dev, unsigned int reg)
 			PCA953x_BANK_CONFIG;
 	}
 
-	if (chip->driver_data & PCA_PCAL)
+	if (pca953x_is_pcal_type(chip))
 		bank |= PCAL9xxx_BANK_IN_LATCH | PCAL9xxx_BANK_PULL_EN |
 			PCAL9xxx_BANK_PULL_SEL | PCAL9xxx_BANK_IRQ_MASK;
 
@@ -390,12 +421,12 @@ static bool pca953x_volatile_register(struct device *dev, unsigned int reg)
 	struct pca953x_chip *chip = dev_get_drvdata(dev);
 	u32 bank;
 
-	if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE)
+	if (chip->type == TYPE_PCA957X)
 		bank = PCA957x_BANK_INPUT;
 	else
 		bank = PCA953x_BANK_INPUT;
 
-	if (chip->driver_data & PCA_PCAL)
+	if (pca953x_is_pcal_type(chip))
 		bank |= PCAL9xxx_BANK_IRQ_STAT;
 
 	return chip->check_reg(chip, reg, bank);
@@ -645,7 +676,7 @@ static int pca953x_gpio_set_pull_up_down(struct pca953x_chip *chip,
 	 * pull-up/pull-down configuration requires PCAL extended
 	 * registers
 	 */
-	if (!(chip->driver_data & PCA_PCAL))
+	if (!pca953x_is_pcal_type(chip))
 		return -ENOTSUPP;
 
 	mutex_lock(&chip->i2c_lock);
@@ -761,7 +792,7 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
 	DECLARE_BITMAP(reg_direction, MAX_LINE);
 	int level;
 
-	if (chip->driver_data & PCA_PCAL) {
+	if (pca953x_is_pcal_type(chip)) {
 		/* Enable latch on interrupt-enabled inputs */
 		pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
 
@@ -843,7 +874,7 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pendin
 	DECLARE_BITMAP(trigger, MAX_LINE);
 	int ret;
 
-	if (chip->driver_data & PCA_PCAL) {
+	if (pca953x_is_pcal_type(chip)) {
 		/* Read the current interrupt status from the device */
 		ret = pca953x_read_regs(chip, PCAL953X_INT_STAT, trigger);
 		if (ret)
@@ -941,7 +972,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, int irq_base)
 	if (irq_base == -1)
 		return 0;
 
-	if (!(chip->driver_data & PCA_INT))
+	if (!pca953x_has_int(chip))
 		return 0;
 
 	ret = pca953x_read_regs(chip, chip->regs->input, irq_stat);
@@ -987,7 +1018,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip,
 {
 	struct i2c_client *client = chip->client;
 
-	if (client->irq && irq_base != -1 && (chip->driver_data & PCA_INT))
+	if (client->irq && irq_base != -1 && pca953x_has_int(chip))
 		dev_warn(&client->dev, "interrupt support not compiled in\n");
 
 	return 0;
@@ -1117,7 +1148,9 @@ static int pca953x_probe(struct i2c_client *client)
 
 	pca953x_setup_gpio(chip, chip->driver_data & PCA_GPIO_MASK);
 
-	if (NBANK(chip) > 2 || PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
+	chip->type = PCA_GET_TYPE(chip->driver_data);
+
+	if (NBANK(chip) > 2 || chip->type == TYPE_PCA957X) {
 		dev_info(&client->dev, "using AI\n");
 		regmap_config = &pca953x_ai_i2c_regmap;
 	} else {
@@ -1125,7 +1158,7 @@ static int pca953x_probe(struct i2c_client *client)
 		regmap_config = &pca953x_i2c_regmap;
 	}
 
-	if (PCA_CHIP_TYPE(chip->driver_data) == PCAL653X_TYPE) {
+	if (chip->type == TYPE_PCAL653X) {
 		chip->recalc_addr = pcal6534_recalc_addr;
 		chip->check_reg = pcal6534_check_register;
 	} else {
@@ -1164,7 +1197,7 @@ static int pca953x_probe(struct i2c_client *client)
 	/* initialize cached registers from their original values.
 	 * we can't share this chip with another i2c master.
 	 */
-	if (PCA_CHIP_TYPE(chip->driver_data) == PCA957X_TYPE) {
+	if (chip->type == TYPE_PCA957X) {
 		chip->regs = &pca957x_regs;
 		ret = device_pca957x_init(chip, invert);
 	} else {
@@ -1235,7 +1268,7 @@ static int pca953x_regcache_sync(struct device *dev)
 	}
 
 #ifdef CONFIG_GPIO_PCA953X_IRQ
-	if (chip->driver_data & PCA_PCAL) {
+	if (pca953x_is_pcal_type(chip)) {
 		regaddr = chip->recalc_addr(chip, PCAL953X_IN_LATCH, 0);
 		ret = regcache_sync_region(chip->regmap, regaddr,
 					   regaddr + NBANK(chip) - 1);
@@ -1309,55 +1342,53 @@ static int pca953x_resume(struct device *dev)
 #endif
 
 /* convenience to stop overlong match-table lines */
-#define OF_653X(__nrgpio, __int) ((void *)(__nrgpio | PCAL653X_TYPE | __int))
-#define OF_953X(__nrgpio, __int) (void *)(__nrgpio | PCA953X_TYPE | __int)
-#define OF_957X(__nrgpio, __int) (void *)(__nrgpio | PCA957X_TYPE | __int)
+#define OF_DATA(__nrgpio, __type) ((void *)(__nrgpio | PCA_SET_TYPE(__type)))
 
 static const struct of_device_id pca953x_dt_ids[] = {
-	{ .compatible = "nxp,pca6408", .data = OF_953X(8, PCA_INT), },
-	{ .compatible = "nxp,pca6416", .data = OF_953X(16, PCA_INT), },
-	{ .compatible = "nxp,pca9505", .data = OF_953X(40, PCA_INT), },
-	{ .compatible = "nxp,pca9506", .data = OF_953X(40, PCA_INT), },
-	{ .compatible = "nxp,pca9534", .data = OF_953X( 8, PCA_INT), },
-	{ .compatible = "nxp,pca9535", .data = OF_953X(16, PCA_INT), },
-	{ .compatible = "nxp,pca9536", .data = OF_953X( 4, 0), },
-	{ .compatible = "nxp,pca9537", .data = OF_953X( 4, PCA_INT), },
-	{ .compatible = "nxp,pca9538", .data = OF_953X( 8, PCA_INT), },
-	{ .compatible = "nxp,pca9539", .data = OF_953X(16, PCA_INT), },
-	{ .compatible = "nxp,pca9554", .data = OF_953X( 8, PCA_INT), },
-	{ .compatible = "nxp,pca9555", .data = OF_953X(16, PCA_INT), },
-	{ .compatible = "nxp,pca9556", .data = OF_953X( 8, 0), },
-	{ .compatible = "nxp,pca9557", .data = OF_953X( 8, 0), },
-	{ .compatible = "nxp,pca9574", .data = OF_957X( 8, PCA_INT), },
-	{ .compatible = "nxp,pca9575", .data = OF_957X(16, PCA_INT), },
-	{ .compatible = "nxp,pca9698", .data = OF_953X(40, 0), },
-
-	{ .compatible = "nxp,pcal6408", .data = OF_953X(8, PCA_LATCH_INT), },
-	{ .compatible = "nxp,pcal6416", .data = OF_953X(16, PCA_LATCH_INT), },
-	{ .compatible = "nxp,pcal6524", .data = OF_953X(24, PCA_LATCH_INT), },
-	{ .compatible = "nxp,pcal6534", .data = OF_653X(34, PCA_LATCH_INT), },
-	{ .compatible = "nxp,pcal9535", .data = OF_953X(16, PCA_LATCH_INT), },
-	{ .compatible = "nxp,pcal9554b", .data = OF_953X( 8, 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), },
-	{ .compatible = "maxim,max7313", .data = OF_953X(16, PCA_INT), },
-	{ .compatible = "maxim,max7315", .data = OF_953X( 8, PCA_INT), },
-	{ .compatible = "maxim,max7318", .data = OF_953X(16, PCA_INT), },
-
-	{ .compatible = "ti,pca6107", .data = OF_953X( 8, PCA_INT), },
-	{ .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,tca6424", .data = OF_953X(24, PCA_INT), },
-	{ .compatible = "ti,tca9539", .data = OF_953X(16, PCA_INT), },
-
-	{ .compatible = "onnn,cat9554", .data = OF_953X( 8, PCA_INT), },
-	{ .compatible = "onnn,pca9654", .data = OF_953X( 8, PCA_INT), },
-	{ .compatible = "onnn,pca9655", .data = OF_953X(16, PCA_INT), },
-
-	{ .compatible = "exar,xra1202", .data = OF_953X( 8, 0), },
+	{ .compatible = "nxp,pca6408", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca6416", .data = OF_DATA(16, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca9505", .data = OF_DATA(40, TYPE_PCA950X), },
+	{ .compatible = "nxp,pca9506", .data = OF_DATA(40, TYPE_PCA950X), },
+	{ .compatible = "nxp,pca9534", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca9535", .data = OF_DATA(16, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca9536", .data = OF_DATA(4, TYPE_PCA953X_NOINT), },
+	{ .compatible = "nxp,pca9537", .data = OF_DATA(4, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca9538", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca9539", .data = OF_DATA(16, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca9554", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca9555", .data = OF_DATA(16, TYPE_PCA953X), },
+	{ .compatible = "nxp,pca9556", .data = OF_DATA(8, TYPE_PCA953X_NOINT), },
+	{ .compatible = "nxp,pca9557", .data = OF_DATA(8, TYPE_PCA953X_NOINT), },
+	{ .compatible = "nxp,pca9574", .data = OF_DATA(8, TYPE_PCA957X), },
+	{ .compatible = "nxp,pca9575", .data = OF_DATA(16, TYPE_PCA957X), },
+	{ .compatible = "nxp,pca9698", .data = OF_DATA(40, TYPE_PCA950X), },
+
+	{ .compatible = "nxp,pcal6408", .data = OF_DATA(8, TYPE_PCAL953X), },
+	{ .compatible = "nxp,pcal6416", .data = OF_DATA(16, TYPE_PCAL953X), },
+	{ .compatible = "nxp,pcal6524", .data = OF_DATA(24, TYPE_PCAL652X), },
+	{ .compatible = "nxp,pcal6534", .data = OF_DATA(34, TYPE_PCAL653X), },
+	{ .compatible = "nxp,pcal9535", .data = OF_DATA(16, TYPE_PCAL953X), },
+	{ .compatible = "nxp,pcal9554b", .data = OF_DATA(8, TYPE_PCAL953X), },
+	{ .compatible = "nxp,pcal9555a", .data = OF_DATA(16, TYPE_PCAL953X), },
+
+	{ .compatible = "maxim,max7310", .data = OF_DATA(8, TYPE_PCA953X_NOINT), },
+	{ .compatible = "maxim,max7312", .data = OF_DATA(16, TYPE_PCA953X), },
+	{ .compatible = "maxim,max7313", .data = OF_DATA(16, TYPE_PCA953X), },
+	{ .compatible = "maxim,max7315", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "maxim,max7318", .data = OF_DATA(16, TYPE_PCA953X), },
+
+	{ .compatible = "ti,pca6107", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "ti,pca9536", .data = OF_DATA(4, TYPE_PCA953X_NOINT), },
+	{ .compatible = "ti,tca6408", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "ti,tca6416", .data = OF_DATA(16, TYPE_PCA953X), },
+	{ .compatible = "ti,tca6424", .data = OF_DATA(24, TYPE_PCA953X), },
+	{ .compatible = "ti,tca9539", .data = OF_DATA(16, TYPE_PCA953X), },
+
+	{ .compatible = "onnn,cat9554", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "onnn,pca9654", .data = OF_DATA(8, TYPE_PCA953X), },
+	{ .compatible = "onnn,pca9655", .data = OF_DATA(16, TYPE_PCA953X), },
+
+	{ .compatible = "exar,xra1202", .data = OF_DATA(8, TYPE_XRA120X), },
 	{ }
 };
 
-- 
2.38.1

[RFC PATCH 2/3] gpio: pca953x: Describe register maps with enums

[Levente Révész]

The driver supports 8 chip types, 6 of which have extended registers
with various functions, e.g. pull-up and pull-down bias for the gpio
lines or interrupt masking. To allow supporting these functions, the
driver has to be able to calculate the addresses of these registers.

Replace the register maps with an enum for each chip type. These do not
contain the same numeric values as the old defines, but the new address
calculating functions (in the next patch) use them appropriately.

Add currently used registers to struct pca953x_reg_config.

Create a reg_config struct for each chip type.

Signed-off-by: Levente Révész <levente.revesz@eilabs.com>
---
 drivers/gpio/gpio-pca953x.c | 422 ++++++++++++++++++++++++++++--------
 1 file changed, 333 insertions(+), 89 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index d6099fc18b01..3d4c3efeaf35 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -25,24 +25,10 @@
 
 #include <asm/unaligned.h>
 
-#define PCA953X_INPUT		0x00
-#define PCA953X_OUTPUT		0x01
-#define PCA953X_INVERT		0x02
-#define PCA953X_DIRECTION	0x03
-
 #define REG_ADDR_MASK		GENMASK(5, 0)
 #define REG_ADDR_EXT		BIT(6)
 #define REG_ADDR_AI		BIT(7)
 
-#define PCA957X_IN		0x00
-#define PCA957X_INVRT		0x01
-#define PCA957X_BKEN		0x02
-#define PCA957X_PUPD		0x03
-#define PCA957X_CFG		0x04
-#define PCA957X_OUT		0x05
-#define PCA957X_MSK		0x06
-#define PCA957X_INTS		0x07
-
 #define PCAL953X_OUT_STRENGTH	0x20
 #define PCAL953X_IN_LATCH	0x22
 #define PCAL953X_PULL_EN	0x23
@@ -191,25 +177,152 @@ MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids);
 
 #define NBANK(chip) DIV_ROUND_UP(chip->gpio_chip.ngpio, BANK_SZ)
 
+enum pca953x_reg {
+	PCA953X_REG_INPUT,
+	PCA953X_REG_OUTPUT,
+	PCA953X_REG_INVERT,
+	PCA953X_REG_DIRECTION,
+};
+
+enum pca950x_reg {
+	PCA950X_REG_INPUT,
+	PCA950X_REG_OUTPUT,
+	PCA950X_REG_INVERT,
+	PCA950X_REG_DIRECTION,
+	PCA950X_REG_INT_MASK,
+};
+
+enum pcal953x_reg {
+	PCAL953X_REG_INPUT,
+	PCAL953X_REG_OUTPUT,
+	PCAL953X_REG_INVERT,
+	PCAL953X_REG_DIRECTION,
+	PCAL953X_REG_DRIVE_STRENGTH,
+	PCAL953X_REG_INPUT_LATCH,
+	PCAL953X_REG_PULL_EN,
+	PCAL953X_REG_PULL_SEL,
+	PCAL953X_REG_INT_MASK,
+	PCAL953X_REG_INT_STATUS,
+	PCAL953X_REG_DRIVE_MODE,
+};
+
+enum pcal65xx_reg {
+	PCAL65XX_REG_INPUT,
+	PCAL65XX_REG_OUTPUT,
+	PCAL65XX_REG_INVERT,
+	PCAL65XX_REG_DIRECTION,
+	PCAL65XX_REG_DRIVE_STRENGTH,
+	PCAL65XX_REG_INPUT_LATCH,
+	PCAL65XX_REG_PULL_EN,
+	PCAL65XX_REG_PULL_SEL,
+	PCAL65XX_REG_INT_MASK,
+	PCAL65XX_REG_INT_STATUS,
+	PCAL65XX_REG_DRIVE_MODE,
+	PCAL65XX_REG_INT_EDGE,
+	PCAL65XX_REG_INT_CLEAR,
+	PCAL65XX_REG_INPUT_STATUS,
+	PCAL65XX_REG_INDIV_DRIVE_MODE,
+	PCAL65XX_REG_DEBOUNCE_EN,
+	PCAL65XX_REG_DEBOUNCE_COUNT,
+};
+
+#define PCAL65XX_MAX_NUM_DEBOUNCE_PORTS	2
+
+enum pca957x_reg {
+	PCA957X_REG_INPUT,
+	PCA957X_REG_INVERT,
+	PCA957X_REG_PULL_EN,
+	PCA957X_REG_PULL_SEL,
+	PCA957X_REG_DIRECTION,
+	PCA957X_REG_OUTPUT,
+	PCA957X_REG_INT_MASK,
+	PCA957X_REG_INT_STATUS,
+};
+
+enum xra120x_reg {
+	XRA120X_REG_INPUT,
+	XRA120X_REG_OUTPUT,
+	XRA120X_REG_INVERT,
+	XRA120X_REG_DIRECTION,
+	XRA120X_REG_PULL_EN,
+	XRA120X_REG_INT_MASK,
+	XRA120X_REG_TRISTATE,
+	XRA120X_REG_INT_STATUS,
+	XRA120X_REG_RISING_MASK,
+	XRA120X_REG_FALLING_MASK,
+	XRA120X_REG_DEBOUNCE_EN,
+};
+
 struct pca953x_reg_config {
 	int direction;
 	int output;
 	int input;
 	int invert;
+	int input_latch;
+	int int_mask;
+	int int_status;
+	int pull_en;
+	int pull_sel;
 };
 
 static const struct pca953x_reg_config pca953x_regs = {
-	.direction = PCA953X_DIRECTION,
-	.output = PCA953X_OUTPUT,
-	.input = PCA953X_INPUT,
-	.invert = PCA953X_INVERT,
+	.direction = PCA953X_REG_DIRECTION,
+	.output = PCA953X_REG_OUTPUT,
+	.input = PCA953X_REG_INPUT,
+	.invert = PCA953X_REG_INVERT,
+};
+
+static const struct pca953x_reg_config pca950x_regs = {
+	.direction = PCA950X_REG_DIRECTION,
+	.output = PCA950X_REG_OUTPUT,
+	.input = PCA950X_REG_INPUT,
+	.invert = PCA950X_REG_INVERT,
+	.int_mask = PCA950X_REG_INT_MASK,
 };
 
 static const struct pca953x_reg_config pca957x_regs = {
-	.direction = PCA957X_CFG,
-	.output = PCA957X_OUT,
-	.input = PCA957X_IN,
-	.invert = PCA957X_INVRT,
+	.direction = PCA957X_REG_DIRECTION,
+	.output = PCA957X_REG_OUTPUT,
+	.input = PCA957X_REG_INPUT,
+	.invert = PCA957X_REG_INVERT,
+	.int_mask = PCA957X_REG_INT_MASK,
+	.int_status = PCA957X_REG_INT_STATUS,
+	.pull_en = PCA957X_REG_PULL_EN,
+	.pull_sel = PCA957X_REG_PULL_SEL,
+};
+
+static const struct pca953x_reg_config xra120x_regs = {
+	.direction = XRA120X_REG_DIRECTION,
+	.output = XRA120X_REG_OUTPUT,
+	.input = XRA120X_REG_INPUT,
+	.invert = XRA120X_REG_INVERT,
+	.int_mask = XRA120X_REG_INT_MASK,
+	.int_status = XRA120X_REG_INT_STATUS,
+	.pull_en = XRA120X_REG_PULL_EN,
+};
+
+static const struct pca953x_reg_config pcal953x_regs = {
+	.direction = PCAL953X_REG_DIRECTION,
+	.output = PCAL953X_REG_OUTPUT,
+	.input = PCAL953X_REG_INPUT,
+	.invert = PCAL953X_REG_INVERT,
+	.input_latch = PCAL953X_REG_INPUT_LATCH,
+	.int_mask = PCAL953X_REG_INT_MASK,
+	.int_status = PCAL953X_REG_INT_STATUS,
+	.pull_en = PCAL953X_REG_PULL_EN,
+	.pull_sel = PCAL953X_REG_PULL_SEL,
+};
+
+static const struct pca953x_reg_config pcal65xx_regs = {
+	.direction = PCAL65XX_REG_DIRECTION,
+	.output = PCAL65XX_REG_OUTPUT,
+	.input = PCAL65XX_REG_INPUT,
+	.invert = PCAL65XX_REG_INVERT,
+	.input_latch = PCAL65XX_REG_INPUT_LATCH,
+	.int_mask = PCAL65XX_REG_INT_MASK,
+	.int_status = PCAL65XX_REG_INT_STATUS,
+	.pull_en = PCAL65XX_REG_PULL_EN,
+	.pull_sel = PCAL65XX_REG_PULL_SEL,
 };
 
 struct pca953x_chip {
@@ -257,23 +370,6 @@ static int pca953x_bank_shift(struct pca953x_chip *chip)
 	return fls((chip->gpio_chip.ngpio - 1) / BANK_SZ);
 }
 
-#define PCA953x_BANK_INPUT	BIT(0)
-#define PCA953x_BANK_OUTPUT	BIT(1)
-#define PCA953x_BANK_POLARITY	BIT(2)
-#define PCA953x_BANK_CONFIG	BIT(3)
-
-#define PCA957x_BANK_INPUT	BIT(0)
-#define PCA957x_BANK_POLARITY	BIT(1)
-#define PCA957x_BANK_BUSHOLD	BIT(2)
-#define PCA957x_BANK_CONFIG	BIT(4)
-#define PCA957x_BANK_OUTPUT	BIT(5)
-
-#define PCAL9xxx_BANK_IN_LATCH	BIT(8 + 2)
-#define PCAL9xxx_BANK_PULL_EN	BIT(8 + 3)
-#define PCAL9xxx_BANK_PULL_SEL	BIT(8 + 4)
-#define PCAL9xxx_BANK_IRQ_MASK	BIT(8 + 5)
-#define PCAL9xxx_BANK_IRQ_STAT	BIT(8 + 6)
-
 /*
  * We care about the following registers:
  * - Standard set, below 0x40, each port can be replicated up to 8 times
@@ -373,63 +469,189 @@ static bool pcal6534_check_register(struct pca953x_chip *chip, unsigned int reg,
 	return true;
 }
 
-static bool pca953x_readable_register(struct device *dev, unsigned int reg)
+static bool pca953x_readable_register(struct device *dev, unsigned int reg_addr)
 {
 	struct pca953x_chip *chip = dev_get_drvdata(dev);
-	u32 bank;
-
-	if (chip->type == TYPE_PCA957X) {
-		bank = PCA957x_BANK_INPUT | PCA957x_BANK_OUTPUT |
-		       PCA957x_BANK_POLARITY | PCA957x_BANK_CONFIG |
-		       PCA957x_BANK_BUSHOLD;
-	} else {
-		bank = PCA953x_BANK_INPUT | PCA953x_BANK_OUTPUT |
-		       PCA953x_BANK_POLARITY | PCA953x_BANK_CONFIG;
-	}
-
-	if (pca953x_is_pcal_type(chip)) {
-		bank |= PCAL9xxx_BANK_IN_LATCH | PCAL9xxx_BANK_PULL_EN |
-			PCAL9xxx_BANK_PULL_SEL | PCAL9xxx_BANK_IRQ_MASK |
-			PCAL9xxx_BANK_IRQ_STAT;
+	u32 registers;
+
+	switch (chip->type) {
+	case TYPE_PCA950X:
+		registers = BIT(PCA950X_REG_INPUT) |
+			    BIT(PCA950X_REG_OUTPUT) |
+			    BIT(PCA950X_REG_INVERT) |
+			    BIT(PCA950X_REG_DIRECTION) |
+			    BIT(PCA950X_REG_INT_MASK);
+		break;
+	case TYPE_PCAL953X:
+		registers = BIT(PCAL953X_REG_INPUT) |
+			    BIT(PCAL953X_REG_OUTPUT) |
+			    BIT(PCAL953X_REG_INVERT) |
+			    BIT(PCAL953X_REG_DIRECTION) |
+			    BIT(PCAL953X_REG_DRIVE_STRENGTH) |
+			    BIT(PCAL953X_REG_INPUT_LATCH) |
+			    BIT(PCAL953X_REG_PULL_EN) |
+			    BIT(PCAL953X_REG_PULL_SEL) |
+			    BIT(PCAL953X_REG_INT_MASK) |
+			    BIT(PCAL953X_REG_INT_STATUS) |
+			    BIT(PCAL953X_REG_DRIVE_MODE);
+		break;
+	case TYPE_PCAL652X:
+	case TYPE_PCAL653X:
+		registers = BIT(PCAL65XX_REG_INPUT) |
+			    BIT(PCAL65XX_REG_OUTPUT) |
+			    BIT(PCAL65XX_REG_INVERT) |
+			    BIT(PCAL65XX_REG_DIRECTION) |
+			    BIT(PCAL65XX_REG_DRIVE_STRENGTH) |
+			    BIT(PCAL65XX_REG_INPUT_LATCH) |
+			    BIT(PCAL65XX_REG_PULL_EN) |
+			    BIT(PCAL65XX_REG_PULL_SEL) |
+			    BIT(PCAL65XX_REG_INT_MASK) |
+			    BIT(PCAL65XX_REG_INT_STATUS) |
+			    BIT(PCAL65XX_REG_DRIVE_MODE) |
+			    BIT(PCAL65XX_REG_INT_EDGE) |
+			    BIT(PCAL65XX_REG_INPUT_STATUS) |
+			    BIT(PCAL65XX_REG_INDIV_DRIVE_MODE) |
+			    BIT(PCAL65XX_REG_DEBOUNCE_EN) |
+			    BIT(PCAL65XX_REG_DEBOUNCE_COUNT);
+		break;
+	case TYPE_PCA957X:
+		registers = BIT(PCA957X_REG_INPUT) |
+			    BIT(PCA957X_REG_INVERT) |
+			    BIT(PCA957X_REG_PULL_EN) |
+			    BIT(PCA957X_REG_PULL_SEL) |
+			    BIT(PCA957X_REG_DIRECTION) |
+			    BIT(PCA957X_REG_OUTPUT) |
+			    BIT(PCA957X_REG_INT_MASK) |
+			    BIT(PCA957X_REG_INT_STATUS);
+		break;
+	case TYPE_XRA120X:
+		registers = BIT(XRA120X_REG_INPUT) |
+			    BIT(XRA120X_REG_OUTPUT) |
+			    BIT(XRA120X_REG_INVERT) |
+			    BIT(XRA120X_REG_DIRECTION) |
+			    BIT(XRA120X_REG_OUTPUT) |
+			    BIT(XRA120X_REG_PULL_EN) |
+			    BIT(XRA120X_REG_INT_MASK) |
+			    BIT(XRA120X_REG_TRISTATE) |
+			    BIT(XRA120X_REG_INT_STATUS) |
+			    BIT(XRA120X_REG_RISING_MASK) |
+			    BIT(XRA120X_REG_FALLING_MASK) |
+			    BIT(XRA120X_REG_DEBOUNCE_EN);
+		break;
+	default:
+		registers = BIT(PCA953X_REG_INPUT) |
+			    BIT(PCA953X_REG_OUTPUT) |
+			    BIT(PCA953X_REG_INVERT) |
+			    BIT(PCAL953X_REG_DIRECTION);
+		break;
 	}
 
-	return chip->check_reg(chip, reg, bank);
+	return chip->check_reg(chip, reg_addr, registers);
 }
 
-static bool pca953x_writeable_register(struct device *dev, unsigned int reg)
+static bool pca953x_writeable_register(struct device *dev, unsigned int reg_addr)
 {
 	struct pca953x_chip *chip = dev_get_drvdata(dev);
-	u32 bank;
-
-	if (chip->type == TYPE_PCA957X) {
-		bank = PCA957x_BANK_OUTPUT | PCA957x_BANK_POLARITY |
-			PCA957x_BANK_CONFIG | PCA957x_BANK_BUSHOLD;
-	} else {
-		bank = PCA953x_BANK_OUTPUT | PCA953x_BANK_POLARITY |
-			PCA953x_BANK_CONFIG;
+	u32 registers;
+
+	switch (chip->type) {
+	case TYPE_PCA950X:
+		registers = BIT(PCA950X_REG_OUTPUT) |
+			    BIT(PCA950X_REG_INVERT) |
+			    BIT(PCA950X_REG_DIRECTION) |
+			    BIT(PCA950X_REG_INT_MASK);
+		break;
+	case TYPE_PCAL953X:
+		registers = BIT(PCAL953X_REG_OUTPUT) |
+			    BIT(PCAL953X_REG_INVERT) |
+			    BIT(PCAL953X_REG_DIRECTION) |
+			    BIT(PCAL953X_REG_DRIVE_STRENGTH) |
+			    BIT(PCAL953X_REG_INPUT_LATCH) |
+			    BIT(PCAL953X_REG_PULL_EN) |
+			    BIT(PCAL953X_REG_PULL_SEL) |
+			    BIT(PCAL953X_REG_INT_MASK) |
+			    BIT(PCAL953X_REG_DRIVE_MODE);
+		break;
+	case TYPE_PCAL652X:
+	case TYPE_PCAL653X:
+		registers = BIT(PCAL65XX_REG_OUTPUT) |
+			    BIT(PCAL65XX_REG_INVERT) |
+			    BIT(PCAL65XX_REG_DIRECTION) |
+			    BIT(PCAL65XX_REG_DRIVE_STRENGTH) |
+			    BIT(PCAL65XX_REG_INPUT_LATCH) |
+			    BIT(PCAL65XX_REG_PULL_EN) |
+			    BIT(PCAL65XX_REG_PULL_SEL) |
+			    BIT(PCAL65XX_REG_INT_MASK) |
+			    BIT(PCAL65XX_REG_DRIVE_MODE) |
+			    BIT(PCAL65XX_REG_INT_EDGE) |
+			    BIT(PCAL65XX_REG_INT_CLEAR) |
+			    BIT(PCAL65XX_REG_INDIV_DRIVE_MODE) |
+			    BIT(PCAL65XX_REG_DEBOUNCE_EN) |
+			    BIT(PCAL65XX_REG_DEBOUNCE_COUNT);
+		break;
+	case TYPE_PCA957X:
+		registers = BIT(PCA957X_REG_INVERT) |
+			    BIT(PCA957X_REG_PULL_EN) |
+			    BIT(PCA957X_REG_PULL_SEL) |
+			    BIT(PCA957X_REG_DIRECTION) |
+			    BIT(PCA957X_REG_OUTPUT) |
+			    BIT(PCA957X_REG_INT_MASK);
+		break;
+	case TYPE_XRA120X:
+		registers = BIT(XRA120X_REG_OUTPUT) |
+			    BIT(XRA120X_REG_INVERT) |
+			    BIT(XRA120X_REG_DIRECTION) |
+			    BIT(XRA120X_REG_OUTPUT) |
+			    BIT(XRA120X_REG_PULL_EN) |
+			    BIT(XRA120X_REG_INT_MASK) |
+			    BIT(XRA120X_REG_TRISTATE) |
+			    BIT(XRA120X_REG_RISING_MASK) |
+			    BIT(XRA120X_REG_FALLING_MASK) |
+			    BIT(XRA120X_REG_DEBOUNCE_EN);
+		break;
+	default:
+		registers = BIT(PCA953X_REG_OUTPUT) |
+			    BIT(PCA953X_REG_INVERT) |
+			    BIT(PCAL953X_REG_DIRECTION);
+		break;
 	}
 
-	if (pca953x_is_pcal_type(chip))
-		bank |= PCAL9xxx_BANK_IN_LATCH | PCAL9xxx_BANK_PULL_EN |
-			PCAL9xxx_BANK_PULL_SEL | PCAL9xxx_BANK_IRQ_MASK;
-
-	return chip->check_reg(chip, reg, bank);
+	return chip->check_reg(chip, reg_addr, registers);
 }
 
 static bool pca953x_volatile_register(struct device *dev, unsigned int reg)
 {
 	struct pca953x_chip *chip = dev_get_drvdata(dev);
-	u32 bank;
-
-	if (chip->type == TYPE_PCA957X)
-		bank = PCA957x_BANK_INPUT;
-	else
-		bank = PCA953x_BANK_INPUT;
+	u32 registers;
 
-	if (pca953x_is_pcal_type(chip))
-		bank |= PCAL9xxx_BANK_IRQ_STAT;
+	switch (chip->type) {
+	case TYPE_PCA950X:
+		registers = BIT(PCA950X_REG_INPUT);
+		break;
+	case TYPE_PCAL953X:
+		registers = BIT(PCAL953X_REG_INPUT) |
+			    BIT(PCAL953X_REG_INT_STATUS);
+		break;
+	case TYPE_PCAL652X:
+	case TYPE_PCAL653X:
+		registers = BIT(PCAL65XX_REG_INPUT) |
+			    BIT(PCAL65XX_REG_INT_STATUS) |
+			    BIT(PCAL65XX_REG_INPUT_STATUS);
+		break;
+	case TYPE_PCA957X:
+		registers = BIT(PCA957X_REG_INPUT) |
+			    BIT(PCA957X_REG_INT_STATUS);
+		break;
+	case TYPE_XRA120X:
+		registers = BIT(XRA120X_REG_INPUT) |
+			    BIT(XRA120X_REG_INT_STATUS);
+		break;
+	default:
+		registers = BIT(PCA953X_REG_INPUT);
+		break;
+	}
 
-	return chip->check_reg(chip, reg, bank);
+	return chip->check_reg(chip, reg, registers);
 }
 
 static const struct regmap_config pca953x_i2c_regmap = {
@@ -667,8 +889,8 @@ static int pca953x_gpio_set_pull_up_down(struct pca953x_chip *chip,
 {
 	enum pin_config_param param = pinconf_to_config_param(config);
 
-	u8 pull_en_reg = chip->recalc_addr(chip, PCAL953X_PULL_EN, offset);
-	u8 pull_sel_reg = chip->recalc_addr(chip, PCAL953X_PULL_SEL, offset);
+	u8 pull_en_reg = chip->recalc_addr(chip, chip->regs->pull_en, offset);
+	u8 pull_sel_reg = chip->recalc_addr(chip, chip->regs->pull_sel, offset);
 	u8 bit = BIT(offset % BANK_SZ);
 	int ret;
 
@@ -794,12 +1016,12 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
 
 	if (pca953x_is_pcal_type(chip)) {
 		/* Enable latch on interrupt-enabled inputs */
-		pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
+		pca953x_write_regs(chip, chip->regs->input_latch, chip->irq_mask);
 
 		bitmap_complement(irq_mask, chip->irq_mask, gc->ngpio);
 
 		/* Unmask enabled interrupts */
-		pca953x_write_regs(chip, PCAL953X_INT_MASK, irq_mask);
+		pca953x_write_regs(chip, chip->regs->int_mask, irq_mask);
 	}
 
 	/* Switch direction to input if needed */
@@ -876,7 +1098,7 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pendin
 
 	if (pca953x_is_pcal_type(chip)) {
 		/* Read the current interrupt status from the device */
-		ret = pca953x_read_regs(chip, PCAL953X_INT_STAT, trigger);
+		ret = pca953x_read_regs(chip, chip->regs->int_status, trigger);
 		if (ret)
 			return false;
 
@@ -1068,7 +1290,7 @@ static int device_pca957x_init(struct pca953x_chip *chip, u32 invert)
 	for (i = 0; i < NBANK(chip); i++)
 		bitmap_set_value8(val, 0x02, i * BANK_SZ);
 
-	ret = pca953x_write_regs(chip, PCA957X_BKEN, val);
+	ret = pca953x_write_regs(chip, chip->regs->pull_en, val);
 	if (ret)
 		goto out;
 
@@ -1158,6 +1380,30 @@ static int pca953x_probe(struct i2c_client *client)
 		regmap_config = &pca953x_i2c_regmap;
 	}
 
+	switch (chip->type) {
+	case TYPE_PCA950X:
+		chip->regs = &pca950x_regs;
+		break;
+	case TYPE_PCAL953X:
+		chip->regs = &pcal953x_regs;
+		break;
+	case TYPE_PCAL652X:
+		chip->regs = &pcal65xx_regs;
+		break;
+	case TYPE_PCAL653X:
+		chip->regs = &pcal65xx_regs;
+		break;
+	case TYPE_PCA957X:
+		chip->regs = &pca957x_regs;
+		break;
+	case TYPE_XRA120X:
+		chip->regs = &xra120x_regs;
+		break;
+	default:
+		chip->regs = &pca953x_regs;
+		break;
+	}
+
 	if (chip->type == TYPE_PCAL653X) {
 		chip->recalc_addr = pcal6534_recalc_addr;
 		chip->check_reg = pcal6534_check_register;
@@ -1198,10 +1444,8 @@ static int pca953x_probe(struct i2c_client *client)
 	 * we can't share this chip with another i2c master.
 	 */
 	if (chip->type == TYPE_PCA957X) {
-		chip->regs = &pca957x_regs;
 		ret = device_pca957x_init(chip, invert);
 	} else {
-		chip->regs = &pca953x_regs;
 		ret = device_pca95xx_init(chip, invert);
 	}
 	if (ret)
@@ -1269,7 +1513,7 @@ static int pca953x_regcache_sync(struct device *dev)
 
 #ifdef CONFIG_GPIO_PCA953X_IRQ
 	if (pca953x_is_pcal_type(chip)) {
-		regaddr = chip->recalc_addr(chip, PCAL953X_IN_LATCH, 0);
+		regaddr = chip->recalc_addr(chip, chip->regs->input_latch, 0);
 		ret = regcache_sync_region(chip->regmap, regaddr,
 					   regaddr + NBANK(chip) - 1);
 		if (ret) {
@@ -1278,7 +1522,7 @@ static int pca953x_regcache_sync(struct device *dev)
 			return ret;
 		}
 
-		regaddr = chip->recalc_addr(chip, PCAL953X_INT_MASK, 0);
+		regaddr = chip->recalc_addr(chip, chip->regs->int_mask, 0);
 		ret = regcache_sync_region(chip->regmap, regaddr,
 					   regaddr + NBANK(chip) - 1);
 		if (ret) {
-- 
2.38.1

[RFC PATCH 3/3] gpio: pca953x: Redesign register address calculation

[Levente Révész]

The driver supports 8 chip types, 6 of which have extended registers
with various functions, e.g. pull-up and pull-down bias for the gpio
lines or interrupt masking. To allow supporting these functions, the
driver has to be able to calculate the addresses of these registers.

Replace the address calculation scheme with new reg_addr() and check_reg()
functions, one for each register layout. The new functions can work with
all of the extended registers. The functions have been tested with the
register layout of each compatible chip.

Signed-off-by: Levente Révész <levente.revesz@eilabs.com>
---
 drivers/gpio/gpio-pca953x.c | 551 ++++++++++++++++++++++++++++--------
 1 file changed, 434 insertions(+), 117 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 3d4c3efeaf35..34326abf8f63 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -26,26 +26,8 @@
 #include <asm/unaligned.h>
 
 #define REG_ADDR_MASK		GENMASK(5, 0)
-#define REG_ADDR_EXT		BIT(6)
 #define REG_ADDR_AI		BIT(7)
 
-#define PCAL953X_OUT_STRENGTH	0x20
-#define PCAL953X_IN_LATCH	0x22
-#define PCAL953X_PULL_EN	0x23
-#define PCAL953X_PULL_SEL	0x24
-#define PCAL953X_INT_MASK	0x25
-#define PCAL953X_INT_STAT	0x26
-#define PCAL953X_OUT_CONF	0x27
-
-#define PCAL6524_INT_EDGE	0x28
-#define PCAL6524_INT_CLR	0x2a
-#define PCAL6524_IN_STATUS	0x2b
-#define PCAL6524_OUT_INDCONF	0x2c
-#define PCAL6524_DEBOUNCE	0x2d
-
-#define PCAL_GPIO_MASK		GENMASK(4, 0)
-#define PCAL_PINCTRL_MASK	GENMASK(6, 5)
-
 /*
  * driver_data
  *
@@ -348,9 +330,10 @@ struct pca953x_chip {
 	enum pca953x_chip_type type;
 	const struct pca953x_reg_config *regs;
 
-	u8 (*recalc_addr)(struct pca953x_chip *chip, int reg, int off);
-	bool (*check_reg)(struct pca953x_chip *chip, unsigned int reg,
-			  u32 checkbank);
+	u8 (*reg_addr)(struct pca953x_chip *chip, unsigned int reg,
+		       unsigned int offset);
+	bool (*check_reg)(struct pca953x_chip *chip, u8 reg_addr,
+			  u32 registers);
 };
 
 static bool pca953x_has_int(struct pca953x_chip *chip)
@@ -398,25 +381,33 @@ static int pca953x_bank_shift(struct pca953x_chip *chip)
  *   categories above.
  */
 
-static bool pca953x_check_register(struct pca953x_chip *chip, unsigned int reg,
-				   u32 checkbank)
+/*
+ * Register types:
+ *   - Single register: 1 byte for each GPIO port.
+ *   - Double register: 2 bytes for each GPIO port.
+ *   - One-byte register: Only a single byte.
+ */
+
+/* Check if register address belongs to any of the registers in a set. */
+static bool pca953x_check_reg(struct pca953x_chip *chip, u8 reg_addr,
+			      u32 registers)
 {
 	int bank_shift = pca953x_bank_shift(chip);
-	int bank = (reg & REG_ADDR_MASK) >> bank_shift;
-	int offset = reg & (BIT(bank_shift) - 1);
+	int reg;
+	int offset;
 
-	/* Special PCAL extended register check. */
-	if (reg & REG_ADDR_EXT) {
-		if (!pca953x_is_pcal_type(chip))
-			return false;
-		bank += 8;
-	}
+	reg_addr &= REG_ADDR_MASK;
+
+	reg = reg_addr >> bank_shift;
+
+	/* Which byte this is in a register bank. */
+	offset = reg_addr & (BIT(bank_shift) - 1);
 
-	/* Register is not in the matching bank. */
-	if (!(BIT(bank) & checkbank))
+	/* Address does not belong to any of the registers. */
+	if (!(BIT(reg) & registers))
 		return false;
 
-	/* Register is not within allowed range of bank. */
+	/* Offset is not within allowed range of the register. */
 	if (offset >= NBANK(chip))
 		return false;
 
@@ -424,46 +415,287 @@ static bool pca953x_check_register(struct pca953x_chip *chip, unsigned int reg,
 }
 
 /*
- * Unfortunately, whilst the PCAL6534 chip (and compatibles) broadly follow the
- * same register layout as the PCAL6524, the spacing of the registers has been
- * fundamentally altered by compacting them and thus does not obey the same
- * rules, including being able to use bit shifting to determine bank. These
- * chips hence need special handling here.
+ * Convert register address to register enum value.
+ * Returns -1 if reg_addr is invalid.
  */
-static bool pcal6534_check_register(struct pca953x_chip *chip, unsigned int reg,
-				    u32 checkbank)
+static int pcal953x_reg_addr_to_reg(int bank_shift, u8 reg_addr)
 {
-	int bank_shift;
-	int bank;
+	int reg;
+
+	if (reg_addr < (4U << bank_shift)) {
+		/* Basic registers. */
+		reg = reg_addr >> bank_shift;
+	} else if (reg_addr < 0x40) {
+		reg = -1;
+	} else if (reg_addr < 0x4F) {
+		/* Extended registers */
+		reg = ((reg_addr - 0x40) >> bank_shift) +
+			PCAL953X_REG_DRIVE_STRENGTH;
+
+		/* Compensate skipped reg numbers due to double registers. */
+		if (reg > PCAL953X_REG_DRIVE_STRENGTH)
+			reg -= 1;
+
+	} else if (reg_addr == 0x4F) {
+		/* Drive Mode register. Always 1 byte. */
+		reg = PCAL953X_REG_DRIVE_MODE;
+	} else {
+		reg = -1;
+	}
+
+	return reg;
+}
+
+static int pcal953x_calculate_reg_offset(int bank_offset, u8 reg_addr,
+					 enum pcal953x_reg reg)
+{
+	/*
+	 * Single registers and one-byte registers always start on a bank
+	 * border, the offset is the distance from reg_addr to the bank border.
+	 *
+	 * Double registers span over two banks, so the offset can be larger
+	 * than a bank. The offset is the distance from reg_addr to either an
+	 * odd or an even bank border, depending on the base address of the
+	 * register.
+	 */
+
+	u8 ext_addr;
+
+	if (reg <= PCAL953X_REG_DIRECTION)
+		return reg_addr % bank_offset;
+
+	/* Count banks from the base address of the extended regs. */
+	ext_addr = reg_addr - 0x40;
+
+	/* Double register in bank (N,N+1), N is even. */
+	if (reg == PCAL953X_REG_DRIVE_STRENGTH)
+		return ext_addr % (bank_offset * 2);
+	/* Drive Mode has no offsets. */
+	else if (reg == PCAL953X_REG_DRIVE_MODE)
+		return 0;
+	else
+		return ext_addr % bank_offset;
+}
+
+/* Check if offset is valid for register reg. */
+static bool pcal953x_check_reg_offset(int bank_size, enum pcal953x_reg reg,
+				      unsigned int offset)
+{
+	int maxoffset = bank_size - 1;
+
+	/* Double register */
+	if (reg == PCAL953X_REG_DRIVE_STRENGTH)
+		maxoffset = 2 * bank_size - 1;
+
+	/* One byte register */
+	if (reg == PCAL953X_REG_DRIVE_MODE)
+		maxoffset = 0;
+
+	/* Offset is not within allowed range of the register. */
+	if (offset > maxoffset)
+		return false;
+
+	return true;
+}
+
+/* Check if register address belongs to any of the registers in a set. */
+static bool pcal953x_check_reg(struct pca953x_chip *chip, u8 reg_addr,
+			       u32 registers)
+{
+	int bank_shift = pca953x_bank_shift(chip);
+	int bank_offset = BIT(bank_shift);
+	int reg;
 	int offset;
 
-	if (reg >= 0x54) {
-		/*
-		 * Handle lack of reserved registers after output port
-		 * configuration register to form a bank.
-		 */
-		reg -= 0x54;
-		bank_shift = 16;
-	} else if (reg >= 0x30) {
-		/*
-		 * Reserved block between 14h and 2Fh does not align on
-		 * expected bank boundaries like other devices.
-		 */
-		reg -= 0x30;
-		bank_shift = 8;
+	reg_addr &= REG_ADDR_MASK;
+
+	reg = pcal953x_reg_addr_to_reg(bank_shift, reg_addr);
+
+	/* Address does not belong to any of the registers. */
+	if (!(BIT(reg) & registers))
+		return false;
+
+	offset = pcal953x_calculate_reg_offset(bank_offset, reg_addr, reg);
+
+	if (!pcal953x_check_reg_offset(NBANK(chip), reg, offset))
+		return false;
+
+	return true;
+}
+
+/*
+ * Convert register address to register enum value.
+ * Returns -1 if reg_addr is invalid.
+ */
+static int pcal65xx_reg_addr_to_reg(int bank_offset, u8 ext_base_addr,
+				    u8 reg_addr)
+{
+	int reg;
+
+	if (reg_addr < 4 * bank_offset) {
+		/* Basic registers. */
+		reg = reg_addr / bank_offset;
+	} else if (reg_addr < ext_base_addr) {
+		reg = -1;
+	} else if (reg_addr < ext_base_addr + 15 * bank_offset) {
+		/* Extended registers */
+		reg = ((reg_addr - ext_base_addr) / bank_offset) +
+			PCAL65XX_REG_DRIVE_STRENGTH;
+
+		/* Compensate for skipped reg numbers after double registers. */
+		if (reg > PCAL65XX_REG_DRIVE_STRENGTH)
+			reg -= 1;
+
+		if (reg > PCAL65XX_REG_INT_EDGE)
+			reg -= 1;
 	} else {
-		bank_shift = 0;
+		reg = -1;
 	}
 
-	bank = bank_shift + reg / NBANK(chip);
-	offset = reg % NBANK(chip);
+	return reg;
+}
+
+static int pcal65xx_calculate_reg_offset(int bank_offset, u8 ext_base_addr,
+					 u8 reg_addr, enum pcal65xx_reg reg)
+{
+	/*
+	 * Single registers and one-byte registers always start on a bank
+	 * border, the offset is the distance from reg_addr to the bank border.
+	 *
+	 * Double registers span over two banks, so the offset can be larger
+	 * than a bank. The offset is the distance from reg_addr to either an
+	 * odd or an even bank border, depending on the base address of the
+	 * register.
+	 */
 
-	/* Register is not in the matching bank. */
-	if (!(BIT(bank) & checkbank))
+	u8 ext_addr;
+
+	if (reg <= PCAL65XX_REG_DIRECTION)
+		return reg_addr % bank_offset;
+
+	/* Count banks in relation to the base address of the extended regs. */
+	ext_addr = reg_addr - ext_base_addr;
+
+	/* Double register in bank (N,N+1), N is even. */
+	if (reg == PCAL65XX_REG_DRIVE_STRENGTH || reg == PCAL65XX_REG_INT_EDGE)
+		return ext_addr % (bank_offset * 2);
+	else
+		return ext_addr % bank_offset;
+}
+
+/* Check if offset is valid for register reg. */
+static bool pcal65xx_check_reg_offset(int bank_size, enum pcal65xx_reg reg,
+				      unsigned int offset)
+{
+	int maxoffset = bank_size - 1;
+	int num_debounce_ports = min(bank_size, PCAL65XX_MAX_NUM_DEBOUNCE_PORTS);
+
+	/* Double registers */
+	if (reg == PCAL65XX_REG_DRIVE_STRENGTH || reg == PCAL65XX_REG_INT_EDGE)
+		maxoffset = 2 * bank_size - 1;
+
+	/* One byte registers */
+	if (reg == PCAL65XX_REG_DRIVE_MODE || reg == PCAL65XX_REG_DEBOUNCE_COUNT)
+		maxoffset = 0;
+
+	if (reg == PCAL65XX_REG_DEBOUNCE_EN)
+		maxoffset = num_debounce_ports;
+
+	/* Offset is not within allowed range of the register. */
+	if (offset > maxoffset)
 		return false;
 
-	/* Register is not within allowed range of bank. */
-	if (offset >= NBANK(chip))
+	return true;
+}
+
+/* Check if register address belongs to any of the registers in a set. */
+static bool pcal652x_check_reg(struct pca953x_chip *chip, u8 reg_addr,
+			       u32 registers)
+{
+	int bank_shift = pca953x_bank_shift(chip);
+	int bank_offset = BIT(bank_shift);
+	int ext_base_addr = 0x40;
+	int num_debounce_ports = min(NBANK(chip), PCAL65XX_MAX_NUM_DEBOUNCE_PORTS);
+	int max_addr;
+	int reg;
+	int offset;
+
+	reg_addr &= REG_ADDR_MASK;
+
+	max_addr = ext_base_addr + (13U << bank_shift) + num_debounce_ports;
+	if (reg_addr > max_addr)
+		return false;
+
+	/*
+	 * The debounce count register is not aligned to bank offset.
+	 * Add enough offset to reg_address to form a bank.
+	 */
+	if (reg_addr == ext_base_addr + (13U << bank_shift) + num_debounce_ports)
+		reg_addr += bank_offset - num_debounce_ports;
+
+
+	reg = pcal65xx_reg_addr_to_reg(bank_offset, ext_base_addr, reg_addr);
+	if (reg < 0)
+		return false;
+
+	/* Address does not belong to any of the registers. */
+	if (!(BIT(reg) & registers))
+		return false;
+
+	offset = pcal65xx_calculate_reg_offset(bank_offset, ext_base_addr,
+					       reg_addr, reg);
+
+	if (!pcal65xx_check_reg_offset(NBANK(chip), reg, offset))
+		return false;
+
+	return true;
+}
+
+/* Check if register address belongs to any of the registers in a set. */
+static bool pcal653x_check_reg(struct pca953x_chip *chip, u8 reg_addr,
+			       u32 registers)
+{
+	/* PCAL653X has no padding (reserved registers) between banks. */
+
+	int bank_offset = NBANK(chip);
+	int ext_base_addr = 0x30;
+	int num_debounce_ports = min(NBANK(chip), PCAL65XX_MAX_NUM_DEBOUNCE_PORTS);
+	int max_addr;
+	int reg;
+	int offset;
+
+	reg_addr &= REG_ADDR_MASK;
+
+	max_addr = ext_base_addr + 13 * bank_offset + num_debounce_ports;
+	if (reg_addr > max_addr)
+		return false;
+
+	/*
+	 * The drive mode register is only 1 byte long with no padding after it.
+	 * Add enough offset to reg_addr to form a bank.
+	 */
+	if (reg_addr > ext_base_addr + 7 * bank_offset)
+		reg_addr += bank_offset - 1;
+
+	/*
+	 * The debounce count register is not aligned to bank offset. Add enough
+	 * offset to reg_address to form a bank.
+	 */
+	if (reg_addr == ext_base_addr + 13 * bank_offset + num_debounce_ports)
+		reg_addr += bank_offset - num_debounce_ports;
+
+	reg = pcal65xx_reg_addr_to_reg(bank_offset, 0x30, reg_addr);
+	if (reg < 0)
+		return false;
+
+	/* Address does not belong to any of the registers. */
+	if (!(BIT(reg) & registers))
+		return false;
+
+	offset = pcal65xx_calculate_reg_offset(bank_offset, ext_base_addr,
+					       reg_addr, reg);
+
+	if (!pcal65xx_check_reg_offset(NBANK(chip), reg, offset))
 		return false;
 
 	return true;
@@ -686,51 +918,125 @@ static const struct regmap_config pca953x_ai_i2c_regmap = {
 	.max_register = 0x7f,
 };
 
-static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off)
+/* Get the i2c address of register reg for line at offset. */
+static u8 pca953x_reg_addr(struct pca953x_chip *chip, unsigned int reg,
+			   unsigned int offset)
 {
 	int bank_shift = pca953x_bank_shift(chip);
-	int addr = (reg & PCAL_GPIO_MASK) << bank_shift;
-	int pinctrl = (reg & PCAL_PINCTRL_MASK) << 1;
-	u8 regaddr = pinctrl | addr | (off / BANK_SZ);
+	u8 reg_addr = (reg << bank_shift) + offset / BANK_SZ;
 
-	return regaddr;
+	return reg_addr;
+}
+
+/* Get the i2c address of register reg for line at offset. */
+static u8 pcal953x_reg_addr(struct pca953x_chip *chip, unsigned int reg,
+			    unsigned int offset)
+{
+	int bank_shift = pca953x_bank_shift(chip);
+	u8 reg_addr;
+
+	if (reg == PCAL953X_REG_DRIVE_MODE) {
+		/* One byte register on a fixed address. */
+		return 0x4F;
+	}
+
+	if (reg < PCAL953X_REG_DRIVE_STRENGTH)
+		reg_addr = reg << bank_shift;
+	else
+		reg_addr = 0x40 + ((reg - 4) << bank_shift);
+
+	/* Compensate for double register. */
+	if (reg > PCAL953X_REG_DRIVE_STRENGTH)
+		reg_addr += BIT(bank_shift);
+
+	if (reg == PCAL953X_REG_DRIVE_STRENGTH)
+		/*
+		 * Double register. Return the specific byte for given offset.
+		 * e.g. offset=0 -> byte 0, offset=4 -> byte 1.
+		 */
+		reg_addr += 2 * offset / BANK_SZ;
+	else
+		reg_addr += offset / BANK_SZ;
+
+	return reg_addr;
+}
+
+static u8 pcal65xx_reg_addr_common(int bank_offset,
+				   unsigned int ext_base_addr,
+				   enum pcal65xx_reg reg,
+				   int offset)
+{
+	u8 reg_addr;
+
+	if (reg < PCAL65XX_REG_DRIVE_STRENGTH)
+		reg_addr = reg * bank_offset;
+	else
+		reg_addr = ext_base_addr + (reg - 4) * bank_offset;
+
+	/* Compensate for double register. */
+	if (reg > PCAL65XX_REG_DRIVE_STRENGTH)
+		reg_addr += bank_offset;
+
+	if (reg > PCAL65XX_REG_INT_EDGE)
+		reg_addr += bank_offset;
+
+	if (reg == PCAL65XX_REG_DRIVE_STRENGTH || reg == PCAL65XX_REG_INT_EDGE)
+		/*
+		 * Double register. Return the specific byte for given offset.
+		 * e.g. offset=0 -> byte 0, offset=4 -> byte 1.
+		 */
+		reg_addr += 2 * offset / BANK_SZ;
+	else if (reg == PCAL65XX_REG_DRIVE_MODE || reg == PCAL65XX_REG_DEBOUNCE_COUNT)
+		/* One byte register. Do not add offset. */
+		;
+	else
+		reg_addr += offset / BANK_SZ;
+
+	return reg_addr;
+}
+
+/* Get the i2c address of register reg for line at offset. */
+static u8 pcal652x_reg_addr(struct pca953x_chip *chip, unsigned int reg,
+			    unsigned int offset)
+{
+	int bank_offset = BIT(pca953x_bank_shift(chip));
+	u8 reg_addr = pcal65xx_reg_addr_common(bank_offset, 0x40, reg, offset);
+	int num_debounce_ports = min(NBANK(chip), PCAL65XX_MAX_NUM_DEBOUNCE_PORTS);
+
+	/* The reserved registers before the Debounce Count register are missing. */
+	if (reg == PCAL65XX_REG_DEBOUNCE_COUNT)
+		reg_addr -= bank_offset - num_debounce_ports;
+
+	return reg_addr;
 }
 
 /*
- * The PCAL6534 and compatible chips have altered bank alignment that doesn't
- * fit within the bit shifting scheme used for other devices.
+ * Get the i2c address of register reg for line at offset.
+ *
+ * PCAL653X does not have padding between the register banks, unlike other chip
+ * types.
  */
-static u8 pcal6534_recalc_addr(struct pca953x_chip *chip, int reg, int off)
+static u8 pcal653x_reg_addr(struct pca953x_chip *chip, unsigned int reg,
+			    unsigned int offset)
 {
-	int addr;
-	int pinctrl;
-
-	addr = (reg & PCAL_GPIO_MASK) * NBANK(chip);
-
-	switch (reg) {
-	case PCAL953X_OUT_STRENGTH:
-	case PCAL953X_IN_LATCH:
-	case PCAL953X_PULL_EN:
-	case PCAL953X_PULL_SEL:
-	case PCAL953X_INT_MASK:
-	case PCAL953X_INT_STAT:
-		pinctrl = ((reg & PCAL_PINCTRL_MASK) >> 1) + 0x20;
-		break;
-	case PCAL6524_INT_EDGE:
-	case PCAL6524_INT_CLR:
-	case PCAL6524_IN_STATUS:
-	case PCAL6524_OUT_INDCONF:
-	case PCAL6524_DEBOUNCE:
-		pinctrl = ((reg & PCAL_PINCTRL_MASK) >> 1) + 0x1c;
-		break;
-	}
+	int bank_offset = NBANK(chip);
+	u8 reg_addr = pcal65xx_reg_addr_common(bank_offset, 0x30, reg, offset);
+	int num_debounce_ports = min(NBANK(chip), PCAL65XX_MAX_NUM_DEBOUNCE_PORTS);
+
+	/* Drive Mode is only one byte, not a full bank. */
+	if (reg > PCAL65XX_REG_DRIVE_MODE)
+		reg_addr -= (bank_offset - 1);
+
+	/* Debounce Count is only num_debounce_ports bytes long, not a full bank. */
+	if (reg == PCAL65XX_REG_DEBOUNCE_COUNT)
+		reg_addr -= bank_offset - num_debounce_ports;
 
-	return pinctrl + addr + (off / BANK_SZ);
+	return reg_addr;
 }
 
 static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long *val)
 {
-	u8 regaddr = chip->recalc_addr(chip, reg, 0);
+	u8 regaddr = chip->reg_addr(chip, reg, 0);
 	u8 value[MAX_BANK];
 	int i, ret;
 
@@ -748,7 +1054,7 @@ static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long
 
 static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long *val)
 {
-	u8 regaddr = chip->recalc_addr(chip, reg, 0);
+	u8 regaddr = chip->reg_addr(chip, reg, 0);
 	u8 value[MAX_BANK];
 	int i, ret;
 
@@ -767,7 +1073,7 @@ static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long *
 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 dirreg = chip->reg_addr(chip, chip->regs->direction, off);
 	u8 bit = BIT(off % BANK_SZ);
 	int ret;
 
@@ -781,8 +1087,8 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 		unsigned off, int val)
 {
 	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 dirreg = chip->reg_addr(chip, chip->regs->direction, off);
+	u8 outreg = chip->reg_addr(chip, chip->regs->output, off);
 	u8 bit = BIT(off % BANK_SZ);
 	int ret;
 
@@ -802,7 +1108,7 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc,
 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 inreg = chip->reg_addr(chip, chip->regs->input, off);
 	u8 bit = BIT(off % BANK_SZ);
 	u32 reg_val;
 	int ret;
@@ -819,7 +1125,7 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off)
 static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val)
 {
 	struct pca953x_chip *chip = gpiochip_get_data(gc);
-	u8 outreg = chip->recalc_addr(chip, chip->regs->output, off);
+	u8 outreg = chip->reg_addr(chip, chip->regs->output, off);
 	u8 bit = BIT(off % BANK_SZ);
 
 	mutex_lock(&chip->i2c_lock);
@@ -830,7 +1136,7 @@ static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val)
 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 dirreg = chip->reg_addr(chip, chip->regs->direction, off);
 	u8 bit = BIT(off % BANK_SZ);
 	u32 reg_val;
 	int ret;
@@ -889,8 +1195,8 @@ static int pca953x_gpio_set_pull_up_down(struct pca953x_chip *chip,
 {
 	enum pin_config_param param = pinconf_to_config_param(config);
 
-	u8 pull_en_reg = chip->recalc_addr(chip, chip->regs->pull_en, offset);
-	u8 pull_sel_reg = chip->recalc_addr(chip, chip->regs->pull_sel, offset);
+	u8 pull_en_reg = chip->reg_addr(chip, chip->regs->pull_en, offset);
+	u8 pull_sel_reg = chip->reg_addr(chip, chip->regs->pull_sel, offset);
 	u8 bit = BIT(offset % BANK_SZ);
 	int ret;
 
@@ -1253,13 +1559,13 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert)
 	u8 regaddr;
 	int ret;
 
-	regaddr = chip->recalc_addr(chip, chip->regs->output, 0);
+	regaddr = chip->reg_addr(chip, chip->regs->output, 0);
 	ret = regcache_sync_region(chip->regmap, regaddr,
 				   regaddr + NBANK(chip) - 1);
 	if (ret)
 		goto out;
 
-	regaddr = chip->recalc_addr(chip, chip->regs->direction, 0);
+	regaddr = chip->reg_addr(chip, chip->regs->direction, 0);
 	ret = regcache_sync_region(chip->regmap, regaddr,
 				   regaddr + NBANK(chip) - 1);
 	if (ret)
@@ -1404,12 +1710,23 @@ static int pca953x_probe(struct i2c_client *client)
 		break;
 	}
 
-	if (chip->type == TYPE_PCAL653X) {
-		chip->recalc_addr = pcal6534_recalc_addr;
-		chip->check_reg = pcal6534_check_register;
-	} else {
-		chip->recalc_addr = pca953x_recalc_addr;
-		chip->check_reg = pca953x_check_register;
+	switch (chip->type) {
+	case TYPE_PCAL953X:
+		chip->reg_addr = pcal953x_reg_addr;
+		chip->check_reg = pcal953x_check_reg;
+		break;
+	case TYPE_PCAL652X:
+		chip->reg_addr = pcal652x_reg_addr;
+		chip->check_reg = pcal652x_check_reg;
+		break;
+	case TYPE_PCAL653X:
+		chip->reg_addr = pcal653x_reg_addr;
+		chip->check_reg = pcal653x_check_reg;
+		break;
+	default:
+		chip->reg_addr = pca953x_reg_addr;
+		chip->check_reg = pca953x_check_reg;
+		break;
 	}
 
 	chip->regmap = devm_regmap_init_i2c(client, regmap_config);
@@ -1497,14 +1814,14 @@ static int pca953x_regcache_sync(struct device *dev)
 	 * The ordering between direction and output is important,
 	 * sync these registers first and only then sync the rest.
 	 */
-	regaddr = chip->recalc_addr(chip, chip->regs->direction, 0);
+	regaddr = chip->reg_addr(chip, chip->regs->direction, 0);
 	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 = chip->recalc_addr(chip, chip->regs->output, 0);
+	regaddr = chip->reg_addr(chip, chip->regs->output, 0);
 	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);
@@ -1513,7 +1830,7 @@ static int pca953x_regcache_sync(struct device *dev)
 
 #ifdef CONFIG_GPIO_PCA953X_IRQ
 	if (pca953x_is_pcal_type(chip)) {
-		regaddr = chip->recalc_addr(chip, chip->regs->input_latch, 0);
+		regaddr = chip->reg_addr(chip, chip->regs->input_latch, 0);
 		ret = regcache_sync_region(chip->regmap, regaddr,
 					   regaddr + NBANK(chip) - 1);
 		if (ret) {
@@ -1522,7 +1839,7 @@ static int pca953x_regcache_sync(struct device *dev)
 			return ret;
 		}
 
-		regaddr = chip->recalc_addr(chip, chip->regs->int_mask, 0);
+		regaddr = chip->reg_addr(chip, chip->regs->int_mask, 0);
 		ret = regcache_sync_region(chip->regmap, regaddr,
 					   regaddr + NBANK(chip) - 1);
 		if (ret) {
-- 
2.38.1

Re: [RFC PATCH 1/3] gpio: pca953x: Replace chip type flags with a type enum

[Andy Shevchenko]

On Mon, Jan 30, 2023 at 09:59:42PM +0100, Levente Révész wrote:
> The driver supports 8 chip types. The chip type is encoded in
> driver_data so that different chip types can be handled appropriately.
> 
> Encoding the type information in separate bit flags was not too
> straightforward, and it didn't make it possible to encode 8 chip
> types.
> 
> Replace bit flags with a pca953x_chip_type enum. Encode this enum in
> driver_data as a number. Add missing chip types based on chip functions
> and register layout.

...

> +/*
> + * driver_data
> + *
> + *   31    24 23    16 15     8 7      0
> + *   xxxxxxxx xxxxxxxx xxxx____ ________
> + *                         ^^^^ ^^^^^^^^
> + *                         |    number of gpio lines
> + *                         chip type
> + */

Not sure we need this as it's self explanatory from the below masks.
Moreover, driver_data is 64-bit or 32-bit depending on the build.

> +#define PCA_GPIO_MASK		GENMASK(7, 0)
> +#define PCA_TYPE_MASK		GENMASK(11, 8)

...

> +enum pca953x_chip_type {

> +	TYPE_PCA953X_NOINT,
> +	TYPE_PCA953X,

Hmm... Since you change this anyway, I think I would distinguish them better.
Perhaps

	TYPE_PCA953X_INT,
	TYPE_PCA953X_NOINT,

?

In any case, please add a kernel doc to this enum to explain each type briefly.

> +	TYPE_PCA950X,

Can we make sorted? (I.e. move this before PCA953X)

> +	TYPE_PCAL953X,
> +	TYPE_PCAL652X,
> +	TYPE_PCAL653X,
> +	TYPE_PCA957X,

Can we make sorted? (I.e. move this after PCA953X)

> +	TYPE_XRA120X,

	TYPE_MAX

> +};

static_assert((PCA_TYPE_MASK + 1) >= (TYPE_MAX << 8));

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 1/3] gpio: pca953x: Replace chip type flags with a type enum

[Andy Shevchenko]

On Mon, Jan 30, 2023 at 11:57:11PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 30, 2023 at 09:59:42PM +0100, Levente Révész wrote:

...

> static_assert((PCA_TYPE_MASK + 1) >= (TYPE_MAX << 8));

Actually better

static_assert(PCA_TYPE_MASK >= ((TYPE_MAX - 1) << 8));

to prevent potential overflow if we move this field to the end of the type.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 2/3] gpio: pca953x: Describe register maps with enums

[Andy Shevchenko]

On Mon, Jan 30, 2023 at 09:59:48PM +0100, Levente Révész wrote:
> The driver supports 8 chip types, 6 of which have extended registers
> with various functions, e.g. pull-up and pull-down bias for the gpio
> lines or interrupt masking. To allow supporting these functions, the
> driver has to be able to calculate the addresses of these registers.
> 
> Replace the register maps with an enum for each chip type. These do not
> contain the same numeric values as the old defines, but the new address
> calculating functions (in the next patch) use them appropriately.
> 
> Add currently used registers to struct pca953x_reg_config.
> 
> Create a reg_config struct for each chip type.

...

> +enum xra120x_reg {
> +};

>  static const struct pca953x_reg_config pca953x_regs = {
> +};

Make those enums and reg_config definitions to be sorted by their
respective names.

...

> +	case TYPE_PCA950X:
> +		registers = BIT(PCA950X_REG_INPUT) |
> +			    BIT(PCA950X_REG_OUTPUT) |
> +			    BIT(PCA950X_REG_INVERT) |
> +			    BIT(PCA950X_REG_DIRECTION) |
> +			    BIT(PCA950X_REG_INT_MASK);
> +		break;

Can't it be simplified if you define something like REG_MAX in each of
the enums and use here simply GENMASK(MAX, 0); ?

...

> +	case TYPE_PCA950X:
> +		registers = BIT(PCA950X_REG_OUTPUT) |
> +			    BIT(PCA950X_REG_INVERT) |
> +			    BIT(PCA950X_REG_DIRECTION) |
> +			    BIT(PCA950X_REG_INT_MASK);

Something similar, maybe with a definition of the volatile registers?

> +		break;

...

>  	if (chip->type == TYPE_PCA957X) {
> -		chip->regs = &pca957x_regs;
>  		ret = device_pca957x_init(chip, invert);
>  	} else {
> -		chip->regs = &pca953x_regs;
>  		ret = device_pca95xx_init(chip, invert);
>  	}

After this the {} may be dropped as well.

-- 
With Best Regards,
Andy Shevchenko

Re: [RFC PATCH 3/3] gpio: pca953x: Redesign register address calculation

[Andy Shevchenko]

On Mon, Jan 30, 2023 at 09:59:55PM +0100, Levente Révész wrote:
> The driver supports 8 chip types, 6 of which have extended registers
> with various functions, e.g. pull-up and pull-down bias for the gpio
> lines or interrupt masking. To allow supporting these functions, the
> driver has to be able to calculate the addresses of these registers.
> 
> Replace the address calculation scheme with new reg_addr() and check_reg()
> functions, one for each register layout. The new functions can work with
> all of the extended registers. The functions have been tested with the
> register layout of each compatible chip.

...

> +/*
> + * Register types:
> + *   - Single register: 1 byte for each GPIO port.
> + *   - Double register: 2 bytes for each GPIO port.
> + *   - One-byte register: Only a single byte.

We use term "bank".

> + */

...

> + * Returns -1 if reg_addr is invalid.

Use proper error code.

...

> + * Convert register address to register enum value.
> + * Returns -1 if reg_addr is invalid.

Ditto.

-- 
With Best Regards,
Andy Shevchenko