[PATCH v2] RFC: pinctrl: bcm2835: switch to GPIOLIB_IRQCHIP

Linus Walleij linus.walleij at linaro.org
Thu Nov 10 09:35:48 PST 2016


It should be possible to use the GPIOLIB_IRQCHIP helper
library with the BCM2835 driver since it is a pretty straight
forward cascaded irqchip.

The only difference from other drivers is that the BCM2835
has several banks for a single gpiochip, and each bank has
a separate IRQ line. Instead of creating one gpiochip per
bank, a single gpiochip covers all banks GPIO lines. This
makes it necessary to resolve the bank ID in the IRQ
handler.

The GPIOLIB_IRQCHIP allows several IRQs to be cascaded off
the same gpiochip by calling gpiochip_set_chained_irqchip()
repeatedly, but we have been a bit short on examples
for how this should be handled in practice, so this is intended
as an example of how this can be achieved.

The old code did not model the chip as a chained interrupt
handler, but this patch also rectifies that situation.

Cc: Stephen Warren <swarren at wwwdotorg.org>
Cc: Stefan Wahren <stefan.wahren at i2se.com>
Cc: Eric Anholt <eric at anholt.net>
Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
---
ChangeLog v1->v2:
- Forgot to convert the driver to chained IRQ handler. Now fixed.

Rpi folks: I have no clue if this works or not, but would be happy
if you could test it to see if IRQs fire as expected and provide
some feedback.
---
 drivers/pinctrl/bcm/Kconfig           |   1 +
 drivers/pinctrl/bcm/pinctrl-bcm2835.c | 111 +++++++++++++++++-----------------
 2 files changed, 57 insertions(+), 55 deletions(-)

diff --git a/drivers/pinctrl/bcm/Kconfig b/drivers/pinctrl/bcm/Kconfig
index 63246770bd74..8968dd7aebed 100644
--- a/drivers/pinctrl/bcm/Kconfig
+++ b/drivers/pinctrl/bcm/Kconfig
@@ -20,6 +20,7 @@ config PINCTRL_BCM2835
 	bool
 	select PINMUX
 	select PINCONF
+	select GPIOLIB_IRQCHIP
 
 config PINCTRL_IPROC_GPIO
 	bool "Broadcom iProc GPIO (with PINCONF) driver"
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm2835.c b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
index b2dd278f18b1..57c524d0adeb 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm2835.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm2835.c
@@ -24,11 +24,9 @@
 #include <linux/device.h>
 #include <linux/err.h>
 #include <linux/gpio/driver.h>
-#include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/irqdesc.h>
-#include <linux/irqdomain.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of.h>
@@ -86,31 +84,23 @@ enum bcm2835_pinconf_pull {
 #define BCM2835_PINCONF_UNPACK_PARAM(_conf_) ((_conf_) >> 16)
 #define BCM2835_PINCONF_UNPACK_ARG(_conf_) ((_conf_) & 0xffff)
 
-struct bcm2835_gpio_irqdata {
-	struct bcm2835_pinctrl *pc;
-	int bank;
-};
-
 struct bcm2835_pinctrl {
 	struct device *dev;
 	void __iomem *base;
 	int irq[BCM2835_NUM_BANKS];
+	unsigned int irq_bank[BCM2835_NUM_BANKS];
 
 	/* note: locking assumes each bank will have its own unsigned long */
 	unsigned long enabled_irq_map[BCM2835_NUM_BANKS];
 	unsigned int irq_type[BCM2835_NUM_GPIOS];
 
 	struct pinctrl_dev *pctl_dev;
-	struct irq_domain *irq_domain;
 	struct gpio_chip gpio_chip;
 	struct pinctrl_gpio_range gpio_range;
 
-	struct bcm2835_gpio_irqdata irq_data[BCM2835_NUM_BANKS];
 	spinlock_t irq_lock[BCM2835_NUM_BANKS];
 };
 
-static struct lock_class_key gpio_lock_class;
-
 /* pins are just named GPIO0..GPIO53 */
 #define BCM2835_GPIO_PIN(a) PINCTRL_PIN(a, "gpio" #a)
 static struct pinctrl_pin_desc bcm2835_gpio_pins[] = {
@@ -368,13 +358,6 @@ static int bcm2835_gpio_direction_output(struct gpio_chip *chip,
 	return pinctrl_gpio_direction_output(chip->base + offset);
 }
 
-static int bcm2835_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
-{
-	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
-
-	return irq_linear_revmap(pc->irq_domain, offset);
-}
-
 static struct gpio_chip bcm2835_gpio_chip = {
 	.label = MODULE_NAME,
 	.owner = THIS_MODULE,
@@ -385,31 +368,48 @@ static struct gpio_chip bcm2835_gpio_chip = {
 	.get_direction = bcm2835_gpio_get_direction,
 	.get = bcm2835_gpio_get,
 	.set = bcm2835_gpio_set,
-	.to_irq = bcm2835_gpio_to_irq,
 	.base = -1,
 	.ngpio = BCM2835_NUM_GPIOS,
 	.can_sleep = false,
 };
 
-static irqreturn_t bcm2835_gpio_irq_handler(int irq, void *dev_id)
+static void bcm2835_gpio_irq_handler(struct irq_desc *desc)
 {
-	struct bcm2835_gpio_irqdata *irqdata = dev_id;
-	struct bcm2835_pinctrl *pc = irqdata->pc;
-	int bank = irqdata->bank;
+	struct gpio_chip *chip = irq_desc_get_handler_data(desc);
+	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
+	struct irq_chip *host_chip = irq_desc_get_chip(desc);
+	int irq = irq_desc_get_irq(desc);
+	unsigned int bank;
 	unsigned long events;
 	unsigned offset;
 	unsigned gpio;
 	unsigned int type;
+	int i;
+
+	/* Figure out the bank number for the IRQ that just fired */
+	for (i = 0; i < ARRAY_SIZE(pc->irq); i++) {
+		if (pc->irq[i] == irq) {
+			bank = pc->irq_bank[i];
+			break;
+		}
+	}
+	/* This should not happen, every IRQ has a bank */
+	if (i == ARRAY_SIZE(pc->irq))
+		BUG();
 
 	events = bcm2835_gpio_rd(pc, GPEDS0 + bank * 4);
 	events &= pc->enabled_irq_map[bank];
+
+	chained_irq_enter(host_chip, desc);
+
 	for_each_set_bit(offset, &events, 32) {
 		gpio = (32 * bank) + offset;
 		type = pc->irq_type[gpio];
 
-		generic_handle_irq(irq_linear_revmap(pc->irq_domain, gpio));
+		generic_handle_irq(irq_find_mapping(chip->irqdomain, gpio));
 	}
-	return events ? IRQ_HANDLED : IRQ_NONE;
+
+	chained_irq_exit(host_chip, desc);
 }
 
 static inline void __bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc,
@@ -455,7 +455,8 @@ static void bcm2835_gpio_irq_config(struct bcm2835_pinctrl *pc,
 
 static void bcm2835_gpio_irq_enable(struct irq_data *data)
 {
-	struct bcm2835_pinctrl *pc = irq_data_get_irq_chip_data(data);
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
 	unsigned gpio = irqd_to_hwirq(data);
 	unsigned offset = GPIO_REG_SHIFT(gpio);
 	unsigned bank = GPIO_REG_OFFSET(gpio);
@@ -469,7 +470,8 @@ static void bcm2835_gpio_irq_enable(struct irq_data *data)
 
 static void bcm2835_gpio_irq_disable(struct irq_data *data)
 {
-	struct bcm2835_pinctrl *pc = irq_data_get_irq_chip_data(data);
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
 	unsigned gpio = irqd_to_hwirq(data);
 	unsigned offset = GPIO_REG_SHIFT(gpio);
 	unsigned bank = GPIO_REG_OFFSET(gpio);
@@ -575,7 +577,8 @@ static int __bcm2835_gpio_irq_set_type_enabled(struct bcm2835_pinctrl *pc,
 
 static int bcm2835_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 {
-	struct bcm2835_pinctrl *pc = irq_data_get_irq_chip_data(data);
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
 	unsigned gpio = irqd_to_hwirq(data);
 	unsigned offset = GPIO_REG_SHIFT(gpio);
 	unsigned bank = GPIO_REG_OFFSET(gpio);
@@ -601,7 +604,8 @@ static int bcm2835_gpio_irq_set_type(struct irq_data *data, unsigned int type)
 
 static void bcm2835_gpio_irq_ack(struct irq_data *data)
 {
-	struct bcm2835_pinctrl *pc = irq_data_get_irq_chip_data(data);
+	struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+	struct bcm2835_pinctrl *pc = gpiochip_get_data(chip);
 	unsigned gpio = irqd_to_hwirq(data);
 
 	bcm2835_gpio_set_bit(pc, GPEDS0, gpio);
@@ -644,10 +648,11 @@ static void bcm2835_pctl_pin_dbg_show(struct pinctrl_dev *pctldev,
 		unsigned offset)
 {
 	struct bcm2835_pinctrl *pc = pinctrl_dev_get_drvdata(pctldev);
+	struct gpio_chip *chip = &pc->gpio_chip;
 	enum bcm2835_fsel fsel = bcm2835_pinctrl_fsel_get(pc, offset);
 	const char *fname = bcm2835_functions[fsel];
 	int value = bcm2835_gpio_get_bit(pc, GPLEV0, offset);
-	int irq = irq_find_mapping(pc->irq_domain, offset);
+	int irq = irq_find_mapping(chip->irqdomain, offset);
 
 	seq_printf(s, "function %s in %s; irq %d (%s)",
 		fname, value ? "hi" : "lo",
@@ -982,21 +987,6 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 	pc->gpio_chip.parent = dev;
 	pc->gpio_chip.of_node = np;
 
-	pc->irq_domain = irq_domain_add_linear(np, BCM2835_NUM_GPIOS,
-			&irq_domain_simple_ops, NULL);
-	if (!pc->irq_domain) {
-		dev_err(dev, "could not create IRQ domain\n");
-		return -ENOMEM;
-	}
-
-	for (i = 0; i < BCM2835_NUM_GPIOS; i++) {
-		int irq = irq_create_mapping(pc->irq_domain, i);
-		irq_set_lockdep_class(irq, &gpio_lock_class);
-		irq_set_chip_and_handler(irq, &bcm2835_gpio_irq_chip,
-				handle_level_irq);
-		irq_set_chip_data(irq, pc);
-	}
-
 	for (i = 0; i < BCM2835_NUM_BANKS; i++) {
 		unsigned long events;
 		unsigned offset;
@@ -1017,8 +1007,7 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 			bcm2835_gpio_wr(pc, GPEDS0 + i * 4, BIT(offset));
 
 		pc->irq[i] = irq_of_parse_and_map(np, i);
-		pc->irq_data[i].pc = pc;
-		pc->irq_data[i].bank = i;
+		pc->irq_bank[i] = i;
 		spin_lock_init(&pc->irq_lock[i]);
 
 		len = strlen(dev_name(pc->dev)) + 16;
@@ -1026,14 +1015,6 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 		if (!name)
 			return -ENOMEM;
 		snprintf(name, len, "%s:bank%d", dev_name(pc->dev), i);
-
-		err = devm_request_irq(dev, pc->irq[i],
-			bcm2835_gpio_irq_handler, IRQF_SHARED,
-			name, &pc->irq_data[i]);
-		if (err) {
-			dev_err(dev, "unable to request IRQ %d\n", pc->irq[i]);
-			return err;
-		}
 	}
 
 	err = gpiochip_add_data(&pc->gpio_chip, pc);
@@ -1042,6 +1023,26 @@ static int bcm2835_pinctrl_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	err = gpiochip_irqchip_add(&pc->gpio_chip, &bcm2835_gpio_irq_chip,
+				   0, handle_level_irq, IRQ_TYPE_NONE);
+	if (err) {
+		dev_info(dev, "could not add irqchip\n");
+		return err;
+	}
+
+	for (i = 0; i < BCM2835_NUM_BANKS; i++) {
+		/*
+		 * Use the same handler for all banks/IRQs: this is necessary
+		 * since we use one gpiochip to cover all lines - the
+		 * irq handler then needs to figure out which bank was firing
+		 * the IRQ and look up the per-bank data.
+		 */
+		gpiochip_set_chained_irqchip(&pc->gpio_chip,
+					     &bcm2835_gpio_irq_chip,
+					     pc->irq[i],
+					     bcm2835_gpio_irq_handler);
+	}
+
 	pc->pctl_dev = devm_pinctrl_register(dev, &bcm2835_pinctrl_desc, pc);
 	if (IS_ERR(pc->pctl_dev)) {
 		gpiochip_remove(&pc->gpio_chip);
-- 
2.7.4




More information about the linux-rpi-kernel mailing list