[PATCH 0/2] gpio: brcm: XGS iProc GPIO driver

[PATCH 0/2] gpio: brcm: XGS iProc GPIO driver

[Chris Packham]

This is ported this from Broadcom's XLDK. There seem to be 3 different
IP blocks for 3 separate banks of GPIOs in the iProc chips.

I've dropped everything except support for the Chip Common A GPIO
controller because the other blocks actually seem to be supportable with
other drivers. The driver itself is halfway between pinctrl-nsp-gpio.c
and pinctrl-iproc-gpio.c.

Chris Packham (2):
  dt-bindings: gpio: brcm: Add bindings for xgs-iproc
  gpio: Add xgs-iproc driver

 .../bindings/gpio/brcm,xgs-iproc.txt          |  41 ++
 drivers/gpio/Kconfig                          |   9 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-xgs-iproc.c                 | 422 ++++++++++++++++++
 4 files changed, 473 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt
 create mode 100644 drivers/gpio/gpio-xgs-iproc.c

-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] dt-bindings: gpio: brcm: Add bindings for xgs-iproc

[Chris Packham]

This GPIO controller is present on a number of Broadcom switch ASICs
with integrated SoCs. It is similar to the nsp-gpio and iproc-gpio
blocks but different enough to require a separate driver.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 .../bindings/gpio/brcm,xgs-iproc.txt          | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt

diff --git a/Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt b/Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt
new file mode 100644
index 000000000000..328b844c82dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt
@@ -0,0 +1,41 @@
+Broadcom XGS iProc GPIO controller
+
+This controller is the Chip Common A GPIO present on a number of Broadcom
+switch ASICs with integrated SoCs.
+
+Required properties:
+- compatible:
+    Must be "brcm,iproc-gpio-cca"
+
+- reg:
+    The first region defines the base I/O address containing
+    the GPIO controller registers. The second region defines
+    the I/O address containing the Chip Common A interrupt
+    registers.
+
+Optional properties:
+
+- interrupts:
+    The interrupt shared by all GPIO lines for this controller.
+
+- #interrupt-cells:
+    Should be <2>.  The first cell is the GPIO number, the second should specify
+    flags.
+
+    See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+
+- interrupt-controller:
+    Marks the device node as an interrupt controller
+
+Example:
+	gpioa: gpio@18000060 {
+		compatible = "brcm,iproc-gpio-cca";
+		#gpio-cells = <2>;
+		reg = <0x18000060 0x50>,
+		      <0x18000000 0x50>;
+		ngpios = <12>;
+		gpio-controller;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+	};
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] gpio: Add xgs-iproc driver

[Chris Packham]

This driver supports the Chip Common A GPIO controller present on a
number of Broadcom switch ASICs with integrated SoCs. The controller is
similar to the pinctrl-nsp-gpio and pinctrl-iproc-gpio blocks but
different enough that a separate driver is required.

This has been ported from Broadcom's XLDK 5.0.3 retaining only the CCA
support (pinctrl-iproc-gpio covers CCB).

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/gpio/Kconfig          |   9 +
 drivers/gpio/Makefile         |   1 +
 drivers/gpio/gpio-xgs-iproc.c | 422 ++++++++++++++++++++++++++++++++++
 3 files changed, 432 insertions(+)
 create mode 100644 drivers/gpio/gpio-xgs-iproc.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 38e096e6925f..4b3c0f8397d7 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -156,6 +156,15 @@ config GPIO_BRCMSTB
 	help
 	  Say yes here to enable GPIO support for Broadcom STB (BCM7XXX) SoCs.
 
+config GPIO_XGS_IPROC
+	tristate "BRCM XGS iProc GPIO support"
+	depends on OF_GPIO && (ARCH_BCM_IPROC || COMPILE_TEST)
+	select GPIO_GENERIC
+	select GPIOLIB_IRQCHIP
+	default ARCH_BCM_IPROC
+	help
+	  Say yes here to enable GPIO support for Broadcom XGS iProc SoCs.
+
 config GPIO_CADENCE
 	tristate "Cadence GPIO support"
 	depends on OF_GPIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d2fd19c15bae..3783c3d43fbe 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -37,6 +37,7 @@ obj-$(CONFIG_GPIO_BCM_KONA)		+= gpio-bcm-kona.o
 obj-$(CONFIG_GPIO_BD70528)		+= gpio-bd70528.o
 obj-$(CONFIG_GPIO_BD9571MWV)		+= gpio-bd9571mwv.o
 obj-$(CONFIG_GPIO_BRCMSTB)		+= gpio-brcmstb.o
+obj-$(CONFIG_GPIO_XGS_IPROC)		+= gpio-xgs-iproc.o
 obj-$(CONFIG_GPIO_BT8XX)		+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CADENCE)		+= gpio-cadence.o
 obj-$(CONFIG_GPIO_CLPS711X)		+= gpio-clps711x.o
diff --git a/drivers/gpio/gpio-xgs-iproc.c b/drivers/gpio/gpio-xgs-iproc.c
new file mode 100644
index 000000000000..12656ca7b9d4
--- /dev/null
+++ b/drivers/gpio/gpio-xgs-iproc.c
@@ -0,0 +1,422 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 Broadcom Corporation
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+
+#define CCA_INT_F_GPIOINT		BIT(0)
+#define CCA_INT_STS			0x20
+#define CCA_INT_MASK			0x24
+
+#define GPIO_CCA_DIN			0x0
+#define GPIO_CCA_DOUT			0x4
+#define GPIO_CCA_OUT_EN			0x8
+#define GPIO_CCA_INT_LEVEL		0x10
+#define GPIO_CCA_INT_LEVEL_MASK		0x14
+#define GPIO_CCA_INT_EVENT		0x18
+#define GPIO_CCA_INT_EVENT_MASK		0x1C
+#define GPIO_CCA_INT_EDGE		0x24
+
+struct iproc_gpio_chip {
+	struct device *dev;
+	void __iomem *base;
+	void __iomem *intr;
+	struct gpio_chip gc;
+	struct irq_chip irqchip;
+	spinlock_t lock;
+	struct irq_domain *irq_domain;
+};
+
+static u32 iproc_gpio_readl(struct iproc_gpio_chip *chip, int reg)
+{
+	return readl(chip->base + reg);
+}
+
+static void iproc_gpio_writel(struct iproc_gpio_chip *chip, u32 val, int reg)
+{
+	writel(val, chip->base + reg);
+}
+
+/* returns the corresponding gpio register bit */
+static int iproc_irq_to_gpio(struct iproc_gpio_chip *chip, u32 irq)
+{
+	struct irq_data *data = irq_domain_get_irq_data(chip->irq_domain, irq);
+
+	return data->hwirq;
+}
+
+static void iproc_gpio_irq_ack(struct irq_data *d)
+{
+	u32 irq = d->irq;
+	struct iproc_gpio_chip *chip = irq_get_chip_data(irq);
+	int pin;
+	u32 irq_type, event_status = 0;
+
+	pin = iproc_irq_to_gpio(chip, d->irq);
+	irq_type = irq_get_trigger_type(irq);
+	if (irq_type & IRQ_TYPE_EDGE_BOTH) {
+		event_status |= BIT(pin);
+		iproc_gpio_writel(chip, event_status, GPIO_CCA_INT_EVENT);
+	}
+}
+
+static void iproc_gpio_irq_unmask(struct irq_data *d)
+{
+	u32 irq = d->irq;
+	struct iproc_gpio_chip *chip = irq_get_chip_data(irq);
+	int pin;
+	u32 int_mask, irq_type, event_mask;
+
+	pin = iproc_irq_to_gpio(chip, irq);
+	irq_type = irq_get_trigger_type(irq);
+	event_mask = iproc_gpio_readl(chip, GPIO_CCA_INT_EVENT_MASK);
+	int_mask = iproc_gpio_readl(chip, GPIO_CCA_INT_LEVEL_MASK);
+
+	if (irq_type & IRQ_TYPE_EDGE_BOTH) {
+		event_mask |= 1 << pin;
+		iproc_gpio_writel(chip, event_mask, GPIO_CCA_INT_EVENT_MASK);
+	} else {
+		int_mask |= 1 << pin;
+		iproc_gpio_writel(chip, int_mask, GPIO_CCA_INT_LEVEL_MASK);
+	}
+}
+
+static void iproc_gpio_irq_mask(struct irq_data *d)
+{
+	u32 irq = d->irq;
+	struct iproc_gpio_chip *chip = irq_get_chip_data(irq);
+	int pin;
+	u32 irq_type, int_mask, event_mask;
+
+	pin = iproc_irq_to_gpio(chip, irq);
+	irq_type = irq_get_trigger_type(irq);
+	event_mask = iproc_gpio_readl(chip, GPIO_CCA_INT_EVENT_MASK);
+	int_mask = iproc_gpio_readl(chip, GPIO_CCA_INT_LEVEL_MASK);
+
+	if (irq_type & IRQ_TYPE_EDGE_BOTH) {
+		event_mask &= ~BIT(pin);
+		iproc_gpio_writel(chip, event_mask, GPIO_CCA_INT_EVENT_MASK);
+	} else {
+		int_mask &= ~BIT(pin);
+		iproc_gpio_writel(chip, int_mask, GPIO_CCA_INT_LEVEL_MASK);
+	}
+}
+
+
+static int iproc_gpio_irq_set_type(struct irq_data *d, u32 type)
+{
+	u32 irq = d->irq;
+	struct iproc_gpio_chip *chip = irq_get_chip_data(irq);
+	int pin;
+	u32 event_pol, int_pol;
+
+	pin = iproc_irq_to_gpio(chip, irq);
+
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_EDGE_RISING:
+		event_pol = iproc_gpio_readl(chip, GPIO_CCA_INT_EDGE);
+		event_pol &= ~BIT(pin);
+		iproc_gpio_writel(chip, event_pol, GPIO_CCA_INT_EDGE);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		event_pol = iproc_gpio_readl(chip, GPIO_CCA_INT_EDGE);
+		event_pol |= BIT(pin);
+		iproc_gpio_writel(chip, event_pol, GPIO_CCA_INT_EDGE);
+		break;
+	case IRQ_TYPE_LEVEL_HIGH:
+		int_pol = iproc_gpio_readl(chip, GPIO_CCA_INT_LEVEL);
+		int_pol &= ~BIT(pin);
+		iproc_gpio_writel(chip, int_pol, GPIO_CCA_INT_LEVEL);
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		int_pol = iproc_gpio_readl(chip, GPIO_CCA_INT_LEVEL);
+		int_pol |= BIT(pin);
+		iproc_gpio_writel(chip, int_pol, GPIO_CCA_INT_LEVEL);
+		break;
+	default:
+		/* should not come here */
+		return -EINVAL;
+	}
+
+	if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_LEVEL_HIGH))
+		irq_set_handler_locked(irq_get_irq_data(irq), handle_level_irq);
+	else if (type & (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
+		irq_set_handler_locked(irq_get_irq_data(irq), handle_edge_irq);
+
+	return 0;
+}
+
+static irqreturn_t iproc_gpio_irq_handler(int irq, void *data)
+{
+	struct iproc_gpio_chip *chip = (struct iproc_gpio_chip *)data;
+	struct gpio_chip gc = chip->gc;
+	int bit;
+	unsigned long int_bits = 0;
+	u32 int_status;
+
+	/* go through the entire GPIOs and handle all interrupts */
+	int_status = readl(chip->intr + CCA_INT_STS);
+	if (int_status & CCA_INT_F_GPIOINT) {
+		u32 event, level;
+
+		/* Get level and edge interrupts */
+		event = readl(chip->base + GPIO_CCA_INT_EVENT_MASK);
+		event &= readl(chip->base + GPIO_CCA_INT_EVENT);
+		level = readl(chip->base + GPIO_CCA_DIN);
+		level ^= readl(chip->base + GPIO_CCA_INT_LEVEL);
+		level &= readl(chip->base + GPIO_CCA_INT_LEVEL_MASK);
+		int_bits = level | event;
+
+		for_each_set_bit(bit, &int_bits, gc.ngpio)
+			generic_handle_irq(
+				irq_linear_revmap(chip->irq_domain, bit));
+	}
+
+	return  int_bits ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int iproc_gpiolib_input(struct gpio_chip *gc, u32 gpio)
+{
+	struct iproc_gpio_chip *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32  val;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	val = iproc_gpio_readl(chip, GPIO_CCA_OUT_EN);
+	val &= ~BIT(gpio);
+	iproc_gpio_writel(chip, val, GPIO_CCA_OUT_EN);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static int iproc_gpiolib_output(struct gpio_chip *gc, u32 gpio, int value)
+{
+	struct iproc_gpio_chip *chip = gpiochip_get_data(gc);
+	unsigned long flags, val;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	val = iproc_gpio_readl(chip, GPIO_CCA_OUT_EN);
+	val |= BIT(gpio);
+	iproc_gpio_writel(chip, val, GPIO_CCA_OUT_EN);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	return 0;
+}
+
+static void iproc_gpiolib_set(struct gpio_chip *gc, u32 gpio, int value)
+{
+	struct iproc_gpio_chip *chip = gpiochip_get_data(gc);
+	unsigned long flags, val;
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	val = iproc_gpio_readl(chip, GPIO_CCA_OUT_EN);
+	val &= BIT(gpio);
+
+	/* this function only applies to output pin */
+	if (!val)
+		return;
+
+	val = iproc_gpio_readl(chip, GPIO_CCA_DOUT);
+	if (value == 0)
+		/* Set the pin to zero */
+		val &= ~BIT(gpio);
+	else
+		/* Set the pin to 1 */
+		val |= BIT(gpio);
+
+	iproc_gpio_writel(chip, val, GPIO_CCA_DOUT);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+}
+
+static int iproc_gpiolib_get(struct gpio_chip *gc, u32 gpio)
+{
+	struct iproc_gpio_chip *chip = gpiochip_get_data(gc);
+	unsigned long flags;
+	u32 val, is_out;
+	/* GPIO register bit */
+
+	spin_lock_irqsave(&chip->lock, flags);
+
+	/* determine the GPIO pin direction */
+	is_out = iproc_gpio_readl(chip, GPIO_CCA_OUT_EN);
+	is_out &= BIT(gpio);
+
+	if (is_out)
+		val = iproc_gpio_readl(chip, GPIO_CCA_DOUT);
+	else
+		val = iproc_gpio_readl(chip, GPIO_CCA_DIN);
+
+	spin_unlock_irqrestore(&chip->lock, flags);
+
+	return !!(val & BIT(gpio));
+}
+
+static int iproc_gpiolib_to_irq(struct gpio_chip *gc, u32 offset)
+{
+	struct iproc_gpio_chip *chip = gpiochip_get_data(gc);
+
+	return irq_linear_revmap(chip->irq_domain, offset);
+}
+
+static int iproc_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *dn = pdev->dev.of_node;
+	struct iproc_gpio_chip *chip;
+	u32 num_gpios;
+	int irq, ret;
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->dev = dev;
+	platform_set_drvdata(pdev, chip);
+
+	chip->gc.label = dev_name(dev);
+
+	chip->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(chip->base))
+		return PTR_ERR(chip->base);
+
+	/* Get number of GPIO pin */
+	if (of_property_read_u32(dn, "ngpios", &num_gpios)) {
+		dev_err(dev, "missing ngpios DT property\n");
+		return -EINVAL;
+	}
+	chip->gc.ngpio = num_gpios;
+	chip->gc.parent = dev;
+	chip->gc.of_node = dn;
+	chip->gc.direction_input = iproc_gpiolib_input;
+	chip->gc.direction_output = iproc_gpiolib_output;
+	chip->gc.set = iproc_gpiolib_set;
+	chip->gc.get = iproc_gpiolib_get;
+	chip->gc.to_irq = iproc_gpiolib_to_irq;
+
+	ret = gpiochip_add_data(&chip->gc, chip);
+	if (ret) {
+		dev_err(dev, "unable to add GPIO chip\n");
+		return ret;
+	}
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq > 0) {
+		u32 val, count;
+		struct irq_chip *irqc;
+
+		irqc = &chip->irqchip;
+		irqc->name = dev_name(dev);
+		irqc->irq_ack = iproc_gpio_irq_ack;
+		irqc->irq_mask = iproc_gpio_irq_mask;
+		irqc->irq_unmask = iproc_gpio_irq_unmask;
+		irqc->irq_set_type = iproc_gpio_irq_set_type;
+		irqc->irq_enable = iproc_gpio_irq_unmask;
+		irqc->irq_disable = iproc_gpio_irq_mask;
+
+		chip->intr = devm_platform_ioremap_resource(pdev, 1);
+		if (IS_ERR(chip->intr))
+			return PTR_ERR(chip->intr);
+
+		/* Create irq domain */
+		chip->irq_domain = irq_domain_add_linear(dn, num_gpios,
+				&irq_domain_simple_ops, chip);
+
+		if (!chip->irq_domain) {
+			dev_err(dev, "Couldn't allocate IRQ domain\n");
+			ret = -ENODEV;
+			goto err_irq_domain;
+		}
+
+		/* Map each gpio pin to an IRQ and set the handler */
+		for (count = 0; count < num_gpios; count++) {
+			int irq;
+
+			irq = irq_create_mapping(chip->irq_domain, count);
+			irq_set_chip_and_handler(irq, irqc, handle_simple_irq);
+			irq_set_chip_data(irq, chip);
+		}
+
+		/* Enable GPIO interrupts for CCA GPIO */
+		val = readl(chip->intr + CCA_INT_MASK);
+		val |= CCA_INT_F_GPIOINT;
+		writel(val, chip->intr + CCA_INT_MASK);
+
+		/* Install ISR for this GPIO controller */
+		ret = devm_request_irq(dev, irq, iproc_gpio_irq_handler,
+				       IRQF_SHARED, chip->gc.label, chip);
+		if (ret) {
+			dev_err(dev, "Fail to request IRQ%d: %d\n", irq, ret);
+			goto err_irq_request;
+		}
+	}
+
+	return 0;
+
+err_irq_request:
+	irq_domain_remove(chip->irq_domain);
+	chip->irq_domain = NULL;
+
+err_irq_domain:
+	gpiochip_remove(&chip->gc);
+
+	return ret;
+}
+
+static int __exit iproc_gpio_remove(struct platform_device *pdev)
+{
+	struct iproc_gpio_chip *chip;
+
+	chip = platform_get_drvdata(pdev);
+	if (!chip)
+		return -ENODEV;
+
+	if (chip->intr) {
+		u32 val;
+
+		val = readl(chip->intr + CCA_INT_MASK);
+		val &= ~CCA_INT_F_GPIOINT;
+		writel(val, chip->intr + CCA_INT_MASK);
+	}
+
+	gpiochip_remove(&chip->gc);
+
+	return 0;
+}
+
+static const struct of_device_id bcm_iproc_gpio_of_match[] __initconst = {
+	{ .compatible = "brcm,iproc-gpio-cca" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, bcm_iproc_gpio_of_match);
+
+static struct platform_driver bcm_iproc_gpio_driver = {
+	.driver = {
+		.name = "iproc-xgs-gpio",
+		.owner = THIS_MODULE,
+		.of_match_table = bcm_iproc_gpio_of_match,
+	},
+	.probe = iproc_gpio_probe,
+	.remove = iproc_gpio_remove,
+};
+
+module_platform_driver(bcm_iproc_gpio_driver);
+
+MODULE_DESCRIPTION("XGS IPROC GPIO driver");
+MODULE_LICENSE("GPL v2");
-- 
2.23.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] gpio: Add xgs-iproc driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

Hi Chris,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[cannot apply to v5.4-rc1 next-20191003]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Chris-Packham/gpio-brcm-XGS-iProc-GPIO-driver/20191004-093320
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=ia64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> ERROR: "ia64_delay_loop" undefined!
>> ERROR: "ia64_delay_loop" undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54589 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] gpio: Add xgs-iproc driver

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]

Hi Chris,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on gpio/for-next]
[cannot apply to v5.4-rc1 next-20191003]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Chris-Packham/gpio-brcm-XGS-iProc-GPIO-driver/20191004-093320
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> WARNING: drivers/gpio/gpio-xgs-iproc.o(.data+0x2a): Section mismatch in reference from the variable bcm_iproc_gpio_driver to the variable .init.rodata:bcm_iproc_gpio_of_match
   The variable bcm_iproc_gpio_driver references
   the variable __initconst bcm_iproc_gpio_of_match
   If the reference is valid then annotate the
   variable with or __refdata (see linux/init.h) or name the variable:
   

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50932 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] gpio: Add xgs-iproc driver

[Linus Walleij]

Hi Chris!

Thanks for your patch!

On Fri, Oct 4, 2019 at 3:25 AM Chris Packham
<chris.packham@alliedtelesis.co.nz> wrote:

> This driver supports the Chip Common A GPIO controller present on a
> number of Broadcom switch ASICs with integrated SoCs. The controller is
> similar to the pinctrl-nsp-gpio and pinctrl-iproc-gpio blocks but
> different enough that a separate driver is required.
>
> This has been ported from Broadcom's XLDK 5.0.3 retaining only the CCA
> support (pinctrl-iproc-gpio covers CCB).
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

(...)

> +config GPIO_XGS_IPROC
> +       tristate "BRCM XGS iProc GPIO support"
> +       depends on OF_GPIO && (ARCH_BCM_IPROC || COMPILE_TEST)
> +       select GPIO_GENERIC
> +       select GPIOLIB_IRQCHIP

Nice reuse of abstractions.

> +static u32 iproc_gpio_readl(struct iproc_gpio_chip *chip, int reg)
> +{
> +       return readl(chip->base + reg);
> +}
> +
> +static void iproc_gpio_writel(struct iproc_gpio_chip *chip, u32 val, int reg)
> +{
> +       writel(val, chip->base + reg);
> +}

These wrappers don't really add anything do they? Just inline the
direct readl()/writel() to base + reg everywhere instead.

> +/* returns the corresponding gpio register bit */
> +static int iproc_irq_to_gpio(struct iproc_gpio_chip *chip, u32 irq)
> +{
> +       struct irq_data *data = irq_domain_get_irq_data(chip->irq_domain, irq);
> +
> +       return data->hwirq;
> +}

I would name it something clearer since "gpio" is pretty ambigous.

Like iproc_irq_to_gpio_offset()

Maybe this is also a bit of unnecessary wrapper?

> +static irqreturn_t iproc_gpio_irq_handler(int irq, void *data)
> +{
> +       struct iproc_gpio_chip *chip = (struct iproc_gpio_chip *)data;
> +       struct gpio_chip gc = chip->gc;
> +       int bit;
> +       unsigned long int_bits = 0;
> +       u32 int_status;
> +
> +       /* go through the entire GPIOs and handle all interrupts */
> +       int_status = readl(chip->intr + CCA_INT_STS);
> +       if (int_status & CCA_INT_F_GPIOINT) {
> +               u32 event, level;
> +
> +               /* Get level and edge interrupts */
> +               event = readl(chip->base + GPIO_CCA_INT_EVENT_MASK);
> +               event &= readl(chip->base + GPIO_CCA_INT_EVENT);
> +               level = readl(chip->base + GPIO_CCA_DIN);
> +               level ^= readl(chip->base + GPIO_CCA_INT_LEVEL);
> +               level &= readl(chip->base + GPIO_CCA_INT_LEVEL_MASK);
> +               int_bits = level | event;
> +
> +               for_each_set_bit(bit, &int_bits, gc.ngpio)
> +                       generic_handle_irq(
> +                               irq_linear_revmap(chip->irq_domain, bit));
> +       }
> +
> +       return  int_bits ? IRQ_HANDLED : IRQ_NONE;
> +}

I think this should be a chained interrupt handler (see below how to
register it).

See e.g. drivers/gpio/gpio-ftgpio010.c for an example:
change function prototype, no return value, use
chained_irq_enter/exit(irqchip, desc); etc.

> +static int iproc_gpiolib_input(struct gpio_chip *gc, u32 gpio)
> +static int iproc_gpiolib_output(struct gpio_chip *gc, u32 gpio, int value)
> +static void iproc_gpiolib_set(struct gpio_chip *gc, u32 gpio, int value)
> +static int iproc_gpiolib_get(struct gpio_chip *gc, u32 gpio)

These callbacks seems to reimplement parts of GPIO_GENERIC
that you should already be using.

Again look at drivers/gpio/gpio-ftgpio010.c() use bgpio_init()
to set up the library callbacks, look in
drivers/gpio/gpio-mmio.c for kerneldoc on the function.

> +static int iproc_gpiolib_to_irq(struct gpio_chip *gc, u32 offset)
> +{
> +       struct iproc_gpio_chip *chip = gpiochip_get_data(gc);
> +
> +       return irq_linear_revmap(chip->irq_domain, offset);
> +}

GPIOLIB_IRQCHIP provides a .to_irq() implementation
so drop this and let the library handle this.

> +static int iproc_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *dn = pdev->dev.of_node;
> +       struct iproc_gpio_chip *chip;
> +       u32 num_gpios;
> +       int irq, ret;
> +
> +       chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +       if (!chip)
> +               return -ENOMEM;
> +
> +       chip->dev = dev;
> +       platform_set_drvdata(pdev, chip);
> +
> +       chip->gc.label = dev_name(dev);
> +
> +       chip->base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(chip->base))
> +               return PTR_ERR(chip->base);
> +
> +       /* Get number of GPIO pin */
> +       if (of_property_read_u32(dn, "ngpios", &num_gpios)) {
> +               dev_err(dev, "missing ngpios DT property\n");
> +               return -EINVAL;
> +       }

Maybe provide a sensible default?

> +       chip->gc.ngpio = num_gpios;
> +       chip->gc.parent = dev;
> +       chip->gc.of_node = dn;
> +       chip->gc.direction_input = iproc_gpiolib_input;
> +       chip->gc.direction_output = iproc_gpiolib_output;
> +       chip->gc.set = iproc_gpiolib_set;
> +       chip->gc.get = iproc_gpiolib_get;

Drop this and call bgpio_init() to set up the callbacks
instead. However, set up .ngpio *after* calling
bgpio_init() so users can't access the nonexisting
gpios.

> +       chip->gc.to_irq = iproc_gpiolib_to_irq;

Drop this and use the GPIOLIB_IRQCHIP.

> +       ret = gpiochip_add_data(&chip->gc, chip);
> +       if (ret) {
> +               dev_err(dev, "unable to add GPIO chip\n");
> +               return ret;
> +       }

Why not use devm_gpiochip_add_data()?

> +       irq = platform_get_irq(pdev, 0);
> +       if (irq > 0) {
> +               u32 val, count;
> +               struct irq_chip *irqc;
> +
> +               irqc = &chip->irqchip;
> +               irqc->name = dev_name(dev);
> +               irqc->irq_ack = iproc_gpio_irq_ack;
> +               irqc->irq_mask = iproc_gpio_irq_mask;
> +               irqc->irq_unmask = iproc_gpio_irq_unmask;
> +               irqc->irq_set_type = iproc_gpio_irq_set_type;
> +               irqc->irq_enable = iproc_gpio_irq_unmask;
> +               irqc->irq_disable = iproc_gpio_irq_mask;
> +               chip->intr = devm_platform_ioremap_resource(pdev, 1);
> +               if (IS_ERR(chip->intr))
> +                       return PTR_ERR(chip->intr);

Move all this above the [devm_]gpiochip_add_data()
call and add:

struct gpio_irq_chip *girq;

girq = &chip->gc.irq;
girq->chip = irqc;

etc follow the pattern from other drivers like the
mentioned ftgpio010.c.

> +               /* Create irq domain */
> +               chip->irq_domain = irq_domain_add_linear(dn, num_gpios,
> +                               &irq_domain_simple_ops, chip);
> +
> +               if (!chip->irq_domain) {
> +                       dev_err(dev, "Couldn't allocate IRQ domain\n");
> +                       ret = -ENODEV;
> +                       goto err_irq_domain;
> +               }
> +
> +               /* Map each gpio pin to an IRQ and set the handler */
> +               for (count = 0; count < num_gpios; count++) {
> +                       int irq;
> +
> +                       irq = irq_create_mapping(chip->irq_domain, count);
> +                       irq_set_chip_and_handler(irq, irqc, handle_simple_irq);
> +                       irq_set_chip_data(irq, chip);
> +               }

Drop this and let the generic GPIO irqchip handle
the domain.

> +               /* Enable GPIO interrupts for CCA GPIO */
> +               val = readl(chip->intr + CCA_INT_MASK);
> +               val |= CCA_INT_F_GPIOINT;
> +               writel(val, chip->intr + CCA_INT_MASK);

Move this before registering the chip.

> +               /* Install ISR for this GPIO controller */
> +               ret = devm_request_irq(dev, irq, iproc_gpio_irq_handler,
> +                                      IRQF_SHARED, chip->gc.label, chip);
> +               if (ret) {
> +                       dev_err(dev, "Fail to request IRQ%d: %d\n", irq, ret);
> +                       goto err_irq_request;
> +               }

Drop this and use the gpiolib irqchip library.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] gpio: Add xgs-iproc driver

[Chris Packham]

On Fri, 2019-10-11 at 09:43 +0200, Linus Walleij wrote:
> Hi Chris!
> 
> Thanks for your patch!
> 
> On Fri, Oct 4, 2019 at 3:25 AM Chris Packham
> <chris.packham@alliedtelesis.co.nz> wrote:
> 
> > This driver supports the Chip Common A GPIO controller present on a
> > number of Broadcom switch ASICs with integrated SoCs. The controller is
> > similar to the pinctrl-nsp-gpio and pinctrl-iproc-gpio blocks but
> > different enough that a separate driver is required.
> > 
> > This has been ported from Broadcom's XLDK 5.0.3 retaining only the CCA
> > support (pinctrl-iproc-gpio covers CCB).
> > 
> > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> 
> (...)
> 
> > +config GPIO_XGS_IPROC
> > +       tristate "BRCM XGS iProc GPIO support"
> > +       depends on OF_GPIO && (ARCH_BCM_IPROC || COMPILE_TEST)
> > +       select GPIO_GENERIC
> > +       select GPIOLIB_IRQCHIP
> 
> Nice reuse of abstractions.
> 
> > +static u32 iproc_gpio_readl(struct iproc_gpio_chip *chip, int reg)
> > +{
> > +       return readl(chip->base + reg);
> > +}
> > +
> > +static void iproc_gpio_writel(struct iproc_gpio_chip *chip, u32 val, int reg)
> > +{
> > +       writel(val, chip->base + reg);
> > +}
> 
> These wrappers don't really add anything do they? Just inline the
> direct readl()/writel() to base + reg everywhere instead.
> 

Will do.

> > +/* returns the corresponding gpio register bit */
> > +static int iproc_irq_to_gpio(struct iproc_gpio_chip *chip, u32 irq)
> > +{
> > +       struct irq_data *data = irq_domain_get_irq_data(chip->irq_domain, irq);
> > +
> > +       return data->hwirq;
> > +}
> 
> I would name it something clearer since "gpio" is pretty ambigous.
> 
> Like iproc_irq_to_gpio_offset()
> 
> Maybe this is also a bit of unnecessary wrapper?
> 

I'll look into it. We might already have the irq_data we need so the
callers could use "pin = d->hwirq;".

> > +static irqreturn_t iproc_gpio_irq_handler(int irq, void *data)
> > +{
> > +       struct iproc_gpio_chip *chip = (struct iproc_gpio_chip *)data;
> > +       struct gpio_chip gc = chip->gc;
> > +       int bit;
> > +       unsigned long int_bits = 0;
> > +       u32 int_status;
> > +
> > +       /* go through the entire GPIOs and handle all interrupts */
> > +       int_status = readl(chip->intr + CCA_INT_STS);
> > +       if (int_status & CCA_INT_F_GPIOINT) {
> > +               u32 event, level;
> > +
> > +               /* Get level and edge interrupts */
> > +               event = readl(chip->base + GPIO_CCA_INT_EVENT_MASK);
> > +               event &= readl(chip->base + GPIO_CCA_INT_EVENT);
> > +               level = readl(chip->base + GPIO_CCA_DIN);
> > +               level ^= readl(chip->base + GPIO_CCA_INT_LEVEL);
> > +               level &= readl(chip->base + GPIO_CCA_INT_LEVEL_MASK);
> > +               int_bits = level | event;
> > +
> > +               for_each_set_bit(bit, &int_bits, gc.ngpio)
> > +                       generic_handle_irq(
> > +                               irq_linear_revmap(chip->irq_domain, bit));
> > +       }
> > +
> > +       return  int_bits ? IRQ_HANDLED : IRQ_NONE;
> > +}
> 
> I think this should be a chained interrupt handler (see below how to
> register it).
> 
> See e.g. drivers/gpio/gpio-ftgpio010.c for an example:
> change function prototype, no return value, use
> chained_irq_enter/exit(irqchip, desc); etc.
> 

I don't think a chained interrupt handler can work. The problem is that
the parent irq on the SoC is shared between the gpio and uart0 (why
it's this way with two IP blocks in the same SoC I'll never know). When
a chained interrupt handler is registered I lose the serial interrupts.
Please correct me if there is some way to make the chained handlers
deal with sharing interrupts.

> > +static int iproc_gpiolib_input(struct gpio_chip *gc, u32 gpio)
> > +static int iproc_gpiolib_output(struct gpio_chip *gc, u32 gpio, int value)
> > +static void iproc_gpiolib_set(struct gpio_chip *gc, u32 gpio, int value)
> > +static int iproc_gpiolib_get(struct gpio_chip *gc, u32 gpio)
> 
> These callbacks seems to reimplement parts of GPIO_GENERIC
> that you should already be using.
> 
> Again look at drivers/gpio/gpio-ftgpio010.c() use bgpio_init()
> to set up the library callbacks, look in
> drivers/gpio/gpio-mmio.c for kerneldoc on the function.
> 

Will look into it.

> > +static int iproc_gpiolib_to_irq(struct gpio_chip *gc, u32 offset)
> > +{
> > +       struct iproc_gpio_chip *chip = gpiochip_get_data(gc);
> > +
> > +       return irq_linear_revmap(chip->irq_domain, offset);
> > +}
> 
> GPIOLIB_IRQCHIP provides a .to_irq() implementation
> so drop this and let the library handle this.
> 

Will that work if I've squirreled the irq_domain away in struct
iproc_gpio_chip? See above as to why I'm not using the embedded
gpio_irq_chip.

> > +static int iproc_gpio_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct device_node *dn = pdev->dev.of_node;
> > +       struct iproc_gpio_chip *chip;
> > +       u32 num_gpios;
> > +       int irq, ret;
> > +
> > +       chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> > +       if (!chip)
> > +               return -ENOMEM;
> > +
> > +       chip->dev = dev;
> > +       platform_set_drvdata(pdev, chip);
> > +
> > +       chip->gc.label = dev_name(dev);
> > +
> > +       chip->base = devm_platform_ioremap_resource(pdev, 0);
> > +       if (IS_ERR(chip->base))
> > +               return PTR_ERR(chip->base);
> > +
> > +       /* Get number of GPIO pin */
> > +       if (of_property_read_u32(dn, "ngpios", &num_gpios)) {
> > +               dev_err(dev, "missing ngpios DT property\n");
> > +               return -EINVAL;
> > +       }
> 
> Maybe provide a sensible default?
> 

I don't know that there is one. On the particular SoC I have the number
is 12 but the datasheets I have access to are really hard to follow
(the SoC part is tacked on to the end of the datasheet for the switch
ASIC). 32 would be vaguely sane since they're all 32-bit wide
registers.

> > +       chip->gc.ngpio = num_gpios;
> > +       chip->gc.parent = dev;
> > +       chip->gc.of_node = dn;
> > +       chip->gc.direction_input = iproc_gpiolib_input;
> > +       chip->gc.direction_output = iproc_gpiolib_output;
> > +       chip->gc.set = iproc_gpiolib_set;
> > +       chip->gc.get = iproc_gpiolib_get;
> 
> Drop this and call bgpio_init() to set up the callbacks
> instead. However, set up .ngpio *after* calling
> bgpio_init() so users can't access the nonexisting
> gpios.
> 
> > +       chip->gc.to_irq = iproc_gpiolib_to_irq;
> 
> Drop this and use the GPIOLIB_IRQCHIP.
> 
> > +       ret = gpiochip_add_data(&chip->gc, chip);
> > +       if (ret) {
> > +               dev_err(dev, "unable to add GPIO chip\n");
> > +               return ret;
> > +       }
> 
> Why not use devm_gpiochip_add_data()?
> 
> > +       irq = platform_get_irq(pdev, 0);
> > +       if (irq > 0) {
> > +               u32 val, count;
> > +               struct irq_chip *irqc;
> > +
> > +               irqc = &chip->irqchip;
> > +               irqc->name = dev_name(dev);
> > +               irqc->irq_ack = iproc_gpio_irq_ack;
> > +               irqc->irq_mask = iproc_gpio_irq_mask;
> > +               irqc->irq_unmask = iproc_gpio_irq_unmask;
> > +               irqc->irq_set_type = iproc_gpio_irq_set_type;
> > +               irqc->irq_enable = iproc_gpio_irq_unmask;
> > +               irqc->irq_disable = iproc_gpio_irq_mask;
> > +               chip->intr = devm_platform_ioremap_resource(pdev, 1);
> > +               if (IS_ERR(chip->intr))
> > +                       return PTR_ERR(chip->intr);
> 
> Move all this above the [devm_]gpiochip_add_data()
> call and add:
> 
> struct gpio_irq_chip *girq;
> 
> girq = &chip->gc.irq;
> girq->chip = irqc;
> 
> etc follow the pattern from other drivers like the
> mentioned ftgpio010.c.
> 
> > +               /* Create irq domain */
> > +               chip->irq_domain = irq_domain_add_linear(dn, num_gpios,
> > +                               &irq_domain_simple_ops, chip);
> > +
> > +               if (!chip->irq_domain) {
> > +                       dev_err(dev, "Couldn't allocate IRQ domain\n");
> > +                       ret = -ENODEV;
> > +                       goto err_irq_domain;
> > +               }
> > +
> > +               /* Map each gpio pin to an IRQ and set the handler */
> > +               for (count = 0; count < num_gpios; count++) {
> > +                       int irq;
> > +
> > +                       irq = irq_create_mapping(chip->irq_domain, count);
> > +                       irq_set_chip_and_handler(irq, irqc, handle_simple_irq);
> > +                       irq_set_chip_data(irq, chip);
> > +               }
> 
> Drop this and let the generic GPIO irqchip handle
> the domain.
> 
> > +               /* Enable GPIO interrupts for CCA GPIO */
> > +               val = readl(chip->intr + CCA_INT_MASK);
> > +               val |= CCA_INT_F_GPIOINT;
> > +               writel(val, chip->intr + CCA_INT_MASK);
> 
> Move this before registering the chip.
> 
> > +               /* Install ISR for this GPIO controller */
> > +               ret = devm_request_irq(dev, irq, iproc_gpio_irq_handler,
> > +                                      IRQF_SHARED, chip->gc.label, chip);
> > +               if (ret) {
> > +                       dev_err(dev, "Fail to request IRQ%d: %d\n", irq, ret);
> > +                       goto err_irq_request;
> > +               }
> 
> Drop this and use the gpiolib irqchip library.

As I've said above I'm not sure I can due to sharing interrupts with
uart0. I'd be happy to be proven wrong as the generic irq code removes
a great deal of boiler plate for me.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: gpio: brcm: Add bindings for xgs-iproc

[Rob Herring]

On Fri, Oct 04, 2019 at 02:25:24PM +1300, Chris Packham wrote:
> This GPIO controller is present on a number of Broadcom switch ASICs
> with integrated SoCs. It is similar to the nsp-gpio and iproc-gpio
> blocks but different enough to require a separate driver.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  .../bindings/gpio/brcm,xgs-iproc.txt          | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt

Please make this a DT schema.

> 
> diff --git a/Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt b/Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt
> new file mode 100644
> index 000000000000..328b844c82dc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/brcm,xgs-iproc.txt
> @@ -0,0 +1,41 @@
> +Broadcom XGS iProc GPIO controller
> +
> +This controller is the Chip Common A GPIO present on a number of Broadcom
> +switch ASICs with integrated SoCs.
> +
> +Required properties:
> +- compatible:
> +    Must be "brcm,iproc-gpio-cca"
> +
> +- reg:
> +    The first region defines the base I/O address containing
> +    the GPIO controller registers. The second region defines
> +    the I/O address containing the Chip Common A interrupt
> +    registers.
> +
> +Optional properties:
> +
> +- interrupts:
> +    The interrupt shared by all GPIO lines for this controller.
> +
> +- #interrupt-cells:
> +    Should be <2>.  The first cell is the GPIO number, the second should specify
> +    flags.
> +
> +    See also Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +
> +- interrupt-controller:
> +    Marks the device node as an interrupt controller
> +
> +Example:
> +	gpioa: gpio@18000060 {
> +		compatible = "brcm,iproc-gpio-cca";
> +		#gpio-cells = <2>;

Not documented...

> +		reg = <0x18000060 0x50>,
> +		      <0x18000000 0x50>;
> +		ngpios = <12>;

Not documented. 

> +		gpio-controller;

Not documented.

> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
> +	};
> -- 
> 2.23.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] gpio: Add xgs-iproc driver

[Linus Walleij]

On Mon, Oct 14, 2019 at 12:08 AM Chris Packham
<Chris.Packham@alliedtelesis.co.nz> wrote:
Me:

> > I think this should be a chained interrupt handler (see below how to
> > register it).
> >
> > See e.g. drivers/gpio/gpio-ftgpio010.c for an example:
> > change function prototype, no return value, use
> > chained_irq_enter/exit(irqchip, desc); etc.
> >
>
> I don't think a chained interrupt handler can work. The problem is that
> the parent irq on the SoC is shared between the gpio and uart0 (why
> it's this way with two IP blocks in the same SoC I'll never know). When
> a chained interrupt handler is registered I lose the serial interrupts.
> Please correct me if there is some way to make the chained handlers
> deal with sharing interrupts.

Aha I see. Look at:
drivers/gpio/gpio-mt7621.c

And how that driver sets the parent handler to NULL in order
to still exploit the core helpers.

I will refactor this to some more elegant API at some point when
I get there, for now follow the example of mt7621.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel