[PATCH 1/5] DT: Add documentation for gpio-rt2880

[PATCH 1/5] DT: Add documentation for gpio-rt2880

[John Crispin]

Describe gpio-rt2880 binding.

Signed-off-by: John Crispin <blogic@openwrt.org>
Cc: linux-mips@linux-mips.org
Cc: devicetree@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
---
 .../devicetree/bindings/gpio/gpio-rt2880.txt       |   40 ++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rt2880.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-rt2880.txt b/Documentation/devicetree/bindings/gpio/gpio-rt2880.txt
new file mode 100644
index 0000000..b4acf02
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-rt2880.txt
@@ -0,0 +1,40 @@
+Ralink SoC GPIO controller bindings
+
+Required properties:
+- compatible:
+  - "ralink,rt2880-gpio" for Ralink controllers
+- #gpio-cells : Should be two.
+  - first cell is the pin number
+  - second cell is used to specify optional parameters (unused)
+- gpio-controller : Marks the device node as a GPIO controller
+- reg : Physical base address and length of the controller's registers
+- interrupt-parent: phandle to the INTC device node
+- interrupts : Specify the INTC interrupt number
+- ralink,num-gpios : Specify the number of GPIOs
+- ralink,register-map : The register layout depends on the GPIO bank and actual
+		SoC type. Register offsets need to be in this order.
+		[ INT, EDGE, RENA, FENA, DATA, DIR, POL, SET, RESET, TOGGLE ]
+
+Optional properties:
+- ralink,gpio-base : Specify the GPIO chips base number
+
+Example:
+
+	gpio0: gpio@600 {
+		compatible = "ralink,rt5350-gpio", "ralink,rt2880-gpio";
+
+		#gpio-cells = <2>;
+		gpio-controller;
+
+		reg = <0x600 0x34>;
+
+		interrupt-parent = <&intc>;
+		interrupts = <6>;
+
+		ralink,gpio-base = <0>;
+		ralink,num-gpios = <24>;
+		ralink,register-map = [ 00 04 08 0c
+				20 24 28 2c
+				30 34 ];
+
+	};
-- 
1.7.10.4

[PATCH 2/5] GPIO: MIPS: ralink: add gpio driver for ralink rt2880 SoC

[John Crispin]

Add gpio driver for Ralink SoC. This driver makes the gpio core on
RT2880, RT305x, rt3352, rt3662, rt3883, rt5350 and mt7620 work.

Signed-off-by: John Crispin <blogic@openwrt.org>
Cc: linux-mips@linux-mips.org
Cc: linux-gpio@vger.kernel.org
---
 drivers/gpio/Kconfig       |    6 +
 drivers/gpio/Makefile      |    1 +
 drivers/gpio/gpio-rt2880.c |  354 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 361 insertions(+)
 create mode 100644 drivers/gpio/gpio-rt2880.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 9de1515..c91b15b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -289,6 +289,12 @@ config GPIO_SCH311X
 	  To compile this driver as a module, choose M here: the module will
 	  be called gpio-sch311x.
 
+config GPIO_RALINK
+	bool "Ralink GPIO Support"
+	depends on RALINK
+	help
+	  Say yes here to support the Ralink SoC GPIO device
+
 config GPIO_SPEAR_SPICS
 	bool "ST SPEAr13xx SPI Chip Select as GPIO support"
 	depends on PLAT_SPEAR
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5d024e3..d8f0f17 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_GPIO_PCF857X)	+= gpio-pcf857x.o
 obj-$(CONFIG_GPIO_PCH)		+= gpio-pch.o
 obj-$(CONFIG_GPIO_PL061)	+= gpio-pl061.o
 obj-$(CONFIG_GPIO_PXA)		+= gpio-pxa.o
+obj-$(CONFIG_GPIO_RALINK)	+= gpio-rt2880.o
 obj-$(CONFIG_GPIO_RC5T583)	+= gpio-rc5t583.o
 obj-$(CONFIG_GPIO_RDC321X)	+= gpio-rdc321x.o
 obj-$(CONFIG_GPIO_RCAR)		+= gpio-rcar.o
diff --git a/drivers/gpio/gpio-rt2880.c b/drivers/gpio/gpio-rt2880.c
new file mode 100644
index 0000000..c375210
--- /dev/null
+++ b/drivers/gpio/gpio-rt2880.c
@@ -0,0 +1,354 @@
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * Copyright (C) 2009-2011 Gabor Juhos <juhosg@openwrt.org>
+ * Copyright (C) 2013 John Crispin <blogic@openwrt.org>
+ */
+
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/gpio.h>
+#include <linux/spinlock.h>
+#include <linux/platform_device.h>
+#include <linux/of_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+
+enum ralink_gpio_reg {
+	GPIO_REG_INT = 0,
+	GPIO_REG_EDGE,
+	GPIO_REG_RENA,
+	GPIO_REG_FENA,
+	GPIO_REG_DATA,
+	GPIO_REG_DIR,
+	GPIO_REG_POL,
+	GPIO_REG_SET,
+	GPIO_REG_RESET,
+	GPIO_REG_TOGGLE,
+	GPIO_REG_MAX
+};
+
+struct ralink_gpio_chip {
+	struct gpio_chip chip;
+	u8 regs[GPIO_REG_MAX];
+
+	spinlock_t lock;
+	void __iomem *membase;
+	struct irq_domain *domain;
+	int irq;
+
+	u32 rising;
+	u32 falling;
+};
+
+#define MAP_MAX	4
+static struct irq_domain *irq_map[MAP_MAX];
+static int irq_map_count;
+static atomic_t irq_refcount = ATOMIC_INIT(0);
+
+static inline struct ralink_gpio_chip *to_ralink_gpio(struct gpio_chip *chip)
+{
+	struct ralink_gpio_chip *rg;
+
+	rg = container_of(chip, struct ralink_gpio_chip, chip);
+
+	return rg;
+}
+
+static inline void rt_gpio_w32(struct ralink_gpio_chip *rg, u8 reg, u32 val)
+{
+	iowrite32(val, rg->membase + rg->regs[reg]);
+}
+
+static inline u32 rt_gpio_r32(struct ralink_gpio_chip *rg, u8 reg)
+{
+	return ioread32(rg->membase + rg->regs[reg]);
+}
+
+static void ralink_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
+
+	rt_gpio_w32(rg, (value) ? GPIO_REG_SET : GPIO_REG_RESET, BIT(offset));
+}
+
+static int ralink_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
+
+	return !!(rt_gpio_r32(rg, GPIO_REG_DATA) & BIT(offset));
+}
+
+static int ralink_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
+	unsigned long flags;
+	u32 t;
+
+	spin_lock_irqsave(&rg->lock, flags);
+	t = rt_gpio_r32(rg, GPIO_REG_DIR);
+	t &= ~BIT(offset);
+	rt_gpio_w32(rg, GPIO_REG_DIR, t);
+	spin_unlock_irqrestore(&rg->lock, flags);
+
+	return 0;
+}
+
+static int ralink_gpio_direction_output(struct gpio_chip *chip,
+					unsigned offset, int value)
+{
+	struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
+	unsigned long flags;
+	u32 t;
+
+	spin_lock_irqsave(&rg->lock, flags);
+	ralink_gpio_set(chip, offset, value);
+	t = rt_gpio_r32(rg, GPIO_REG_DIR);
+	t |= BIT(offset);
+	rt_gpio_w32(rg, GPIO_REG_DIR, t);
+	spin_unlock_irqrestore(&rg->lock, flags);
+
+	return 0;
+}
+
+static int ralink_gpio_to_irq(struct gpio_chip *chip, unsigned pin)
+{
+	struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
+
+	if (rg->irq < 1)
+		return -1;
+
+	return irq_create_mapping(rg->domain, pin);
+}
+
+static void ralink_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
+{
+	int i;
+
+	for (i = 0; i < irq_map_count; i++) {
+		struct irq_domain *domain = irq_map[i];
+		struct ralink_gpio_chip *rg;
+		unsigned long pending;
+		int bit;
+
+		rg = (struct ralink_gpio_chip *) domain->host_data;
+		pending = rt_gpio_r32(rg, GPIO_REG_INT);
+
+		for_each_set_bit(bit, &pending, rg->chip.ngpio) {
+			u32 map = irq_find_mapping(domain, bit);
+
+			generic_handle_irq(map);
+			rt_gpio_w32(rg, GPIO_REG_INT, BIT(bit));
+		}
+	}
+}
+
+static void ralink_gpio_irq_unmask(struct irq_data *d)
+{
+	struct ralink_gpio_chip *rg;
+	unsigned long flags;
+	u32 val;
+
+	rg = (struct ralink_gpio_chip *) d->domain->host_data;
+	val = rt_gpio_r32(rg, GPIO_REG_RENA);
+
+	spin_lock_irqsave(&rg->lock, flags);
+	rt_gpio_w32(rg, GPIO_REG_RENA, val | (BIT(d->hwirq) & rg->rising));
+	rt_gpio_w32(rg, GPIO_REG_FENA, val | (BIT(d->hwirq) & rg->falling));
+	spin_unlock_irqrestore(&rg->lock, flags);
+}
+
+static void ralink_gpio_irq_mask(struct irq_data *d)
+{
+	struct ralink_gpio_chip *rg;
+	unsigned long flags;
+	u32 val;
+
+	rg = (struct ralink_gpio_chip *) d->domain->host_data;
+	val = rt_gpio_r32(rg, GPIO_REG_RENA);
+
+	spin_lock_irqsave(&rg->lock, flags);
+	rt_gpio_w32(rg, GPIO_REG_FENA, val & ~BIT(d->hwirq));
+	rt_gpio_w32(rg, GPIO_REG_RENA, val & ~BIT(d->hwirq));
+	spin_unlock_irqrestore(&rg->lock, flags);
+}
+
+static int ralink_gpio_irq_type(struct irq_data *d, unsigned int type)
+{
+	struct ralink_gpio_chip *rg;
+	u32 mask = BIT(d->hwirq);
+
+	rg = (struct ralink_gpio_chip *) d->domain->host_data;
+
+	if (type == IRQ_TYPE_PROBE) {
+		if ((rg->rising | rg->falling) & mask)
+			return 0;
+
+		type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
+	}
+
+	if (type & IRQ_TYPE_EDGE_RISING)
+		rg->rising |= mask;
+	else
+		rg->rising &= ~mask;
+
+	if (type & IRQ_TYPE_EDGE_FALLING)
+		rg->falling |= mask;
+	else
+		rg->falling &= ~mask;
+
+	return 0;
+}
+
+static struct irq_chip ralink_gpio_irq_chip = {
+	.name		= "GPIO",
+	.irq_unmask	= ralink_gpio_irq_unmask,
+	.irq_mask	= ralink_gpio_irq_mask,
+	.irq_mask_ack	= ralink_gpio_irq_mask,
+	.irq_set_type	= ralink_gpio_irq_type,
+};
+
+static int gpio_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
+{
+	irq_set_chip_and_handler(irq, &ralink_gpio_irq_chip, handle_level_irq);
+	irq_set_handler_data(irq, d);
+
+	return 0;
+}
+
+static const struct irq_domain_ops irq_domain_ops = {
+	.xlate = irq_domain_xlate_onecell,
+	.map = gpio_map,
+};
+
+static void ralink_gpio_irq_init(struct device_node *np,
+				 struct ralink_gpio_chip *rg)
+{
+	if (irq_map_count >= MAP_MAX)
+		return;
+
+	rg->irq = irq_of_parse_and_map(np, 0);
+	if (!rg->irq)
+		return;
+
+	rg->domain = irq_domain_add_linear(np, rg->chip.ngpio,
+					   &irq_domain_ops, rg);
+	if (!rg->domain) {
+		dev_err(rg->chip.dev, "irq_domain_add_linear failed\n");
+		return;
+	}
+
+	irq_map[irq_map_count++] = rg->domain;
+
+	rt_gpio_w32(rg, GPIO_REG_RENA, 0x0);
+	rt_gpio_w32(rg, GPIO_REG_FENA, 0x0);
+
+	if (!atomic_read(&irq_refcount))
+		irq_set_chained_handler(rg->irq, ralink_gpio_irq_handler);
+	atomic_inc(&irq_refcount);
+
+	dev_info(rg->chip.dev, "registering %d irq handlers\n", rg->chip.ngpio);
+}
+
+static int ralink_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	int gpio = chip->base + offset;
+
+	return pinctrl_request_gpio(gpio);
+}
+
+static void ralink_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+	int gpio = chip->base + offset;
+
+	pinctrl_free_gpio(gpio);
+}
+
+static int ralink_gpio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	struct ralink_gpio_chip *rg;
+	const __be32 *ngpio, *gpiobase;
+
+	if (!res) {
+		dev_err(&pdev->dev, "failed to find resource\n");
+		return -ENOMEM;
+	}
+
+	rg = devm_kzalloc(&pdev->dev,
+			sizeof(struct ralink_gpio_chip), GFP_KERNEL);
+	if (!rg)
+		return -ENOMEM;
+
+	rg->membase = devm_ioremap_resource(&pdev->dev, res);
+	if (!rg->membase) {
+		dev_err(&pdev->dev, "cannot remap I/O memory region\n");
+		return -ENOMEM;
+	}
+
+	if (of_property_read_u8_array(np, "ralink,register-map",
+			rg->regs, GPIO_REG_MAX)) {
+		dev_err(&pdev->dev, "failed to read register definition\n");
+		return -EINVAL;
+	}
+
+	ngpio = of_get_property(np, "ralink,num-gpios", NULL);
+	if (!ngpio) {
+		dev_err(&pdev->dev, "failed to read number of pins\n");
+		return -EINVAL;
+	}
+
+	gpiobase = of_get_property(np, "ralink,gpio-base", NULL);
+	if (gpiobase)
+		rg->chip.base = be32_to_cpu(*gpiobase);
+	else
+		rg->chip.base = -1;
+
+	spin_lock_init(&rg->lock);
+
+	rg->chip.dev = &pdev->dev;
+	rg->chip.label = dev_name(&pdev->dev);
+	rg->chip.of_node = np;
+	rg->chip.ngpio = be32_to_cpu(*ngpio);
+	rg->chip.direction_input = ralink_gpio_direction_input;
+	rg->chip.direction_output = ralink_gpio_direction_output;
+	rg->chip.get = ralink_gpio_get;
+	rg->chip.set = ralink_gpio_set;
+	rg->chip.request = ralink_gpio_request;
+	rg->chip.to_irq = ralink_gpio_to_irq;
+	rg->chip.free = ralink_gpio_free;
+
+	/* set polarity to low for all lines */
+	rt_gpio_w32(rg, GPIO_REG_POL, 0);
+
+	dev_info(&pdev->dev, "registering %d gpios\n", rg->chip.ngpio);
+
+	ralink_gpio_irq_init(np, rg);
+
+	return gpiochip_add(&rg->chip);
+}
+
+static const struct of_device_id ralink_gpio_match[] = {
+	{ .compatible = "ralink,rt2880-gpio" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ralink_gpio_match);
+
+static struct platform_driver ralink_gpio_driver = {
+	.probe = ralink_gpio_probe,
+	.driver = {
+		.name = "rt2880_gpio",
+		.owner = THIS_MODULE,
+		.of_match_table = ralink_gpio_match,
+	},
+};
+
+static int __init ralink_gpio_init(void)
+{
+	return platform_driver_register(&ralink_gpio_driver);
+}
+
+subsys_initcall(ralink_gpio_init);
-- 
1.7.10.4

[PATCH 3/5] DT: Add documentation for gpio-mt7621

[John Crispin]

Describe gpio-mt7621 binding.

Signed-off-by: John Crispin <blogic@openwrt.org>
Cc: linux-mips@linux-mips.org
Cc: devicetree@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
---
 .../devicetree/bindings/gpio/gpio-mt7621.txt       |   45 ++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mt7621.txt

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mt7621.txt b/Documentation/devicetree/bindings/gpio/gpio-mt7621.txt
new file mode 100644
index 0000000..ade0efe
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-mt7621.txt
@@ -0,0 +1,45 @@
+Mediatek SoC GPIO controller bindings
+
+The IP core used inside these SoCs has 1-N banks of 32 GPIOs each. Unfortunately
+the registers of all the banks are interwoven inside one single IO range. We
+really want to load one GPIO controller instance per bank. to make this possible
+we support 2 types of nodes. The parent node defines the memory I/O range and
+has N children each describing a single bank.
+
+Required properties for the top level node:
+- compatible:
+  - "mediatek,mt7621-gpio" for Mediatek controllers
+- reg : Physical base address and length of the controller's registers
+
+Required properties for the GPIO bank node:
+- compatible:
+  - "mediatek,mt7621-gpio-bank" for Mediatek banks
+- #gpio-cells : Should be two.
+  - first cell is the pin number
+  - second cell is used to specify optional parameters (unused)
+- gpio-controller : Marks the device node as a GPIO controller
+- reg : The id of the bank that the node describes.
+
+
+Example:
+	gpio@600 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		compatible = "mediatek,mt7621-gpio";
+		reg = <0x600 0x100>;
+
+		gpio0: bank@0 {
+			reg = <0>;
+			compatible = "mediatek,mt7621-gpio-bank";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+
+		gpio1: bank@1 {
+			reg = <1>;
+			compatible = "mediatek,mt7621-gpio-bank";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+	};
-- 
1.7.10.4

[PATCH 4/5] GPIO: MIPS: ralink: add gpio driver for ralink MT762x SoC

[John Crispin]

Add gpio driver for Ralink SoC. This driver makes the gpio core on
MT7621 and MT7628 work.

Signed-off-by: John Crispin <blogic@openwrt.org>
Cc: linux-mips@linux-mips.org
Cc: linux-gpio@vger.kernel.org
---
 drivers/gpio/Kconfig       |    6 ++
 drivers/gpio/Makefile      |    1 +
 drivers/gpio/gpio-mt7621.c |  195 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 202 insertions(+)
 create mode 100644 drivers/gpio/gpio-mt7621.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c91b15b..1ef83a0 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -523,6 +523,12 @@ config GPIO_MC9S08DZ60
 	help
 	  Select this to enable the MC9S08DZ60 GPIO driver
 
+config GPIO_MT7621
+	bool "Mediatek GPIO Support"
+	depends on MIPS && (SOC_MT7620 || SOC_MT7621)
+	help
+	  Say yes here to support the Mediatek SoC GPIO device
+
 config GPIO_PCA953X
 	tristate "PCA95[357]x, PCA9698, TCA64xx, and MAX7310 I/O ports"
 	depends on I2C
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d8f0f17..60a9e0e 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_GPIO_MPC8XXX)	+= gpio-mpc8xxx.o
 obj-$(CONFIG_GPIO_MSIC)		+= gpio-msic.o
 obj-$(CONFIG_GPIO_MSM_V1)	+= gpio-msm-v1.o
 obj-$(CONFIG_GPIO_MSM_V2)	+= gpio-msm-v2.o
+obj-$(CONFIG_GPIO_MT7621)	+= gpio-mt7621.o
 obj-$(CONFIG_GPIO_MVEBU)        += gpio-mvebu.o
 obj-$(CONFIG_GPIO_MXC)		+= gpio-mxc.o
 obj-$(CONFIG_GPIO_MXS)		+= gpio-mxs.o
diff --git a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c
new file mode 100644
index 0000000..7faf2c0
--- /dev/null
+++ b/drivers/gpio/gpio-mt7621.c
@@ -0,0 +1,195 @@
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ *
+ * Copyright (C) 2009-2011 Gabor Juhos <juhosg@openwrt.org>
+ * Copyright (C) 2013 John Crispin <blogic@openwrt.org>
+ */
+
+#include <linux/io.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/spinlock.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+
+#define MTK_BANK_WIDTH		32
+
+enum mediatek_gpio_reg {
+	GPIO_REG_CTRL = 0,
+	GPIO_REG_POL,
+	GPIO_REG_DATA,
+	GPIO_REG_DSET,
+	GPIO_REG_DCLR,
+};
+
+static void __iomem *mtk_gc_membase;
+
+struct mtk_gc {
+	struct gpio_chip chip;
+	spinlock_t lock;
+	int bank;
+};
+
+static inline struct mtk_gc
+*to_mediatek_gpio(struct gpio_chip *chip)
+{
+	struct mtk_gc *mgc;
+
+	mgc = container_of(chip, struct mtk_gc, chip);
+
+	return mgc;
+}
+
+static inline void
+mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
+{
+	iowrite32(val, mtk_gc_membase + (reg * 0x10) + (rg->bank * 0x4));
+}
+
+static inline u32
+mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
+{
+	return ioread32(mtk_gc_membase + (reg * 0x10) + (rg->bank * 0x4));
+}
+
+static void
+mediatek_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+	struct mtk_gc *rg = to_mediatek_gpio(chip);
+
+	mtk_gpio_w32(rg, (value) ? GPIO_REG_DSET : GPIO_REG_DCLR, BIT(offset));
+}
+
+static int
+mediatek_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+	struct mtk_gc *rg = to_mediatek_gpio(chip);
+
+	return !!(mtk_gpio_r32(rg, GPIO_REG_DATA) & BIT(offset));
+}
+
+static int
+mediatek_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+	struct mtk_gc *rg = to_mediatek_gpio(chip);
+	unsigned long flags;
+	u32 t;
+
+	spin_lock_irqsave(&rg->lock, flags);
+	t = mtk_gpio_r32(rg, GPIO_REG_CTRL);
+	t &= ~BIT(offset);
+	mtk_gpio_w32(rg, GPIO_REG_CTRL, t);
+	spin_unlock_irqrestore(&rg->lock, flags);
+
+	return 0;
+}
+
+static int
+mediatek_gpio_direction_output(struct gpio_chip *chip,
+					unsigned offset, int value)
+{
+	struct mtk_gc *rg = to_mediatek_gpio(chip);
+	unsigned long flags;
+	u32 t;
+
+	spin_lock_irqsave(&rg->lock, flags);
+	t = mtk_gpio_r32(rg, GPIO_REG_CTRL);
+	t |= BIT(offset);
+	mtk_gpio_w32(rg, GPIO_REG_CTRL, t);
+	mediatek_gpio_set(chip, offset, value);
+	spin_unlock_irqrestore(&rg->lock, flags);
+
+	return 0;
+}
+
+static int
+mediatek_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	int gpio = chip->base + offset;
+
+	return pinctrl_request_gpio(gpio);
+}
+
+static void
+mediatek_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+	int gpio = chip->base + offset;
+
+	pinctrl_free_gpio(gpio);
+}
+
+static int
+mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
+{
+	const __be32 *id = of_get_property(bank, "reg", NULL);
+	struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
+				sizeof(struct mtk_gc), GFP_KERNEL);
+	if (!rg || !id)
+		return -ENOMEM;
+
+	spin_lock_init(&rg->lock);
+
+	rg->chip.dev = &pdev->dev;
+	rg->chip.label = dev_name(&pdev->dev);
+	rg->chip.of_node = bank;
+	rg->chip.base = MTK_BANK_WIDTH * be32_to_cpu(*id);
+	rg->chip.ngpio = MTK_BANK_WIDTH;
+	rg->chip.direction_input = mediatek_gpio_direction_input;
+	rg->chip.direction_output = mediatek_gpio_direction_output;
+	rg->chip.get = mediatek_gpio_get;
+	rg->chip.set = mediatek_gpio_set;
+	rg->chip.request = mediatek_gpio_request;
+	rg->chip.free = mediatek_gpio_free;
+
+	/* set polarity to low for all gpios */
+	mtk_gpio_w32(rg, GPIO_REG_POL, 0);
+
+	dev_info(&pdev->dev, "registering %d gpios\n", rg->chip.ngpio);
+
+	return gpiochip_add(&rg->chip);
+}
+
+static int
+mediatek_gpio_probe(struct platform_device *pdev)
+{
+	struct device_node *bank, *np = pdev->dev.of_node;
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	mtk_gc_membase = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mtk_gc_membase))
+		return PTR_ERR(mtk_gc_membase);
+
+	for_each_child_of_node(np, bank)
+		if (of_device_is_compatible(bank, "mediatek,mt7621-gpio-bank"))
+			mediatek_gpio_bank_probe(pdev, bank);
+
+	return 0;
+}
+
+static const struct of_device_id mediatek_gpio_match[] = {
+	{ .compatible = "mediatek,mt7621-gpio" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mediatek_gpio_match);
+
+static struct platform_driver mediatek_gpio_driver = {
+	.probe = mediatek_gpio_probe,
+	.driver = {
+		.name = "mt7621_gpio",
+		.owner = THIS_MODULE,
+		.of_match_table = mediatek_gpio_match,
+	},
+};
+
+static int __init
+mediatek_gpio_init(void)
+{
+	return platform_driver_register(&mediatek_gpio_driver);
+}
+
+subsys_initcall(mediatek_gpio_init);
-- 
1.7.10.4

[PATCH 5/5] MIPS: ralink: we require gpiolib

[John Crispin]

Select ARCH_REQUIRE_GPIOLIB by default when building a kernel for RALINK.

Signed-off-by: John Crispin <blogic@openwrt.org>
---
 arch/mips/Kconfig                        |    1 +
 arch/mips/include/asm/mach-ralink/gpio.h |   24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)
 create mode 100644 arch/mips/include/asm/mach-ralink/gpio.h

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 2d255e8..380cce3 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -453,6 +453,7 @@ config RALINK
 	select RESET_CONTROLLER
 	select PINCTRL
 	select PINCTRL_RT2880
+	select ARCH_REQUIRE_GPIOLIB
 
 config SGI_IP22
 	bool "SGI IP22 (Indy/Indigo2)"
diff --git a/arch/mips/include/asm/mach-ralink/gpio.h b/arch/mips/include/asm/mach-ralink/gpio.h
new file mode 100644
index 0000000..f68ee16
--- /dev/null
+++ b/arch/mips/include/asm/mach-ralink/gpio.h
@@ -0,0 +1,24 @@
+/*
+ *  Ralink SoC GPIO API support
+ *
+ *  Copyright (C) 2008-2009 Gabor Juhos <juhosg@openwrt.org>
+ *  Copyright (C) 2008 Imre Kaloz <kaloz@openwrt.org>
+ *
+ *  This program is free software; you can redistribute it and/or modify it
+ *  under the terms of the GNU General Public License version 2 as published
+ *  by the Free Software Foundation.
+ *
+ */
+
+#ifndef __ASM_MACH_RALINK_GPIO_H
+#define __ASM_MACH_RALINK_GPIO_H
+
+#define ARCH_NR_GPIOS	128
+#include <asm-generic/gpio.h>
+
+#define gpio_get_value	__gpio_get_value
+#define gpio_set_value	__gpio_set_value
+#define gpio_cansleep	__gpio_cansleep
+#define gpio_to_irq	__gpio_to_irq
+
+#endif /* __ASM_MACH_RALINK_GPIO_H */
-- 
1.7.10.4

Re: [PATCH 2/5] GPIO: MIPS: ralink: add gpio driver for ralink rt2880 SoC

[Alexandre Courbot]

On Sat, Oct 11, 2014 at 5:28 AM, John Crispin <blogic@openwrt.org> wrote:
> Add gpio driver for Ralink SoC. This driver makes the gpio core on

Makes the gpio core what?

> RT2880, RT305x, rt3352, rt3662, rt3883, rt5350 and mt7620 work.
>
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Cc: linux-mips@linux-mips.org
> Cc: linux-gpio@vger.kernel.org
> ---
>  drivers/gpio/Kconfig       |    6 +
>  drivers/gpio/Makefile      |    1 +
>  drivers/gpio/gpio-rt2880.c |  354 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 361 insertions(+)
>  create mode 100644 drivers/gpio/gpio-rt2880.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 9de1515..c91b15b 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -289,6 +289,12 @@ config GPIO_SCH311X
>           To compile this driver as a module, choose M here: the module will
>           be called gpio-sch311x.
>
> +config GPIO_RALINK
> +       bool "Ralink GPIO Support"
> +       depends on RALINK
> +       help
> +         Say yes here to support the Ralink SoC GPIO device
> +
>  config GPIO_SPEAR_SPICS
>         bool "ST SPEAr13xx SPI Chip Select as GPIO support"
>         depends on PLAT_SPEAR
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 5d024e3..d8f0f17 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -67,6 +67,7 @@ obj-$(CONFIG_GPIO_PCF857X)    += gpio-pcf857x.o
>  obj-$(CONFIG_GPIO_PCH)         += gpio-pch.o
>  obj-$(CONFIG_GPIO_PL061)       += gpio-pl061.o
>  obj-$(CONFIG_GPIO_PXA)         += gpio-pxa.o
> +obj-$(CONFIG_GPIO_RALINK)      += gpio-rt2880.o
>  obj-$(CONFIG_GPIO_RC5T583)     += gpio-rc5t583.o
>  obj-$(CONFIG_GPIO_RDC321X)     += gpio-rdc321x.o
>  obj-$(CONFIG_GPIO_RCAR)                += gpio-rcar.o
> diff --git a/drivers/gpio/gpio-rt2880.c b/drivers/gpio/gpio-rt2880.c
> new file mode 100644
> index 0000000..c375210
> --- /dev/null
> +++ b/drivers/gpio/gpio-rt2880.c
> @@ -0,0 +1,354 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * Copyright (C) 2009-2011 Gabor Juhos <juhosg@openwrt.org>
> + * Copyright (C) 2013 John Crispin <blogic@openwrt.org>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/io.h>
> +#include <linux/gpio.h>

Please use <linux/gpio/driver.h> for new drivers.

> +#include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> +#include <linux/of_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/interrupt.h>
> +
> +enum ralink_gpio_reg {
> +       GPIO_REG_INT = 0,
> +       GPIO_REG_EDGE,
> +       GPIO_REG_RENA,
> +       GPIO_REG_FENA,
> +       GPIO_REG_DATA,
> +       GPIO_REG_DIR,
> +       GPIO_REG_POL,
> +       GPIO_REG_SET,
> +       GPIO_REG_RESET,
> +       GPIO_REG_TOGGLE,
> +       GPIO_REG_MAX
> +};

You have a slight possibility for identifier collision with such
generic names. Maybe use RT2880_REG_ as a prefix?

> +
> +struct ralink_gpio_chip {
> +       struct gpio_chip chip;
> +       u8 regs[GPIO_REG_MAX];
> +
> +       spinlock_t lock;
> +       void __iomem *membase;
> +       struct irq_domain *domain;
> +       int irq;
> +
> +       u32 rising;
> +       u32 falling;
> +};
> +
> +#define MAP_MAX        4
> +static struct irq_domain *irq_map[MAP_MAX];
> +static int irq_map_count;
> +static atomic_t irq_refcount = ATOMIC_INIT(0);
> +
> +static inline struct ralink_gpio_chip *to_ralink_gpio(struct gpio_chip *chip)
> +{
> +       struct ralink_gpio_chip *rg;
> +
> +       rg = container_of(chip, struct ralink_gpio_chip, chip);
> +
> +       return rg;
> +}
> +
> +static inline void rt_gpio_w32(struct ralink_gpio_chip *rg, u8 reg, u32 val)

nit: reg could (should?) be of enum ralink_gpio_reg (same for other
functions that use a register index).

> +{
> +       iowrite32(val, rg->membase + rg->regs[reg]);
> +}
> +
> +static inline u32 rt_gpio_r32(struct ralink_gpio_chip *rg, u8 reg)
> +{
> +       return ioread32(rg->membase + rg->regs[reg]);
> +}
> +
> +static void ralink_gpio_set(struct gpio_chip *chip, unsigned offset, int value)

Isn't "offset" a gpio index? In that case renaming it to "gpio" might
made the code easier to understand (same for other functions).

> +{
> +       struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
> +
> +       rt_gpio_w32(rg, (value) ? GPIO_REG_SET : GPIO_REG_RESET, BIT(offset));
> +}
> +
> +static int ralink_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
> +
> +       return !!(rt_gpio_r32(rg, GPIO_REG_DATA) & BIT(offset));
> +}
> +
> +static int ralink_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
> +       unsigned long flags;
> +       u32 t;
> +
> +       spin_lock_irqsave(&rg->lock, flags);
> +       t = rt_gpio_r32(rg, GPIO_REG_DIR);
> +       t &= ~BIT(offset);
> +       rt_gpio_w32(rg, GPIO_REG_DIR, t);
> +       spin_unlock_irqrestore(&rg->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int ralink_gpio_direction_output(struct gpio_chip *chip,
> +                                       unsigned offset, int value)
> +{
> +       struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
> +       unsigned long flags;
> +       u32 t;
> +
> +       spin_lock_irqsave(&rg->lock, flags);
> +       ralink_gpio_set(chip, offset, value);
> +       t = rt_gpio_r32(rg, GPIO_REG_DIR);
> +       t |= BIT(offset);
> +       rt_gpio_w32(rg, GPIO_REG_DIR, t);
> +       spin_unlock_irqrestore(&rg->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int ralink_gpio_to_irq(struct gpio_chip *chip, unsigned pin)
> +{
> +       struct ralink_gpio_chip *rg = to_ralink_gpio(chip);
> +
> +       if (rg->irq < 1)
> +               return -1;
> +
> +       return irq_create_mapping(rg->domain, pin);
> +}
> +
> +static void ralink_gpio_irq_handler(unsigned int irq, struct irq_desc *desc)
> +{
> +       int i;
> +
> +       for (i = 0; i < irq_map_count; i++) {
> +               struct irq_domain *domain = irq_map[i];
> +               struct ralink_gpio_chip *rg;
> +               unsigned long pending;
> +               int bit;
> +
> +               rg = (struct ralink_gpio_chip *) domain->host_data;
> +               pending = rt_gpio_r32(rg, GPIO_REG_INT);
> +
> +               for_each_set_bit(bit, &pending, rg->chip.ngpio) {
> +                       u32 map = irq_find_mapping(domain, bit);
> +
> +                       generic_handle_irq(map);
> +                       rt_gpio_w32(rg, GPIO_REG_INT, BIT(bit));
> +               }
> +       }
> +}
> +
> +static void ralink_gpio_irq_unmask(struct irq_data *d)
> +{
> +       struct ralink_gpio_chip *rg;
> +       unsigned long flags;
> +       u32 val;
> +
> +       rg = (struct ralink_gpio_chip *) d->domain->host_data;
> +       val = rt_gpio_r32(rg, GPIO_REG_RENA);
> +
> +       spin_lock_irqsave(&rg->lock, flags);
> +       rt_gpio_w32(rg, GPIO_REG_RENA, val | (BIT(d->hwirq) & rg->rising));
> +       rt_gpio_w32(rg, GPIO_REG_FENA, val | (BIT(d->hwirq) & rg->falling));
> +       spin_unlock_irqrestore(&rg->lock, flags);
> +}
> +
> +static void ralink_gpio_irq_mask(struct irq_data *d)
> +{
> +       struct ralink_gpio_chip *rg;
> +       unsigned long flags;
> +       u32 val;
> +
> +       rg = (struct ralink_gpio_chip *) d->domain->host_data;
> +       val = rt_gpio_r32(rg, GPIO_REG_RENA);
> +
> +       spin_lock_irqsave(&rg->lock, flags);
> +       rt_gpio_w32(rg, GPIO_REG_FENA, val & ~BIT(d->hwirq));
> +       rt_gpio_w32(rg, GPIO_REG_RENA, val & ~BIT(d->hwirq));
> +       spin_unlock_irqrestore(&rg->lock, flags);
> +}
> +
> +static int ralink_gpio_irq_type(struct irq_data *d, unsigned int type)
> +{
> +       struct ralink_gpio_chip *rg;
> +       u32 mask = BIT(d->hwirq);
> +
> +       rg = (struct ralink_gpio_chip *) d->domain->host_data;
> +
> +       if (type == IRQ_TYPE_PROBE) {
> +               if ((rg->rising | rg->falling) & mask)
> +                       return 0;
> +
> +               type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
> +       }
> +
> +       if (type & IRQ_TYPE_EDGE_RISING)
> +               rg->rising |= mask;
> +       else
> +               rg->rising &= ~mask;
> +
> +       if (type & IRQ_TYPE_EDGE_FALLING)
> +               rg->falling |= mask;
> +       else
> +               rg->falling &= ~mask;
> +
> +       return 0;
> +}
> +
> +static struct irq_chip ralink_gpio_irq_chip = {
> +       .name           = "GPIO",
> +       .irq_unmask     = ralink_gpio_irq_unmask,
> +       .irq_mask       = ralink_gpio_irq_mask,
> +       .irq_mask_ack   = ralink_gpio_irq_mask,
> +       .irq_set_type   = ralink_gpio_irq_type,
> +};
> +
> +static int gpio_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> +{
> +       irq_set_chip_and_handler(irq, &ralink_gpio_irq_chip, handle_level_irq);
> +       irq_set_handler_data(irq, d);
> +
> +       return 0;
> +}
> +
> +static const struct irq_domain_ops irq_domain_ops = {
> +       .xlate = irq_domain_xlate_onecell,
> +       .map = gpio_map,
> +};
> +
> +static void ralink_gpio_irq_init(struct device_node *np,
> +                                struct ralink_gpio_chip *rg)
> +{
> +       if (irq_map_count >= MAP_MAX)
> +               return;
> +
> +       rg->irq = irq_of_parse_and_map(np, 0);
> +       if (!rg->irq)
> +               return;
> +
> +       rg->domain = irq_domain_add_linear(np, rg->chip.ngpio,
> +                                          &irq_domain_ops, rg);
> +       if (!rg->domain) {
> +               dev_err(rg->chip.dev, "irq_domain_add_linear failed\n");
> +               return;
> +       }
> +
> +       irq_map[irq_map_count++] = rg->domain;
> +
> +       rt_gpio_w32(rg, GPIO_REG_RENA, 0x0);
> +       rt_gpio_w32(rg, GPIO_REG_FENA, 0x0);
> +
> +       if (!atomic_read(&irq_refcount))
> +               irq_set_chained_handler(rg->irq, ralink_gpio_irq_handler);
> +       atomic_inc(&irq_refcount);
> +
> +       dev_info(rg->chip.dev, "registering %d irq handlers\n", rg->chip.ngpio);
> +}
> +
> +static int ralink_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> +       int gpio = chip->base + offset;
> +
> +       return pinctrl_request_gpio(gpio);
> +}
> +
> +static void ralink_gpio_free(struct gpio_chip *chip, unsigned offset)
> +{
> +       int gpio = chip->base + offset;
> +
> +       pinctrl_free_gpio(gpio);
> +}
> +
> +static int ralink_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       struct ralink_gpio_chip *rg;
> +       const __be32 *ngpio, *gpiobase;
> +
> +       if (!res) {
> +               dev_err(&pdev->dev, "failed to find resource\n");
> +               return -ENOMEM;
> +       }
> +
> +       rg = devm_kzalloc(&pdev->dev,
> +                       sizeof(struct ralink_gpio_chip), GFP_KERNEL);
> +       if (!rg)
> +               return -ENOMEM;
> +
> +       rg->membase = devm_ioremap_resource(&pdev->dev, res);
> +       if (!rg->membase) {
> +               dev_err(&pdev->dev, "cannot remap I/O memory region\n");
> +               return -ENOMEM;
> +       }
> +
> +       if (of_property_read_u8_array(np, "ralink,register-map",

This (and the device tree bindings) seems the indicate that the
registers offset can vary depending on the chip and bank. The chip can
be specified using the compatible property, as for the bank you can
also require a property giving the bank number. With these two bits of
information, this driver should be able to pick the right register
layout out of an in-driver table. This would be much cleaner that
letting the DT specify whatever layout it wants.

> +                       rg->regs, GPIO_REG_MAX)) {
> +               dev_err(&pdev->dev, "failed to read register definition\n");
> +               return -EINVAL;
> +       }
> +
> +       ngpio = of_get_property(np, "ralink,num-gpios", NULL);
> +       if (!ngpio) {
> +               dev_err(&pdev->dev, "failed to read number of pins\n");
> +               return -EINVAL;
> +       }
> +
> +       gpiobase = of_get_property(np, "ralink,gpio-base", NULL);

Mmm I am not sure this is a good idea - it let's the DT specify a
potentially already-used base for this GPIO chip. I'd get rid of this
option if it is not absolutely needed.

I haven't reviewed the IRQ part of this driver, being ignorant on the
matter. I will try to learn from Linus' review. :P

Re: [PATCH 4/5] GPIO: MIPS: ralink: add gpio driver for ralink MT762x SoC

[Alexandre Courbot]

On Sat, Oct 11, 2014 at 5:28 AM, John Crispin <blogic@openwrt.org> wrote:
> Add gpio driver for Ralink SoC. This driver makes the gpio core on
> MT7621 and MT7628 work.

Basically I have the same remarks as for the rt2880 driver. Both seem
to be very similar, and to work kind of like the gpio-generic driver.
I wonder if there wouldn't be a way to leverage this driver instead of
rewriting the same logic X times?

Some more comments below...

>
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Cc: linux-mips@linux-mips.org
> Cc: linux-gpio@vger.kernel.org
> ---
>  drivers/gpio/Kconfig       |    6 ++
>  drivers/gpio/Makefile      |    1 +
>  drivers/gpio/gpio-mt7621.c |  195 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 202 insertions(+)
>  create mode 100644 drivers/gpio/gpio-mt7621.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index c91b15b..1ef83a0 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -523,6 +523,12 @@ config GPIO_MC9S08DZ60
>         help
>           Select this to enable the MC9S08DZ60 GPIO driver
>
> +config GPIO_MT7621
> +       bool "Mediatek GPIO Support"
> +       depends on MIPS && (SOC_MT7620 || SOC_MT7621)
> +       help
> +         Say yes here to support the Mediatek SoC GPIO device
> +
>  config GPIO_PCA953X
>         tristate "PCA95[357]x, PCA9698, TCA64xx, and MAX7310 I/O ports"
>         depends on I2C
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index d8f0f17..60a9e0e 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_GPIO_MPC8XXX)    += gpio-mpc8xxx.o
>  obj-$(CONFIG_GPIO_MSIC)                += gpio-msic.o
>  obj-$(CONFIG_GPIO_MSM_V1)      += gpio-msm-v1.o
>  obj-$(CONFIG_GPIO_MSM_V2)      += gpio-msm-v2.o
> +obj-$(CONFIG_GPIO_MT7621)      += gpio-mt7621.o
>  obj-$(CONFIG_GPIO_MVEBU)        += gpio-mvebu.o
>  obj-$(CONFIG_GPIO_MXC)         += gpio-mxc.o
>  obj-$(CONFIG_GPIO_MXS)         += gpio-mxs.o
> diff --git a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c
> new file mode 100644
> index 0000000..7faf2c0
> --- /dev/null
> +++ b/drivers/gpio/gpio-mt7621.c
> @@ -0,0 +1,195 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * Copyright (C) 2009-2011 Gabor Juhos <juhosg@openwrt.org>
> + * Copyright (C) 2013 John Crispin <blogic@openwrt.org>
> + */
> +
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/module.h>
> +#include <linux/of_irq.h>
> +#include <linux/spinlock.h>
> +#include <linux/irqdomain.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +
> +#define MTK_BANK_WIDTH         32
> +
> +enum mediatek_gpio_reg {
> +       GPIO_REG_CTRL = 0,
> +       GPIO_REG_POL,
> +       GPIO_REG_DATA,
> +       GPIO_REG_DSET,
> +       GPIO_REG_DCLR,
> +};
> +
> +static void __iomem *mtk_gc_membase;
> +
> +struct mtk_gc {
> +       struct gpio_chip chip;
> +       spinlock_t lock;
> +       int bank;
> +};
> +
> +static inline struct mtk_gc
> +*to_mediatek_gpio(struct gpio_chip *chip)
> +{
> +       struct mtk_gc *mgc;
> +
> +       mgc = container_of(chip, struct mtk_gc, chip);
> +
> +       return mgc;
> +}
> +
> +static inline void
> +mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
> +{
> +       iowrite32(val, mtk_gc_membase + (reg * 0x10) + (rg->bank * 0x4));
> +}
> +
> +static inline u32
> +mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
> +{
> +       return ioread32(mtk_gc_membase + (reg * 0x10) + (rg->bank * 0x4));
> +}
> +
> +static void
> +mediatek_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> +       struct mtk_gc *rg = to_mediatek_gpio(chip);
> +
> +       mtk_gpio_w32(rg, (value) ? GPIO_REG_DSET : GPIO_REG_DCLR, BIT(offset));
> +}
> +
> +static int
> +mediatek_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct mtk_gc *rg = to_mediatek_gpio(chip);
> +
> +       return !!(mtk_gpio_r32(rg, GPIO_REG_DATA) & BIT(offset));
> +}
> +
> +static int
> +mediatek_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct mtk_gc *rg = to_mediatek_gpio(chip);
> +       unsigned long flags;
> +       u32 t;
> +
> +       spin_lock_irqsave(&rg->lock, flags);
> +       t = mtk_gpio_r32(rg, GPIO_REG_CTRL);
> +       t &= ~BIT(offset);
> +       mtk_gpio_w32(rg, GPIO_REG_CTRL, t);
> +       spin_unlock_irqrestore(&rg->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int
> +mediatek_gpio_direction_output(struct gpio_chip *chip,
> +                                       unsigned offset, int value)
> +{
> +       struct mtk_gc *rg = to_mediatek_gpio(chip);
> +       unsigned long flags;
> +       u32 t;
> +
> +       spin_lock_irqsave(&rg->lock, flags);
> +       t = mtk_gpio_r32(rg, GPIO_REG_CTRL);
> +       t |= BIT(offset);
> +       mtk_gpio_w32(rg, GPIO_REG_CTRL, t);
> +       mediatek_gpio_set(chip, offset, value);
> +       spin_unlock_irqrestore(&rg->lock, flags);
> +
> +       return 0;
> +}
> +
> +static int
> +mediatek_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> +       int gpio = chip->base + offset;
> +
> +       return pinctrl_request_gpio(gpio);
> +}
> +
> +static void
> +mediatek_gpio_free(struct gpio_chip *chip, unsigned offset)
> +{
> +       int gpio = chip->base + offset;
> +
> +       pinctrl_free_gpio(gpio);
> +}
> +
> +static int
> +mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
> +{
> +       const __be32 *id = of_get_property(bank, "reg", NULL);
> +       struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
> +                               sizeof(struct mtk_gc), GFP_KERNEL);
> +       if (!rg || !id)
> +               return -ENOMEM;
> +
> +       spin_lock_init(&rg->lock);
> +
> +       rg->chip.dev = &pdev->dev;
> +       rg->chip.label = dev_name(&pdev->dev);
> +       rg->chip.of_node = bank;
> +       rg->chip.base = MTK_BANK_WIDTH * be32_to_cpu(*id);

Argh, no, I don't think so. The GPIO integer space is not something
you can arbitrarily use like this. Any value that is not -1 is
dangerous here.

If you add your banks one after the other, like you are seemingly
doing here, then you have a high probability of ending with
consecutive numbers. This cannot be guaranteed 100% though. That's why
we are trying to get away from the integer numberspace and to use GPIO
descriptors instead.

> +       rg->chip.ngpio = MTK_BANK_WIDTH;
> +       rg->chip.direction_input = mediatek_gpio_direction_input;
> +       rg->chip.direction_output = mediatek_gpio_direction_output;
> +       rg->chip.get = mediatek_gpio_get;
> +       rg->chip.set = mediatek_gpio_set;
> +       rg->chip.request = mediatek_gpio_request;
> +       rg->chip.free = mediatek_gpio_free;
> +
> +       /* set polarity to low for all gpios */
> +       mtk_gpio_w32(rg, GPIO_REG_POL, 0);
> +
> +       dev_info(&pdev->dev, "registering %d gpios\n", rg->chip.ngpio);
> +
> +       return gpiochip_add(&rg->chip);
> +}
> +
> +static int
> +mediatek_gpio_probe(struct platform_device *pdev)
> +{
> +       struct device_node *bank, *np = pdev->dev.of_node;
> +       struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +       mtk_gc_membase = devm_ioremap_resource(&pdev->dev, res);
> +       if (IS_ERR(mtk_gc_membase))
> +               return PTR_ERR(mtk_gc_membase);
> +
> +       for_each_child_of_node(np, bank)
> +               if (of_device_is_compatible(bank, "mediatek,mt7621-gpio-bank"))
> +                       mediatek_gpio_bank_probe(pdev, bank);

Here you may want to make sure the banks are parsed in the right order
if the order of their base number matters to you. Another solution
would be to just have a property with the number of banks, and use
that instead of sub-nodes for each bank. This should be doable here
since your banks do not have special properties of their own, nor do
they differ from each other.

Re: [PATCH 2/5] GPIO: MIPS: ralink: add gpio driver for ralink rt2880 SoC

[John Crispin]

Hi,


On 20/10/2014 06:41, Alexandre Courbot wrote:

On Sat, Oct 11, 2014 at 5:28 AM, John Crispin <blogic@openwrt.org> wrote:
> Add gpio driver for Ralink SoC. This driver makes the gpio core on

Makes the gpio core what?

> RT2880, RT305x, rt3352, rt3662, rt3883, rt5350 and mt7620 work.

work as it says in the last work of the sentence

[snip..]

> This (and the device tree bindings) seems the indicate that the 
> registers offset can vary depending on the chip and bank. The chip
> can be specified using the compatible property, as for the bank you
> can also require a property giving the bank number. With these two
> bits of information, this driver should be able to pick the right
> register layout out of an in-driver table. This would be much
> cleaner that letting the DT specify whatever layout it wants.

i tend to disagree. if we put the register offsets into the driver we
will have lots of static arrays (5 or 6) and with each new soc we need
to potentially need to patch the driver causing us in openwrt to carry
lots of patches and have to worry about upstreaming them. From my
understanding, the dts has this exact purpose, describing the hardware
and in turn reducing the boiler plate and static code in the drivers.
If have sent other drivers that do the same and was told there that
this is totally legit.

	John

Re: [PATCH 4/5] GPIO: MIPS: ralink: add gpio driver for ralink MT762x SoC

[John Crispin]

On 20/10/2014 06:57, Alexandre Courbot wrote:
> On Sat, Oct 11, 2014 at 5:28 AM, John Crispin <blogic@openwrt.org> 
> wrote:
>> Add gpio driver for Ralink SoC. This driver makes the gpio core 
>> on MT7621 and MT7628 work.
> 
> Basically I have the same remarks as for the rt2880 driver. Both 
> seem to be very similar, and to work kind of like the gpio-generic 
> driver. I wonder if there wouldn't be a way to leverage this
> driver instead of rewriting the same logic X times?
> 
> Some more comments below...
> 
>> 
>> Signed-off-by: John Crispin <blogic@openwrt.org> Cc: 
>> linux-mips@linux-mips.org Cc: linux-gpio@vger.kernel.org --- 
>> drivers/gpio/Kconfig       |    6 ++ drivers/gpio/Makefile
>> | 1 + drivers/gpio/gpio-mt7621.c |  195 
>> ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed,
>> 202 insertions(+) create mode 100644 drivers/gpio/gpio-mt7621.c
>> 
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 
>> c91b15b..1ef83a0 100644 --- a/drivers/gpio/Kconfig +++ 
>> b/drivers/gpio/Kconfig @@ -523,6 +523,12 @@ config 
>> GPIO_MC9S08DZ60 help Select this to enable the MC9S08DZ60 GPIO 
>> driver
>> 
>> +config GPIO_MT7621 +       bool "Mediatek GPIO Support" + 
>> depends on MIPS && (SOC_MT7620 || SOC_MT7621) +       help + Say
>> yes here to support the Mediatek SoC GPIO device + config 
>> GPIO_PCA953X tristate "PCA95[357]x, PCA9698, TCA64xx, and
>> MAX7310 I/O ports" depends on I2C diff --git
>> a/drivers/gpio/Makefile b/drivers/gpio/Makefile index
>> d8f0f17..60a9e0e 100644 --- a/drivers/gpio/Makefile +++
>> b/drivers/gpio/Makefile @@ -57,6 +57,7 @@
>> obj-$(CONFIG_GPIO_MPC8XXX)    += gpio-mpc8xxx.o 
>> obj-$(CONFIG_GPIO_MSIC)                += gpio-msic.o 
>> obj-$(CONFIG_GPIO_MSM_V1)      += gpio-msm-v1.o 
>> obj-$(CONFIG_GPIO_MSM_V2)      += gpio-msm-v2.o 
>> +obj-$(CONFIG_GPIO_MT7621)      += gpio-mt7621.o 
>> obj-$(CONFIG_GPIO_MVEBU)        += gpio-mvebu.o 
>> obj-$(CONFIG_GPIO_MXC)         += gpio-mxc.o 
>> obj-$(CONFIG_GPIO_MXS)         += gpio-mxs.o diff --git 
>> a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c new 
>> file mode 100644 index 0000000..7faf2c0 --- /dev/null +++ 
>> b/drivers/gpio/gpio-mt7621.c @@ -0,0 +1,195 @@ +/* + * This 
>> program is free software; you can redistribute it and/or modify 
>> it + * under the terms of the GNU General Public License version 
>> 2 as published + * by the Free Software Foundation. + * + * 
>> Copyright (C) 2009-2011 Gabor Juhos <juhosg@openwrt.org> + * 
>> Copyright (C) 2013 John Crispin <blogic@openwrt.org> + */ + 
>> +#include <linux/io.h> +#include <linux/err.h> +#include 
>> <linux/gpio.h> +#include <linux/module.h> +#include 
>> <linux/of_irq.h> +#include <linux/spinlock.h> +#include 
>> <linux/irqdomain.h> +#include <linux/interrupt.h> +#include 
>> <linux/platform_device.h> + +#define MTK_BANK_WIDTH         32 + 
>> +enum mediatek_gpio_reg { +       GPIO_REG_CTRL = 0, + 
>> GPIO_REG_POL, +       GPIO_REG_DATA, +       GPIO_REG_DSET, + 
>> GPIO_REG_DCLR, +}; + +static void __iomem *mtk_gc_membase; + 
>> +struct mtk_gc { +       struct gpio_chip chip; + spinlock_t
>> lock; +       int bank; +}; + +static inline struct mtk_gc
>> +*to_mediatek_gpio(struct gpio_chip *chip) +{ + struct mtk_gc
>> *mgc; + +       mgc = container_of(chip, struct mtk_gc, chip); +
>> +       return mgc; +} + +static inline void +mtk_gpio_w32(struct
>> mtk_gc *rg, u8 reg, u32 val) +{ + iowrite32(val, mtk_gc_membase +
>> (reg * 0x10) + (rg->bank * 0x4)); +} + +static inline u32
>> +mtk_gpio_r32(struct mtk_gc *rg, u8 reg) +{ +       return
>> ioread32(mtk_gc_membase + (reg * 0x10) + (rg->bank * 0x4)); +} +
>> +static void +mediatek_gpio_set(struct gpio_chip *chip, unsigned
>> offset, int value) +{ +       struct mtk_gc *rg =
>> to_mediatek_gpio(chip); + +       mtk_gpio_w32(rg, (value) ?
>> GPIO_REG_DSET : GPIO_REG_DCLR, BIT(offset)); +} + +static int
>> +mediatek_gpio_get(struct gpio_chip *chip, unsigned offset) +{ +
>> struct mtk_gc *rg = to_mediatek_gpio(chip); + +       return
>> !!(mtk_gpio_r32(rg, GPIO_REG_DATA) & BIT(offset)); +} + +static
>> int +mediatek_gpio_direction_input(struct gpio_chip *chip,
>> unsigned offset) +{ +       struct mtk_gc *rg =
>> to_mediatek_gpio(chip); + unsigned long flags; +       u32 t; +
>> + spin_lock_irqsave(&rg->lock, flags); +       t =
>> mtk_gpio_r32(rg, GPIO_REG_CTRL); +       t &= ~BIT(offset); + 
>> mtk_gpio_w32(rg, GPIO_REG_CTRL, t); + 
>> spin_unlock_irqrestore(&rg->lock, flags); + +       return 0; +} 
>> + +static int +mediatek_gpio_direction_output(struct gpio_chip 
>> *chip, +                                       unsigned offset, 
>> int value) +{ +       struct mtk_gc *rg = to_mediatek_gpio(chip);
>> +       unsigned long flags; +       u32 t; + +
>> spin_lock_irqsave(&rg->lock, flags); +       t = mtk_gpio_r32(rg,
>> GPIO_REG_CTRL); +       t |= BIT(offset); + mtk_gpio_w32(rg,
>> GPIO_REG_CTRL, t); + mediatek_gpio_set(chip, offset, value); + 
>> spin_unlock_irqrestore(&rg->lock, flags); + +       return 0; +} 
>> + +static int +mediatek_gpio_request(struct gpio_chip *chip, 
>> unsigned offset) +{ +       int gpio = chip->base + offset; + + 
>> return pinctrl_request_gpio(gpio); +} + +static void 
>> +mediatek_gpio_free(struct gpio_chip *chip, unsigned offset) +{
>> + int gpio = chip->base + offset; + + pinctrl_free_gpio(gpio); +}
>> + +static int +mediatek_gpio_bank_probe(struct platform_device
>> *pdev, struct device_node *bank) +{ +       const __be32 *id = 
>> of_get_property(bank, "reg", NULL); +       struct mtk_gc *rg = 
>> devm_kzalloc(&pdev->dev, + sizeof(struct mtk_gc), GFP_KERNEL); +
>> if (!rg || !id) + return -ENOMEM; + +
>> spin_lock_init(&rg->lock); + + rg->chip.dev = &pdev->dev; +
>> rg->chip.label = dev_name(&pdev->dev); +       rg->chip.of_node =
>> bank; + rg->chip.base = MTK_BANK_WIDTH * be32_to_cpu(*id);
> 
> Argh, no, I don't think so. The GPIO integer space is not something
> you can arbitrarily use like this. Any value that is not -1 is
> dangerous here.
> 
> If you add your banks one after the other, like you are seemingly 
> doing here, then you have a high probability of ending with 
> consecutive numbers. This cannot be guaranteed 100% though. That's 
> why we are trying to get away from the integer numberspace and to 
> use GPIO descriptors instead.

dou you have an example of a driver already doing so ?



> 
>> +       rg->chip.ngpio = MTK_BANK_WIDTH; + 
>> rg->chip.direction_input = mediatek_gpio_direction_input; + 
>> rg->chip.direction_output = mediatek_gpio_direction_output; + 
>> rg->chip.get = mediatek_gpio_get; +       rg->chip.set = 
>> mediatek_gpio_set; +       rg->chip.request = 
>> mediatek_gpio_request; +       rg->chip.free = 
>> mediatek_gpio_free; + +       /* set polarity to low for all 
>> gpios */ +       mtk_gpio_w32(rg, GPIO_REG_POL, 0); + + 
>> dev_info(&pdev->dev, "registering %d gpios\n", rg->chip.ngpio); +
>> +       return gpiochip_add(&rg->chip); +} + +static int 
>> +mediatek_gpio_probe(struct platform_device *pdev) +{ + struct
>> device_node *bank, *np = pdev->dev.of_node; +       struct 
>> resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); +
>> +       mtk_gc_membase = devm_ioremap_resource(&pdev->dev, res);
>> +       if (IS_ERR(mtk_gc_membase)) +               return 
>> PTR_ERR(mtk_gc_membase); + +       for_each_child_of_node(np, 
>> bank) +               if (of_device_is_compatible(bank, 
>> "mediatek,mt7621-gpio-bank")) + mediatek_gpio_bank_probe(pdev,
>> bank);
> 
> Here you may want to make sure the banks are parsed in the right 
> order if the order of their base number matters to you. Another 
> solution would be to just have a property with the number of
> banks, and use that instead of sub-nodes for each bank. This should
> be doable here since your banks do not have special properties of 
> their own, nor do they differ from each other.
> 
> 

that would be redundant or not "i will now list <4> banks", "here are
the 4 banks". that is what the count helpers are for if we need to be
aware of the number of properties prior to parsing the node.

i am not sure i have seen another instance of dts using a count index
for the following properties array/table/... do you have an example of
a driver doing so ?

	John

Re: [PATCH 2/5] GPIO: MIPS: ralink: add gpio driver for ralink rt2880 SoC

[Alexandre Courbot]

On Mon, Oct 20, 2014 at 2:27 PM, John Crispin <blogic@openwrt.org> wrote:
> Hi,
>
>
> On 20/10/2014 06:41, Alexandre Courbot wrote:
>
> On Sat, Oct 11, 2014 at 5:28 AM, John Crispin <blogic@openwrt.org> wrote:
>> Add gpio driver for Ralink SoC. This driver makes the gpio core on
>
> Makes the gpio core what?
>
>> RT2880, RT305x, rt3352, rt3662, rt3883, rt5350 and mt7620 work.
>
> work as it says in the last work of the sentence

Right, I stopped reading after the 4th or 5th chip name. Never mind. :)

>
> [snip..]
>
>> This (and the device tree bindings) seems the indicate that the
>> registers offset can vary depending on the chip and bank. The chip
>> can be specified using the compatible property, as for the bank you
>> can also require a property giving the bank number. With these two
>> bits of information, this driver should be able to pick the right
>> register layout out of an in-driver table. This would be much
>> cleaner that letting the DT specify whatever layout it wants.
>
> i tend to disagree. if we put the register offsets into the driver we
> will have lots of static arrays (5 or 6) and with each new soc we need
> to potentially need to patch the driver causing us in openwrt to carry
> lots of patches and have to worry about upstreaming them. From my
> understanding, the dts has this exact purpose, describing the hardware
> and in turn reducing the boiler plate and static code in the drivers.
> If have sent other drivers that do the same and was told there that
> this is totally legit.

With each new SoC you would have to patch the driver to add the new
compatible property anyway. If your devices differ as much as by
having a different register layout, they need a dedicated compatible
property. In that case, you may as well add the register layout for
this new property into the driver.

The purpose of the DT is to describe the hardware layout, not its
internal workings. Saying "this function is fulfilled by this GPIO
because that's how the components are wired on my board" is something
that belongs to the DT. However, "This device is almost the same as
this one but with this particular register layout", is just moving the
boilerplate from one place to another. The DT has no vocation of
taking care of the differences between several revisions of a device ;
that's the job of the driver.

Re: [PATCH 4/5] GPIO: MIPS: ralink: add gpio driver for ralink MT762x SoC

[Alexandre Courbot]

On Mon, Oct 20, 2014 at 2:31 PM, John Crispin <blogic@openwrt.org> wrote:
>
>
> On 20/10/2014 06:57, Alexandre Courbot wrote:
>> On Sat, Oct 11, 2014 at 5:28 AM, John Crispin <blogic@openwrt.org>
>> wrote:
>>> Add gpio driver for Ralink SoC. This driver makes the gpio core
>>> on MT7621 and MT7628 work.
>>
>> Basically I have the same remarks as for the rt2880 driver. Both
>> seem to be very similar, and to work kind of like the gpio-generic
>> driver. I wonder if there wouldn't be a way to leverage this
>> driver instead of rewriting the same logic X times?
>>
>> Some more comments below...
>>
>>>
>>> Signed-off-by: John Crispin <blogic@openwrt.org> Cc:
>>> linux-mips@linux-mips.org Cc: linux-gpio@vger.kernel.org ---
>>> drivers/gpio/Kconfig       |    6 ++ drivers/gpio/Makefile
>>> | 1 + drivers/gpio/gpio-mt7621.c |  195
>>> ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed,
>>> 202 insertions(+) create mode 100644 drivers/gpio/gpio-mt7621.c
>>>
>>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index
>>> c91b15b..1ef83a0 100644 --- a/drivers/gpio/Kconfig +++
>>> b/drivers/gpio/Kconfig @@ -523,6 +523,12 @@ config
>>> GPIO_MC9S08DZ60 help Select this to enable the MC9S08DZ60 GPIO
>>> driver
>>>
>>> +config GPIO_MT7621 +       bool "Mediatek GPIO Support" +
>>> depends on MIPS && (SOC_MT7620 || SOC_MT7621) +       help + Say
>>> yes here to support the Mediatek SoC GPIO device + config
>>> GPIO_PCA953X tristate "PCA95[357]x, PCA9698, TCA64xx, and
>>> MAX7310 I/O ports" depends on I2C diff --git
>>> a/drivers/gpio/Makefile b/drivers/gpio/Makefile index
>>> d8f0f17..60a9e0e 100644 --- a/drivers/gpio/Makefile +++
>>> b/drivers/gpio/Makefile @@ -57,6 +57,7 @@
>>> obj-$(CONFIG_GPIO_MPC8XXX)    += gpio-mpc8xxx.o
>>> obj-$(CONFIG_GPIO_MSIC)                += gpio-msic.o
>>> obj-$(CONFIG_GPIO_MSM_V1)      += gpio-msm-v1.o
>>> obj-$(CONFIG_GPIO_MSM_V2)      += gpio-msm-v2.o
>>> +obj-$(CONFIG_GPIO_MT7621)      += gpio-mt7621.o
>>> obj-$(CONFIG_GPIO_MVEBU)        += gpio-mvebu.o
>>> obj-$(CONFIG_GPIO_MXC)         += gpio-mxc.o
>>> obj-$(CONFIG_GPIO_MXS)         += gpio-mxs.o diff --git
>>> a/drivers/gpio/gpio-mt7621.c b/drivers/gpio/gpio-mt7621.c new
>>> file mode 100644 index 0000000..7faf2c0 --- /dev/null +++
>>> b/drivers/gpio/gpio-mt7621.c @@ -0,0 +1,195 @@ +/* + * This
>>> program is free software; you can redistribute it and/or modify
>>> it + * under the terms of the GNU General Public License version
>>> 2 as published + * by the Free Software Foundation. + * + *
>>> Copyright (C) 2009-2011 Gabor Juhos <juhosg@openwrt.org> + *
>>> Copyright (C) 2013 John Crispin <blogic@openwrt.org> + */ +
>>> +#include <linux/io.h> +#include <linux/err.h> +#include
>>> <linux/gpio.h> +#include <linux/module.h> +#include
>>> <linux/of_irq.h> +#include <linux/spinlock.h> +#include
>>> <linux/irqdomain.h> +#include <linux/interrupt.h> +#include
>>> <linux/platform_device.h> + +#define MTK_BANK_WIDTH         32 +
>>> +enum mediatek_gpio_reg { +       GPIO_REG_CTRL = 0, +
>>> GPIO_REG_POL, +       GPIO_REG_DATA, +       GPIO_REG_DSET, +
>>> GPIO_REG_DCLR, +}; + +static void __iomem *mtk_gc_membase; +
>>> +struct mtk_gc { +       struct gpio_chip chip; + spinlock_t
>>> lock; +       int bank; +}; + +static inline struct mtk_gc
>>> +*to_mediatek_gpio(struct gpio_chip *chip) +{ + struct mtk_gc
>>> *mgc; + +       mgc = container_of(chip, struct mtk_gc, chip); +
>>> +       return mgc; +} + +static inline void +mtk_gpio_w32(struct
>>> mtk_gc *rg, u8 reg, u32 val) +{ + iowrite32(val, mtk_gc_membase +
>>> (reg * 0x10) + (rg->bank * 0x4)); +} + +static inline u32
>>> +mtk_gpio_r32(struct mtk_gc *rg, u8 reg) +{ +       return
>>> ioread32(mtk_gc_membase + (reg * 0x10) + (rg->bank * 0x4)); +} +
>>> +static void +mediatek_gpio_set(struct gpio_chip *chip, unsigned
>>> offset, int value) +{ +       struct mtk_gc *rg =
>>> to_mediatek_gpio(chip); + +       mtk_gpio_w32(rg, (value) ?
>>> GPIO_REG_DSET : GPIO_REG_DCLR, BIT(offset)); +} + +static int
>>> +mediatek_gpio_get(struct gpio_chip *chip, unsigned offset) +{ +
>>> struct mtk_gc *rg = to_mediatek_gpio(chip); + +       return
>>> !!(mtk_gpio_r32(rg, GPIO_REG_DATA) & BIT(offset)); +} + +static
>>> int +mediatek_gpio_direction_input(struct gpio_chip *chip,
>>> unsigned offset) +{ +       struct mtk_gc *rg =
>>> to_mediatek_gpio(chip); + unsigned long flags; +       u32 t; +
>>> + spin_lock_irqsave(&rg->lock, flags); +       t =
>>> mtk_gpio_r32(rg, GPIO_REG_CTRL); +       t &= ~BIT(offset); +
>>> mtk_gpio_w32(rg, GPIO_REG_CTRL, t); +
>>> spin_unlock_irqrestore(&rg->lock, flags); + +       return 0; +}
>>> + +static int +mediatek_gpio_direction_output(struct gpio_chip
>>> *chip, +                                       unsigned offset,
>>> int value) +{ +       struct mtk_gc *rg = to_mediatek_gpio(chip);
>>> +       unsigned long flags; +       u32 t; + +
>>> spin_lock_irqsave(&rg->lock, flags); +       t = mtk_gpio_r32(rg,
>>> GPIO_REG_CTRL); +       t |= BIT(offset); + mtk_gpio_w32(rg,
>>> GPIO_REG_CTRL, t); + mediatek_gpio_set(chip, offset, value); +
>>> spin_unlock_irqrestore(&rg->lock, flags); + +       return 0; +}
>>> + +static int +mediatek_gpio_request(struct gpio_chip *chip,
>>> unsigned offset) +{ +       int gpio = chip->base + offset; + +
>>> return pinctrl_request_gpio(gpio); +} + +static void
>>> +mediatek_gpio_free(struct gpio_chip *chip, unsigned offset) +{
>>> + int gpio = chip->base + offset; + + pinctrl_free_gpio(gpio); +}
>>> + +static int +mediatek_gpio_bank_probe(struct platform_device
>>> *pdev, struct device_node *bank) +{ +       const __be32 *id =
>>> of_get_property(bank, "reg", NULL); +       struct mtk_gc *rg =
>>> devm_kzalloc(&pdev->dev, + sizeof(struct mtk_gc), GFP_KERNEL); +
>>> if (!rg || !id) + return -ENOMEM; + +
>>> spin_lock_init(&rg->lock); + + rg->chip.dev = &pdev->dev; +
>>> rg->chip.label = dev_name(&pdev->dev); +       rg->chip.of_node =
>>> bank; + rg->chip.base = MTK_BANK_WIDTH * be32_to_cpu(*id);
>>
>> Argh, no, I don't think so. The GPIO integer space is not something
>> you can arbitrarily use like this. Any value that is not -1 is
>> dangerous here.
>>
>> If you add your banks one after the other, like you are seemingly
>> doing here, then you have a high probability of ending with
>> consecutive numbers. This cannot be guaranteed 100% though. That's
>> why we are trying to get away from the integer numberspace and to
>> use GPIO descriptors instead.
>
> dou you have an example of a driver already doing so ?

AFAICT all recent GPIO drivers set chip.base to -1. This member has
not been used in age - relying on a GPIO being given a particular
number is dangerous and a bad practice generally speaking.

>
>
>
>>
>>> +       rg->chip.ngpio = MTK_BANK_WIDTH; +
>>> rg->chip.direction_input = mediatek_gpio_direction_input; +
>>> rg->chip.direction_output = mediatek_gpio_direction_output; +
>>> rg->chip.get = mediatek_gpio_get; +       rg->chip.set =
>>> mediatek_gpio_set; +       rg->chip.request =
>>> mediatek_gpio_request; +       rg->chip.free =
>>> mediatek_gpio_free; + +       /* set polarity to low for all
>>> gpios */ +       mtk_gpio_w32(rg, GPIO_REG_POL, 0); + +
>>> dev_info(&pdev->dev, "registering %d gpios\n", rg->chip.ngpio); +
>>> +       return gpiochip_add(&rg->chip); +} + +static int
>>> +mediatek_gpio_probe(struct platform_device *pdev) +{ + struct
>>> device_node *bank, *np = pdev->dev.of_node; +       struct
>>> resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); +
>>> +       mtk_gc_membase = devm_ioremap_resource(&pdev->dev, res);
>>> +       if (IS_ERR(mtk_gc_membase)) +               return
>>> PTR_ERR(mtk_gc_membase); + +       for_each_child_of_node(np,
>>> bank) +               if (of_device_is_compatible(bank,
>>> "mediatek,mt7621-gpio-bank")) + mediatek_gpio_bank_probe(pdev,
>>> bank);
>>
>> Here you may want to make sure the banks are parsed in the right
>> order if the order of their base number matters to you. Another
>> solution would be to just have a property with the number of
>> banks, and use that instead of sub-nodes for each bank. This should
>> be doable here since your banks do not have special properties of
>> their own, nor do they differ from each other.
>>
>>
>
> that would be redundant or not "i will now list <4> banks", "here are
> the 4 banks". that is what the count helpers are for if we need to be
> aware of the number of properties prior to parsing the node.

My suggestion was to have a property indicating the number of banks,
and not listing them further.

>
> i am not sure i have seen another instance of dts using a count index
> for the following properties array/table/... do you have an example of
> a driver doing so ?

gpio-bcm-kona does something close: the number of interrupts specified
indicate how many banks the chip has. I would not rely too much on
existing bindings to decide what a "good" practice is.

Note that my question is only relevant if you care about the numbers
that your GPIOs are going to receive. The GPIO integer interface is
deprecated and our recommendation is to build your system in such a
way that you don't need to use it (something that is made easier by
DT) ; in this case your current binding would work.

Re: [PATCH 1/5] DT: Add documentation for gpio-rt2880

[Linus Walleij]

On Fri, Oct 10, 2014 at 10:28 PM, John Crispin <blogic@openwrt.org> wrote:

> Describe gpio-rt2880 binding.
>
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Cc: linux-mips@linux-mips.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-gpio@vger.kernel.org
> ---
>  .../devicetree/bindings/gpio/gpio-rt2880.txt       |   40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-rt2880.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-rt2880.txt b/Documentation/devicetree/bindings/gpio/gpio-rt2880.txt
> new file mode 100644
> index 0000000..b4acf02
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-rt2880.txt
> @@ -0,0 +1,40 @@
> +Ralink SoC GPIO controller bindings
> +
> +Required properties:
> +- compatible:
> +  - "ralink,rt2880-gpio" for Ralink controllers
> +- #gpio-cells : Should be two.
> +  - first cell is the pin number
> +  - second cell is used to specify optional parameters (unused)
> +- gpio-controller : Marks the device node as a GPIO controller
> +- reg : Physical base address and length of the controller's registers
> +- interrupt-parent: phandle to the INTC device node
> +- interrupts : Specify the INTC interrupt number
> +- ralink,num-gpios : Specify the number of GPIOs
> +- ralink,register-map : The register layout depends on the GPIO bank and actual
> +               SoC type. Register offsets need to be in this order.
> +               [ INT, EDGE, RENA, FENA, DATA, DIR, POL, SET, RESET, TOGGLE ]
> +
> +Optional properties:
> +- ralink,gpio-base : Specify the GPIO chips base number

NAK. This is a Linux-internal number. It is not OS-neutral.

Yours,
Linus Walleij

Re: [PATCH 5/5] MIPS: ralink: we require gpiolib

[Linus Walleij]

On Fri, Oct 10, 2014 at 10:28 PM, John Crispin <blogic@openwrt.org> wrote:

> Select ARCH_REQUIRE_GPIOLIB by default when building a kernel for RALINK.
>
> Signed-off-by: John Crispin <blogic@openwrt.org>

Happy with selecting this, but don't think you need the altered
mach gpio.h header. Do like x86 does.

Yours,
Linus Walleij

Re: [PATCH 3/5] DT: Add documentation for gpio-mt7621

[Linus Walleij]

On Fri, Oct 10, 2014 at 10:28 PM, John Crispin <blogic@openwrt.org> wrote:

> Describe gpio-mt7621 binding.
>
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Cc: linux-mips@linux-mips.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-gpio@vger.kernel.org

These bindings look fine.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 2/5] GPIO: MIPS: ralink: add gpio driver for ralink rt2880 SoC

[Linus Walleij]

On Mon, Oct 20, 2014 at 8:19 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Mon, Oct 20, 2014 at 2:27 PM, John Crispin <blogic@openwrt.org> wrote:
>> On 20/10/2014 06:41, Alexandre Courbot wrote:
>> On Sat, Oct 11, 2014 at 5:28 AM, John Crispin <blogic@openwrt.org> wrote:

>>> This (and the device tree bindings) seems the indicate that the
>>> registers offset can vary depending on the chip and bank. The chip
>>> can be specified using the compatible property, as for the bank you
>>> can also require a property giving the bank number. With these two
>>> bits of information, this driver should be able to pick the right
>>> register layout out of an in-driver table. This would be much
>>> cleaner that letting the DT specify whatever layout it wants.
>>
>> i tend to disagree. if we put the register offsets into the driver we
>> will have lots of static arrays (5 or 6) and with each new soc we need
>> to potentially need to patch the driver causing us in openwrt to carry
>> lots of patches and have to worry about upstreaming them. From my
>> understanding, the dts has this exact purpose, describing the hardware
>> and in turn reducing the boiler plate and static code in the drivers.
>> If have sent other drivers that do the same and was told there that
>> this is totally legit.
>
> With each new SoC you would have to patch the driver to add the new
> compatible property anyway. If your devices differ as much as by
> having a different register layout, they need a dedicated compatible
> property. In that case, you may as well add the register layout for
> this new property into the driver.

I agree. +1 on this.

Yours,
Linus Walleij

Re: [PATCH 4/5] GPIO: MIPS: ralink: add gpio driver for ralink MT762x SoC

[Linus Walleij]

On Mon, Oct 20, 2014 at 8:27 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Mon, Oct 20, 2014 at 2:31 PM, John Crispin <blogic@openwrt.org> wrote:

>> i am not sure i have seen another instance of dts using a count index
>> for the following properties array/table/... do you have an example of
>> a driver doing so ?
>
> gpio-bcm-kona does something close: the number of interrupts specified
> indicate how many banks the chip has. I would not rely too much on
> existing bindings to decide what a "good" practice is.

Overall honestly I think too much responsibility of DT bindings
(and now also ACPI!) is pushed to the subsystem maintainers,
we are Linux GPIO experts, not really hardware description experts.

We do a best effort but the current practice of letting the subsystem
maintainers have the final word on all device tree bindings is
not optimal, I would like to have a DT/OF persons Reviewed-by tag
on any substantially new binding that is not just a copy of what
every other GPIO driver is doing.

Yours,
Linus Walleij