[PATCH v3 0/7] gpio: ep93xx: fixes series patch

[PATCH v3 0/7] gpio: ep93xx: fixes series patch

[Nikita Shubin]

v2: 
https://lore.kernel.org/linux-gpio/20210127104617.1173-1-nikita.shubin@maquefel.me/

v2->v3 changes

[PATCH v3 1/7] gpio: ep93xx: fix BUG_ON port F usage
As suggested Andy Shevchenko dropped  most of commit message

[PATCH v3 2/7] gpio: ep93xx: Fix single irqchip with multi
 gpiochips
- Andy Shevchenko:
	- added coma
- Alexander Sverdlin:
	- changed to fixes commit d2b091961510

[PATCH v2 6/9] gpio: ep93xx: refactor ep93xx_gpio_add_bank
- Alexander Sverdlin:
	- use ->num_parents instead of ARRAY_SIZE()

Alexander - i think you are right about these two patches
they have no meaning currently, so i dropped them.

[PATCH v2 7/9] gpio: ep93xx: separate IRQ's setup
- Alexander Sverdlin:
	- drop patch entirely

[PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports
- Alexander Sverdlin:
	- drop patch entirely

[PATCH v3 1/7] gpio: gpio-ep93xx: fix BUG_ON port F usage

[Nikita Shubin]

The port F is index 2 not 5.

------------[ cut here ]------------
kernel BUG at drivers/gpio/gpio-ep93xx.c:64!
Internal error: Oops - BUG: 0 [#1] ARM
Modules linked in:
CPU: 0 PID: 403 Comm: gpio-event-mon Not tainted 5.9.10-00011-ge93e9618628b-dirty #19
Hardware name: Technologic Systems TS-72xx SBC
PC is at ep93xx_gpio_update_int_params+0x1c/0x80
LR is at ep93xx_gpio_update_int_params+0x14/0x80
pc : [<c03abc44>] lr : [<c03abc3c>] psr: 20000093
sp : c158de00 ip : 00000000 fp : 00000001
r10: c44154d4 r9 : 00000000 r8 : c4415020
r7 : c04ef884 r6 : c051c842 r5 : c4415020 r4 : 00000005
r3 : 00000000 r2 : 00000000 r1 : c04eb768 r0 : 00000008
Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
Control: 0000717f Table: 01684000 DAC: 00000051
Process gpio-event-mon (pid: 403, stack limit = 0x(ptrval))
Stack: (0xc158de00 to 0xc158e000)
de00: 00000005 00000002 c051c842 c0238dc0 c0238c98 c0238c98 c04ef874 00000000
de20: 00000003 c04fcfcc 60000013 c04ef910 c04ef8d4 c00456f0 c04ef874 c15f1e00
de40: 00000000 00000000 00000001 c0045d40 c15f1e00 c4400160 c0044ca8 c04ef8a8
de60: 60000013 00000000 c15f1e00 c04ef874 c04ef884 00000001 c0235d70 c158b800
de80: be825f0f c0045ec8 00000003 c158b800 c440aa00 be825bc8 00000003 00000001
dea0: 00000000 c0236f00 c44ed3a0 c158b800 c45c2015 00000000 00000001 00000003
dec0: 6f697067 6576652d 6d2d746e 00006e6f 00000000 00000000 00000000 00000000
dee0: be825df4 c00abb0c c440c500 c00aabd4 c440c500 c528b840 c45c2010 c04e1228
df00: 00000ff0 c4478d28 c030b404 be825bc8 c1550e20 00000003 c1550e20 c00c3388
df20: c4478d28 c00c3d48 be825f0f c00abd00 c45c2000 c45c2000 c1550e20 c00bfea8
df40: 00000003 c00b0714 00000000 c4450000 00000004 00000100 00000001 c04e1228
df60: c158dfb0 ffffff9c 000231f8 00000003 00000142 c00b085c 00000000 c04e1228
df80: 00000000 be825f0f 00000003 00000003 00000036 c00083c4 c158c000 00000000
dfa0: be825f0f c00081e0 be825f0f 00000003 00000003 c030b404 be825bc8 00000000
dfc0: be825f0f 00000003 00000003 00000036 00000001 00000000 00022070 be825f0f
dfe0: b6f2e4e0 be825bac 00010acc b6f2e4ec 60000010 00000003 00000000 00000000
[<c03abc44>] (ep93xx_gpio_update_int_params) from [<c0238dc0>] (ep93xx_gpio_irq_type+0x128/0x1c0)
[<c0238dc0>] (ep93xx_gpio_irq_type) from [<c00456f0>] (__irq_set_trigger+0x6c/0x128)
[<c00456f0>] (__irq_set_trigger) from [<c0045d40>] (__setup_irq+0x594/0x678)
[<c0045d40>] (__setup_irq) from [<c0045ec8>] (request_threaded_irq+0xa4/0x128)
[<c0045ec8>] (request_threaded_irq) from [<c0236f00>] (gpio_ioctl+0x300/0x4d8)
[<c0236f00>] (gpio_ioctl) from [<c00c3388>] (vfs_ioctl+0x24/0x3c)
[<c00c3388>] (vfs_ioctl) from [<c00c3d48>] (sys_ioctl+0xbc/0x768)
[<c00c3d48>] (sys_ioctl) from [<c00081e0>] (ret_fast_syscall+0x0/0x50)
Exception stack(0xc158dfa8 to 0xc158dff0)
dfa0: be825f0f 00000003 00000003 c030b404 be825bc8 00000000
dfc0: be825f0f 00000003 00000003 00000036 00000001 00000000 00022070 be825f0f
dfe0: b6f2e4e0 be825bac 00010acc b6f2e4ec
Code: e59f0060 ebfff3e1 e3540002 9a000000 (e7f001f2)
---[ end trace 3f6544e133e9f5ae ]---

Fixes: fd935fc421e74 ("gpio: ep93xx: Do not pingpong irq numbers")
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
v2->v3:
Reworded commit message.
---
 drivers/gpio/gpio-ep93xx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index 226da8df6f10..0d5a9a90cf48 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -39,6 +39,12 @@ struct ep93xx_gpio {
 	struct gpio_chip	gc[8];
 };
 
+/*
+ * F Port index in GPIOCHIP'S array is 5
+ * but we use index 2 for stored values and offsets
+ */
+#define EP93XX_GPIO_F_PORT_INDEX 5
+
 /*************************************************************************
  * Interrupt handling for EP93xx on-chip GPIOs
  *************************************************************************/
@@ -85,6 +91,9 @@ static int ep93xx_gpio_port(struct gpio_chip *gc)
 		return 0;
 	}
 
+	if (port == EP93XX_GPIO_F_PORT_INDEX)
+		port = 2;
+
 	return port;
 }
 
-- 
2.29.2

[PATCH v3 2/7] gpio: gpio-ep93xx: Fix single irqchip with multi gpiochips

[Nikita Shubin]

Fixes the following warnings which results in interrupts disabled on
port B/F:

gpio gpiochip1: (B): detected irqchip that is shared with multiple gpiochips: please fix the driver.
gpio gpiochip5: (F): detected irqchip that is shared with multiple gpiochips: please fix the driver.

- added separate irqchip for each interrupt capable gpiochip
- provided unique names for each irqchip
- reworked ep93xx_gpio_port to make it usable before chip_add_data
  in ep93xx_init_irq_chips

Fixes: d2b091961510 ("gpio: ep93xx: Pass irqchip when adding gpiochip")
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
v2->v3
- Added a coma
- changed to fixes commit d2b091961510
---
 drivers/gpio/gpio-ep93xx.c | 45 ++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index 0d5a9a90cf48..b990d37da143 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -34,9 +34,12 @@
  */
 #define EP93XX_GPIO_F_IRQ_BASE 80
 
+#define EP93XX_GPIO_IRQ_CHIPS_NUM 3
+
 struct ep93xx_gpio {
 	void __iomem		*base;
 	struct gpio_chip	gc[8];
+	struct irq_chip		ic[EP93XX_GPIO_IRQ_CHIPS_NUM];
 };
 
 /*
@@ -55,6 +58,11 @@ static unsigned char gpio_int_type2[3];
 static unsigned char gpio_int_debounce[3];
 
 /* Port ordering is: A B F */
+static const char * const irq_chip_names[] = {
+	"gpio-irq-a",
+	"gpio-irq-b",
+	"gpio-irq-f",
+};
 static const u8 int_type1_register_offset[3]	= { 0x90, 0xac, 0x4c };
 static const u8 int_type2_register_offset[3]	= { 0x94, 0xb0, 0x50 };
 static const u8 eoi_register_offset[3]		= { 0x98, 0xb4, 0x54 };
@@ -77,9 +85,8 @@ static void ep93xx_gpio_update_int_params(struct ep93xx_gpio *epg, unsigned port
 	       epg->base + int_en_register_offset[port]);
 }
 
-static int ep93xx_gpio_port(struct gpio_chip *gc)
+static int ep93xx_gpio_port(struct ep93xx_gpio *epg, struct gpio_chip *gc)
 {
-	struct ep93xx_gpio *epg = gpiochip_get_data(gc);
 	int port = 0;
 
 	while (port < ARRAY_SIZE(epg->gc) && gc != &epg->gc[port])
@@ -101,7 +108,7 @@ static void ep93xx_gpio_int_debounce(struct gpio_chip *gc,
 				     unsigned int offset, bool enable)
 {
 	struct ep93xx_gpio *epg = gpiochip_get_data(gc);
-	int port = ep93xx_gpio_port(gc);
+	int port = ep93xx_gpio_port(epg, gc);
 	int port_mask = BIT(offset);
 
 	if (enable)
@@ -163,7 +170,7 @@ static void ep93xx_gpio_irq_ack(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct ep93xx_gpio *epg = gpiochip_get_data(gc);
-	int port = ep93xx_gpio_port(gc);
+	int port = ep93xx_gpio_port(epg, gc);
 	int port_mask = BIT(d->irq & 7);
 
 	if (irqd_get_trigger_type(d) == IRQ_TYPE_EDGE_BOTH) {
@@ -178,7 +185,7 @@ static void ep93xx_gpio_irq_mask_ack(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct ep93xx_gpio *epg = gpiochip_get_data(gc);
-	int port = ep93xx_gpio_port(gc);
+	int port = ep93xx_gpio_port(epg, gc);
 	int port_mask = BIT(d->irq & 7);
 
 	if (irqd_get_trigger_type(d) == IRQ_TYPE_EDGE_BOTH)
@@ -194,7 +201,7 @@ static void ep93xx_gpio_irq_mask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct ep93xx_gpio *epg = gpiochip_get_data(gc);
-	int port = ep93xx_gpio_port(gc);
+	int port = ep93xx_gpio_port(epg, gc);
 
 	gpio_int_unmasked[port] &= ~BIT(d->irq & 7);
 	ep93xx_gpio_update_int_params(epg, port);
@@ -204,7 +211,7 @@ static void ep93xx_gpio_irq_unmask(struct irq_data *d)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct ep93xx_gpio *epg = gpiochip_get_data(gc);
-	int port = ep93xx_gpio_port(gc);
+	int port = ep93xx_gpio_port(epg, gc);
 
 	gpio_int_unmasked[port] |= BIT(d->irq & 7);
 	ep93xx_gpio_update_int_params(epg, port);
@@ -219,7 +226,7 @@ static int ep93xx_gpio_irq_type(struct irq_data *d, unsigned int type)
 {
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct ep93xx_gpio *epg = gpiochip_get_data(gc);
-	int port = ep93xx_gpio_port(gc);
+	int port = ep93xx_gpio_port(epg, gc);
 	int offset = d->irq & 7;
 	int port_mask = BIT(offset);
 	irq_flow_handler_t handler;
@@ -335,6 +342,22 @@ static int ep93xx_gpio_f_to_irq(struct gpio_chip *gc, unsigned offset)
 	return EP93XX_GPIO_F_IRQ_BASE + offset;
 }
 
+static void ep93xx_init_irq_chips(struct ep93xx_gpio *epg)
+{
+	int i;
+
+	for (i = 0; i < EP93XX_GPIO_IRQ_CHIPS_NUM; i++) {
+		struct irq_chip *ic = &epg->ic[i];
+
+		ic->name = irq_chip_names[i];
+		ic->irq_ack = ep93xx_gpio_irq_ack;
+		ic->irq_mask_ack = ep93xx_gpio_irq_mask_ack;
+		ic->irq_mask = ep93xx_gpio_irq_mask;
+		ic->irq_unmask = ep93xx_gpio_irq_unmask;
+		ic->irq_set_type = ep93xx_gpio_irq_type;
+	}
+}
+
 static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 				struct platform_device *pdev,
 				struct ep93xx_gpio *epg,
@@ -345,6 +368,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 	struct device *dev = &pdev->dev;
 	struct gpio_irq_chip *girq;
 	int err;
+	int port;
 
 	err = bgpio_init(gc, dev, 1, data, NULL, NULL, dir, NULL, 0);
 	if (err)
@@ -356,7 +380,8 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 	girq = &gc->irq;
 	if (bank->has_irq || bank->has_hierarchical_irq) {
 		gc->set_config = ep93xx_gpio_set_config;
-		girq->chip = &ep93xx_gpio_irq_chip;
+		port = ep93xx_gpio_port(epg, gc);
+		girq->chip = &epg->ic[port];
 	}
 
 	if (bank->has_irq) {
@@ -423,6 +448,8 @@ static int ep93xx_gpio_probe(struct platform_device *pdev)
 	if (IS_ERR(epg->base))
 		return PTR_ERR(epg->base);
 
+	ep93xx_init_irq_chips(epg);
+
 	for (i = 0; i < ARRAY_SIZE(ep93xx_gpio_banks); i++) {
 		struct gpio_chip *gc = &epg->gc[i];
 		struct ep93xx_gpio_bank *bank = &ep93xx_gpio_banks[i];
-- 
2.29.2

[PATCH v3 3/7] gpio: gpio-ep93xx: Fix wrong irq numbers in port F

[Nikita Shubin]

Port F irq's should be statically mapped to EP93XX_GPIO_F_IRQ_BASE.

So we need to specify girq->first otherwise:

"If device tree is used, then first_irq will be 0 and
irqs get mapped dynamically on the fly"

And that's not the thing we want.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/gpio/gpio-ep93xx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index b990d37da143..dc88115e34da 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -430,6 +430,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 		girq->default_type = IRQ_TYPE_NONE;
 		girq->handler = handle_level_irq;
 		gc->to_irq = ep93xx_gpio_f_to_irq;
+		girq->first = EP93XX_GPIO_F_IRQ_BASE;
 	}
 
 	return devm_gpiochip_add_data(dev, gc, epg);
-- 
2.29.2

[PATCH v3 4/7] gpio: ep93xx: drop to_irq binding

[Nikita Shubin]

As ->to_irq is redefined in gpiochip_add_irqchip, having it defined in
driver is useless, so let's drop it.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/gpio/gpio-ep93xx.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index dc88115e34da..ee1cb3b894db 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -337,11 +337,6 @@ static int ep93xx_gpio_set_config(struct gpio_chip *gc, unsigned offset,
 	return 0;
 }
 
-static int ep93xx_gpio_f_to_irq(struct gpio_chip *gc, unsigned offset)
-{
-	return EP93XX_GPIO_F_IRQ_BASE + offset;
-}
-
 static void ep93xx_init_irq_chips(struct ep93xx_gpio *epg)
 {
 	int i;
@@ -429,7 +424,6 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 		}
 		girq->default_type = IRQ_TYPE_NONE;
 		girq->handler = handle_level_irq;
-		gc->to_irq = ep93xx_gpio_f_to_irq;
 		girq->first = EP93XX_GPIO_F_IRQ_BASE;
 	}
 
-- 
2.29.2

[PATCH v3 5/7] gpio: ep93xx: Fix typo s/hierarchial/hierarchical

[Nikita Shubin]

Fix typo in comment.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/gpio/gpio-ep93xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index ee1cb3b894db..d69ec09cd618 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -402,7 +402,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 
 		/*
 		 * FIXME: convert this to use hierarchical IRQ support!
-		 * this requires fixing the root irqchip to be hierarchial.
+		 * this requires fixing the root irqchip to be hierarchical.
 		 */
 		girq->parent_handler = ep93xx_gpio_f_irq_handler;
 		girq->num_parents = 8;
-- 
2.29.2

[PATCH v3 6/7] gpio: ep93xx: refactor ep93xx_gpio_add_bank

[Nikita Shubin]

- replace plain numbers with girq->num_parents in devm_kcalloc
- replace plain numbers with girq->num_parents for port F
- refactor i - 1 to i + 1 to make loop more readable
- combine getting IRQ's loop and setting handler's into single loop

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
v2->v3
- use ->num_parents instead of ARRAY_SIZE()
---
 drivers/gpio/gpio-ep93xx.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index d69ec09cd618..df55aa13bd9a 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -384,7 +384,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 
 		girq->parent_handler = ep93xx_gpio_ab_irq_handler;
 		girq->num_parents = 1;
-		girq->parents = devm_kcalloc(dev, 1,
+		girq->parents = devm_kcalloc(dev, girq->num_parents,
 					     sizeof(*girq->parents),
 					     GFP_KERNEL);
 		if (!girq->parents)
@@ -406,15 +406,14 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 		 */
 		girq->parent_handler = ep93xx_gpio_f_irq_handler;
 		girq->num_parents = 8;
-		girq->parents = devm_kcalloc(dev, 8,
+		girq->parents = devm_kcalloc(dev, girq->num_parents,
 					     sizeof(*girq->parents),
 					     GFP_KERNEL);
 		if (!girq->parents)
 			return -ENOMEM;
 		/* Pick resources 1..8 for these IRQs */
-		for (i = 1; i <= 8; i++)
-			girq->parents[i - 1] = platform_get_irq(pdev, i);
-		for (i = 0; i < 8; i++) {
+		for (i = 0; i < girq->num_parents; i++) {
+			girq->parents[i] = platform_get_irq(pdev, i + 1);
 			gpio_irq = EP93XX_GPIO_F_IRQ_BASE + i;
 			irq_set_chip_data(gpio_irq, &epg->gc[5]);
 			irq_set_chip_and_handler(gpio_irq,
-- 
2.29.2

[PATCH v3 7/7] gpiod: ep93xx: refactor base IRQ number

[Nikita Shubin]

- use predefined constants instead of plain numbers
- use provided bank IRQ number instead of defined constant
  for port F

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/gpio/gpio-ep93xx.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index df55aa13bd9a..b1501607e8ed 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -28,6 +28,8 @@
 /* Maximum value for irq capable line identifiers */
 #define EP93XX_GPIO_LINE_MAX_IRQ 23
 
+#define EP93XX_GPIO_A_IRQ_BASE 64
+#define EP93XX_GPIO_B_IRQ_BASE 72
 /*
  * Static mapping of GPIO bank F IRQS:
  * F0..F7 (16..24) to irq 80..87.
@@ -311,14 +313,14 @@ struct ep93xx_gpio_bank {
 
 static struct ep93xx_gpio_bank ep93xx_gpio_banks[] = {
 	/* Bank A has 8 IRQs */
-	EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true, false, 64),
+	EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true, false, EP93XX_GPIO_A_IRQ_BASE),
 	/* Bank B has 8 IRQs */
-	EP93XX_GPIO_BANK("B", 0x04, 0x14, 8, true, false, 72),
+	EP93XX_GPIO_BANK("B", 0x04, 0x14, 8, true, false, EP93XX_GPIO_B_IRQ_BASE),
 	EP93XX_GPIO_BANK("C", 0x08, 0x18, 40, false, false, 0),
 	EP93XX_GPIO_BANK("D", 0x0c, 0x1c, 24, false, false, 0),
 	EP93XX_GPIO_BANK("E", 0x20, 0x24, 32, false, false, 0),
 	/* Bank F has 8 IRQs */
-	EP93XX_GPIO_BANK("F", 0x30, 0x34, 16, false, true, 0),
+	EP93XX_GPIO_BANK("F", 0x30, 0x34, 16, false, true, EP93XX_GPIO_F_IRQ_BASE),
 	EP93XX_GPIO_BANK("G", 0x38, 0x3c, 48, false, false, 0),
 	EP93XX_GPIO_BANK("H", 0x40, 0x44, 56, false, false, 0),
 };
@@ -414,7 +416,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 		/* Pick resources 1..8 for these IRQs */
 		for (i = 0; i < girq->num_parents; i++) {
 			girq->parents[i] = platform_get_irq(pdev, i + 1);
-			gpio_irq = EP93XX_GPIO_F_IRQ_BASE + i;
+			gpio_irq = bank->irq_base + i;
 			irq_set_chip_data(gpio_irq, &epg->gc[5]);
 			irq_set_chip_and_handler(gpio_irq,
 						 &ep93xx_gpio_irq_chip,
@@ -423,7 +425,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 		}
 		girq->default_type = IRQ_TYPE_NONE;
 		girq->handler = handle_level_irq;
-		girq->first = EP93XX_GPIO_F_IRQ_BASE;
+		girq->first = bank->irq_base;
 	}
 
 	return devm_gpiochip_add_data(dev, gc, epg);
-- 
2.29.2

Re: [PATCH v3 1/7] gpio: gpio-ep93xx: fix BUG_ON port F usage

[Andy Shevchenko]

On Thu, Jan 28, 2021 at 2:21 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> The port F is index 2 not 5.
>
> ------------[ cut here ]------------
> kernel BUG at drivers/gpio/gpio-ep93xx.c:64!

Perhaps you missed my message, please cut this to have only related
information and not be so noisy!

> Internal error: Oops - BUG: 0 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 403 Comm: gpio-event-mon Not tainted 5.9.10-00011-ge93e9618628b-dirty #19
> Hardware name: Technologic Systems TS-72xx SBC
> PC is at ep93xx_gpio_update_int_params+0x1c/0x80
> LR is at ep93xx_gpio_update_int_params+0x14/0x80
> pc : [<c03abc44>] lr : [<c03abc3c>] psr: 20000093
> sp : c158de00 ip : 00000000 fp : 00000001
> r10: c44154d4 r9 : 00000000 r8 : c4415020
> r7 : c04ef884 r6 : c051c842 r5 : c4415020 r4 : 00000005
> r3 : 00000000 r2 : 00000000 r1 : c04eb768 r0 : 00000008
> Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 0000717f Table: 01684000 DAC: 00000051
> Process gpio-event-mon (pid: 403, stack limit = 0x(ptrval))
> Stack: (0xc158de00 to 0xc158e000)
> de00: 00000005 00000002 c051c842 c0238dc0 c0238c98 c0238c98 c04ef874 00000000
> de20: 00000003 c04fcfcc 60000013 c04ef910 c04ef8d4 c00456f0 c04ef874 c15f1e00
> de40: 00000000 00000000 00000001 c0045d40 c15f1e00 c4400160 c0044ca8 c04ef8a8
> de60: 60000013 00000000 c15f1e00 c04ef874 c04ef884 00000001 c0235d70 c158b800
> de80: be825f0f c0045ec8 00000003 c158b800 c440aa00 be825bc8 00000003 00000001
> dea0: 00000000 c0236f00 c44ed3a0 c158b800 c45c2015 00000000 00000001 00000003
> dec0: 6f697067 6576652d 6d2d746e 00006e6f 00000000 00000000 00000000 00000000
> dee0: be825df4 c00abb0c c440c500 c00aabd4 c440c500 c528b840 c45c2010 c04e1228
> df00: 00000ff0 c4478d28 c030b404 be825bc8 c1550e20 00000003 c1550e20 c00c3388
> df20: c4478d28 c00c3d48 be825f0f c00abd00 c45c2000 c45c2000 c1550e20 c00bfea8
> df40: 00000003 c00b0714 00000000 c4450000 00000004 00000100 00000001 c04e1228
> df60: c158dfb0 ffffff9c 000231f8 00000003 00000142 c00b085c 00000000 c04e1228
> df80: 00000000 be825f0f 00000003 00000003 00000036 c00083c4 c158c000 00000000
> dfa0: be825f0f c00081e0 be825f0f 00000003 00000003 c030b404 be825bc8 00000000
> dfc0: be825f0f 00000003 00000003 00000036 00000001 00000000 00022070 be825f0f
> dfe0: b6f2e4e0 be825bac 00010acc b6f2e4ec 60000010 00000003 00000000 00000000
> [<c03abc44>] (ep93xx_gpio_update_int_params) from [<c0238dc0>] (ep93xx_gpio_irq_type+0x128/0x1c0)
> [<c0238dc0>] (ep93xx_gpio_irq_type) from [<c00456f0>] (__irq_set_trigger+0x6c/0x128)
> [<c00456f0>] (__irq_set_trigger) from [<c0045d40>] (__setup_irq+0x594/0x678)
> [<c0045d40>] (__setup_irq) from [<c0045ec8>] (request_threaded_irq+0xa4/0x128)
> [<c0045ec8>] (request_threaded_irq) from [<c0236f00>] (gpio_ioctl+0x300/0x4d8)
> [<c0236f00>] (gpio_ioctl) from [<c00c3388>] (vfs_ioctl+0x24/0x3c)
> [<c00c3388>] (vfs_ioctl) from [<c00c3d48>] (sys_ioctl+0xbc/0x768)
> [<c00c3d48>] (sys_ioctl) from [<c00081e0>] (ret_fast_syscall+0x0/0x50)
> Exception stack(0xc158dfa8 to 0xc158dff0)
> dfa0: be825f0f 00000003 00000003 c030b404 be825bc8 00000000
> dfc0: be825f0f 00000003 00000003 00000036 00000001 00000000 00022070 be825f0f
> dfe0: b6f2e4e0 be825bac 00010acc b6f2e4ec
> Code: e59f0060 ebfff3e1 e3540002 9a000000 (e7f001f2)
> ---[ end trace 3f6544e133e9f5ae ]---
>
> Fixes: fd935fc421e74 ("gpio: ep93xx: Do not pingpong irq numbers")
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

...

> +/*
> + * F Port index in GPIOCHIP'S array is 5
> + * but we use index 2 for stored values and offsets
> + */
> +#define EP93XX_GPIO_F_PORT_INDEX 5

Hmm... Why not to use an array with holes instead.

...

> +       if (port == EP93XX_GPIO_F_PORT_INDEX)
> +               port = 2;

Sorry, but I'm not in favour of this as it adds confusion.
See above for the potential way to solve.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/7] gpio: gpio-ep93xx: Fix single irqchip with multi gpiochips

[Andy Shevchenko]

On Thu, Jan 28, 2021 at 2:21 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> Fixes the following warnings which results in interrupts disabled on
> port B/F:
>
> gpio gpiochip1: (B): detected irqchip that is shared with multiple gpiochips: please fix the driver.
> gpio gpiochip5: (F): detected irqchip that is shared with multiple gpiochips: please fix the driver.
>
> - added separate irqchip for each interrupt capable gpiochip
> - provided unique names for each irqchip
> - reworked ep93xx_gpio_port to make it usable before chip_add_data
>   in ep93xx_init_irq_chips
>
> Fixes: d2b091961510 ("gpio: ep93xx: Pass irqchip when adding gpiochip")
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

...

> +#define EP93XX_GPIO_IRQ_CHIPS_NUM 3
> +
>  struct ep93xx_gpio {
>         void __iomem            *base;
>         struct gpio_chip        gc[8];
> +       struct irq_chip         ic[EP93XX_GPIO_IRQ_CHIPS_NUM];
>  };

>  /* Port ordering is: A B F */
> +static const char * const irq_chip_names[] = {
> +       "gpio-irq-a",
> +       "gpio-irq-b",
> +       "gpio-irq-f",
> +};

Depending on the solution from the previous patch I would also go here
rather with holes, than messing up with mapping between port index and
index in this array.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/7] gpio: ep93xx: fixes series patch

[Andy Shevchenko]

On Thu, Jan 28, 2021 at 2:21 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> v2:
> https://lore.kernel.org/linux-gpio/20210127104617.1173-1-nikita.shubin@maquefel.me/
>
> v2->v3 changes

I stopped reviewing at some point, b/c I have a feeling that I gave
you tags (and others) against some patches and none of them has any.
Am I missing something?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/7] gpio: ep93xx: refactor ep93xx_gpio_add_bank

[Andy Shevchenko]

On Thu, Jan 28, 2021 at 2:21 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:

> - replace plain numbers with girq->num_parents in devm_kcalloc

devm_kcalloc()

> - replace plain numbers with girq->num_parents for port F
> - refactor i - 1 to i + 1 to make loop more readable
> - combine getting IRQ's loop and setting handler's into single loop

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/7] gpio: gpio-ep93xx: fix BUG_ON port F usage

[Alexander Sverdlin]

Hello Nikita,

On Thu, 2021-01-28 at 18:11 +0200, Andy Shevchenko wrote:
> > +/*
> > + * F Port index in GPIOCHIP'S array is 5
> > + * but we use index 2 for stored values and offsets
> > + */
> > +#define EP93XX_GPIO_F_PORT_INDEX 5
> 
> Hmm... Why not to use an array with holes instead.
> 
> ...
> 
> > +       if (port == EP93XX_GPIO_F_PORT_INDEX)
> > +               port = 2;
> 
> Sorry, but I'm not in favour of this as it adds confusion.
> See above for the potential way to solve.

well, I was thinking the same yesterday. It just adds another
level on confusion into the code, which even the author got
wrong :)

Array with holes would be more obvious, but one can also embedd
the necessary values into struct ep93xx_gpio_bank.

-- 
Alexander Sverdlin.

Re: [PATCH v3 7/7] gpiod: ep93xx: refactor base IRQ number

[Alexander Sverdlin]

Hi!

On Thu, 2021-01-28 at 15:21 +0300, Nikita Shubin wrote:
> - use predefined constants instead of plain numbers
> - use provided bank IRQ number instead of defined constant
>   for port F
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Acked-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>

> ---
>  drivers/gpio/gpio-ep93xx.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
> index df55aa13bd9a..b1501607e8ed 100644
> --- a/drivers/gpio/gpio-ep93xx.c
> +++ b/drivers/gpio/gpio-ep93xx.c
> @@ -28,6 +28,8 @@
>  /* Maximum value for irq capable line identifiers */
>  #define EP93XX_GPIO_LINE_MAX_IRQ 23
>  
> +#define EP93XX_GPIO_A_IRQ_BASE 64
> +#define EP93XX_GPIO_B_IRQ_BASE 72
>  /*
>   * Static mapping of GPIO bank F IRQS:
>   * F0..F7 (16..24) to irq 80..87.
> @@ -311,14 +313,14 @@ struct ep93xx_gpio_bank {
>  
>  static struct ep93xx_gpio_bank ep93xx_gpio_banks[] = {
>         /* Bank A has 8 IRQs */
> -       EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true, false, 64),
> +       EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true, false,
> EP93XX_GPIO_A_IRQ_BASE),
>         /* Bank B has 8 IRQs */
> -       EP93XX_GPIO_BANK("B", 0x04, 0x14, 8, true, false, 72),
> +       EP93XX_GPIO_BANK("B", 0x04, 0x14, 8, true, false,
> EP93XX_GPIO_B_IRQ_BASE),
>         EP93XX_GPIO_BANK("C", 0x08, 0x18, 40, false, false, 0),
>         EP93XX_GPIO_BANK("D", 0x0c, 0x1c, 24, false, false, 0),
>         EP93XX_GPIO_BANK("E", 0x20, 0x24, 32, false, false, 0),
>         /* Bank F has 8 IRQs */
> -       EP93XX_GPIO_BANK("F", 0x30, 0x34, 16, false, true, 0),
> +       EP93XX_GPIO_BANK("F", 0x30, 0x34, 16, false, true,
> EP93XX_GPIO_F_IRQ_BASE),
>         EP93XX_GPIO_BANK("G", 0x38, 0x3c, 48, false, false, 0),
>         EP93XX_GPIO_BANK("H", 0x40, 0x44, 56, false, false, 0),
>  };
> @@ -414,7 +416,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip
> *gc,
>                 /* Pick resources 1..8 for these IRQs */
>                 for (i = 0; i < girq->num_parents; i++) {
>                         girq->parents[i] = platform_get_irq(pdev, i +
> 1);
> -                       gpio_irq = EP93XX_GPIO_F_IRQ_BASE + i;
> +                       gpio_irq = bank->irq_base + i;
>                         irq_set_chip_data(gpio_irq, &epg->gc[5]);
>                         irq_set_chip_and_handler(gpio_irq,
>                                                 
> &ep93xx_gpio_irq_chip,
> @@ -423,7 +425,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip
> *gc,
>                 }
>                 girq->default_type = IRQ_TYPE_NONE;
>                 girq->handler = handle_level_irq;
> -               girq->first = EP93XX_GPIO_F_IRQ_BASE;
> +               girq->first = bank->irq_base;
>         }
>  
>         return devm_gpiochip_add_data(dev, gc, epg);

-- 
Alexander Sverdlin.

Re: [PATCH v3 3/7] gpio: gpio-ep93xx: Fix wrong irq numbers in port F

[Alexander Sverdlin]

Hi!

On Thu, 2021-01-28 at 15:21 +0300, Nikita Shubin wrote:
> Port F irq's should be statically mapped to EP93XX_GPIO_F_IRQ_BASE.
> 
> So we need to specify girq->first otherwise:
> 
> "If device tree is used, then first_irq will be 0 and
> irqs get mapped dynamically on the fly"
> 
> And that's not the thing we want.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Acked-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>

> ---
>  drivers/gpio/gpio-ep93xx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
> index b990d37da143..dc88115e34da 100644
> --- a/drivers/gpio/gpio-ep93xx.c
> +++ b/drivers/gpio/gpio-ep93xx.c
> @@ -430,6 +430,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip
> *gc,
>                 girq->default_type = IRQ_TYPE_NONE;
>                 girq->handler = handle_level_irq;
>                 gc->to_irq = ep93xx_gpio_f_to_irq;
> +               girq->first = EP93XX_GPIO_F_IRQ_BASE;
>         }
>  
>         return devm_gpiochip_add_data(dev, gc, epg);

-- 
Alexander Sverdlin.

Re: [PATCH v3 6/7] gpio: ep93xx: refactor ep93xx_gpio_add_bank

[Alexander Sverdlin]

On Thu, 2021-01-28 at 15:21 +0300, Nikita Shubin wrote:
> - replace plain numbers with girq->num_parents in devm_kcalloc
> - replace plain numbers with girq->num_parents for port F
> - refactor i - 1 to i + 1 to make loop more readable
> - combine getting IRQ's loop and setting handler's into single loop
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Acked-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>

> ---
> v2->v3
> - use ->num_parents instead of ARRAY_SIZE()
> ---
>  drivers/gpio/gpio-ep93xx.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
> index d69ec09cd618..df55aa13bd9a 100644
> --- a/drivers/gpio/gpio-ep93xx.c
> +++ b/drivers/gpio/gpio-ep93xx.c
> @@ -384,7 +384,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip
> *gc,
>  
>                 girq->parent_handler = ep93xx_gpio_ab_irq_handler;
>                 girq->num_parents = 1;
> -               girq->parents = devm_kcalloc(dev, 1,
> +               girq->parents = devm_kcalloc(dev, girq->num_parents,
>                                              sizeof(*girq->parents),
>                                              GFP_KERNEL);
>                 if (!girq->parents)
> @@ -406,15 +406,14 @@ static int ep93xx_gpio_add_bank(struct
> gpio_chip *gc,
>                  */
>                 girq->parent_handler = ep93xx_gpio_f_irq_handler;
>                 girq->num_parents = 8;
> -               girq->parents = devm_kcalloc(dev, 8,
> +               girq->parents = devm_kcalloc(dev, girq->num_parents,
>                                              sizeof(*girq->parents),
>                                              GFP_KERNEL);
>                 if (!girq->parents)
>                         return -ENOMEM;
>                 /* Pick resources 1..8 for these IRQs */
> -               for (i = 1; i <= 8; i++)
> -                       girq->parents[i - 1] = platform_get_irq(pdev,
> i);
> -               for (i = 0; i < 8; i++) {
> +               for (i = 0; i < girq->num_parents; i++) {
> +                       girq->parents[i] = platform_get_irq(pdev, i +
> 1);
>                         gpio_irq = EP93XX_GPIO_F_IRQ_BASE + i;
>                         irq_set_chip_data(gpio_irq, &epg->gc[5]);
>                         irq_set_chip_and_handler(gpio_irq,

-- 
Alexander Sverdlin.

Re: [PATCH v3 4/7] gpio: ep93xx: drop to_irq binding

[Alexander Sverdlin]

Hi!

On Thu, 2021-01-28 at 15:21 +0300, Nikita Shubin wrote:
> As ->to_irq is redefined in gpiochip_add_irqchip, having it defined
> in
> driver is useless, so let's drop it.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Acked-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>

> ---
>  drivers/gpio/gpio-ep93xx.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
> index dc88115e34da..ee1cb3b894db 100644
> --- a/drivers/gpio/gpio-ep93xx.c
> +++ b/drivers/gpio/gpio-ep93xx.c
> @@ -337,11 +337,6 @@ static int ep93xx_gpio_set_config(struct
> gpio_chip *gc, unsigned offset,
>         return 0;
>  }
>  
> -static int ep93xx_gpio_f_to_irq(struct gpio_chip *gc, unsigned
> offset)
> -{
> -       return EP93XX_GPIO_F_IRQ_BASE + offset;
> -}
> -
>  static void ep93xx_init_irq_chips(struct ep93xx_gpio *epg)
>  {
>         int i;
> @@ -429,7 +424,6 @@ static int ep93xx_gpio_add_bank(struct gpio_chip
> *gc,
>                 }
>                 girq->default_type = IRQ_TYPE_NONE;
>                 girq->handler = handle_level_irq;
> -               gc->to_irq = ep93xx_gpio_f_to_irq;
>                 girq->first = EP93XX_GPIO_F_IRQ_BASE;
>         }
>  

-- 
Alexander Sverdlin.

Re: [PATCH v3 0/7] gpio: ep93xx: fixes series patch

[Linus Walleij]

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

For the entire series.

Yours,
Linus Walleij

Re: [PATCH v3 1/7] gpio: gpio-ep93xx: fix BUG_ON port F usage

[Alexander Sverdlin]

Hi Nikita,

On Thu, 2021-02-04 at 15:55 +0300, nikita.shubin@maquefel.me wrote:
> I considered your offer of using array with holes.
>  
> It looks pretty ugly to me, couse it leads to bloated arrays:
>  
> static unsigned char gpio_int_unmasked[EP93XX_GPIO_CHIP_NUM];
> static unsigned char gpio_int_enabled[EP93XX_GPIO_CHIP_NUM];
> static unsigned char gpio_int_type1[EP93XX_GPIO_CHIP_NUM];
> static unsigned char gpio_int_type2[EP93XX_GPIO_CHIP_NUM];
> static unsigned char gpio_int_debounce[EP93XX_GPIO_CHIP_NUM];
>  
> /* Port ordering is: A B F */
> static const u8 int_type1_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0x90, 0xac, 0x0, 0x0, 0x0, 0x4c };
> static const u8 int_type2_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0x94, 0xb0, 0x0, 0x0, 0x0, 0x50 };
> static const u8 eoi_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0x98, 0xb4, 0x0, 0x0, 0x0, 0x54 };
> static const u8 int_en_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0x9c, 0xb8, 0x0, 0x0, 0x0, 0x58 };
> static const u8 int_debounce_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0xa8, 0xc4, 0x0, 0x0, 0x0, 0x64 };
>  
> Is this really the thing we want ?

Even in this form it's less error-prone than to have two
index-spaces, and hidden conversion from one numbering scheme
to other.

Alternatives that I see are:
1.
https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html

2.
Embedd the necessary values into struct ep93xx_gpio_bank.
This option can probably simplify the handling of the names
for irq chips as well.
 
> 28.01.2021, 19:19, "Alexander Sverdlin" <alexander.sverdlin@gmail.com>:
> > Hello Nikita,
> > 
> > On Thu, 2021-01-28 at 18:11 +0200, Andy Shevchenko wrote:
> > >  > +/*
> > >  > + * F Port index in GPIOCHIP'S array is 5
> > >  > + * but we use index 2 for stored values and offsets
> > >  > + */
> > >  > +#define EP93XX_GPIO_F_PORT_INDEX 5
> > >  
> > >  Hmm... Why not to use an array with holes instead.
> > >  
> > >  ...
> > >  
> > >  > +       if (port == EP93XX_GPIO_F_PORT_INDEX)
> > >  > +               port = 2;
> > >  
> > >  Sorry, but I'm not in favour of this as it adds confusion.
> > >  See above for the potential way to solve.
> > 
> > well, I was thinking the same yesterday. It just adds another
> > level on confusion into the code, which even the author got
> > wrong :)
> > 
> > Array with holes would be more obvious, but one can also embedd
> > the necessary values into struct ep93xx_gpio_bank.
> >  
> > --
> > Alexander Sverdlin.
> > 
> >  

-- 
Alexander Sverdlin.

Re: [PATCH v3 1/7] gpio: gpio-ep93xx: fix BUG_ON port F usage

[Alexander Sverdlin]

Hi Nikita,

On Thu, 2021-02-04 at 17:00 +0300, nikita.shubin@maquefel.me wrote:
> >  I considered your offer of using array with holes.
> >   
> >  It looks pretty ugly to me, couse it leads to bloated arrays:
> >   
> >  static unsigned char gpio_int_unmasked[EP93XX_GPIO_CHIP_NUM];
> >  static unsigned char gpio_int_enabled[EP93XX_GPIO_CHIP_NUM];
> >  static unsigned char gpio_int_type1[EP93XX_GPIO_CHIP_NUM];
> >  static unsigned char gpio_int_type2[EP93XX_GPIO_CHIP_NUM];
> >  static unsigned char gpio_int_debounce[EP93XX_GPIO_CHIP_NUM];
> >   
> >  /* Port ordering is: A B F */
> >  static const u8 int_type1_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0x90, 0xac, 0x0, 0x0, 0x0, 0x4c };
> >  static const u8 int_type2_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0x94, 0xb0, 0x0, 0x0, 0x0, 0x50 };
> >  static const u8 eoi_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0x98, 0xb4, 0x0, 0x0, 0x0, 0x54 };
> >  static const u8 int_en_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0x9c, 0xb8, 0x0, 0x0, 0x0, 0x58 };
> >  static const u8 int_debounce_register_offset[EP93XX_GPIO_CHIP_NUM]    = { 0xa8, 0xc4, 0x0, 0x0, 0x0, 0x64 };
> >   
> >  Is this really the thing we want ?
> 
> Even in this form it's less error-prone than to have two
> index-spaces, and hidden conversion from one numbering scheme
> to other.
> 
> Alternatives that I see are:
> 1.
> https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
> 
> 2.
> Embedd the necessary values into struct ep93xx_gpio_bank.
> This option can probably simplify the handling of the names
> for irq chips as well. 
> Thank you very much for your comments, and how about a 3rd option ? :
>  
> It also makes easier to add 'struct irqchip' in following patch.
>  struct ep93xx_gpio_irq_chip {
>        u8 irq_offset;
>        u8 int_unmasked;
>        u8 int_enabled;
>        u8 int_type1;
>        u8 int_type2;
>        u8 int_debounce;
> };
>  
> struct ep93xx_gpio_chip {
>        struct gpio_chip                gc;
>        struct ep93xx_gpio_irq_chip     *eic;
> };
>  
> struct ep93xx_gpio {
>        void __iomem            *base;
>        struct ep93xx_gpio_chip gc[8];
> };
> 
> static const u8 int_register_offset[8]   = { 0x90, 0xac, [5] = 0x4c };
> #define EP93XX_INT_TYPE1_OFFSET        0x00
> #define EP93XX_INT_TYPE2_OFFSET        0x04
> #define EP93XX_INT_EOI_OFFSET          0x08
> #define EP93XX_INT_EN_OFFSET           0x0c
> #define EP93XX_INT_STATUS_OFFSET       0x10
> #define EP93XX_INT_RAW_STATUS_OFFSET   0x14
> #define EP93XX_INT_DEBOUNCE_OFFSET     0x18

Makes sense to me.

-- 
Alexander Sverdlin.