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

[John Crispin]

Describe gpio-mt7621 binding.

Signed-off-by: John Crispin <***@openwrt.org>
Cc: linux-***@linux-mips.org
Cc: ***@vger.kernel.org
Cc: linux-***@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:
+ ***@600 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ compatible = "mediatek,mt7621-gpio";
+ reg = <0x600 0x100>;
+
+ gpio0: ***@0 {
+ reg = <0>;
+ compatible = "mediatek,mt7621-gpio-bank";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ gpio1: ***@1 {
+ reg = <1>;
+ compatible = "mediatek,mt7621-gpio-bank";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+ };

[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 <***@openwrt.org>
Cc: linux-***@linux-mips.org
Cc: linux-***@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 <***@openwrt.org>
+ * Copyright (C) 2013 John Crispin <***@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);

[Alexandre Courbot]

Post by John Crispin
Add gpio driver for Ralink SoC. This driver makes the gpio core on

Makes the gpio core what?

Post by John Crispin
RT2880, RT305x, rt3352, rt3662, rt3883, rt5350 and mt7620 work.
---
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.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/gpio.h>

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

Post by John Crispin
+#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?

Post by John Crispin
+
+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).

Post by John Crispin
+{
+ 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).

Post by John Crispin
+{
+ 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.

Post by John Crispin
+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[John Crispin]

Hi,

Post by John Crispin
Add gpio driver for Ralink SoC. This driver makes the gpio core on

Makes the gpio core what?

Post by John Crispin
RT2880, RT305x, rt3352, rt3662, rt3883, rt5350 and mt7620 work.

work as it says in the last work of the sentence

[snip..]

Post by John Crispin
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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Alexandre Courbot]

Post by Alexandre Courbot
Hi,

Post by John Crispin
Add gpio driver for Ralink SoC. This driver makes the gpio core on

Makes the gpio core what?

Post by John Crispin
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. :)

Post by Alexandre Courbot
[snip..]

Post by John Crispin
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[John Crispin]

Select ARCH_REQUIRE_GPIOLIB by default when building a kernel for RALINK.

Signed-off-by: John Crispin <***@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 <***@openwrt.org>
+ * Copyright (C) 2008 Imre 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 */

[John Crispin]

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

Signed-off-by: John Crispin <***@openwrt.org>
Cc: linux-***@linux-mips.org
Cc: linux-***@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 <***@openwrt.org>
+ * Copyright (C) 2013 John Crispin <***@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);

[Alexandre Courbot]

Post by John Crispin
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...

Post by John Crispin
---
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.
+ *
+ */
+
+#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.

Post by John Crispin
+ 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[John Crispin]

Post by Alexandre Courbot

Post by John Crispin
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...

Post by John Crispin
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 +++
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 +++
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 +++
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. + * + *
+#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 ?

Post by Alexandre Courbot

Post by John Crispin
+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Alexandre Courbot]

Post by John Crispin

Post by Alexandre Courbot

Post by John Crispin
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...

Post by John Crispin
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 +++
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 +++
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 +++
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. + * + *
+#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.

Post by John Crispin

Post by Alexandre Courbot

Post by John Crispin
+ 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.

Post by John Crispin
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html