[PATCH v3 0/3] ARM: bcm281xx: GPIO driver

[PATCH v3 0/3] ARM: bcm281xx: GPIO driver

[Markus Mayer]

This patch series adds the GPIO driver, and its device tree bindings,
for the Broadcom bcm281xx family of chips.

Markus Mayer (3):
  ARM: bcm281xx: Add GPIO driver
  ARM: bcm281xx: Device Tree bindings for GPIO driver
  ARM: bcm281xx: Enable GPIO driver

 .../devicetree/bindings/gpio/gpio-bcm-kona.txt     |   41 ++
 arch/arm/boot/dts/bcm11351.dtsi                    |   16 +
 arch/arm/mach-bcm/Kconfig                          |    2 +-
 drivers/gpio/Kconfig                               |    6 +
 drivers/gpio/Makefile                              |    1 +
 drivers/gpio/gpio-bcm-kona.c                       |  697 ++++++++++++++++++++
 6 files changed, 762 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt
 create mode 100644 drivers/gpio/gpio-bcm-kona.c

-- 
1.7.9.5

[PATCH v3 1/3] ARM: bcm281xx: Add GPIO driver

[Markus Mayer]

From: Markus Mayer <mmayer@broadcom.com>

This patch adds the GPIO driver for the bcm281xx family of chips.

Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Christian Daudt <csd@broadcom.com>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
Reviewed-by: Alex Elder <alex.elder@linaro.org>
---
 drivers/gpio/gpio-bcm-kona.c |  697 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 697 insertions(+)
 create mode 100644 drivers/gpio/gpio-bcm-kona.c

diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
new file mode 100644
index 0000000..ca6cb7e
--- /dev/null
+++ b/drivers/gpio/gpio-bcm-kona.c
@@ -0,0 +1,697 @@
+/*
+ * Copyright (C) 2012-2013 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
+
+#define BCM_GPIO_PASSWD	0x00a5a501
+#define GPIO_PER_BANK	32
+
+#define GPIO_BANK(gpio)	((gpio) >> 5)
+#define GPIO_BIT(gpio)	((gpio) & (GPIO_PER_BANK - 1))
+
+#define GPIO_GPOR0_OFFSET					0x00000000
+#define GPIO_GPIR0_OFFSET					0x00000020
+#define GPIO_GPORS0_OFFSET					0x00000040
+#define GPIO_GPORC0_OFFSET					0x00000060
+#define GPIO_ISR0_OFFSET					0x00000080
+#define GPIO_IMR0_OFFSET					0x000000A0
+#define GPIO_IMRC0_OFFSET					0x000000C0
+#define GPIO_GPCTR0_OFFSET					0x00000100
+#define GPIO_GPPLSR0_OFFSET					0x00000500
+#define GPIO_GPPWR_OFFSET					0x00000520
+
+#define GPIO_GPCTR0_DBR_SHIFT					5
+#define GPIO_GPCTR0_DBR_MASK					0x000001E0
+
+#define GPIO_GPCTR0_ITR_SHIFT					3
+#define GPIO_GPCTR0_ITR_MASK					0x00000018
+#define GPIO_GPCTR0_ITR_CMD_RISING_EDGE				0x00000001
+#define GPIO_GPCTR0_ITR_CMD_FALLING_EDGE			0x00000002
+#define GPIO_GPCTR0_ITR_CMD_BOTH_EDGE				0x00000003
+
+#define GPIO_GPCTR0_IOTR_MASK					0x00000001
+#define GPIO_GPCTR0_IOTR_CMD_0UTPUT				0x00000000
+#define GPIO_GPCTR0_IOTR_CMD_INPUT				0x00000001
+
+#define GPIO_GPCTR0_DB_ENABLE_MASK				0x00000100
+
+#define LOCK_CODE	0xFFFFFFFF
+#define UNLOCK_CODE	0x0
+
+struct bcm_kona_gpio_bank {
+	int id;
+	int irq;
+};
+
+struct bcm_kona_gpio {
+	struct irq_domain *irq_domain;
+	void __iomem *reg_base;
+	int num_bank;
+	int irq_base;
+	spinlock_t lock;
+	struct bcm_kona_gpio_bank *banks;
+};
+
+static struct gpio_chip bcm_gpio_chip;
+
+static void bcm_kona_gpio_irq_mask(struct irq_data *d);
+static void bcm_kona_gpio_irq_unmask(struct irq_data *d);
+
+static struct bcm_kona_gpio bcm_kona_gpio;
+
+static inline u32 bcm_kona_gpio_out_status(int bank)
+{
+	return GPIO_GPOR0_OFFSET + (bank << 2);
+}
+
+static inline u32 bcm_kona_gpio_in_status(int bank)
+{
+	return GPIO_GPIR0_OFFSET + (bank << 2);
+}
+
+static inline u32 bcm_kona_gpio_out_set(int bank)
+{
+	return GPIO_GPORS0_OFFSET + (bank << 2);
+}
+
+static inline u32 bcm_kona_gpio_out_clear(int bank)
+{
+	return GPIO_GPORC0_OFFSET + (bank << 2);
+}
+
+static inline u32 bcm_kona_gpio_int_status(int bank)
+{
+	return GPIO_ISR0_OFFSET + (bank << 2);
+}
+
+static inline u32 bcm_kona_gpio_int_mask(int bank)
+{
+	return GPIO_IMR0_OFFSET + (bank << 2);
+}
+
+static inline u32 bcm_kona_gpio_int_mskclr(int bank)
+{
+	return GPIO_IMRC0_OFFSET + (bank << 2);
+}
+
+static inline u32 bcm_kona_gpio_pwd_status(int bank)
+{
+	return GPIO_GPPLSR0_OFFSET + (bank << 2);
+}
+
+static inline u32 bcm_kona_gpio_control(int gpio)
+{
+	return GPIO_GPCTR0_OFFSET + (gpio << 2);
+}
+
+static int bcm_kona_count_irq_resources(struct platform_device *pdev)
+{
+	struct resource *res;
+	int count = 0;
+
+	for (;;) {
+		res = platform_get_resource(pdev, IORESOURCE_IRQ, count);
+		if (!res)
+			break;
+		count++;
+	}
+	return count;
+}
+
+static void  bcm_kona_gpio_set_lockcode_bank(int bank_id, int lockcode)
+{
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	/* This register is password protected  */
+	writel(BCM_GPIO_PASSWD, reg_base + GPIO_GPPWR_OFFSET);
+	/* Lock the bank */
+	writel(lockcode, reg_base + bcm_kona_gpio_pwd_status(bank_id));
+}
+
+static inline void bcm_kona_gpio_lock_bank(int bank_id)
+{
+	bcm_kona_gpio_set_lockcode_bank(bank_id, LOCK_CODE);
+}
+
+static inline void bcm_kona_gpio_unlock_bank(int bank_id)
+{
+	bcm_kona_gpio_set_lockcode_bank(bank_id, UNLOCK_CODE);
+}
+
+static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio,
+	int value)
+{
+	int bank_id = GPIO_BANK(gpio);
+	int bit = GPIO_BIT(gpio);
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	u32 val, reg_offset;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	/* determine the GPIO pin direction */
+	val = readl(reg_base + bcm_kona_gpio_control(gpio));
+	val &= GPIO_GPCTR0_IOTR_MASK;
+
+	/* this function only applies to output pin */
+	if (GPIO_GPCTR0_IOTR_CMD_INPUT == val)
+		return;
+
+	reg_offset = value ? bcm_kona_gpio_out_set(bank_id) :
+				bcm_kona_gpio_out_clear(bank_id);
+
+	val = readl(reg_base + reg_offset);
+	val |= 1 << bit;
+	writel(val, reg_base + reg_offset);
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+}
+
+static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio)
+{
+	int bank_id = GPIO_BANK(gpio);
+	int bit = GPIO_BIT(gpio);
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	u32 val, reg_offset;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	/* determine the GPIO pin direction */
+	val = readl(reg_base + bcm_kona_gpio_control(gpio));
+	val &= GPIO_GPCTR0_IOTR_MASK;
+
+	/* read the GPIO bank status */
+	reg_offset = (GPIO_GPCTR0_IOTR_CMD_INPUT == val) ?
+					bcm_kona_gpio_in_status(bank_id) :
+					bcm_kona_gpio_out_status(bank_id);
+	val = readl(reg_base + reg_offset);
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+
+	/* return the specified bit status */
+	return (val >> bit) & 1;
+}
+
+static int bcm_kona_gpio_direction_input(struct gpio_chip *chip,
+	unsigned gpio)
+{
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	u32 val;
+	unsigned long flags;
+	int bank_id = GPIO_BANK(gpio);
+
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	val = readl(reg_base + bcm_kona_gpio_control(gpio));
+	val &= ~GPIO_GPCTR0_IOTR_MASK;
+	val |= GPIO_GPCTR0_IOTR_CMD_INPUT;
+	writel(val, reg_base + bcm_kona_gpio_control(gpio));
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+
+	return 0;
+}
+
+static int bcm_kona_gpio_direction_output(struct gpio_chip *chip,
+	unsigned gpio, int value)
+{
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	int bank_id = GPIO_BANK(gpio);
+	int bit = GPIO_BIT(gpio);
+	u32 val, reg_offset;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	val = readl(reg_base + bcm_kona_gpio_control(gpio));
+	val &= ~GPIO_GPCTR0_IOTR_MASK;
+	val |= GPIO_GPCTR0_IOTR_CMD_0UTPUT;
+	writel(val, reg_base + bcm_kona_gpio_control(gpio));
+	reg_offset = value ? bcm_kona_gpio_out_set(bank_id) :
+				bcm_kona_gpio_out_clear(bank_id);
+
+	val = readl(reg_base + reg_offset);
+	val |= 1 << bit;
+	writel(val, reg_base + reg_offset);
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+
+	return 0;
+}
+
+static int bcm_kona_gpio_to_irq(struct gpio_chip *chip, unsigned gpio)
+{
+	if (gpio >= bcm_gpio_chip.ngpio)
+		return -ENXIO;
+	return irq_create_mapping(bcm_kona_gpio.irq_domain, gpio);
+}
+
+static int bcm_kona_gpio_set_debounce(struct gpio_chip *chip, unsigned gpio,
+	unsigned debounce)
+{
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	u32 val, res;
+	unsigned long flags;
+	int bank_id = GPIO_BANK(gpio);
+
+	/* debounce must be 1-128ms (or 0) */
+	if ((debounce > 0 && debounce < 1000) || debounce > 128000) {
+		dev_err(chip->dev, "Debounce value %u not in range\n",
+			debounce);
+		return -EINVAL;
+	}
+
+	/* calculate debounce bit value */
+	if (debounce != 0) {
+		/* Convert to ms */
+		debounce /= 1000;
+
+		/* find the MSB */
+		res = fls(debounce) - 1;
+
+		/* Check if MSB-1 is set (round up or down) */
+		if (res > 0 && (debounce & 1 << (res - 1)))
+			res++;
+	}
+
+	/* spin lock for read-modify-write of the GPIO register */
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	val = readl(reg_base + bcm_kona_gpio_control(gpio));
+	val &= ~GPIO_GPCTR0_DBR_MASK;
+
+	if (debounce == 0) {
+		/* disable debounce */
+		val &= ~GPIO_GPCTR0_DB_ENABLE_MASK;
+	} else {
+		val |= GPIO_GPCTR0_DB_ENABLE_MASK |
+			  (res << GPIO_GPCTR0_DBR_SHIFT);
+	}
+
+	writel(val, reg_base + bcm_kona_gpio_control(gpio));
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+
+	return 0;
+}
+
+static int bcm_kona_gpio_request(struct gpio_chip *chip, unsigned gpio)
+{
+	struct irq_data d;
+
+	d.hwirq = gpio;
+	bcm_kona_gpio_irq_unmask(&d);
+
+	return 0;
+}
+
+static void bcm_kona_gpio_free(struct gpio_chip *chip, unsigned gpio)
+{
+	struct irq_data d;
+
+	d.hwirq = gpio;
+	bcm_kona_gpio_irq_mask(&d);
+}
+
+static struct gpio_chip bcm_gpio_chip = {
+	.label = "bcm-kona-gpio",
+	.request = bcm_kona_gpio_request,
+	.free = bcm_kona_gpio_free,
+	.direction_input = bcm_kona_gpio_direction_input,
+	.get = bcm_kona_gpio_get,
+	.direction_output = bcm_kona_gpio_direction_output,
+	.set = bcm_kona_gpio_set,
+	.set_debounce = bcm_kona_gpio_set_debounce,
+	.to_irq = bcm_kona_gpio_to_irq,
+	.base = 0,
+};
+
+static void bcm_kona_gpio_irq_ack(struct irq_data *d)
+{
+	int gpio = d->hwirq;
+	int bank_id = GPIO_BANK(gpio);
+	int bit = GPIO_BIT(gpio);
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	val = readl(reg_base + bcm_kona_gpio_int_status(bank_id));
+	val |= 1 << bit;
+	writel(val, reg_base + bcm_kona_gpio_int_status(bank_id));
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+}
+
+static void bcm_kona_gpio_irq_mask(struct irq_data *d)
+{
+	int gpio = d->hwirq;
+	int bank_id = GPIO_BANK(gpio);
+	int bit = GPIO_BIT(gpio);
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	val = readl(reg_base + bcm_kona_gpio_int_mask(bank_id));
+	val |= 1 << bit;
+	writel(val, reg_base + bcm_kona_gpio_int_mask(bank_id));
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+}
+
+static void bcm_kona_gpio_irq_unmask(struct irq_data *d)
+{
+	int gpio = d->hwirq;
+	int bank_id = GPIO_BANK(gpio);
+	int bit = GPIO_BIT(gpio);
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	u32 val;
+	unsigned long flags;
+
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	val = readl(reg_base + bcm_kona_gpio_int_mskclr(bank_id));
+	val |= 1 << bit;
+	writel(val, reg_base + bcm_kona_gpio_int_mskclr(bank_id));
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+}
+
+static int bcm_kona_gpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+	int gpio = d->hwirq;
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	u32 lvl_type;
+	u32 val;
+	unsigned long flags;
+	int bank_id = GPIO_BANK(gpio);
+
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_EDGE_RISING:
+		lvl_type = GPIO_GPCTR0_ITR_CMD_RISING_EDGE;
+		break;
+
+	case IRQ_TYPE_EDGE_FALLING:
+		lvl_type = GPIO_GPCTR0_ITR_CMD_FALLING_EDGE;
+		break;
+
+	case IRQ_TYPE_EDGE_BOTH:
+		lvl_type = GPIO_GPCTR0_ITR_CMD_BOTH_EDGE;
+		break;
+
+	case IRQ_TYPE_LEVEL_HIGH:
+	case IRQ_TYPE_LEVEL_LOW:
+		/* BCM GPIO doesn't support level triggering */
+	default:
+		dev_err(bcm_gpio_chip.dev, "Invalid BCM GPIO irq type 0x%x\n",
+			type);
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	val = readl(reg_base + bcm_kona_gpio_control(gpio));
+	val &= ~GPIO_GPCTR0_ITR_MASK;
+	val |= lvl_type << GPIO_GPCTR0_ITR_SHIFT;
+	writel(val, reg_base + bcm_kona_gpio_control(gpio));
+
+	bcm_kona_gpio_lock_bank(bank_id);
+	spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
+
+	return 0;
+}
+
+static void bcm_kona_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+	int bit, bank_id;
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+	unsigned long sta;
+	struct bcm_kona_gpio_bank *bank = irq_get_handler_data(irq);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+
+	chained_irq_enter(chip, desc);
+
+	bank_id = bank->id;
+	bcm_kona_gpio_unlock_bank(bank_id);
+
+	sta = readl(reg_base + bcm_kona_gpio_int_status(bank_id)) &
+	    (~(readl(reg_base + bcm_kona_gpio_int_mask(bank_id))));
+
+	for_each_set_bit(bit, &sta, 32) {
+		/*
+		 * Clear interrupt before handler is called so we don't
+		 * miss any interrupt occurred during executing them.
+		 */
+		writel(readl(reg_base + bcm_kona_gpio_int_status(bank_id)) |
+				(1 << bit),
+				reg_base + bcm_kona_gpio_int_status(bank_id));
+
+		/* Invoke interrupt handler */
+		generic_handle_irq(gpio_to_irq(GPIO_PER_BANK * bank_id + bit));
+	}
+
+	bcm_kona_gpio_lock_bank(bank_id);
+
+	chained_irq_exit(chip, desc);
+}
+
+static struct irq_chip bcm_gpio_irq_chip = {
+	.name = "bcm-kona-gpio",
+	.irq_ack = bcm_kona_gpio_irq_ack,
+	.irq_mask = bcm_kona_gpio_irq_mask,
+	.irq_unmask = bcm_kona_gpio_irq_unmask,
+	.irq_set_type = bcm_kona_gpio_irq_set_type,
+};
+
+static struct __initconst of_device_id bcm_kona_gpio_of_match[] = {
+	{ .compatible = "brcm,kona-gpio" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, kona_gpio_match);
+
+/*
+ * This lock class tells lockdep that GPIO irqs are in a different
+ * category than their parents, so it won't report false recursion.
+ */
+static struct lock_class_key gpio_lock_class;
+
+static int bcm_kona_gpio_irq_map(struct irq_domain *d, unsigned int virq,
+				irq_hw_number_t hwirq)
+{
+	irq_set_chip_data(virq, &d->host_data);
+	irq_set_lockdep_class(virq, &gpio_lock_class);
+	irq_set_chip_and_handler(virq, &bcm_gpio_irq_chip, handle_simple_irq);
+	irq_set_nested_thread(virq, 1);
+	set_irq_flags(virq, IRQF_VALID);
+
+	return 0;
+}
+
+static void bcm_kona_gpio_irq_unmap(struct irq_domain *d, unsigned int virq)
+{
+	irq_set_chip_and_handler(virq, NULL, NULL);
+	irq_set_chip_data(virq, NULL);
+}
+
+static struct irq_domain_ops bcm_kona_irq_ops = {
+	.map    = bcm_kona_gpio_irq_map,
+	.unmap  = bcm_kona_gpio_irq_unmap,
+	.xlate  = irq_domain_xlate_twocell,
+};
+
+static void bcm_kona_gpio_reset(int num_bank)
+{
+	int i;
+	void __iomem *reg_base = bcm_kona_gpio.reg_base;
+
+	/* disable interrupts and clear status */
+	for (i = 0; i < num_bank; i++) {
+		bcm_kona_gpio_unlock_bank(i);
+		writel(0xFFFFFFFF, reg_base + bcm_kona_gpio_int_mask(i));
+		writel(0xFFFFFFFF, reg_base + bcm_kona_gpio_int_status(i));
+		bcm_kona_gpio_lock_bank(i);
+	}
+}
+
+static int bcm_kona_gpio_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+	struct resource *res;
+	struct bcm_kona_gpio_bank *bank;
+	int ret;
+	int i;
+
+	match = of_match_device(bcm_kona_gpio_of_match, &pdev->dev);
+	if (!match) {
+		dev_err(&pdev->dev, "Failed to find gpio controller\n");
+		return -ENODEV;
+	}
+
+	bcm_kona_gpio.num_bank = bcm_kona_count_irq_resources(pdev);
+	if (bcm_kona_gpio.num_bank == 0) {
+		dev_err(&pdev->dev, "Couldn't determine # GPIO banks\n");
+		return -ENOENT;
+	}
+	bcm_gpio_chip.of_node = pdev->dev.of_node;
+	bcm_gpio_chip.dev = &pdev->dev;
+	bcm_gpio_chip.ngpio = bcm_kona_gpio.num_bank * GPIO_PER_BANK;
+	bcm_kona_gpio.banks = devm_kzalloc(&pdev->dev,
+				       bcm_kona_gpio.num_bank *
+				       sizeof(*bcm_kona_gpio.banks),
+				       GFP_KERNEL);
+	if (!bcm_kona_gpio.banks) {
+		dev_err(&pdev->dev, "Couldn't allocate bank structure\n");
+		return -ENOMEM;
+	}
+
+	bcm_kona_gpio.irq_base = irq_alloc_descs(-1, 0, bcm_gpio_chip.ngpio,
+						0);
+	if (bcm_kona_gpio.irq_base < 0) {
+		dev_err(&pdev->dev, "Couldn't allocate IRQ numbers\n");
+		return -ENXIO;
+	}
+	bcm_kona_gpio.irq_domain = irq_domain_add_linear(pdev->dev.of_node,
+						bcm_gpio_chip.ngpio,
+						&bcm_kona_irq_ops,
+						&bcm_kona_gpio);
+	if (!bcm_kona_gpio.irq_domain) {
+		dev_err(&pdev->dev, "Couldn't allocate IRQ domain\n");
+		ret = -ENXIO;
+		goto err_irq_descs;
+	}
+	for (i = 0; i < bcm_kona_gpio.num_bank; i++) {
+		bank = &bcm_kona_gpio.banks[i];
+		bank->id = i;
+		bank->irq = platform_get_irq(pdev, i);
+		if (bank->irq < 0) {
+			dev_err(&pdev->dev, "Couldn't get IRQ for bank %d", i);
+			ret = -ENOENT;
+			goto err_irq_domain;
+		}
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "Missing MEM resource\n");
+		ret = -ENXIO;
+		goto err_irq_domain;
+	}
+
+	bcm_kona_gpio.reg_base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!bcm_kona_gpio.reg_base) {
+		dev_err(&pdev->dev, "Couldn't ioremap regs\n");
+		ret = -ENXIO;
+		goto err_irq_domain;
+	}
+	dev_info(&pdev->dev, "Setting up Kona GPIO at 0x%p (phys %#x)\n",
+		bcm_kona_gpio.reg_base, res->start);
+
+	bcm_kona_gpio_reset(bcm_kona_gpio.num_bank);
+
+	ret = gpiochip_add(&bcm_gpio_chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Couldn't add GPIO chip -- %d\n", ret);
+		goto err_irq_domain;
+	}
+	for (i = 0; i < bcm_gpio_chip.ngpio; i++) {
+		int irq = bcm_kona_gpio_to_irq(&bcm_gpio_chip, i);
+		bank = &bcm_kona_gpio.banks[GPIO_BANK(i)];
+		irq_set_lockdep_class(irq, &gpio_lock_class);
+		irq_set_chip_data(irq, bank);
+		irq_set_chip_and_handler(irq, &bcm_gpio_irq_chip,
+					 handle_simple_irq);
+		set_irq_flags(irq, IRQF_VALID);
+	}
+	for (i = 0; i < bcm_kona_gpio.num_bank; i++) {
+		bank = &bcm_kona_gpio.banks[i];
+
+		irq_set_chained_handler(bank->irq, bcm_kona_gpio_irq_handler);
+		irq_set_handler_data(bank->irq, bank);
+	}
+
+	spin_lock_init(&bcm_kona_gpio.lock);
+
+	return 0;
+
+err_irq_domain:
+	irq_domain_remove(bcm_kona_gpio.irq_domain);
+
+err_irq_descs:
+	irq_free_descs(bcm_kona_gpio.irq_base, bcm_gpio_chip.ngpio);
+
+	return ret;
+}
+
+static int bcm_kona_gpio_remove(struct platform_device *pdev)
+{
+	int i, ret;
+	struct bcm_kona_gpio_bank *bank;
+
+	for (i = 0; i < bcm_gpio_chip.ngpio; i++)
+		gpio_free(i);
+	ret = gpiochip_remove(&bcm_gpio_chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Couldn't remove GPIO chip -- %d\n", ret);
+		return ret;
+	}
+	for (i = 0; i < bcm_kona_gpio.num_bank; i++) {
+		bank = &bcm_kona_gpio.banks[i];
+		irq_set_chained_handler(bank->irq, NULL);
+		irq_set_handler_data(bank->irq, NULL);
+	}
+	irq_domain_remove(bcm_kona_gpio.irq_domain);
+	irq_free_descs(bcm_kona_gpio.irq_base, bcm_gpio_chip.ngpio);
+
+	return 0;
+}
+
+static struct platform_driver bcm_kona_gpio_driver = {
+	.driver = {
+		   .name = "bcm-kona-gpio",
+		   .owner = THIS_MODULE,
+		   .of_match_table = bcm_kona_gpio_of_match,
+		   },
+	.probe = bcm_kona_gpio_probe,
+	.remove = bcm_kona_gpio_remove,
+};
+
+module_platform_driver(bcm_kona_gpio_driver);
+
+MODULE_AUTHOR("Broadcom");
+MODULE_DESCRIPTION("Broadcom Kona GPIO Driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

[PATCH v3 2/3] ARM: bcm281xx: Device Tree bindings for GPIO driver

[Markus Mayer]

Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Christian Daudt <csd@broadcom.com>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---
 .../devicetree/bindings/gpio/gpio-bcm-kona.txt     |   41 ++++++++++++++++++++
 arch/arm/boot/dts/bcm11351.dtsi                    |   16 ++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt
new file mode 100644
index 0000000..08e9b5a
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt
@@ -0,0 +1,41 @@
+Broadcom Kona Family GPIO
+-------------------------
+
+This GPIO driver is used in the following Broadcom SoCs:
+  BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
+
+The GPIO controller only supports edge, not level triggering.
+
+Required properties:
+  - compatible: "brcm,kona-gpio"
+  - reg: Physical base address and length of the controller's registers.
+  - interrupts: The interrupt outputs from the controller.
+  - #gpio-cells: Should be <2>. The first cell is the pin number, the second
+    cell is used to specify optional parameters:
+    - bit 0 specifies polarity (0 for normal, 1 for inverted)
+  - #interrupt-cells: Should be <2>. The first cell is the GPIO number.
+    The second cell is used to specify flags:
+    - trigger type (bits[1:0]):
+        1 = low-to-high edge triggered.
+        2 = high-to-low edge triggered.
+        3 = low-to-high or high-to-low edge triggered
+        Valid values are 1, 2, 3
+  - gpio-controller: Marks the device node as a GPIO controller.
+  - interrupt-controller: Marks the device node as an interrupt controller.
+
+Example:
+	gpio: gpio at 35003000 {
+		compatible = "brcm,kona-gpio";
+		reg = <0x35003000 0x800>;
+		interrupts =
+		       <0x0 106 0x4
+			0x0 115 0x4
+			0x0 114 0x4
+			0x0 113 0x4
+			0x0 112 0x4
+			0x0 111 0x4>;
+		#gpio-cells = <2>;
+		#interrupt-cells = <2>;
+		gpio-controller;
+		interrupt-controller;
+	};
diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index c0cdf66..13aaa06 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -63,6 +63,22 @@
 		clock-frequency = <32768>;
 	};
 
+	gpio: gpio at 35003000 {
+		compatible = "brcm,kona-gpio";
+		reg = <0x35003000 0x800>;
+		interrupts =
+		       <0x0 106 0x4
+			0x0 115 0x4
+			0x0 114 0x4
+			0x0 113 0x4
+			0x0 112 0x4
+			0x0 111 0x4>;
+		#gpio-cells = <2>;
+		#interrupt-cells = <2>;
+		gpio-controller;
+		interrupt-controller;
+	};
+
 	sdio0: sdio at 0x3f180000 {
 		compatible = "bcm,kona-sdhci";
 		reg = <0x3f180000 0x10000>;
-- 
1.7.9.5

[PATCH v3 3/3] ARM: bcm281xx: Enable GPIO driver

[Markus Mayer]

This patch adds the Kconfig and Makefile glue to compile the GPIO
driver for bcm281xx when CONFIG_GPIO_BCM=y.

Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
Reviewed-by: Christian Daudt <csd@broadcom.com>
Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
Reviewed-by: Matt Porter <matt.porter@linaro.org>
---
 arch/arm/mach-bcm/Kconfig |    2 +-
 drivers/gpio/Kconfig      |    6 ++++++
 drivers/gpio/Makefile     |    1 +
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index f112895..5819126 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -9,7 +9,7 @@ config ARCH_BCM
 	select CLKSRC_OF
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_TIME
-	select GPIO_BCM
+	select GPIO_BCM_KONA
 	select SPARSE_IRQ
 	select TICK_ONESHOT
 	help
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b2450ba..87d3a23 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -714,6 +714,12 @@ config GPIO_MSIC
 	  Enable support for GPIO on intel MSIC controllers found in
 	  intel MID devices
 
+config GPIO_BCM_KONA
+	bool "Broadcom Kona GPIO"
+	depends on ARCH_BCM
+	help
+	  Turn on GPIO support for Broadcom "Kona" chips.
+
 comment "USB GPIO expanders:"
 
 config GPIO_VIPERBOARD
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index ef3e983..fcd0751 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_GPIO_ADP5520)	+= gpio-adp5520.o
 obj-$(CONFIG_GPIO_ADP5588)	+= gpio-adp5588.o
 obj-$(CONFIG_GPIO_AMD8111)	+= gpio-amd8111.o
 obj-$(CONFIG_GPIO_ARIZONA)	+= gpio-arizona.o
+obj-$(CONFIG_GPIO_BCM_KONA)	+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CLPS711X)	+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_CS5535)	+= gpio-cs5535.o
-- 
1.7.9.5

[PATCH v3 2/3] ARM: bcm281xx: Device Tree bindings for GPIO driver

[Stephen Warren]

(CC'ing the DT bindings maintainers hence quoting the binding in full)

On 07/26/2013 02:25 PM, Markus Mayer wrote:

Some kind of patch description is always useful.

>  .../devicetree/bindings/gpio/gpio-bcm-kona.txt     |   41 ++++++++++++++++++++
>  arch/arm/boot/dts/bcm11351.dtsi                    |   16 ++++++++

It's more usual to put the DT binding doc change and driver change in
one patch, then "make use of that" in a later separate patch which edits
*.dts and *.dtsi.

> diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt

> +Broadcom Kona Family GPIO
> +-------------------------
> +
> +This GPIO driver is used in the following Broadcom SoCs:
> +  BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
> +
> +The GPIO controller only supports edge, not level triggering.

add "... of interrupts"? This might not be worth mentioning; it's
clearly spelled out in the description of the interrupt cells below.

> +Required properties:
> +  - compatible: "brcm,kona-gpio"
> +  - reg: Physical base address and length of the controller's registers.
> +  - interrupts: The interrupt outputs from the controller.

How many entries are required? I notice there's more than 1 in the
example below.

> +  - #gpio-cells: Should be <2>. The first cell is the pin number, the second
> +    cell is used to specify optional parameters:
> +    - bit 0 specifies polarity (0 for normal, 1 for inverted)
> +  - #interrupt-cells: Should be <2>. The first cell is the GPIO number.
> +    The second cell is used to specify flags:
> +    - trigger type (bits[1:0]):
> +        1 = low-to-high edge triggered.
> +        2 = high-to-low edge triggered.
> +        3 = low-to-high or high-to-low edge triggered
> +        Valid values are 1, 2, 3
> +  - gpio-controller: Marks the device node as a GPIO controller.
> +  - interrupt-controller: Marks the device node as an interrupt controller.
> +
> +Example:
> +	gpio: gpio at 35003000 {
> +		compatible = "brcm,kona-gpio";
> +		reg = <0x35003000 0x800>;
> +		interrupts =
> +		       <0x0 106 0x4
> +			0x0 115 0x4
> +			0x0 114 0x4
> +			0x0 113 0x4
> +			0x0 112 0x4
> +			0x0 111 0x4>;
> +		#gpio-cells = <2>;
> +		#interrupt-cells = <2>;
> +		gpio-controller;
> +		interrupt-controller;
> +	};

> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi

> +	gpio: gpio at 35003000 {
> +		compatible = "brcm,kona-gpio";

In order to enable any later chip-specific quirking requirements, that
compatible property should both specify the IP block and the specific
chip it's included in, so I'd expect to see something like:

		compatible = "brcm,brcm11351-gpio", "brcm,kona-gpio";

[PATCH v3 2/3] ARM: bcm281xx: Device Tree bindings for GPIO driver

[Markus Mayer]

On 26 July 2013 15:08, Stephen Warren <swarren@wwwdotorg.org> wrote:
> (CC'ing the DT bindings maintainers hence quoting the binding in full)
>
> On 07/26/2013 02:25 PM, Markus Mayer wrote:
>
> Some kind of patch description is always useful.

I will add something along the lines of "GPIO device tree bindings for
the Broadcom bcm281xx family of chips."

>>  .../devicetree/bindings/gpio/gpio-bcm-kona.txt     |   41 ++++++++++++++++++++
>>  arch/arm/boot/dts/bcm11351.dtsi                    |   16 ++++++++
>
> It's more usual to put the DT binding doc change and driver change in
> one patch, then "make use of that" in a later separate patch which edits
> *.dts and *.dtsi.

Sure. I have no issue moving the gpio-bcm-kona.txt into the other
patch, together with the code.

>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt
>
>> +Broadcom Kona Family GPIO
>> +-------------------------
>> +
>> +This GPIO driver is used in the following Broadcom SoCs:
>> +  BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
>> +
>> +The GPIO controller only supports edge, not level triggering.
>
> add "... of interrupts"? This might not be worth mentioning; it's
> clearly spelled out in the description of the interrupt cells below.

Added.

>> +Required properties:
>> +  - compatible: "brcm,kona-gpio"
>> +  - reg: Physical base address and length of the controller's registers.
>> +  - interrupts: The interrupt outputs from the controller.
>
> How many entries are required? I notice there's more than 1 in the
> example below.

You only "need" the ones you want to use, i.e. at least one. The board
supports up to 6. I can mention as much in the description.

[...]

>> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
>
>> +     gpio: gpio at 35003000 {
>> +             compatible = "brcm,kona-gpio";
>
> In order to enable any later chip-specific quirking requirements, that
> compatible property should both specify the IP block and the specific
> chip it's included in, so I'd expect to see something like:
>
>                 compatible = "brcm,brcm11351-gpio", "brcm,kona-gpio";

Added.

Thanks,
-Markus

-- 
Markus Mayer
Broadcom Landing Team

[PATCH v3 2/3] ARM: bcm281xx: Device Tree bindings for GPIO driver

[Stephen Warren]

On 07/26/2013 04:51 PM, Markus Mayer wrote:
> On 26 July 2013 15:08, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> (CC'ing the DT bindings maintainers hence quoting the binding in full)
>>
>> On 07/26/2013 02:25 PM, Markus Mayer wrote:

>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt

>>> +Required properties:
>>> +  - compatible: "brcm,kona-gpio"
>>> +  - reg: Physical base address and length of the controller's registers.
>>> +  - interrupts: The interrupt outputs from the controller.
>>
>> How many entries are required? I notice there's more than 1 in the
>> example below.
> 
> You only "need" the ones you want to use, i.e. at least one. The board
> supports up to 6. I can mention as much in the description.

Isn't this a property of the SoC's IP block, not of the board?

[PATCH v3 2/3] ARM: bcm281xx: Device Tree bindings for GPIO driver

[Markus Mayer]

On 26 July 2013 16:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/26/2013 04:51 PM, Markus Mayer wrote:
>> On 26 July 2013 15:08, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> (CC'ing the DT bindings maintainers hence quoting the binding in full)
>>>
>>> On 07/26/2013 02:25 PM, Markus Mayer wrote:
>
>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt b/Documentation/devicetree/bindings/gpio/gpio-bcm-kona.txt
>
>>>> +Required properties:
>>>> +  - compatible: "brcm,kona-gpio"
>>>> +  - reg: Physical base address and length of the controller's registers.
>>>> +  - interrupts: The interrupt outputs from the controller.
>>>
>>> How many entries are required? I notice there's more than 1 in the
>>> example below.
>>
>> You only "need" the ones you want to use, i.e. at least one. The board
>> supports up to 6. I can mention as much in the description.
>
> Isn't this a property of the SoC's IP block, not of the board?

Yes. I wasn't very precise with my statement.

What I added to the description is "Specify at least 1 GPIO interrupt.
The maximum number is 6."

-- 
Markus Mayer
Broadcom Landing Team

[PATCH v3 2/3] ARM: bcm281xx: Device Tree bindings for GPIO driver

[Tim Kryger]

On Fri, Jul 26, 2013 at 4:22 PM, Markus Mayer <markus.mayer@linaro.org> wrote:
> On 26 July 2013 16:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> Isn't this a property of the SoC's IP block, not of the board?
>
> Yes. I wasn't very precise with my statement.
>
> What I added to the description is "Specify at least 1 GPIO interrupt.
> The maximum number is 6."

The IP has a configurable number of banks, each of which has its own
interrupt.  If a particular SoC was configured to have six banks then
you need to list six interrupts in the DTS.  Perhaps it would help to
spell that out that relationship explicitly in the documentation
rather than talking about a minimum or maximum number of interrupts?
Also, you may want to mention that interrupts should to be ordered by
bank number.

[PATCH v3 1/3] ARM: bcm281xx: Add GPIO driver

[Sudeep KarkadaNagesha]

On 26/07/13 21:25, Markus Mayer wrote:
> From: Markus Mayer <mmayer@broadcom.com>
> 
> This patch adds the GPIO driver for the bcm281xx family of chips.
> 
May be I am missing my mails, but I could not find even in archives.
The patch says its v3, I could not trace v1/v2 and not change log.

> Signed-off-by: Markus Mayer <markus.mayer@linaro.org>
> Reviewed-by: Ray Jui <rjui@broadcom.com>
> Reviewed-by: Christian Daudt <csd@broadcom.com>
> Reviewed-by: Tim Kryger <tim.kryger@linaro.org>
> Reviewed-by: Matt Porter <matt.porter@linaro.org>
> Reviewed-by: Alex Elder <alex.elder@linaro.org>
> ---
>  drivers/gpio/gpio-bcm-kona.c |  697 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 697 insertions(+)
>  create mode 100644 drivers/gpio/gpio-bcm-kona.c
> 
> diff --git a/drivers/gpio/gpio-bcm-kona.c b/drivers/gpio/gpio-bcm-kona.c
> new file mode 100644
> index 0000000..ca6cb7e
> --- /dev/null
> +++ b/drivers/gpio/gpio-bcm-kona.c
> @@ -0,0 +1,697 @@
> +/*
> + * Copyright (C) 2012-2013 Broadcom Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/gpio.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/irqdomain.h>
> +#include <linux/irqchip/chained_irq.h>
> +
> +#define BCM_GPIO_PASSWD        0x00a5a501
> +#define GPIO_PER_BANK  32
> +
> +#define GPIO_BANK(gpio)        ((gpio) >> 5)
> +#define GPIO_BIT(gpio) ((gpio) & (GPIO_PER_BANK - 1))
> +
> +#define GPIO_GPOR0_OFFSET                                      0x00000000
> +#define GPIO_GPIR0_OFFSET                                      0x00000020
> +#define GPIO_GPORS0_OFFSET                                     0x00000040
> +#define GPIO_GPORC0_OFFSET                                     0x00000060
> +#define GPIO_ISR0_OFFSET                                       0x00000080
> +#define GPIO_IMR0_OFFSET                                       0x000000A0
> +#define GPIO_IMRC0_OFFSET                                      0x000000C0
> +#define GPIO_GPCTR0_OFFSET                                     0x00000100
> +#define GPIO_GPPLSR0_OFFSET                                    0x00000500
> +#define GPIO_GPPWR_OFFSET                                      0x00000520
> +
Why these macros can't be something like this ?
#define GPIO_GPOR_OFFSET(bank)	(0x00000000 + (bank) << 2)

> +#define GPIO_GPCTR0_DBR_SHIFT                                  5
> +#define GPIO_GPCTR0_DBR_MASK                                   0x000001E0
> +
> +#define GPIO_GPCTR0_ITR_SHIFT                                  3
> +#define GPIO_GPCTR0_ITR_MASK                                   0x00000018
> +#define GPIO_GPCTR0_ITR_CMD_RISING_EDGE                                0x00000001
> +#define GPIO_GPCTR0_ITR_CMD_FALLING_EDGE                       0x00000002
> +#define GPIO_GPCTR0_ITR_CMD_BOTH_EDGE                          0x00000003
> +
> +#define GPIO_GPCTR0_IOTR_MASK                                  0x00000001
> +#define GPIO_GPCTR0_IOTR_CMD_0UTPUT                            0x00000000
> +#define GPIO_GPCTR0_IOTR_CMD_INPUT                             0x00000001
> +
> +#define GPIO_GPCTR0_DB_ENABLE_MASK                             0x00000100
> +
> +#define LOCK_CODE      0xFFFFFFFF
> +#define UNLOCK_CODE    0x0
> +
> +struct bcm_kona_gpio_bank {
> +       int id;
> +       int irq;
> +};
> +
> +struct bcm_kona_gpio {
> +       struct irq_domain *irq_domain;
> +       void __iomem *reg_base;
> +       int num_bank;
> +       int irq_base;
> +       spinlock_t lock;
> +       struct bcm_kona_gpio_bank *banks;
> +};
> +
> +static struct gpio_chip bcm_gpio_chip;
> +
Can this be embed in bcm_kona_gpio?
You can then get rid of all the accesses to global variable
bcm_kona_gpio.

> +static void bcm_kona_gpio_irq_mask(struct irq_data *d);
> +static void bcm_kona_gpio_irq_unmask(struct irq_data *d);
> +
> +static struct bcm_kona_gpio bcm_kona_gpio;
> +
Can this be allocated dynamically ?
And the changed global pointer can be accessed only in probe.
You can remove all direct access once you embed as mentioned above.

> +static inline u32 bcm_kona_gpio_out_status(int bank)
> +{
> +       return GPIO_GPOR0_OFFSET + (bank << 2);
> +}
> +
> +static inline u32 bcm_kona_gpio_in_status(int bank)
> +{
> +       return GPIO_GPIR0_OFFSET + (bank << 2);
> +}
> +
> +static inline u32 bcm_kona_gpio_out_set(int bank)
> +{
> +       return GPIO_GPORS0_OFFSET + (bank << 2);
> +}
> +
> +static inline u32 bcm_kona_gpio_out_clear(int bank)
> +{
> +       return GPIO_GPORC0_OFFSET + (bank << 2);
> +}
> +
> +static inline u32 bcm_kona_gpio_int_status(int bank)
> +{
> +       return GPIO_ISR0_OFFSET + (bank << 2);
> +}
> +
> +static inline u32 bcm_kona_gpio_int_mask(int bank)
> +{
> +       return GPIO_IMR0_OFFSET + (bank << 2);
> +}
> +
> +static inline u32 bcm_kona_gpio_int_mskclr(int bank)
> +{
> +       return GPIO_IMRC0_OFFSET + (bank << 2);
> +}
> +
> +static inline u32 bcm_kona_gpio_pwd_status(int bank)
> +{
> +       return GPIO_GPPLSR0_OFFSET + (bank << 2);
> +}
> +
> +static inline u32 bcm_kona_gpio_control(int gpio)
> +{
> +       return GPIO_GPCTR0_OFFSET + (gpio << 2);
> +}
All these functions can go away if you can have the macros as shown above ?

> +
> +static int bcm_kona_count_irq_resources(struct platform_device *pdev)
> +{
> +       struct resource *res;
> +       int count = 0;
> +
> +       for (;;) {
> +               res = platform_get_resource(pdev, IORESOURCE_IRQ, count);
platform_get_irq instead

> +               if (!res)
> +                       break;
> +               count++;
> +       }
> +       return count;
> +}
> +
> +static void  bcm_kona_gpio_set_lockcode_bank(int bank_id, int lockcode)
> +{
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
pass gpio_chip to avoid using global

> +       /* This register is password protected  */
> +       writel(BCM_GPIO_PASSWD, reg_base + GPIO_GPPWR_OFFSET);
> +       /* Lock the bank */
> +       writel(lockcode, reg_base + bcm_kona_gpio_pwd_status(bank_id));
> +}
> +
> +static inline void bcm_kona_gpio_lock_bank(int bank_id)
ditto

> +{
> +       bcm_kona_gpio_set_lockcode_bank(bank_id, LOCK_CODE);
> +}
> +
> +static inline void bcm_kona_gpio_unlock_bank(int bank_id)
ditto

> +{
> +       bcm_kona_gpio_set_lockcode_bank(bank_id, UNLOCK_CODE);
> +}
> +
> +static void bcm_kona_gpio_set(struct gpio_chip *chip, unsigned gpio,
> +       int value)
> +{
> +       int bank_id = GPIO_BANK(gpio);
> +       int bit = GPIO_BIT(gpio);
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
fetch bcm_kona_gpio from gpio_chip

> +       u32 val, reg_offset;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       /* determine the GPIO pin direction */
> +       val = readl(reg_base + bcm_kona_gpio_control(gpio));
> +       val &= GPIO_GPCTR0_IOTR_MASK;
> +
> +       /* this function only applies to output pin */
> +       if (GPIO_GPCTR0_IOTR_CMD_INPUT == val)
> +               return;
> +
> +       reg_offset = value ? bcm_kona_gpio_out_set(bank_id) :
> +                               bcm_kona_gpio_out_clear(bank_id);
> +
> +       val = readl(reg_base + reg_offset);
> +       val |= 1 << bit;
> +       writel(val, reg_base + reg_offset);
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +}
> +
> +static int bcm_kona_gpio_get(struct gpio_chip *chip, unsigned gpio)
> +{
> +       int bank_id = GPIO_BANK(gpio);
> +       int bit = GPIO_BIT(gpio);
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +       u32 val, reg_offset;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       /* determine the GPIO pin direction */
> +       val = readl(reg_base + bcm_kona_gpio_control(gpio));
> +       val &= GPIO_GPCTR0_IOTR_MASK;
> +
> +       /* read the GPIO bank status */
> +       reg_offset = (GPIO_GPCTR0_IOTR_CMD_INPUT == val) ?
> +                                       bcm_kona_gpio_in_status(bank_id) :
> +                                       bcm_kona_gpio_out_status(bank_id);
> +       val = readl(reg_base + reg_offset);
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +
> +       /* return the specified bit status */
> +       return (val >> bit) & 1;
> +}
> +
> +static int bcm_kona_gpio_direction_input(struct gpio_chip *chip,
> +       unsigned gpio)
> +{
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +       u32 val;
> +       unsigned long flags;
> +       int bank_id = GPIO_BANK(gpio);
> +
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       val = readl(reg_base + bcm_kona_gpio_control(gpio));
> +       val &= ~GPIO_GPCTR0_IOTR_MASK;
> +       val |= GPIO_GPCTR0_IOTR_CMD_INPUT;
> +       writel(val, reg_base + bcm_kona_gpio_control(gpio));
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +
> +       return 0;
> +}
> +
> +static int bcm_kona_gpio_direction_output(struct gpio_chip *chip,
> +       unsigned gpio, int value)
> +{
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +       int bank_id = GPIO_BANK(gpio);
> +       int bit = GPIO_BIT(gpio);
> +       u32 val, reg_offset;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       val = readl(reg_base + bcm_kona_gpio_control(gpio));
> +       val &= ~GPIO_GPCTR0_IOTR_MASK;
> +       val |= GPIO_GPCTR0_IOTR_CMD_0UTPUT;
> +       writel(val, reg_base + bcm_kona_gpio_control(gpio));
> +       reg_offset = value ? bcm_kona_gpio_out_set(bank_id) :
> +                               bcm_kona_gpio_out_clear(bank_id);
> +
> +       val = readl(reg_base + reg_offset);
> +       val |= 1 << bit;
> +       writel(val, reg_base + reg_offset);
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +
> +       return 0;
> +}
> +
> +static int bcm_kona_gpio_to_irq(struct gpio_chip *chip, unsigned gpio)
> +{
> +       if (gpio >= bcm_gpio_chip.ngpio)
is bcm_gpio_chip != chip ?
Globals are even if you have that value passed as argument ?

> +               return -ENXIO;
> +       return irq_create_mapping(bcm_kona_gpio.irq_domain, gpio);
> +}
> +
> +static int bcm_kona_gpio_set_debounce(struct gpio_chip *chip, unsigned gpio,
> +       unsigned debounce)
> +{
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +       u32 val, res;
> +       unsigned long flags;
> +       int bank_id = GPIO_BANK(gpio);
> +
> +       /* debounce must be 1-128ms (or 0) */
> +       if ((debounce > 0 && debounce < 1000) || debounce > 128000) {
> +               dev_err(chip->dev, "Debounce value %u not in range\n",
> +                       debounce);
> +               return -EINVAL;
> +       }
> +
> +       /* calculate debounce bit value */
> +       if (debounce != 0) {
> +               /* Convert to ms */
> +               debounce /= 1000;
> +
> +               /* find the MSB */
> +               res = fls(debounce) - 1;
> +
> +               /* Check if MSB-1 is set (round up or down) */
> +               if (res > 0 && (debounce & 1 << (res - 1)))
> +                       res++;
> +       }
> +
> +       /* spin lock for read-modify-write of the GPIO register */
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       val = readl(reg_base + bcm_kona_gpio_control(gpio));
> +       val &= ~GPIO_GPCTR0_DBR_MASK;
> +
> +       if (debounce == 0) {
> +               /* disable debounce */
> +               val &= ~GPIO_GPCTR0_DB_ENABLE_MASK;
> +       } else {
> +               val |= GPIO_GPCTR0_DB_ENABLE_MASK |
> +                         (res << GPIO_GPCTR0_DBR_SHIFT);
> +       }
> +
> +       writel(val, reg_base + bcm_kona_gpio_control(gpio));
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +
> +       return 0;
> +}
> +
> +static int bcm_kona_gpio_request(struct gpio_chip *chip, unsigned gpio)
> +{
> +       struct irq_data d;
> +
> +       d.hwirq = gpio;
> +       bcm_kona_gpio_irq_unmask(&d);
> +
> +       return 0;
> +}
> +
> +static void bcm_kona_gpio_free(struct gpio_chip *chip, unsigned gpio)
> +{
> +       struct irq_data d;
> +
> +       d.hwirq = gpio;
> +       bcm_kona_gpio_irq_mask(&d);
> +}
> +
> +static struct gpio_chip bcm_gpio_chip = {
> +       .label = "bcm-kona-gpio",
> +       .request = bcm_kona_gpio_request,
> +       .free = bcm_kona_gpio_free,
> +       .direction_input = bcm_kona_gpio_direction_input,
> +       .get = bcm_kona_gpio_get,
> +       .direction_output = bcm_kona_gpio_direction_output,
> +       .set = bcm_kona_gpio_set,
> +       .set_debounce = bcm_kona_gpio_set_debounce,
> +       .to_irq = bcm_kona_gpio_to_irq,
> +       .base = 0,
> +};
> +
> +static void bcm_kona_gpio_irq_ack(struct irq_data *d)
> +{
> +       int gpio = d->hwirq;
> +       int bank_id = GPIO_BANK(gpio);
> +       int bit = GPIO_BIT(gpio);
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +       u32 val;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       val = readl(reg_base + bcm_kona_gpio_int_status(bank_id));
> +       val |= 1 << bit;
> +       writel(val, reg_base + bcm_kona_gpio_int_status(bank_id));
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +}
> +
> +static void bcm_kona_gpio_irq_mask(struct irq_data *d)
> +{
> +       int gpio = d->hwirq;
> +       int bank_id = GPIO_BANK(gpio);
> +       int bit = GPIO_BIT(gpio);
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +       u32 val;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       val = readl(reg_base + bcm_kona_gpio_int_mask(bank_id));
> +       val |= 1 << bit;
> +       writel(val, reg_base + bcm_kona_gpio_int_mask(bank_id));
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +}
> +
> +static void bcm_kona_gpio_irq_unmask(struct irq_data *d)
> +{
> +       int gpio = d->hwirq;
> +       int bank_id = GPIO_BANK(gpio);
> +       int bit = GPIO_BIT(gpio);
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +       u32 val;
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       val = readl(reg_base + bcm_kona_gpio_int_mskclr(bank_id));
> +       val |= 1 << bit;
> +       writel(val, reg_base + bcm_kona_gpio_int_mskclr(bank_id));
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +}
> +
> +static int bcm_kona_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +       int gpio = d->hwirq;
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
irq_*_get_irq_chip_* series of helpers will be useful in these case
You have chip, chip_data, handler_data in irq_data to avoid all these
global data.

> +       u32 lvl_type;
> +       u32 val;
> +       unsigned long flags;
> +       int bank_id = GPIO_BANK(gpio);
> +
> +       switch (type & IRQ_TYPE_SENSE_MASK) {
> +       case IRQ_TYPE_EDGE_RISING:
> +               lvl_type = GPIO_GPCTR0_ITR_CMD_RISING_EDGE;
> +               break;
> +
> +       case IRQ_TYPE_EDGE_FALLING:
> +               lvl_type = GPIO_GPCTR0_ITR_CMD_FALLING_EDGE;
> +               break;
> +
> +       case IRQ_TYPE_EDGE_BOTH:
> +               lvl_type = GPIO_GPCTR0_ITR_CMD_BOTH_EDGE;
> +               break;
> +
> +       case IRQ_TYPE_LEVEL_HIGH:
> +       case IRQ_TYPE_LEVEL_LOW:
> +               /* BCM GPIO doesn't support level triggering */
> +       default:
> +               dev_err(bcm_gpio_chip.dev, "Invalid BCM GPIO irq type 0x%x\n",
> +                       type);
> +               return -EINVAL;
> +       }
> +
> +       spin_lock_irqsave(&bcm_kona_gpio.lock, flags);
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       val = readl(reg_base + bcm_kona_gpio_control(gpio));
> +       val &= ~GPIO_GPCTR0_ITR_MASK;
> +       val |= lvl_type << GPIO_GPCTR0_ITR_SHIFT;
> +       writel(val, reg_base + bcm_kona_gpio_control(gpio));
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +       spin_unlock_irqrestore(&bcm_kona_gpio.lock, flags);
> +
> +       return 0;
> +}
> +
> +static void bcm_kona_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> +{
> +       int bit, bank_id;
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +       unsigned long sta;
> +       struct bcm_kona_gpio_bank *bank = irq_get_handler_data(irq);
> +       struct irq_chip *chip = irq_desc_get_chip(desc);
> +
> +       chained_irq_enter(chip, desc);
> +
> +       bank_id = bank->id;
> +       bcm_kona_gpio_unlock_bank(bank_id);
> +
> +       sta = readl(reg_base + bcm_kona_gpio_int_status(bank_id)) &
> +           (~(readl(reg_base + bcm_kona_gpio_int_mask(bank_id))));
> +
> +       for_each_set_bit(bit, &sta, 32) {
> +               /*
> +                * Clear interrupt before handler is called so we don't
> +                * miss any interrupt occurred during executing them.
> +                */
> +               writel(readl(reg_base + bcm_kona_gpio_int_status(bank_id)) |
> +                               (1 << bit),
> +                               reg_base + bcm_kona_gpio_int_status(bank_id));
> +
> +               /* Invoke interrupt handler */
> +               generic_handle_irq(gpio_to_irq(GPIO_PER_BANK * bank_id + bit));
> +       }
> +
> +       bcm_kona_gpio_lock_bank(bank_id);
> +
> +       chained_irq_exit(chip, desc);
> +}
> +
> +static struct irq_chip bcm_gpio_irq_chip = {
> +       .name = "bcm-kona-gpio",
> +       .irq_ack = bcm_kona_gpio_irq_ack,
> +       .irq_mask = bcm_kona_gpio_irq_mask,
> +       .irq_unmask = bcm_kona_gpio_irq_unmask,
> +       .irq_set_type = bcm_kona_gpio_irq_set_type,
> +};
> +
> +static struct __initconst of_device_id bcm_kona_gpio_of_match[] = {
> +       { .compatible = "brcm,kona-gpio" },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, kona_gpio_match);
> +
> +/*
> + * This lock class tells lockdep that GPIO irqs are in a different
> + * category than their parents, so it won't report false recursion.
> + */
> +static struct lock_class_key gpio_lock_class;
> +
> +static int bcm_kona_gpio_irq_map(struct irq_domain *d, unsigned int virq,
> +                               irq_hw_number_t hwirq)
> +{
> +       irq_set_chip_data(virq, &d->host_data);
> +       irq_set_lockdep_class(virq, &gpio_lock_class);
> +       irq_set_chip_and_handler(virq, &bcm_gpio_irq_chip, handle_simple_irq);
> +       irq_set_nested_thread(virq, 1);
> +       set_irq_flags(virq, IRQF_VALID);
> +
> +       return 0;
> +}
> +
> +static void bcm_kona_gpio_irq_unmap(struct irq_domain *d, unsigned int virq)
> +{
> +       irq_set_chip_and_handler(virq, NULL, NULL);
> +       irq_set_chip_data(virq, NULL);
> +}
> +
> +static struct irq_domain_ops bcm_kona_irq_ops = {
> +       .map    = bcm_kona_gpio_irq_map,
> +       .unmap  = bcm_kona_gpio_irq_unmap,
> +       .xlate  = irq_domain_xlate_twocell,
> +};
> +
> +static void bcm_kona_gpio_reset(int num_bank)
> +{
> +       int i;
> +       void __iomem *reg_base = bcm_kona_gpio.reg_base;
> +
> +       /* disable interrupts and clear status */
> +       for (i = 0; i < num_bank; i++) {
> +               bcm_kona_gpio_unlock_bank(i);
> +               writel(0xFFFFFFFF, reg_base + bcm_kona_gpio_int_mask(i));
> +               writel(0xFFFFFFFF, reg_base + bcm_kona_gpio_int_status(i));
> +               bcm_kona_gpio_lock_bank(i);
> +       }
> +}
> +
> +static int bcm_kona_gpio_probe(struct platform_device *pdev)
> +{
> +       const struct of_device_id *match;
> +       struct resource *res;
> +       struct bcm_kona_gpio_bank *bank;
> +       int ret;
> +       int i;
> +
> +       match = of_match_device(bcm_kona_gpio_of_match, &pdev->dev);
> +       if (!match) {
> +               dev_err(&pdev->dev, "Failed to find gpio controller\n");
> +               return -ENODEV;
> +       }
> +
> +       bcm_kona_gpio.num_bank = bcm_kona_count_irq_resources(pdev);
> +       if (bcm_kona_gpio.num_bank == 0) {
> +               dev_err(&pdev->dev, "Couldn't determine # GPIO banks\n");
> +               return -ENOENT;
> +       }
> +       bcm_gpio_chip.of_node = pdev->dev.of_node;
> +       bcm_gpio_chip.dev = &pdev->dev;
> +       bcm_gpio_chip.ngpio = bcm_kona_gpio.num_bank * GPIO_PER_BANK;
> +       bcm_kona_gpio.banks = devm_kzalloc(&pdev->dev,
> +                                      bcm_kona_gpio.num_bank *
> +                                      sizeof(*bcm_kona_gpio.banks),
> +                                      GFP_KERNEL);
> +       if (!bcm_kona_gpio.banks) {
> +               dev_err(&pdev->dev, "Couldn't allocate bank structure\n");
> +               return -ENOMEM;
> +       }
> +
> +       bcm_kona_gpio.irq_base = irq_alloc_descs(-1, 0, bcm_gpio_chip.ngpio,
> +                                               0);
> +       if (bcm_kona_gpio.irq_base < 0) {
> +               dev_err(&pdev->dev, "Couldn't allocate IRQ numbers\n");
> +               return -ENXIO;
> +       }
> +       bcm_kona_gpio.irq_domain = irq_domain_add_linear(pdev->dev.of_node,
> +                                               bcm_gpio_chip.ngpio,
> +                                               &bcm_kona_irq_ops,
> +                                               &bcm_kona_gpio);
> +       if (!bcm_kona_gpio.irq_domain) {
> +               dev_err(&pdev->dev, "Couldn't allocate IRQ domain\n");
> +               ret = -ENXIO;
> +               goto err_irq_descs;
> +       }
> +       for (i = 0; i < bcm_kona_gpio.num_bank; i++) {
> +               bank = &bcm_kona_gpio.banks[i];
> +               bank->id = i;
> +               bank->irq = platform_get_irq(pdev, i);
> +               if (bank->irq < 0) {
> +                       dev_err(&pdev->dev, "Couldn't get IRQ for bank %d", i);
> +                       ret = -ENOENT;
> +                       goto err_irq_domain;
> +               }
> +       }
> +
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!res) {
> +               dev_err(&pdev->dev, "Missing MEM resource\n");
> +               ret = -ENXIO;
> +               goto err_irq_domain;
> +       }
> +
> +       bcm_kona_gpio.reg_base = devm_request_and_ioremap(&pdev->dev, res);
> +       if (!bcm_kona_gpio.reg_base) {
> +               dev_err(&pdev->dev, "Couldn't ioremap regs\n");
> +               ret = -ENXIO;
> +               goto err_irq_domain;
> +       }
> +       dev_info(&pdev->dev, "Setting up Kona GPIO at 0x%p (phys %#x)\n",
> +               bcm_kona_gpio.reg_base, res->start);
> +
> +       bcm_kona_gpio_reset(bcm_kona_gpio.num_bank);
> +
> +       ret = gpiochip_add(&bcm_gpio_chip);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "Couldn't add GPIO chip -- %d\n", ret);
> +               goto err_irq_domain;
> +       }
> +       for (i = 0; i < bcm_gpio_chip.ngpio; i++) {
> +               int irq = bcm_kona_gpio_to_irq(&bcm_gpio_chip, i);
> +               bank = &bcm_kona_gpio.banks[GPIO_BANK(i)];
> +               irq_set_lockdep_class(irq, &gpio_lock_class);
> +               irq_set_chip_data(irq, bank);
> +               irq_set_chip_and_handler(irq, &bcm_gpio_irq_chip,
> +                                        handle_simple_irq);
> +               set_irq_flags(irq, IRQF_VALID);
> +       }
> +       for (i = 0; i < bcm_kona_gpio.num_bank; i++) {
> +               bank = &bcm_kona_gpio.banks[i];
> +
> +               irq_set_chained_handler(bank->irq, bcm_kona_gpio_irq_handler);
> +               irq_set_handler_data(bank->irq, bank);
> +       }
> +
> +       spin_lock_init(&bcm_kona_gpio.lock);
> +
platform_set_drvdata to set it as bcm_kona_gpio so that you need not
access the global pointer anywhere else. In fact it can removed I think.

> +       return 0;
> +
> +err_irq_domain:
> +       irq_domain_remove(bcm_kona_gpio.irq_domain);
> +
> +err_irq_descs:
> +       irq_free_descs(bcm_kona_gpio.irq_base, bcm_gpio_chip.ngpio);
> +
> +       return ret;
> +}
> +
> +static int bcm_kona_gpio_remove(struct platform_device *pdev)
> +{
> +       int i, ret;
> +       struct bcm_kona_gpio_bank *bank;
> +
> +       for (i = 0; i < bcm_gpio_chip.ngpio; i++)
may be bcm_kona_gpio as driver data will help here ?

> +               gpio_free(i);
> +       ret = gpiochip_remove(&bcm_gpio_chip);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "Couldn't remove GPIO chip -- %d\n", ret);
> +               return ret;
> +       }
> +       for (i = 0; i < bcm_kona_gpio.num_bank; i++) {
> +               bank = &bcm_kona_gpio.banks[i];
> +               irq_set_chained_handler(bank->irq, NULL);
> +               irq_set_handler_data(bank->irq, NULL);
> +       }
> +       irq_domain_remove(bcm_kona_gpio.irq_domain);
> +       irq_free_descs(bcm_kona_gpio.irq_base, bcm_gpio_chip.ngpio);
> +
> +       return 0;
> +}
> +
> +static struct platform_driver bcm_kona_gpio_driver = {
> +       .driver = {
> +                  .name = "bcm-kona-gpio",
> +                  .owner = THIS_MODULE,
> +                  .of_match_table = bcm_kona_gpio_of_match,
> +                  },
> +       .probe = bcm_kona_gpio_probe,
> +       .remove = bcm_kona_gpio_remove,
> +};
> +
> +module_platform_driver(bcm_kona_gpio_driver);
> +
> +MODULE_AUTHOR("Broadcom");
> +MODULE_DESCRIPTION("Broadcom Kona GPIO Driver");
> +MODULE_LICENSE("GPL v2");

IMO, you don't require any global data or pointers if probe populates
all the pdev, irq/gpio_chip data member appropriately

Regards,
Sudeep

[PATCH v3 1/3] ARM: bcm281xx: Add GPIO driver

[Markus Mayer]

On 1 August 2013 11:20, Sudeep KarkadaNagesha
<Sudeep.KarkadaNagesha@arm.com> wrote:
> On 26/07/13 21:25, Markus Mayer wrote:
>> From: Markus Mayer <mmayer@broadcom.com>
>>
>> This patch adds the GPIO driver for the bcm281xx family of chips.
>>
> May be I am missing my mails, but I could not find even in archives.
> The patch says its v3, I could not trace v1/v2 and not change log.

No, you didn't miss any e-mails. We had some internal reviews and the
plan was for me to renumber the patches and start from v1 again for
the public review, but I clearly forgot. Hence v3 is really the first
public version of this patch.

> Why these macros can't be something like this ?
> #define GPIO_GPOR_OFFSET(bank)  (0x00000000 + (bank) << 2)

Done.

> Can this be embed in bcm_kona_gpio?
> You can then get rid of all the accesses to global variable
> bcm_kona_gpio.

Still working on this one, but the global variable will go away. The
same is true of the other globals.

>> +static inline u32 bcm_kona_gpio_out_status(int bank)
>> +{
>> +       return GPIO_GPOR0_OFFSET + (bank << 2);
>> +}
>> +
>> +static inline u32 bcm_kona_gpio_in_status(int bank)
>> +{
>> +       return GPIO_GPIR0_OFFSET + (bank << 2);
>> +}
>> +
>> +static inline u32 bcm_kona_gpio_out_set(int bank)
>> +{
>> +       return GPIO_GPORS0_OFFSET + (bank << 2);
>> +}
>> +
>> +static inline u32 bcm_kona_gpio_out_clear(int bank)
>> +{
>> +       return GPIO_GPORC0_OFFSET + (bank << 2);
>> +}
>> +
>> +static inline u32 bcm_kona_gpio_int_status(int bank)
>> +{
>> +       return GPIO_ISR0_OFFSET + (bank << 2);
>> +}
>> +
>> +static inline u32 bcm_kona_gpio_int_mask(int bank)
>> +{
>> +       return GPIO_IMR0_OFFSET + (bank << 2);
>> +}
>> +
>> +static inline u32 bcm_kona_gpio_int_mskclr(int bank)
>> +{
>> +       return GPIO_IMRC0_OFFSET + (bank << 2);
>> +}
>> +
>> +static inline u32 bcm_kona_gpio_pwd_status(int bank)
>> +{
>> +       return GPIO_GPPLSR0_OFFSET + (bank << 2);
>> +}
>> +
>> +static inline u32 bcm_kona_gpio_control(int gpio)
>> +{
>> +       return GPIO_GPCTR0_OFFSET + (gpio << 2);
>> +}
> All these functions can go away if you can have the macros as shown above ?

I took those functions out.

>> +static int bcm_kona_count_irq_resources(struct platform_device *pdev)
>> +{
>> +       struct resource *res;
>> +       int count = 0;
>> +
>> +       for (;;) {
>> +               res = platform_get_resource(pdev, IORESOURCE_IRQ, count);
> platform_get_irq instead

Done.

>> +               if (!res)
>> +                       break;
>> +               count++;
>> +       }
>> +       return count;
>> +}

> IMO, you don't require any global data or pointers if probe populates
> all the pdev, irq/gpio_chip data member appropriately

Yes, they'll go away in the next revision of this driver which I am
hoping to post shortly.

Thanks,
-Markus

-- 
Markus Mayer
Broadcom Landing Team

[PATCH v3 2/3] ARM: bcm281xx: Device Tree bindings for GPIO driver

[Linus Walleij]

On Fri, Jul 26, 2013 at 10:25 PM, Markus Mayer <markus.mayer@linaro.org> wrote:

> --- a/arch/arm/boot/dts/bcm11351.dtsi
> +++ b/arch/arm/boot/dts/bcm11351.dtsi
> @@ -63,6 +63,22 @@
>                 clock-frequency = <32768>;
>         };
>
> +       gpio: gpio at 35003000 {
> +               compatible = "brcm,kona-gpio";
> +               reg = <0x35003000 0x800>;
> +               interrupts =
> +                      <0x0 106 0x4
> +                       0x0 115 0x4
> +                       0x0 114 0x4
> +                       0x0 113 0x4
> +                       0x0 112 0x4
> +                       0x0 111 0x4>;
> +               #gpio-cells = <2>;
> +               #interrupt-cells = <2>;
> +               gpio-controller;
> +               interrupt-controller;
> +       };

Do this:

#include <dt-bindings//interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>

Rewrite like this:

+               interrupts =
+                      <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH
+                       GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH

As you can see we can now understand the dts file.

Possibly this should also be done in the examples.

Yours,
Linus Walleij

[PATCH v3 2/3] ARM: bcm281xx: Device Tree bindings for GPIO driver

[Markus Mayer]

On 16 August 2013 05:51, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Fri, Jul 26, 2013 at 10:25 PM, Markus Mayer <markus.mayer@linaro.org> wrote:
>
>> --- a/arch/arm/boot/dts/bcm11351.dtsi
>> +++ b/arch/arm/boot/dts/bcm11351.dtsi
>> @@ -63,6 +63,22 @@
>>                 clock-frequency = <32768>;
>>         };
>>
>> +       gpio: gpio at 35003000 {
>> +               compatible = "brcm,kona-gpio";
>> +               reg = <0x35003000 0x800>;
>> +               interrupts =
>> +                      <0x0 106 0x4
>> +                       0x0 115 0x4
>> +                       0x0 114 0x4
>> +                       0x0 113 0x4
>> +                       0x0 112 0x4
>> +                       0x0 111 0x4>;
>> +               #gpio-cells = <2>;
>> +               #interrupt-cells = <2>;
>> +               gpio-controller;
>> +               interrupt-controller;
>> +       };
>
> Do this:
>
> #include <dt-bindings//interrupt-controller/irq.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
>
> Rewrite like this:
>
> +               interrupts =
> +                      <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH
> +                       GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH
>
> As you can see we can now understand the dts file.
>
> Possibly this should also be done in the examples.

Thanks for the feedback. This change will be part of the next revision
that I am about to send out (in dtsi & examples).

Regards,
-Markus

-- 
Markus Mayer
Broadcom Landing Team