[PATCH v2 0/7] staging: mt7621-gpio: last cleanups

[PATCH v2 0/7] staging: mt7621-gpio: last cleanups

[Sergio Paracuellos]

After submiting this driver to try to get mainlined and get
out of staging some new cleanups seems to be necessary.
According to this main of Linus Walleij:

http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-June/121742.html

this series tries to fix all of the issues in order to send
v2 and give it a new try. Because I don't have to hardware to
test my changes I send new cleanups first in staging to make
easier to NeilBrown test it and get a feedback about them.

Changes in v2:
    - Patch where GPIOLIB_IRQCHIP was used avoiding
      the use of a custom irq domain has been dropped to
      be sure after this changes all is working properly.
      (This was PATCH 7 in previous series)
    - PATCH 1: 
         * avoid introducing new macros and use 'bank'
           field of mtk_gc with register offset.
         * Make correct use of bgpio_init passing new
           void __iomem pointers instead of use the
           macros. 
    - Previous series PATCH 8 now is PATCH 7. Avoid the
      use of a switch-case statement which was wrong and
      distinc if we have RISSING AND FALLING EDGE interrupt
      or HIGH LOW level ones. This last two are exclusive and
      cannot be generated at the same time.

Also, I think is we finally avoid to use a new irq_domain the
need for the new functions introduced for request and release
resources dissapears. I was diving down the other drivers code
and I see that these two only are used in drivers which use its
own irq_domain. Correct me if I am wrong, please.

Hope this helps.

Thanks in advance.

Best regards,
    Sergio Paracuellos

Sergio Paracuellos (7):
  staging: mt7621-gpio: make use 'bgpio_init' from GPIO_GENERIC
  staging: mt7621-gpio: avoid including 'gpio.h'
  staging: mt7621-gpio: make use of 'builtin_platform_driver'
  staging: mt7621-gpio: implement '.irq_[request|release]_resources'
    functions
  staging: mt7621-gpio: add COMPILE_TEST
  staging: mt7621-gpio: add kerneldoc for state data containers
  staging: mt7621-gpio: implement high level and low level irqs

 drivers/staging/mt7621-gpio/Kconfig       |   3 +-
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 251 ++++++++++++++++--------------
 2 files changed, 138 insertions(+), 116 deletions(-)

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 1/7] staging: mt7621-gpio: make use 'bgpio_init' from GPIO_GENERIC

[Sergio Paracuellos]

Gpio complexity is just masking the fact that offset is always
0..n and writes to bits 0..n of some memory address. Because
of this whole thing can just me converted to use GPIO_GENERIC
and avoid duplications of a lot of driver custom functions.
So use bgpio_init instead of custom code adding GPIO_GENERIC
dependency to the Kconfig file. Also to make easier using
bgpio_init function offset for each gpio bank, enumeration
where register were defined has been replaced in favour of
some macros which handle each gpio offset taking into account
the bank where are located. Because of this change write and
read functions which are being used for remaining irq handling
stuff have been updated also as well as its dependencies along
the code.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/Kconfig       |   1 +
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 136 +++++++++---------------------
 2 files changed, 42 insertions(+), 95 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/Kconfig b/drivers/staging/mt7621-gpio/Kconfig
index c741ec3..f4453e8 100644
--- a/drivers/staging/mt7621-gpio/Kconfig
+++ b/drivers/staging/mt7621-gpio/Kconfig
@@ -1,6 +1,7 @@
 config GPIO_MT7621
 	bool "Mediatek GPIO Support"
 	depends on SOC_MT7620 || SOC_MT7621
+	select GPIO_GENERIC
 	select ARCH_REQUIRE_GPIOLIB
 	help
 	  Say yes here to support the Mediatek SoC GPIO device
diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 0c4fb4a..eb60dd4 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -19,19 +19,18 @@
 #define MTK_BANK_WIDTH		32
 #define PIN_MASK(nr)		(1UL << ((nr % MTK_BANK_WIDTH)))
 
-enum mediatek_gpio_reg {
-	GPIO_REG_CTRL = 0,
-	GPIO_REG_POL,
-	GPIO_REG_DATA,
-	GPIO_REG_DSET,
-	GPIO_REG_DCLR,
-	GPIO_REG_REDGE,
-	GPIO_REG_FEDGE,
-	GPIO_REG_HLVL,
-	GPIO_REG_LLVL,
-	GPIO_REG_STAT,
-	GPIO_REG_EDGE,
-};
+#define GPIO_BANK_WIDE	0x04
+#define GPIO_REG_CTRL	0x00
+#define GPIO_REG_POL	0x10
+#define GPIO_REG_DATA	0x20
+#define GPIO_REG_DSET	0x30
+#define GPIO_REG_DCLR	0x40
+#define GPIO_REG_REDGE	0x50
+#define GPIO_REG_FEDGE	0x60
+#define GPIO_REG_HLVL	0x70
+#define GPIO_REG_LLVL	0x80
+#define GPIO_REG_STAT	0x90
+#define GPIO_REG_EDGE	0xA0
 
 struct mtk_gc {
 	struct gpio_chip chip;
@@ -55,80 +54,23 @@ to_mediatek_gpio(struct gpio_chip *chip)
 }
 
 static inline void
-mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
+mtk_gpio_w32(struct mtk_gc *rg, u32 offset, u32 val)
 {
-	struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
-	u32 offset = (reg * 0x10) + (rg->bank * 0x4);
+	struct gpio_chip *gc = &rg->chip;
+	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 
-	iowrite32(val, gpio_data->gpio_membase + offset);
+	offset = (rg->bank * GPIO_BANK_WIDE) + offset;
+	gc->write_reg(gpio_data->gpio_membase + offset, val);
 }
 
 static inline u32
-mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
-{
-	struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
-	u32 offset = (reg * 0x10) + (rg->bank * 0x4);
-
-	return ioread32(gpio_data->gpio_membase + offset);
-}
-
-static void
-mediatek_gpio_set(struct gpio_chip *chip, unsigned int 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 int 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 int 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 int offset, int value)
+mtk_gpio_r32(struct mtk_gc *rg, u32 offset)
 {
-	struct mtk_gc *rg = to_mediatek_gpio(chip);
-	unsigned long flags;
-	u32 t;
+	struct gpio_chip *gc = &rg->chip;
+	struct mtk_data *gpio_data = gpiochip_get_data(gc);
 
-	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_get_direction(struct gpio_chip *chip, unsigned int offset)
-{
-	struct mtk_gc *rg = to_mediatek_gpio(chip);
-	u32 t = mtk_gpio_r32(rg, GPIO_REG_CTRL);
-
-	return (t & BIT(offset)) ? GPIOF_DIR_OUT : GPIOF_DIR_IN;
+	offset = (rg->bank * GPIO_BANK_WIDE) + offset;
+	return gc->read_reg(gpio_data->gpio_membase + offset);
 }
 
 static int
@@ -144,34 +86,38 @@ mediatek_gpio_to_irq(struct gpio_chip *chip, unsigned int pin)
 static int
 mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node *bank)
 {
-	struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
+	struct mtk_data *gpio = dev_get_drvdata(&pdev->dev);
 	const __be32 *id = of_get_property(bank, "reg", NULL);
 	struct mtk_gc *rg;
+	void __iomem *dat, *set, *ctrl, *diro;
 	int ret;
 
 	if (!id || be32_to_cpu(*id) >= MTK_BANK_CNT)
 		return -EINVAL;
 
-	rg = &gpio_data->gc_map[be32_to_cpu(*id)];
+	rg = &gpio->gc_map[be32_to_cpu(*id)];
 	memset(rg, 0, sizeof(*rg));
 
 	spin_lock_init(&rg->lock);
-
-	rg->chip.parent = &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_direction = mediatek_gpio_get_direction;
-	rg->chip.get = mediatek_gpio_get;
-	rg->chip.set = mediatek_gpio_set;
-	if (gpio_data->gpio_irq_domain)
-		rg->chip.to_irq = mediatek_gpio_to_irq;
 	rg->bank = be32_to_cpu(*id);
 
-	ret = devm_gpiochip_add_data(&pdev->dev, &rg->chip, gpio_data);
+	dat = gpio->gpio_membase + GPIO_REG_DATA + (rg->bank * GPIO_BANK_WIDE);
+	set = gpio->gpio_membase + GPIO_REG_DSET + (rg->bank * GPIO_BANK_WIDE);
+	ctrl = gpio->gpio_membase + GPIO_REG_DCLR + (rg->bank * GPIO_BANK_WIDE);
+	diro = gpio->gpio_membase + GPIO_REG_CTRL + (rg->bank * GPIO_BANK_WIDE);
+
+	ret = bgpio_init(&rg->chip, &pdev->dev, 4,
+			 dat, set, ctrl, diro, NULL, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "bgpio_init() failed\n");
+		return ret;
+	}
+
+	if (gpio->gpio_irq_domain)
+		rg->chip.to_irq = mediatek_gpio_to_irq;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &rg->chip, gpio);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Could not register gpio %d, ret=%d\n",
 			rg->chip.ngpio, ret);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/7] staging: mt7621-gpio: avoid including 'gpio.h'

[Sergio Paracuellos]

Including file '<linux/gpio.h>' should be avoided in
new drivers code, so just remove it because it is
no necessary at all.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index eb60dd4..f95310c 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -5,7 +5,6 @@
  */
 
 #include <linux/err.h>
-#include <linux/gpio.h>
 #include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 3/7] staging: mt7621-gpio: make use of 'builtin_platform_driver'

[Sergio Paracuellos]

This driver was being registered using 'module_platform_driver'
but it is not a module at all. Instead of this use
'builtin_platform_driver' which seems to be the correct one.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index f95310c..6a31f60 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -312,4 +312,4 @@ static struct platform_driver mediatek_gpio_driver = {
 	},
 };
 
-module_platform_driver(mediatek_gpio_driver);
+builtin_platform_driver(mediatek_gpio_driver);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 4/7] staging: mt7621-gpio: implement '.irq_[request|release]_resources' functions

[Sergio Paracuellos]

When implementing custom irqchips it is important to also
implement .irq_request_resources() and .irq_release_resources()
and make sure these call gpiochip_[un]lock_as_irq().
Add those two for this driver. Also store struct device pointer
in global state structure to be able to use 'dev_err' with the
device from 'mediatek_request_resources' function.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 42 +++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 6a31f60..e5a7979 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -40,6 +40,7 @@ struct mtk_gc {
 };
 
 struct mtk_data {
+	struct device *dev;
 	void __iomem *gpio_membase;
 	int gpio_irq;
 	struct irq_domain *gpio_irq_domain;
@@ -231,12 +232,42 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
 	return 0;
 }
 
+static int mediatek_irq_reqres(struct irq_data *d)
+{
+	struct mtk_data *gpio_data = irq_data_get_irq_chip_data(d);
+	int bank = irqd_to_hwirq(d) / MTK_BANK_WIDTH;
+	struct mtk_gc *rg = &gpio_data->gc_map[bank];
+	struct gpio_chip *gc = &rg->chip;
+	int ret;
+
+	ret = gpiochip_lock_as_irq(gc, irqd_to_hwirq(d));
+	if (ret) {
+		dev_err(gpio_data->dev, "unable to lock HW IRQ %lu for IRQ\n",
+			irqd_to_hwirq(d));
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void mediatek_irq_relres(struct irq_data *d)
+{
+	struct mtk_data *gpio_data = irq_data_get_irq_chip_data(d);
+	int bank = irqd_to_hwirq(d) / MTK_BANK_WIDTH;
+	struct mtk_gc *rg = &gpio_data->gc_map[bank];
+	struct gpio_chip *gc = &rg->chip;
+
+	gpiochip_unlock_as_irq(gc, irqd_to_hwirq(d));
+}
+
 static struct irq_chip mediatek_gpio_irq_chip = {
-	.name		= "GPIO",
-	.irq_unmask	= mediatek_gpio_irq_unmask,
-	.irq_mask	= mediatek_gpio_irq_mask,
-	.irq_mask_ack	= mediatek_gpio_irq_mask,
-	.irq_set_type	= mediatek_gpio_irq_type,
+	.name			= "GPIO",
+	.irq_unmask		= mediatek_gpio_irq_unmask,
+	.irq_mask		= mediatek_gpio_irq_mask,
+	.irq_mask_ack		= mediatek_gpio_irq_mask,
+	.irq_set_type		= mediatek_gpio_irq_type,
+	.irq_request_resources	= mediatek_irq_reqres,
+	.irq_release_resources	= mediatek_irq_relres,
 };
 
 static int
@@ -284,6 +315,7 @@ mediatek_gpio_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "irq_domain_add_linear failed\n");
 	}
 
+	gpio_data->dev = &pdev->dev;
 	platform_set_drvdata(pdev, gpio_data);
 
 	for_each_child_of_node(np, bank)
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 5/7] staging: mt7621-gpio: add COMPILE_TEST

[Sergio Paracuellos]

This driver is actually platform-agnostic.  Add COMPILE_TEST for
the compilation test coverage.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-gpio/Kconfig b/drivers/staging/mt7621-gpio/Kconfig
index f4453e8..835998a 100644
--- a/drivers/staging/mt7621-gpio/Kconfig
+++ b/drivers/staging/mt7621-gpio/Kconfig
@@ -1,6 +1,6 @@
 config GPIO_MT7621
 	bool "Mediatek GPIO Support"
-	depends on SOC_MT7620 || SOC_MT7621
+	depends on SOC_MT7620 || SOC_MT7621 || COMPILE_TEST
 	select GPIO_GENERIC
 	select ARCH_REQUIRE_GPIOLIB
 	help
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 6/7] staging: mt7621-gpio: add kerneldoc for state data containers

[Sergio Paracuellos]

This commit adds kerneldoc for the two data containers in
order to better understanding of its existence.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index e5a7979..54c18c1 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -31,6 +31,14 @@
 #define GPIO_REG_STAT	0x90
 #define GPIO_REG_EDGE	0xA0
 
+/**
+ * struct mtk_gc - data for a single gpio-chip
+ * @chip: gpio chip instance
+ * @lock: spinlock to protect the chip
+ * @bank: gpio bank number for the chip
+ * @rising: mask for rising irqs
+ * @falling: mask for falling irqs
+ */
 struct mtk_gc {
 	struct gpio_chip chip;
 	spinlock_t lock;
@@ -39,6 +47,16 @@ struct mtk_gc {
 	u32 falling;
 };
 
+/**
+ * struct mtk_data - state container for
+ * data of the platform driver. It is a
+ * single irq-chip but 3 separate gpio-chip.
+ * @dev: device instance
+ * @gpio_membase: memory base address
+ * @gpio_irq: irq number from the device tree
+ * @gpio_irq_domain: irq domain for this chip
+ * @gc_map: array of the gpio chips
+ */
 struct mtk_data {
 	struct device *dev;
 	void __iomem *gpio_membase;
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 7/7] staging: mt7621-gpio: implement high level and low level irqs

[Sergio Paracuellos]

This chip support high level and low level interrupts. Those
have to be implemented also to get a complete and clean driver.

Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 51 +++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 54c18c1..39874cb 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -38,6 +38,8 @@
  * @bank: gpio bank number for the chip
  * @rising: mask for rising irqs
  * @falling: mask for falling irqs
+ * @hlevel: mask for high level irqs
+ * @llevel: mask for low level irqs
  */
 struct mtk_gc {
 	struct gpio_chip chip;
@@ -45,6 +47,8 @@ struct mtk_gc {
 	int bank;
 	u32 rising;
 	u32 falling;
+	u32 hlevel;
+	u32 llevel;
 };
 
 /**
@@ -184,7 +188,7 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
 	int bank = pin / MTK_BANK_WIDTH;
 	struct mtk_gc *rg = &gpio_data->gc_map[bank];
 	unsigned long flags;
-	u32 rise, fall;
+	u32 rise, fall, high, low;
 
 	if (!rg)
 		return;
@@ -192,8 +196,12 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
 	spin_lock_irqsave(&rg->lock, flags);
 	rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
 	fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
+	high = mtk_gpio_r32(rg, GPIO_REG_HLVL);
+	low = mtk_gpio_r32(rg, GPIO_REG_LLVL);
 	mtk_gpio_w32(rg, GPIO_REG_REDGE, rise | (PIN_MASK(pin) & rg->rising));
 	mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall | (PIN_MASK(pin) & rg->falling));
+	mtk_gpio_w32(rg, GPIO_REG_HLVL, high | (PIN_MASK(pin) & rg->hlevel));
+	mtk_gpio_w32(rg, GPIO_REG_LLVL, low | (PIN_MASK(pin) & rg->llevel));
 	spin_unlock_irqrestore(&rg->lock, flags);
 }
 
@@ -205,7 +213,7 @@ mediatek_gpio_irq_mask(struct irq_data *d)
 	int bank = pin / MTK_BANK_WIDTH;
 	struct mtk_gc *rg = &gpio_data->gc_map[bank];
 	unsigned long flags;
-	u32 rise, fall;
+	u32 rise, fall, high, low;
 
 	if (!rg)
 		return;
@@ -213,8 +221,12 @@ mediatek_gpio_irq_mask(struct irq_data *d)
 	spin_lock_irqsave(&rg->lock, flags);
 	rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
 	fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
+	high = mtk_gpio_r32(rg, GPIO_REG_HLVL);
+	low = mtk_gpio_r32(rg, GPIO_REG_LLVL);
 	mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall & ~PIN_MASK(pin));
 	mtk_gpio_w32(rg, GPIO_REG_REDGE, rise & ~PIN_MASK(pin));
+	mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~PIN_MASK(pin));
+	mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~PIN_MASK(pin));
 	spin_unlock_irqrestore(&rg->lock, flags);
 }
 
@@ -231,21 +243,34 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
 		return -1;
 
 	if (type == IRQ_TYPE_PROBE) {
-		if ((rg->rising | rg->falling) & mask)
+		if ((rg->rising | rg->falling |
+		     rg->hlevel | rg->llevel) & mask)
 			return 0;
 
-		type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
+		type = (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
+			IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW);
 	}
 
-	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;
+	if (type & IRQ_TYPE_EDGE_RISING ||
+	    type & 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;
+	} else {
+		if (type & IRQ_TYPE_LEVEL_HIGH) {
+			rg->hlevel |= mask;
+			rg->llevel &= ~mask;
+		} else {
+			rg->llevel |= mask;
+			rg->hlevel &= ~mask;
+		}
+	}
 
 	return 0;
 }
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 7/7] staging: mt7621-gpio: implement high level and low level irqs

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 5249 bytes --]

On Mon, Jun 11 2018, Sergio Paracuellos wrote:

> This chip support high level and low level interrupts. Those
> have to be implemented also to get a complete and clean driver.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
> ---
>  drivers/staging/mt7621-gpio/gpio-mt7621.c | 51 +++++++++++++++++++++++--------
>  1 file changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> index 54c18c1..39874cb 100644
> --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> @@ -38,6 +38,8 @@
>   * @bank: gpio bank number for the chip
>   * @rising: mask for rising irqs
>   * @falling: mask for falling irqs
> + * @hlevel: mask for high level irqs
> + * @llevel: mask for low level irqs
>   */
>  struct mtk_gc {
>  	struct gpio_chip chip;
> @@ -45,6 +47,8 @@ struct mtk_gc {
>  	int bank;
>  	u32 rising;
>  	u32 falling;
> +	u32 hlevel;
> +	u32 llevel;
>  };
>  
>  /**
> @@ -184,7 +188,7 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
>  	int bank = pin / MTK_BANK_WIDTH;
>  	struct mtk_gc *rg = &gpio_data->gc_map[bank];
>  	unsigned long flags;
> -	u32 rise, fall;
> +	u32 rise, fall, high, low;
>  
>  	if (!rg)
>  		return;
> @@ -192,8 +196,12 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
>  	spin_lock_irqsave(&rg->lock, flags);
>  	rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
>  	fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
> +	high = mtk_gpio_r32(rg, GPIO_REG_HLVL);
> +	low = mtk_gpio_r32(rg, GPIO_REG_LLVL);
>  	mtk_gpio_w32(rg, GPIO_REG_REDGE, rise | (PIN_MASK(pin) & rg->rising));
>  	mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall | (PIN_MASK(pin) & rg->falling));
> +	mtk_gpio_w32(rg, GPIO_REG_HLVL, high | (PIN_MASK(pin) & rg->hlevel));
> +	mtk_gpio_w32(rg, GPIO_REG_LLVL, low | (PIN_MASK(pin) & rg->llevel));
>  	spin_unlock_irqrestore(&rg->lock, flags);
>  }
>  
> @@ -205,7 +213,7 @@ mediatek_gpio_irq_mask(struct irq_data *d)
>  	int bank = pin / MTK_BANK_WIDTH;
>  	struct mtk_gc *rg = &gpio_data->gc_map[bank];
>  	unsigned long flags;
> -	u32 rise, fall;
> +	u32 rise, fall, high, low;
>  
>  	if (!rg)
>  		return;
> @@ -213,8 +221,12 @@ mediatek_gpio_irq_mask(struct irq_data *d)
>  	spin_lock_irqsave(&rg->lock, flags);
>  	rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
>  	fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
> +	high = mtk_gpio_r32(rg, GPIO_REG_HLVL);
> +	low = mtk_gpio_r32(rg, GPIO_REG_LLVL);
>  	mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall & ~PIN_MASK(pin));
>  	mtk_gpio_w32(rg, GPIO_REG_REDGE, rise & ~PIN_MASK(pin));
> +	mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~PIN_MASK(pin));
> +	mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~PIN_MASK(pin));
>  	spin_unlock_irqrestore(&rg->lock, flags);
>  }
>  
> @@ -231,21 +243,34 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
>  		return -1;
>  
>  	if (type == IRQ_TYPE_PROBE) {
> -		if ((rg->rising | rg->falling) & mask)
> +		if ((rg->rising | rg->falling |
> +		     rg->hlevel | rg->llevel) & mask)
>  			return 0;
>  
> -		type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
> +		type = (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
> +			IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW);
>  	}

This doesn't look right.
IRQ_TYPE_PROBE isn't very well documented and there aren't a lot of
examples of usage, but I think the idea is that if the interrupt type
is already set, then leave it along, otherwise choose a sane default,
which is IRQ_TYPE_EDGE_BOTH
(aka IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING) in all the cases
that I've looked at.

Certainly setting the type to RISING and FALLING and LOW and HIGH cannot
be right as that would cause constant interrupts.

>  
> -	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;
> +	if (type & IRQ_TYPE_EDGE_RISING ||
> +	    type & 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;
> +	} else {
> +		if (type & IRQ_TYPE_LEVEL_HIGH) {
> +			rg->hlevel |= mask;
> +			rg->llevel &= ~mask;
> +		} else {
> +			rg->llevel |= mask;
> +			rg->hlevel &= ~mask;
> +		}
> +	}

I wonder if we should be clearing the mask bit for hlevel and llevel
when setting either edge - and clearing the edge bits when setting a
level.

Actually, you might have been right about using a switch statement.
Several drivers have
  switch (type & IRQ_TYPE_SENSE_MASK) {
or similar.
The cope with the possiblity that both RISING and FALLING are set, they
have a case for IRQ_TYPE_EDGE_BOTH.
Maybe do
 rg->rising &= ~mask;
 rg->falling &= ~mask;
 rg->hlevel &= ~mask;
 rg->llevel &= ~mask;
 switch (type & IRQ_TYPE_SENSE_MASK) {
 case IRQ_TYPE_EDGE_BOTH:
   rg->rising |= mask;
   rg->falling |= mask;
   break;
 ....
 }


The current code works for my test-cases, but maybe it could be better.

Thanks,
NeilBrown

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 0/7] staging: mt7621-gpio: last cleanups

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 2838 bytes --]

On Mon, Jun 11 2018, Sergio Paracuellos wrote:

> After submiting this driver to try to get mainlined and get
> out of staging some new cleanups seems to be necessary.
> According to this main of Linus Walleij:
>
> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-June/121742.html
>
> this series tries to fix all of the issues in order to send
> v2 and give it a new try. Because I don't have to hardware to
> test my changes I send new cleanups first in staging to make
> easier to NeilBrown test it and get a feedback about them.
>
> Changes in v2:
>     - Patch where GPIOLIB_IRQCHIP was used avoiding
>       the use of a custom irq domain has been dropped to
>       be sure after this changes all is working properly.
>       (This was PATCH 7 in previous series)
>     - PATCH 1: 
>          * avoid introducing new macros and use 'bank'
>            field of mtk_gc with register offset.
>          * Make correct use of bgpio_init passing new
>            void __iomem pointers instead of use the
>            macros. 
>     - Previous series PATCH 8 now is PATCH 7. Avoid the
>       use of a switch-case statement which was wrong and
>       distinc if we have RISSING AND FALLING EDGE interrupt
>       or HIGH LOW level ones. This last two are exclusive and
>       cannot be generated at the same time.
>
> Also, I think is we finally avoid to use a new irq_domain the
> need for the new functions introduced for request and release
> resources dissapears. I was diving down the other drivers code
> and I see that these two only are used in drivers which use its
> own irq_domain. Correct me if I am wrong, please.
>
> Hope this helps.
>
> Thanks in advance.

Thanks a lot.
This series appears to work, though I sent a separate comment on one
piece of code.
However the gpio are numbers
480-511
448-479
416-447

instead of
0-31
32-63
64-95

which would be more normal.
Maybe when you resubmit I'll raid it with Linus Walleij and see if he
can explain why I can't have 0-95.

Thanks,
NeilBrown


>
> Best regards,
>     Sergio Paracuellos
>
> Sergio Paracuellos (7):
>   staging: mt7621-gpio: make use 'bgpio_init' from GPIO_GENERIC
>   staging: mt7621-gpio: avoid including 'gpio.h'
>   staging: mt7621-gpio: make use of 'builtin_platform_driver'
>   staging: mt7621-gpio: implement '.irq_[request|release]_resources'
>     functions
>   staging: mt7621-gpio: add COMPILE_TEST
>   staging: mt7621-gpio: add kerneldoc for state data containers
>   staging: mt7621-gpio: implement high level and low level irqs
>
>  drivers/staging/mt7621-gpio/Kconfig       |   3 +-
>  drivers/staging/mt7621-gpio/gpio-mt7621.c | 251 ++++++++++++++++--------------
>  2 files changed, 138 insertions(+), 116 deletions(-)
>
> -- 
> 2.7.4

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 0/7] staging: mt7621-gpio: last cleanups

[Sergio Paracuellos]

On Mon, Jun 11, 2018 at 06:33:44PM +1000, NeilBrown wrote:
> On Mon, Jun 11 2018, Sergio Paracuellos wrote:
> 
> > After submiting this driver to try to get mainlined and get
> > out of staging some new cleanups seems to be necessary.
> > According to this main of Linus Walleij:
> >
> > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-June/121742.html
> >
> > this series tries to fix all of the issues in order to send
> > v2 and give it a new try. Because I don't have to hardware to
> > test my changes I send new cleanups first in staging to make
> > easier to NeilBrown test it and get a feedback about them.
> >
> > Changes in v2:
> >     - Patch where GPIOLIB_IRQCHIP was used avoiding
> >       the use of a custom irq domain has been dropped to
> >       be sure after this changes all is working properly.
> >       (This was PATCH 7 in previous series)
> >     - PATCH 1: 
> >          * avoid introducing new macros and use 'bank'
> >            field of mtk_gc with register offset.
> >          * Make correct use of bgpio_init passing new
> >            void __iomem pointers instead of use the
> >            macros. 
> >     - Previous series PATCH 8 now is PATCH 7. Avoid the
> >       use of a switch-case statement which was wrong and
> >       distinc if we have RISSING AND FALLING EDGE interrupt
> >       or HIGH LOW level ones. This last two are exclusive and
> >       cannot be generated at the same time.
> >
> > Also, I think is we finally avoid to use a new irq_domain the
> > need for the new functions introduced for request and release
> > resources dissapears. I was diving down the other drivers code
> > and I see that these two only are used in drivers which use its
> > own irq_domain. Correct me if I am wrong, please.
> >
> > Hope this helps.
> >
> > Thanks in advance.
> 
> Thanks a lot.
> This series appears to work, though I sent a separate comment on one
> piece of code.

Thanks for testing, review and feedback.

I have resent a complete v3 of this series taking into account
your comments there.

> However the gpio are numbers
> 480-511
> 448-479
> 416-447
> 
> instead of
> 0-31
> 32-63
> 64-95
> 
> which would be more normal.
> Maybe when you resubmit I'll raid it with Linus Walleij and see if he
> can explain why I can't have 0-95.

This change is because the chip.base property changed to be -1 for
dynamic enumeration of the gpio's. In this mail there is some explanation
about it:

http://lists.infradead.org/pipermail/linux-rpi-kernel/2014-October/001045.html

Best regards,
    Sergio Paracuellos
> 
> Thanks,
> NeilBrown
> 
> 
> >
> > Best regards,
> >     Sergio Paracuellos
> >
> > Sergio Paracuellos (7):
> >   staging: mt7621-gpio: make use 'bgpio_init' from GPIO_GENERIC
> >   staging: mt7621-gpio: avoid including 'gpio.h'
> >   staging: mt7621-gpio: make use of 'builtin_platform_driver'
> >   staging: mt7621-gpio: implement '.irq_[request|release]_resources'
> >     functions
> >   staging: mt7621-gpio: add COMPILE_TEST
> >   staging: mt7621-gpio: add kerneldoc for state data containers
> >   staging: mt7621-gpio: implement high level and low level irqs
> >
> >  drivers/staging/mt7621-gpio/Kconfig       |   3 +-
> >  drivers/staging/mt7621-gpio/gpio-mt7621.c | 251 ++++++++++++++++--------------
> >  2 files changed, 138 insertions(+), 116 deletions(-)
> >
> > -- 
> > 2.7.4


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 0/7] staging: mt7621-gpio: last cleanups

[NeilBrown]

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

On Mon, Jun 11 2018, Sergio Paracuellos wrote:

> On Mon, Jun 11, 2018 at 06:33:44PM +1000, NeilBrown wrote:
>> On Mon, Jun 11 2018, Sergio Paracuellos wrote:
>> 
>> > After submiting this driver to try to get mainlined and get
>> > out of staging some new cleanups seems to be necessary.
>> > According to this main of Linus Walleij:
>> >
>> > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-June/121742.html
>> >
>> > this series tries to fix all of the issues in order to send
>> > v2 and give it a new try. Because I don't have to hardware to
>> > test my changes I send new cleanups first in staging to make
>> > easier to NeilBrown test it and get a feedback about them.
>> >
>> > Changes in v2:
>> >     - Patch where GPIOLIB_IRQCHIP was used avoiding
>> >       the use of a custom irq domain has been dropped to
>> >       be sure after this changes all is working properly.
>> >       (This was PATCH 7 in previous series)
>> >     - PATCH 1: 
>> >          * avoid introducing new macros and use 'bank'
>> >            field of mtk_gc with register offset.
>> >          * Make correct use of bgpio_init passing new
>> >            void __iomem pointers instead of use the
>> >            macros. 
>> >     - Previous series PATCH 8 now is PATCH 7. Avoid the
>> >       use of a switch-case statement which was wrong and
>> >       distinc if we have RISSING AND FALLING EDGE interrupt
>> >       or HIGH LOW level ones. This last two are exclusive and
>> >       cannot be generated at the same time.
>> >
>> > Also, I think is we finally avoid to use a new irq_domain the
>> > need for the new functions introduced for request and release
>> > resources dissapears. I was diving down the other drivers code
>> > and I see that these two only are used in drivers which use its
>> > own irq_domain. Correct me if I am wrong, please.
>> >
>> > Hope this helps.
>> >
>> > Thanks in advance.
>> 
>> Thanks a lot.
>> This series appears to work, though I sent a separate comment on one
>> piece of code.
>
> Thanks for testing, review and feedback.
>
> I have resent a complete v3 of this series taking into account
> your comments there.
>
>> However the gpio are numbers
>> 480-511
>> 448-479
>> 416-447
>> 
>> instead of
>> 0-31
>> 32-63
>> 64-95
>> 
>> which would be more normal.
>> Maybe when you resubmit I'll raid it with Linus Walleij and see if he
>> can explain why I can't have 0-95.
>
> This change is because the chip.base property changed to be -1 for
> dynamic enumeration of the gpio's. In this mail there is some explanation
> about it:
>
> http://lists.infradead.org/pipermail/linux-rpi-kernel/2014-October/001045.html

Interesting - thanks for the link.

It seems that the goal is to focus more on names than on numbers, and
that probably makes sense.
It means that we need to make sure that the names are useful.

Currently the driver defines 3 gpiochips, one for each bank.

# grep . /sys/class/gpio/gpiochip4*/label
/sys/class/gpio/gpiochip416/label:1e000600.gpio
/sys/class/gpio/gpiochip448/label:1e000600.gpio
/sys/class/gpio/gpiochip480/label:1e000600.gpio

Unfortunately they all have the same label :-(
It would be good if there was a way to add 0, 1, and 2 to the labels, or
something like that.

In /proc/interrupts we now have:

 17:          0          0          0          0  MIPS GIC  19  mt7621, mt7621, mt7621

which is the interrupt from the GPIO controller.
It is a little weird that all three banks are named "mt7621" here.
We also have:

 26:          0          0          0          0      GPIO  18  reset

which is the interrupt from GPIO which provides the "reset" button.
I suspect that if I had interrupts form two different banks they would
both be called "GPIO" which would be a little confusing.
We could declare three different 'struct irq_chip' with three different
names, but that would be ugly.  Hopefully there is a better way.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH v2 0/7] staging: mt7621-gpio: last cleanups

[Sergio Paracuellos]

On Tue, Jun 12, 2018 at 12:01:38PM +1000, NeilBrown wrote:
> On Mon, Jun 11 2018, Sergio Paracuellos wrote:
> 
> > On Mon, Jun 11, 2018 at 06:33:44PM +1000, NeilBrown wrote:
> >> On Mon, Jun 11 2018, Sergio Paracuellos wrote:
> >> 
> >> > After submiting this driver to try to get mainlined and get
> >> > out of staging some new cleanups seems to be necessary.
> >> > According to this main of Linus Walleij:
> >> >
> >> > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-June/121742.html
> >> >
> >> > this series tries to fix all of the issues in order to send
> >> > v2 and give it a new try. Because I don't have to hardware to
> >> > test my changes I send new cleanups first in staging to make
> >> > easier to NeilBrown test it and get a feedback about them.
> >> >
> >> > Changes in v2:
> >> >     - Patch where GPIOLIB_IRQCHIP was used avoiding
> >> >       the use of a custom irq domain has been dropped to
> >> >       be sure after this changes all is working properly.
> >> >       (This was PATCH 7 in previous series)
> >> >     - PATCH 1: 
> >> >          * avoid introducing new macros and use 'bank'
> >> >            field of mtk_gc with register offset.
> >> >          * Make correct use of bgpio_init passing new
> >> >            void __iomem pointers instead of use the
> >> >            macros. 
> >> >     - Previous series PATCH 8 now is PATCH 7. Avoid the
> >> >       use of a switch-case statement which was wrong and
> >> >       distinc if we have RISSING AND FALLING EDGE interrupt
> >> >       or HIGH LOW level ones. This last two are exclusive and
> >> >       cannot be generated at the same time.
> >> >
> >> > Also, I think is we finally avoid to use a new irq_domain the
> >> > need for the new functions introduced for request and release
> >> > resources dissapears. I was diving down the other drivers code
> >> > and I see that these two only are used in drivers which use its
> >> > own irq_domain. Correct me if I am wrong, please.
> >> >
> >> > Hope this helps.
> >> >
> >> > Thanks in advance.
> >> 
> >> Thanks a lot.
> >> This series appears to work, though I sent a separate comment on one
> >> piece of code.
> >
> > Thanks for testing, review and feedback.
> >
> > I have resent a complete v3 of this series taking into account
> > your comments there.
> >
> >> However the gpio are numbers
> >> 480-511
> >> 448-479
> >> 416-447
> >> 
> >> instead of
> >> 0-31
> >> 32-63
> >> 64-95
> >> 
> >> which would be more normal.
> >> Maybe when you resubmit I'll raid it with Linus Walleij and see if he
> >> can explain why I can't have 0-95.
> >
> > This change is because the chip.base property changed to be -1 for
> > dynamic enumeration of the gpio's. In this mail there is some explanation
> > about it:
> >
> > http://lists.infradead.org/pipermail/linux-rpi-kernel/2014-October/001045.html
> 
> Interesting - thanks for the link.
> 
> It seems that the goal is to focus more on names than on numbers, and
> that probably makes sense.
> It means that we need to make sure that the names are useful.
> 
> Currently the driver defines 3 gpiochips, one for each bank.
> 
> # grep . /sys/class/gpio/gpiochip4*/label
> /sys/class/gpio/gpiochip416/label:1e000600.gpio
> /sys/class/gpio/gpiochip448/label:1e000600.gpio
> /sys/class/gpio/gpiochip480/label:1e000600.gpio
> 
> Unfortunately they all have the same label :-(
> It would be good if there was a way to add 0, 1, and 2 to the labels, or
> something like that.

There might be a way to set that labels from dts. I'll check other
drivers code to change those.

> 
> In /proc/interrupts we now have:
> 
>  17:          0          0          0          0  MIPS GIC  19  mt7621, mt7621, mt7621
> 
> which is the interrupt from the GPIO controller.
> It is a little weird that all three banks are named "mt7621" here.

This is the name which is being set from devm_request_irq to mark
line as shared. Maybe set mt7621-bank[0-2] is more usefull. 

> We also have:
> 
>  26:          0          0          0          0      GPIO  18  reset

This is the name from the one from the static initialization of the
gpio_chip where callbacks are defined. Let's see what can I do to
change that also.

> 
> which is the interrupt from GPIO which provides the "reset" button.
> I suspect that if I had interrupts form two different banks they would
> both be called "GPIO" which would be a little confusing.
> We could declare three different 'struct irq_chip' with three different
> names, but that would be ugly.  Hopefully there is a better way.
> 
> Thanks,
> NeilBrown

Best regards,
    Sergio Paracuellos