linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Levente Révész" <levente.revesz@eilabs.com>
To: Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <brgl@bgdev.pl>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Martyn Welch <martyn.welch@collabora.com>,
	Haibo Chen <haibo.chen@nxp.com>, Puyou Lu <puyou.lu@gmail.com>,
	Justin Chen <justinpopo6@gmail.com>,
	Andrey Gusakov <andrey.gusakov@cogentembedded.com>,
	Nate Drude <nate.d@variscite.com>
Cc: linux-gpio@vger.kernel.org
Subject: [PATCH v2 4/6] gpio: pca953x: Generalize interrupt mask register handling
Date: Wed, 26 Oct 2022 13:23:58 +0200	[thread overview]
Message-ID: <edd3f359-effb-0951-f75f-b03ebb8b7b29@eilabs.com> (raw)
In-Reply-To: <cc987520-d95b-01b9-5b65-53442ce122f6@eilabs.com>

This change is necessary for a following patch, which introduces an
interrupt mask register different from what is already in the driver.

The driver already handles an interrupt mask register for pcal
chips, PCAL953x_INT_MASK. Extend this implementation to support
interrupt mask registers at other addresses.

Add bit flag PCA_HAS_INT_MASK, which is set for each chip with an
interrupt mask register (including pcal chips).

Define a convenience bitmask PCA_MASKED_INT.

Add an int_mask member to struct pca953x_reg_config. This way interrupt
mask handling code can work with registers at different addresses.

Add separate pca953x_reg_config for pcal953x chips. This differs from
the pca953x_regs in the new int_mask field.

Signed-off-by: Levente Révész <levente.revesz@eilabs.com>
---
Changes in v2

    1. New function pca953x_has_int_mask_reg().
    2. In pca953x_probe() replaced if-else with a switch.
	3. Fix recalc_addr inconsistency pointed out by Martyn Welch

Question:

    This patch uses the PCA_HAS_INT_MASK bit to encode if the chip has
	interrupt register mask. An alternatice approach would be to create
	a new chip type for PCA953X_TYPE chips with the mask register.
	What do you think?

 drivers/gpio/gpio-pca953x.c | 84 ++++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 28 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 1725c1000445..2cf9541057a8 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -62,6 +62,8 @@
 #define PCAL_PINCTRL_MASK	GENMASK(6, 5)
 
 #define PCA_INT			BIT(8)
+#define PCA_HAS_INT_MASK	BIT(9)
+#define PCA_MASKED_INT		(PCA_INT | PCA_HAS_INT_MASK)
 #define PCA953X_TYPE		(0x00 << 12)
 #define PCAL953X_TYPE		(0x01 << 12)
 #define PCAL653X_TYPE		(0x02 << 12)
@@ -89,13 +91,13 @@ static const struct i2c_device_id pca953x_id[] = {
 	{ "pca9575", 16 | PCA957X_TYPE | PCA_INT, },
 	{ "pca9698", 40 | PCA953X_TYPE, },
 
-	{ "pcal6408", 8 | PCAL953X_TYPE | PCA_INT, },
-	{ "pcal6416", 16 | PCAL953X_TYPE | PCA_INT, },
-	{ "pcal6524", 24 | PCAL953X_TYPE | PCA_INT, },
-	{ "pcal6534", 34 | PCAL653X_TYPE | PCA_INT, },
-	{ "pcal9535", 16 | PCAL953X_TYPE | PCA_INT, },
-	{ "pcal9554b", 8  | PCAL953X_TYPE | PCA_INT, },
-	{ "pcal9555a", 16 | PCAL953X_TYPE | PCA_INT, },
+	{ "pcal6408", 8 | PCAL953X_TYPE | PCA_MASKED_INT, },
+	{ "pcal6416", 16 | PCAL953X_TYPE | PCA_MASKED_INT, },
+	{ "pcal6524", 24 | PCAL953X_TYPE | PCA_MASKED_INT, },
+	{ "pcal6534", 34 | PCAL653X_TYPE | PCA_MASKED_INT, },
+	{ "pcal9535", 16 | PCAL953X_TYPE | PCA_MASKED_INT, },
+	{ "pcal9554b", 8  | PCAL953X_TYPE | PCA_MASKED_INT, },
+	{ "pcal9555a", 16 | PCAL953X_TYPE | PCA_MASKED_INT, },
 
 	{ "max7310", 8  | PCA953X_TYPE, },
 	{ "max7312", 16 | PCA953X_TYPE | PCA_INT, },
@@ -176,6 +178,7 @@ struct pca953x_reg_config {
 	int output;
 	int input;
 	int invert;
+	int int_mask;
 };
 
 static const struct pca953x_reg_config pca953x_regs = {
@@ -185,6 +188,14 @@ static const struct pca953x_reg_config pca953x_regs = {
 	.invert = PCA953X_INVERT,
 };
 
+static const struct pca953x_reg_config pcal953x_regs = {
+	.direction = PCA953X_DIRECTION,
+	.output = PCA953X_OUTPUT,
+	.input = PCA953X_INPUT,
+	.invert = PCA953X_INVERT,
+	.int_mask = PCAL953X_INT_MASK,
+};
+
 static const struct pca953x_reg_config pca957x_regs = {
 	.direction = PCA957X_CFG,
 	.output = PCA957X_OUT,
@@ -235,6 +246,11 @@ static inline bool pca953x_has_interrupt(const struct pca953x_chip *chip)
 	return chip->driver_data & PCA_INT;
 }
 
+static inline bool pca953x_has_int_mask_reg(const struct pca953x_chip *chip)
+{
+	return chip->driver_data & PCA_HAS_INT_MASK;
+}
+
 #define PCA953x_BANK_INPUT	BIT(0)
 #define PCA953x_BANK_OUTPUT	BIT(1)
 #define PCA953x_BANK_POLARITY	BIT(2)
@@ -790,14 +806,16 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d)
 	DECLARE_BITMAP(reg_direction, MAX_LINE);
 	int level;
 
-	if (pca953x_is_pcal_type(chip)) {
-		/* Enable latch on interrupt-enabled inputs */
-		pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
-
+	if (pca953x_has_int_mask_reg(chip)) {
 		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);
+	}
+
+	if (pca953x_is_pcal_type(chip)) {
+		/* Enable latch on interrupt-enabled inputs */
+		pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask);
 	}
 
 	/* Switch direction to input if needed */
@@ -1193,12 +1211,20 @@ 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) {
-		chip->regs = &pca957x_regs;
-		ret = device_pca957x_init(chip, invert);
-	} else {
+	switch (PCA_CHIP_TYPE(chip->driver_data)) {
+	case PCA953X_TYPE:
 		chip->regs = &pca953x_regs;
 		ret = device_pca95xx_init(chip, invert);
+		break;
+	case PCAL953X_TYPE:
+	case PCAL653X_TYPE:
+		chip->regs = &pcal953x_regs;
+		ret = device_pca95xx_init(chip, invert);
+		break;
+	case PCA957X_TYPE:
+		chip->regs = &pca957x_regs;
+		ret = device_pca957x_init(chip, invert);
+		break;
 	}
 	if (ret)
 		goto err_exit;
@@ -1264,21 +1290,23 @@ 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);
+	if (pca953x_has_int_mask_reg(chip)) {
+		regaddr = chip->recalc_addr(chip, chip->regs->int_mask, 0);
 		ret = regcache_sync_region(chip->regmap, regaddr,
 					   regaddr + NBANK(chip) - 1);
 		if (ret) {
-			dev_err(dev, "Failed to sync INT latch registers: %d\n",
+			dev_err(dev, "Failed to sync INT mask registers: %d\n",
 				ret);
 			return ret;
 		}
+	}
 
-		regaddr = chip->recalc_addr(chip, PCAL953X_INT_MASK, 0);
+	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);
 		if (ret) {
-			dev_err(dev, "Failed to sync INT mask registers: %d\n",
+			dev_err(dev, "Failed to sync INT latch registers: %d\n",
 				ret);
 			return ret;
 		}
@@ -1362,13 +1390,13 @@ 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,pcal6408", .data = OF_L953X( 8, PCA_INT), },
-	{ .compatible = "nxp,pcal6416", .data = OF_L953X(16, PCA_INT), },
-	{ .compatible = "nxp,pcal6524", .data = OF_L953X(24, PCA_INT), },
-	{ .compatible = "nxp,pcal6534", .data = OF_L653X(34, PCA_INT), },
-	{ .compatible = "nxp,pcal9535", .data = OF_L953X(16, PCA_INT), },
-	{ .compatible = "nxp,pcal9554b", .data = OF_L953X( 8, PCA_INT), },
-	{ .compatible = "nxp,pcal9555a", .data = OF_L953X(16, PCA_INT), },
+	{ .compatible = "nxp,pcal6408", .data = OF_L953X( 8, PCA_MASKED_INT), },
+	{ .compatible = "nxp,pcal6416", .data = OF_L953X(16, PCA_MASKED_INT), },
+	{ .compatible = "nxp,pcal6524", .data = OF_L953X(24, PCA_MASKED_INT), },
+	{ .compatible = "nxp,pcal6534", .data = OF_L653X(34, PCA_MASKED_INT), },
+	{ .compatible = "nxp,pcal9535", .data = OF_L953X(16, PCA_MASKED_INT), },
+	{ .compatible = "nxp,pcal9554b", .data = OF_L953X( 8, PCA_MASKED_INT), },
+	{ .compatible = "nxp,pcal9555a", .data = OF_L953X(16, PCA_MASKED_INT), },
 
 	{ .compatible = "maxim,max7310", .data = OF_953X( 8, 0), },
 	{ .compatible = "maxim,max7312", .data = OF_953X(16, PCA_INT), },
-- 
2.37.3



  parent reply	other threads:[~2022-10-26 11:24 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-26 11:17 [PATCH v2 0/6] gpio: pca953x: Add interrupt mask support for pca953x chips Levente Révész
2022-10-26 11:18 ` [PATCH v2 1/6] gpio: pca953x: Convert PCA_TYPE from bits to number Levente Révész
2022-10-26 17:18   ` Andy Shevchenko
2022-10-26 17:21     ` Andy Shevchenko
2022-10-26 11:21 ` [PATCH v2 2/6] gpio: pca953x: Add PCAL953X as a separate chip type Levente Révész
2022-10-26 17:25   ` Andy Shevchenko
2022-10-26 17:28   ` Andy Shevchenko
2022-10-27 13:36     ` Levente Révész
2022-10-27 13:54       ` Martyn Welch
2022-10-27 17:03         ` Andy Shevchenko
2022-10-28 18:57           ` Levente Révész
2022-11-16 13:51             ` Levente Révész
2025-02-26 14:02             ` Andy Shevchenko
2022-10-26 11:22 ` [PATCH v2 3/6] gpio: pca953x: Add helper function to check if chip has interrupts Levente Révész
2022-10-26 11:23 ` Levente Révész [this message]
2022-10-26 17:31   ` [PATCH v2 4/6] gpio: pca953x: Generalize interrupt mask register handling Andy Shevchenko
2022-10-26 11:25 ` [PATCH v2 5/6] gpio: pca953x: Add interrupt mask support for chips with the standard register set Levente Révész
2022-10-26 16:49   ` kernel test robot
2022-10-26 17:32   ` Andy Shevchenko
2022-10-26 22:13   ` kernel test robot
2022-10-26 22:13   ` kernel test robot
2022-10-26 11:26 ` [PATCH v2 6/6] gpio: pca953x: Enable interrupt for pca9698 Levente Révész

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=edd3f359-effb-0951-f75f-b03ebb8b7b29@eilabs.com \
    --to=levente.revesz@eilabs.com \
    --cc=andrey.gusakov@cogentembedded.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=brgl@bgdev.pl \
    --cc=haibo.chen@nxp.com \
    --cc=justinpopo6@gmail.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=martyn.welch@collabora.com \
    --cc=nate.d@variscite.com \
    --cc=puyou.lu@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).