linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] gpiolib: warning on gpiochip->to_irq defined
@ 2020-12-28 15:00 Nikita Shubin
  2021-01-04 14:15 ` Bartosz Golaszewski
                   ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Nikita Shubin @ 2020-12-28 15:00 UTC (permalink / raw)
  Cc: Nikita Shubin, Linus Walleij, Bartosz Golaszewski,
	Grygorii Strashko, Thierry Reding, linux-gpio, linux-kernel

gpiochip->to_irq method is redefined in gpiochip_add_irqchip.

A lot of gpiod driver's still define ->to_irq method, let's give
a gentle warning that they can no longer rely on it, so they can remove
it on ocassion.

Fixes: e0d8972898139 ("gpio: Implement tighter IRQ chip integration")
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/gpio/gpiolib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5ce0c14c637b..44538d1a771a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1489,6 +1489,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
 		type = IRQ_TYPE_NONE;
 	}
 
+	if (gc->to_irq)
+		chip_err(gc, "to_irq is redefined in %s and you shouldn't rely on it\n", __func__);
+
 	gc->to_irq = gpiochip_to_irq;
 	gc->irq.default_type = type;
 	gc->irq.lock_key = lock_key;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH] gpiolib: warning on gpiochip->to_irq defined
  2020-12-28 15:00 [PATCH] gpiolib: warning on gpiochip->to_irq defined Nikita Shubin
@ 2021-01-04 14:15 ` Bartosz Golaszewski
  2021-01-18  9:05 ` [PATCH v2] " Nikita Shubin
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
  2 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2021-01-04 14:15 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Linus Walleij, Grygorii Strashko, Thierry Reding, linux-gpio, LKML

On Mon, Dec 28, 2020 at 4:02 PM Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> gpiochip->to_irq method is redefined in gpiochip_add_irqchip.
>
> A lot of gpiod driver's still define ->to_irq method, let's give
> a gentle warning that they can no longer rely on it, so they can remove
> it on ocassion.
>
> Fixes: e0d8972898139 ("gpio: Implement tighter IRQ chip integration")
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  drivers/gpio/gpiolib.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5ce0c14c637b..44538d1a771a 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1489,6 +1489,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>                 type = IRQ_TYPE_NONE;
>         }
>
> +       if (gc->to_irq)
> +               chip_err(gc, "to_irq is redefined in %s and you shouldn't rely on it\n", __func__);
> +
>         gc->to_irq = gpiochip_to_irq;
>         gc->irq.default_type = type;
>         gc->irq.lock_key = lock_key;
> --
> 2.29.2
>

I know Linus suggested using chip_err() here but since this doesn't
cause the function to fail, I'd say this should be chip_warn().

Unless Linus has any objections, please change it.

Bartosz

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2] gpiolib: warning on gpiochip->to_irq defined
  2020-12-28 15:00 [PATCH] gpiolib: warning on gpiochip->to_irq defined Nikita Shubin
  2021-01-04 14:15 ` Bartosz Golaszewski
@ 2021-01-18  9:05 ` Nikita Shubin
  2021-01-19 10:51   ` Bartosz Golaszewski
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
  2 siblings, 1 reply; 27+ messages in thread
From: Nikita Shubin @ 2021-01-18  9:05 UTC (permalink / raw)
  Cc: Nikita Shubin, Linus Walleij, Bartosz Golaszewski,
	Grygorii Strashko, Thierry Reding, linux-gpio, linux-kernel

gpiochip->to_irq method is redefined in gpiochip_add_irqchip.

A lot of gpiod driver's still define ->to_irq method, let's give
a gentle warning that they can no longer rely on it, so they can remove
it on ocassion.

Fixes: e0d8972898139 ("gpio: Implement tighter IRQ chip integration")
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
v1->v2:
- Change chip_err to chip_warn
---
 drivers/gpio/gpiolib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5ce0c14c637b..5a9410c2537d 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1489,6 +1489,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
 		type = IRQ_TYPE_NONE;
 	}
 
+	if (gc->to_irq)
+		chip_warn(gc, "to_irq is redefined in %s and you shouldn't rely on it\n", __func__);
+
 	gc->to_irq = gpiochip_to_irq;
 	gc->irq.default_type = type;
 	gc->irq.lock_key = lock_key;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v2] gpiolib: warning on gpiochip->to_irq defined
  2021-01-18  9:05 ` [PATCH v2] " Nikita Shubin
@ 2021-01-19 10:51   ` Bartosz Golaszewski
  0 siblings, 0 replies; 27+ messages in thread
From: Bartosz Golaszewski @ 2021-01-19 10:51 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Linus Walleij, Grygorii Strashko, Thierry Reding, linux-gpio, LKML

On Mon, Jan 18, 2021 at 10:05 AM Nikita Shubin
<nikita.shubin@maquefel.me> wrote:
>
> gpiochip->to_irq method is redefined in gpiochip_add_irqchip.
>
> A lot of gpiod driver's still define ->to_irq method, let's give
> a gentle warning that they can no longer rely on it, so they can remove
> it on ocassion.
>
> Fixes: e0d8972898139 ("gpio: Implement tighter IRQ chip integration")
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> v1->v2:
> - Change chip_err to chip_warn
> ---
>  drivers/gpio/gpiolib.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5ce0c14c637b..5a9410c2537d 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1489,6 +1489,9 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>                 type = IRQ_TYPE_NONE;
>         }
>
> +       if (gc->to_irq)
> +               chip_warn(gc, "to_irq is redefined in %s and you shouldn't rely on it\n", __func__);
> +
>         gc->to_irq = gpiochip_to_irq;
>         gc->irq.default_type = type;
>         gc->irq.lock_key = lock_key;
> --
> 2.29.2
>

Applied to fixes, thanks!

Bartosz

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 0/9] gpio: ep93xx: fixes series patch
  2020-12-28 15:00 [PATCH] gpiolib: warning on gpiochip->to_irq defined Nikita Shubin
  2021-01-04 14:15 ` Bartosz Golaszewski
  2021-01-18  9:05 ` [PATCH v2] " Nikita Shubin
@ 2021-01-27 10:46 ` Nikita Shubin
  2021-01-27 10:46   ` [PATCH v2 1/9] gpio: ep93xx: fix BUG_ON port F usage Nikita Shubin
                     ` (9 more replies)
  2 siblings, 10 replies; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

Series of patches to fix ep93xx gpio driver to make IRQ's working.

The following are fix patches (these are enough to get gpio-ep93xx
working with modern kernel):

[PATCH v2 1/9] gpio: ep93xx: fix BUG_ON port F usage
[PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi
 gpiochips
[PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F

The following are cleanup and style patches:

[PATCH v2 4/9] gpio: ep93xx: drop to_irq binding
[PATCH v2 5/9] gpio: ep93xx: Fix typo s/hierarchial/hierarchical
[PATCH v2 6/9] gpio: ep93xx: refactor ep93xx_gpio_add_bank
[PATCH v2 7/9] gpio: ep93xx: separate IRQ's setup
[PATCH v2 8/9] gpio: ep93xx: refactor base IRQ number
[PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports

The futher work can be done by removing ep93xx_gpio_port function
and, maybe, ep93xx_gpio_update_int_params. But i think it's better 
to be done in conjunction with converting ep93xx to DT support.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 1/9] gpio: ep93xx: fix BUG_ON port F usage
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 10:46   ` [PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi gpiochips Nikita Shubin
                     ` (8 subsequent siblings)
  9 siblings, 0 replies; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

The port F is actually index 2 not 5 as it is calculated 
by ep93xx_gpio_port.

Looks like it happened because of the confusing ep93xx_gpio_banks 
table that shows like port F is having index 5, but is actually 
index 2 if we rely on .base instead of index position.

------------[ 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>
---
 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..0d0435c07a5a 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


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi gpiochips
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
  2021-01-27 10:46   ` [PATCH v2 1/9] gpio: ep93xx: fix BUG_ON port F usage Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 21:39     ` Alexander Sverdlin
  2021-01-28 10:17     ` Alexander Sverdlin
  2021-01-27 10:46   ` [PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F Nikita Shubin
                     ` (7 subsequent siblings)
  9 siblings, 2 replies; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, Maulik Shah, linux-gpio,
	linux-kernel

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: a8173820f441 ("gpio: gpiolib: Allow GPIO IRQs to lazy disable")
Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 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 0d0435c07a5a..2eea02c906e0 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


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
  2021-01-27 10:46   ` [PATCH v2 1/9] gpio: ep93xx: fix BUG_ON port F usage Nikita Shubin
  2021-01-27 10:46   ` [PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi gpiochips Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 21:50     ` Linus Walleij
  2021-01-28 10:15     ` Alexander Sverdlin
  2021-01-27 10:46   ` [PATCH v2 4/9] gpio: ep93xx: drop to_irq binding Nikita Shubin
                     ` (6 subsequent siblings)
  9 siblings, 2 replies; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

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 2eea02c906e0..9c3d049e5af7 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


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 4/9] gpio: ep93xx: drop to_irq binding
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
                     ` (2 preceding siblings ...)
  2021-01-27 10:46   ` [PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 12:21     ` Alexander Sverdlin
  2021-01-27 10:46   ` [PATCH v2 5/9] gpio: ep93xx: Fix typo s/hierarchial/hierarchical Nikita Shubin
                     ` (5 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

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 9c3d049e5af7..dee19372ebbd 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


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 5/9] gpio: ep93xx: Fix typo s/hierarchial/hierarchical
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
                     ` (3 preceding siblings ...)
  2021-01-27 10:46   ` [PATCH v2 4/9] gpio: ep93xx: drop to_irq binding Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 13:20     ` Alexander Sverdlin
  2021-01-27 10:46   ` [PATCH v2 6/9] gpio: ep93xx: refactor ep93xx_gpio_add_bank Nikita Shubin
                     ` (4 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

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 dee19372ebbd..8f66e3ca0cfb 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


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 6/9] gpio: ep93xx: refactor ep93xx_gpio_add_bank
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
                     ` (4 preceding siblings ...)
  2021-01-27 10:46   ` [PATCH v2 5/9] gpio: ep93xx: Fix typo s/hierarchial/hierarchical Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 13:24     ` Alexander Sverdlin
  2021-01-27 10:46   ` [PATCH v2 7/9] gpio: ep93xx: separate IRQ's setup Nikita Shubin
                     ` (3 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

- replace plain numbers with girq->num_parents in devm_kcalloc
- replace plain numbers with ARRAY_SIZE(girq->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>
---
 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 8f66e3ca0cfb..e4270b4e5f26 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 < ARRAY_SIZE(girq->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


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 7/9] gpio: ep93xx: separate IRQ's setup
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
                     ` (5 preceding siblings ...)
  2021-01-27 10:46   ` [PATCH v2 6/9] gpio: ep93xx: refactor ep93xx_gpio_add_bank Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 20:48     ` Alexander Sverdlin
  2021-01-27 10:46   ` [PATCH v2 8/9] gpio: ep93xx: refactor base IRQ number Nikita Shubin
                     ` (2 subsequent siblings)
  9 siblings, 1 reply; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

Separate IRQ's setup for port A,B,F.

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

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index e4270b4e5f26..b212c007240e 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -353,6 +353,64 @@ static void ep93xx_init_irq_chips(struct ep93xx_gpio *epg)
 	}
 }
 
+static int ep93xx_gpio_add_ab_irq_chip(struct platform_device *pdev,
+					struct gpio_irq_chip *girq,
+					unsigned int irq_base)
+{
+	struct device *dev = &pdev->dev;
+	int ab_parent_irq = platform_get_irq(pdev, 0);
+
+	girq->parent_handler = ep93xx_gpio_ab_irq_handler;
+	girq->num_parents = 1;
+	girq->parents = devm_kcalloc(dev, girq->num_parents,
+				     sizeof(*girq->parents),
+				     GFP_KERNEL);
+	if (!girq->parents)
+		return -ENOMEM;
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_level_irq;
+	girq->parents[0] = ab_parent_irq;
+	girq->first = irq_base;
+
+	return 0;
+}
+
+static int ep93xx_gpio_add_f_irq_chip(struct platform_device *pdev,
+					struct gpio_irq_chip *girq,
+					unsigned int irq_base)
+{
+	int gpio_irq;
+	int i;
+	struct device *dev = &pdev->dev;
+
+	/*
+	 * FIXME: convert this to use hierarchical IRQ support!
+	 * this requires fixing the root irqchip to be hierarchical.
+	 */
+	girq->parent_handler = ep93xx_gpio_f_irq_handler;
+	girq->num_parents = 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 = 0; i < ARRAY_SIZE(girq->parents); i++) {
+		girq->parents[i] = platform_get_irq(pdev, i + 1);
+		gpio_irq = irq_base + i;
+		irq_set_chip_data(gpio_irq, &epg->gc[5]);
+		irq_set_chip_and_handler(gpio_irq,
+					 &ep93xx_gpio_irq_chip,
+					 handle_level_irq);
+		irq_clear_status_flags(gpio_irq, IRQ_NOREQUEST);
+	}
+	girq->default_type = IRQ_TYPE_NONE;
+	girq->handler = handle_level_irq;
+	girq->first = irq_base;
+
+	return 0;
+}
+
 static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 				struct platform_device *pdev,
 				struct ep93xx_gpio *epg,
@@ -380,50 +438,16 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 	}
 
 	if (bank->has_irq) {
-		int ab_parent_irq = platform_get_irq(pdev, 0);
-
-		girq->parent_handler = ep93xx_gpio_ab_irq_handler;
-		girq->num_parents = 1;
-		girq->parents = devm_kcalloc(dev, girq->num_parents,
-					     sizeof(*girq->parents),
-					     GFP_KERNEL);
-		if (!girq->parents)
-			return -ENOMEM;
-		girq->default_type = IRQ_TYPE_NONE;
-		girq->handler = handle_level_irq;
-		girq->parents[0] = ab_parent_irq;
-		girq->first = bank->irq_base;
+		err = ep93xx_gpio_add_ab_irq_chip(pdev, girq, bank->irq_base);
+		if (err)
+			return err;
 	}
 
 	/* Only bank F has especially funky IRQ handling */
 	if (bank->has_hierarchical_irq) {
-		int gpio_irq;
-		int i;
-
-		/*
-		 * FIXME: convert this to use hierarchical IRQ support!
-		 * this requires fixing the root irqchip to be hierarchical.
-		 */
-		girq->parent_handler = ep93xx_gpio_f_irq_handler;
-		girq->num_parents = 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 = 0; i < ARRAY_SIZE(girq->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,
-						 &ep93xx_gpio_irq_chip,
-						 handle_level_irq);
-			irq_clear_status_flags(gpio_irq, IRQ_NOREQUEST);
-		}
-		girq->default_type = IRQ_TYPE_NONE;
-		girq->handler = handle_level_irq;
-		girq->first = EP93XX_GPIO_F_IRQ_BASE;
+		err = ep93xx_gpio_add_f_irq_chip(pdev, girq, EP93XX_GPIO_F_IRQ_BASE);
+		if (err)
+			return err;
 	}
 
 	return devm_gpiochip_add_data(dev, gc, epg);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 8/9] gpio: ep93xx: refactor base IRQ number
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
                     ` (6 preceding siblings ...)
  2021-01-27 10:46   ` [PATCH v2 7/9] gpio: ep93xx: separate IRQ's setup Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 11:36     ` Alexander Sverdlin
  2021-01-27 10:46   ` [PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports Nikita Shubin
  2021-01-27 21:54   ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Linus Walleij
  9 siblings, 1 reply; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

- 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 | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index b212c007240e..2aee13b5067d 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),
 };
@@ -445,7 +447,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 
 	/* Only bank F has especially funky IRQ handling */
 	if (bank->has_hierarchical_irq) {
-		err = ep93xx_gpio_add_f_irq_chip(pdev, girq, EP93XX_GPIO_F_IRQ_BASE);
+		err = ep93xx_gpio_add_f_irq_chip(pdev, girq, bank->irq_base);
 		if (err)
 			return err;
 	}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
                     ` (7 preceding siblings ...)
  2021-01-27 10:46   ` [PATCH v2 8/9] gpio: ep93xx: refactor base IRQ number Nikita Shubin
@ 2021-01-27 10:46   ` Nikita Shubin
  2021-01-27 11:34     ` Alexander Sverdlin
  2021-01-27 21:54   ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Linus Walleij
  9 siblings, 1 reply; 27+ messages in thread
From: Nikita Shubin @ 2021-01-27 10:46 UTC (permalink / raw)
  Cc: Andy Shevchenko, Nikita Shubin, Linus Walleij,
	Bartosz Golaszewski, Alexander Sverdlin, linux-gpio,
	linux-kernel

Replace boolean values used for determining if gpiochip is IRQ capable
or not with index.

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

diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
index 2aee13b5067d..f75a33b79504 100644
--- a/drivers/gpio/gpio-ep93xx.c
+++ b/drivers/gpio/gpio-ep93xx.c
@@ -44,6 +44,8 @@ struct ep93xx_gpio {
 	struct irq_chip		ic[EP93XX_GPIO_IRQ_CHIPS_NUM];
 };
 
+#define EP93XX_GPIO_A_PORT_INDEX 0
+#define EP93XX_GPIO_B_PORT_INDEX 1
 /*
  * F Port index in GPIOCHIP'S array is 5
  * but we use index 2 for stored values and offsets
@@ -291,38 +293,36 @@ static struct irq_chip ep93xx_gpio_irq_chip = {
  * gpiolib interface for EP93xx on-chip GPIOs
  *************************************************************************/
 struct ep93xx_gpio_bank {
+	u8		idx;
 	const char	*label;
 	int		data;
 	int		dir;
 	int		base;
-	bool		has_irq;
-	bool		has_hierarchical_irq;
 	unsigned int	irq_base;
 };
 
-#define EP93XX_GPIO_BANK(_label, _data, _dir, _base, _has_irq, _has_hier, _irq_base) \
+#define EP93XX_GPIO_BANK(_index, _label, _data, _dir, _base, _irq_base) \
 	{							\
+		.idx		= _index,			\
 		.label		= _label,			\
 		.data		= _data,			\
 		.dir		= _dir,				\
 		.base		= _base,			\
-		.has_irq	= _has_irq,			\
-		.has_hierarchical_irq = _has_hier,		\
 		.irq_base	= _irq_base,			\
 	}
 
 static struct ep93xx_gpio_bank ep93xx_gpio_banks[] = {
 	/* Bank A has 8 IRQs */
-	EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true, false, EP93XX_GPIO_A_IRQ_BASE),
+	EP93XX_GPIO_BANK(0, "A", 0x00, 0x10, 0, EP93XX_GPIO_A_IRQ_BASE),
 	/* Bank B has 8 IRQs */
-	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),
+	EP93XX_GPIO_BANK(1, "B", 0x04, 0x14, 8, EP93XX_GPIO_B_IRQ_BASE),
+	EP93XX_GPIO_BANK(2, "C", 0x08, 0x18, 40, 0),
+	EP93XX_GPIO_BANK(3, "D", 0x0c, 0x1c, 24, 0),
+	EP93XX_GPIO_BANK(4, "E", 0x20, 0x24, 32, 0),
 	/* Bank F has 8 IRQs */
-	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),
+	EP93XX_GPIO_BANK(5, "F", 0x30, 0x34, 16, EP93XX_GPIO_F_IRQ_BASE),
+	EP93XX_GPIO_BANK(6, "G", 0x38, 0x3c, 48, 0),
+	EP93XX_GPIO_BANK(7, "H", 0x40, 0x44, 56, 0),
 };
 
 static int ep93xx_gpio_set_config(struct gpio_chip *gc, unsigned offset,
@@ -356,10 +356,11 @@ static void ep93xx_init_irq_chips(struct ep93xx_gpio *epg)
 }
 
 static int ep93xx_gpio_add_ab_irq_chip(struct platform_device *pdev,
-					struct gpio_irq_chip *girq,
+					struct gpio_chip *gc,
 					unsigned int irq_base)
 {
 	struct device *dev = &pdev->dev;
+	struct gpio_irq_chip *girq = &gc->irq;
 	int ab_parent_irq = platform_get_irq(pdev, 0);
 
 	girq->parent_handler = ep93xx_gpio_ab_irq_handler;
@@ -378,12 +379,13 @@ static int ep93xx_gpio_add_ab_irq_chip(struct platform_device *pdev,
 }
 
 static int ep93xx_gpio_add_f_irq_chip(struct platform_device *pdev,
-					struct gpio_irq_chip *girq,
+					struct gpio_chip *gc,
 					unsigned int irq_base)
 {
 	int gpio_irq;
 	int i;
 	struct device *dev = &pdev->dev;
+	struct gpio_irq_chip *girq = &gc->irq;
 
 	/*
 	 * FIXME: convert this to use hierarchical IRQ support!
@@ -397,10 +399,10 @@ static int ep93xx_gpio_add_f_irq_chip(struct platform_device *pdev,
 	if (!girq->parents)
 		return -ENOMEM;
 	/* Pick resources 1..8 for these IRQs */
-	for (i = 0; i < ARRAY_SIZE(girq->parents); i++) {
+	for (i = 0; i < girq->num_parents; i++) {
 		girq->parents[i] = platform_get_irq(pdev, i + 1);
 		gpio_irq = irq_base + i;
-		irq_set_chip_data(gpio_irq, &epg->gc[5]);
+		irq_set_chip_data(gpio_irq, gc);
 		irq_set_chip_and_handler(gpio_irq,
 					 &ep93xx_gpio_irq_chip,
 					 handle_level_irq);
@@ -433,21 +435,22 @@ static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
 	gc->base = bank->base;
 
 	girq = &gc->irq;
-	if (bank->has_irq || bank->has_hierarchical_irq) {
+	if (bank->irq_base != 0) {
 		gc->set_config = ep93xx_gpio_set_config;
 		port = ep93xx_gpio_port(epg, gc);
 		girq->chip = &epg->ic[port];
 	}
 
-	if (bank->has_irq) {
-		err = ep93xx_gpio_add_ab_irq_chip(pdev, girq, bank->irq_base);
+	if (bank->idx == EP93XX_GPIO_A_PORT_INDEX ||
+		bank->idx == EP93XX_GPIO_B_PORT_INDEX) {
+		err = ep93xx_gpio_add_ab_irq_chip(pdev, gc, bank->irq_base);
 		if (err)
 			return err;
 	}
 
 	/* Only bank F has especially funky IRQ handling */
-	if (bank->has_hierarchical_irq) {
-		err = ep93xx_gpio_add_f_irq_chip(pdev, girq, bank->irq_base);
+	if (bank->idx == EP93XX_GPIO_F_PORT_INDEX) {
+		err = ep93xx_gpio_add_f_irq_chip(pdev, gc, bank->irq_base);
 		if (err)
 			return err;
 	}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports
  2021-01-27 10:46   ` [PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports Nikita Shubin
@ 2021-01-27 11:34     ` Alexander Sverdlin
  0 siblings, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-27 11:34 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	linux-kernel

Hello Nikita!

On Wed, 2021-01-27 at 13:46 +0300, Nikita Shubin wrote:
> Replace boolean values used for determining if gpiochip is IRQ
> capable
> or not with index.

What's the purpose of this patch?
It neither makes the code more effective nor does it make
the code more readable IMO. There are also some changes in the
code which were not mentioned in the commit message.

> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  drivers/gpio/gpio-ep93xx.c | 47 ++++++++++++++++++++----------------
> --
>  1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
> index 2aee13b5067d..f75a33b79504 100644
> --- a/drivers/gpio/gpio-ep93xx.c
> +++ b/drivers/gpio/gpio-ep93xx.c
> @@ -44,6 +44,8 @@ struct ep93xx_gpio {
>         struct irq_chip         ic[EP93XX_GPIO_IRQ_CHIPS_NUM];
>  };
>  
> +#define EP93XX_GPIO_A_PORT_INDEX 0
> +#define EP93XX_GPIO_B_PORT_INDEX 1
>  /*
>   * F Port index in GPIOCHIP'S array is 5
>   * but we use index 2 for stored values and offsets
> @@ -291,38 +293,36 @@ static struct irq_chip ep93xx_gpio_irq_chip = {
>   * gpiolib interface for EP93xx on-chip GPIOs
>  
> *********************************************************************
> ****/
>  struct ep93xx_gpio_bank {
> +       u8              idx;
>         const char      *label;
>         int             data;
>         int             dir;
>         int             base;
> -       bool            has_irq;
> -       bool            has_hierarchical_irq;
>         unsigned int    irq_base;
>  };
>  
> -#define EP93XX_GPIO_BANK(_label, _data, _dir, _base, _has_irq,
> _has_hier, _irq_base) \
> +#define EP93XX_GPIO_BANK(_index, _label, _data, _dir, _base,
> _irq_base) \
>         {                                                       \
> +               .idx            = _index,                       \
>                 .label          = _label,                       \
>                 .data           = _data,                        \
>                 .dir            = _dir,                         \
>                 .base           = _base,                        \
> -               .has_irq        = _has_irq,                     \
> -               .has_hierarchical_irq = _has_hier,              \
>                 .irq_base       = _irq_base,                    \
>         }
>  
>  static struct ep93xx_gpio_bank ep93xx_gpio_banks[] = {
>         /* Bank A has 8 IRQs */
> -       EP93XX_GPIO_BANK("A", 0x00, 0x10, 0, true, false,
> EP93XX_GPIO_A_IRQ_BASE),
> +       EP93XX_GPIO_BANK(0, "A", 0x00, 0x10, 0,
> EP93XX_GPIO_A_IRQ_BASE),
>         /* Bank B has 8 IRQs */
> -       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),
> +       EP93XX_GPIO_BANK(1, "B", 0x04, 0x14, 8,
> EP93XX_GPIO_B_IRQ_BASE),
> +       EP93XX_GPIO_BANK(2, "C", 0x08, 0x18, 40, 0),
> +       EP93XX_GPIO_BANK(3, "D", 0x0c, 0x1c, 24, 0),
> +       EP93XX_GPIO_BANK(4, "E", 0x20, 0x24, 32, 0),
>         /* Bank F has 8 IRQs */
> -       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),
> +       EP93XX_GPIO_BANK(5, "F", 0x30, 0x34, 16,
> EP93XX_GPIO_F_IRQ_BASE),
> +       EP93XX_GPIO_BANK(6, "G", 0x38, 0x3c, 48, 0),
> +       EP93XX_GPIO_BANK(7, "H", 0x40, 0x44, 56, 0),
>  };
>  
>  static int ep93xx_gpio_set_config(struct gpio_chip *gc, unsigned
> offset,
> @@ -356,10 +356,11 @@ static void ep93xx_init_irq_chips(struct
> ep93xx_gpio *epg)
>  }
>  
>  static int ep93xx_gpio_add_ab_irq_chip(struct platform_device *pdev,
> -                                       struct gpio_irq_chip *girq,
> +                                       struct gpio_chip *gc,
>                                         unsigned int irq_base)
>  {
>         struct device *dev = &pdev->dev;
> +       struct gpio_irq_chip *girq = &gc->irq;
>         int ab_parent_irq = platform_get_irq(pdev, 0);
>  
>         girq->parent_handler = ep93xx_gpio_ab_irq_handler;
> @@ -378,12 +379,13 @@ static int ep93xx_gpio_add_ab_irq_chip(struct
> platform_device *pdev,
>  }
>  
>  static int ep93xx_gpio_add_f_irq_chip(struct platform_device *pdev,
> -                                       struct gpio_irq_chip *girq,
> +                                       struct gpio_chip *gc,
>                                         unsigned int irq_base)
>  {
>         int gpio_irq;
>         int i;
>         struct device *dev = &pdev->dev;
> +       struct gpio_irq_chip *girq = &gc->irq;
>  
>         /*
>          * FIXME: convert this to use hierarchical IRQ support!
> @@ -397,10 +399,10 @@ static int ep93xx_gpio_add_f_irq_chip(struct
> platform_device *pdev,
>         if (!girq->parents)
>                 return -ENOMEM;
>         /* Pick resources 1..8 for these IRQs */
> -       for (i = 0; i < ARRAY_SIZE(girq->parents); i++) {
> +       for (i = 0; i < girq->num_parents; i++) {
>                 girq->parents[i] = platform_get_irq(pdev, i + 1);
>                 gpio_irq = irq_base + i;
> -               irq_set_chip_data(gpio_irq, &epg->gc[5]);
> +               irq_set_chip_data(gpio_irq, gc);
>                 irq_set_chip_and_handler(gpio_irq,
>                                          &ep93xx_gpio_irq_chip,
>                                          handle_level_irq);
> @@ -433,21 +435,22 @@ static int ep93xx_gpio_add_bank(struct
> gpio_chip *gc,
>         gc->base = bank->base;
>  
>         girq = &gc->irq;
> -       if (bank->has_irq || bank->has_hierarchical_irq) {
> +       if (bank->irq_base != 0) {
>                 gc->set_config = ep93xx_gpio_set_config;
>                 port = ep93xx_gpio_port(epg, gc);
>                 girq->chip = &epg->ic[port];
>         }
>  
> -       if (bank->has_irq) {
> -               err = ep93xx_gpio_add_ab_irq_chip(pdev, girq, bank-
> >irq_base);
> +       if (bank->idx == EP93XX_GPIO_A_PORT_INDEX ||
> +               bank->idx == EP93XX_GPIO_B_PORT_INDEX) {
> +               err = ep93xx_gpio_add_ab_irq_chip(pdev, gc, bank-
> >irq_base);
>                 if (err)
>                         return err;
>         }
>  
>         /* Only bank F has especially funky IRQ handling */
> -       if (bank->has_hierarchical_irq) {
> -               err = ep93xx_gpio_add_f_irq_chip(pdev, girq, bank-
> >irq_base);
> +       if (bank->idx == EP93XX_GPIO_F_PORT_INDEX) {
> +               err = ep93xx_gpio_add_f_irq_chip(pdev, gc, bank-
> >irq_base);
>                 if (err)
>                         return err;
>         }

-- 
Alexander Sverdlin.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 8/9] gpio: ep93xx: refactor base IRQ number
  2021-01-27 10:46   ` [PATCH v2 8/9] gpio: ep93xx: refactor base IRQ number Nikita Shubin
@ 2021-01-27 11:36     ` Alexander Sverdlin
  0 siblings, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-27 11:36 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	linux-kernel

Hi!

On Wed, 2021-01-27 at 13:46 +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>

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

> ---
>  drivers/gpio/gpio-ep93xx.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
> index b212c007240e..2aee13b5067d 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),
>  };
> @@ -445,7 +447,7 @@ static int ep93xx_gpio_add_bank(struct gpio_chip
> *gc,
>  
>         /* Only bank F has especially funky IRQ handling */
>         if (bank->has_hierarchical_irq) {
> -               err = ep93xx_gpio_add_f_irq_chip(pdev, girq,
> EP93XX_GPIO_F_IRQ_BASE);
> +               err = ep93xx_gpio_add_f_irq_chip(pdev, girq, bank-
> >irq_base);
>                 if (err)
>                         return err;
>         }

-- 
Alexander Sverdlin.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 4/9] gpio: ep93xx: drop to_irq binding
  2021-01-27 10:46   ` [PATCH v2 4/9] gpio: ep93xx: drop to_irq binding Nikita Shubin
@ 2021-01-27 12:21     ` Alexander Sverdlin
  0 siblings, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-27 12:21 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	linux-kernel

Hi!

On Wed, 2021-01-27 at 13:46 +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>

Reviewed-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 9c3d049e5af7..dee19372ebbd 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.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 5/9] gpio: ep93xx: Fix typo s/hierarchial/hierarchical
  2021-01-27 10:46   ` [PATCH v2 5/9] gpio: ep93xx: Fix typo s/hierarchial/hierarchical Nikita Shubin
@ 2021-01-27 13:20     ` Alexander Sverdlin
  0 siblings, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-27 13:20 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	linux-kernel

Hi!

On Wed, 2021-01-27 at 13:46 +0300, Nikita Shubin wrote:
> Fix typo in comment.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

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

> ---
>  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 dee19372ebbd..8f66e3ca0cfb 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;

-- 
Alexander Sverdlin.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 6/9] gpio: ep93xx: refactor ep93xx_gpio_add_bank
  2021-01-27 10:46   ` [PATCH v2 6/9] gpio: ep93xx: refactor ep93xx_gpio_add_bank Nikita Shubin
@ 2021-01-27 13:24     ` Alexander Sverdlin
  0 siblings, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-27 13:24 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	linux-kernel

Hello Nikita,

On Wed, 2021-01-27 at 13:46 +0300, Nikita Shubin wrote:
> - replace plain numbers with girq->num_parents in devm_kcalloc
> - replace plain numbers with ARRAY_SIZE(girq->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>
> ---
>  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 8f66e3ca0cfb..e4270b4e5f26 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 < ARRAY_SIZE(girq->parents); i++) {

Why do you use ARRAY_SIZE() here instead of ->num_parents like above?

> +                       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.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 7/9] gpio: ep93xx: separate IRQ's setup
  2021-01-27 10:46   ` [PATCH v2 7/9] gpio: ep93xx: separate IRQ's setup Nikita Shubin
@ 2021-01-27 20:48     ` Alexander Sverdlin
  0 siblings, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-27 20:48 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	linux-kernel

Hello Nikita,

On Wed, 2021-01-27 at 13:46 +0300, Nikita Shubin wrote:
> Separate IRQ's setup for port A,B,F.

Somehow I don't feel that moving "FIXME" code around makes much
sense. Maybe the anticipated conversion to hierarhical IRQ chip
will result in a cleanup automatically?

> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  drivers/gpio/gpio-ep93xx.c | 104 +++++++++++++++++++++++------------
> --
>  1 file changed, 64 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-ep93xx.c b/drivers/gpio/gpio-ep93xx.c
> index e4270b4e5f26..b212c007240e 100644
> --- a/drivers/gpio/gpio-ep93xx.c
> +++ b/drivers/gpio/gpio-ep93xx.c
> @@ -353,6 +353,64 @@ static void ep93xx_init_irq_chips(struct
> ep93xx_gpio *epg)
>         }
>  }
>  
> +static int ep93xx_gpio_add_ab_irq_chip(struct platform_device *pdev,
> +                                       struct gpio_irq_chip *girq,
> +                                       unsigned int irq_base)
> +{
> +       struct device *dev = &pdev->dev;
> +       int ab_parent_irq = platform_get_irq(pdev, 0);
> +
> +       girq->parent_handler = ep93xx_gpio_ab_irq_handler;
> +       girq->num_parents = 1;
> +       girq->parents = devm_kcalloc(dev, girq->num_parents,
> +                                    sizeof(*girq->parents),
> +                                    GFP_KERNEL);
> +       if (!girq->parents)
> +               return -ENOMEM;
> +       girq->default_type = IRQ_TYPE_NONE;
> +       girq->handler = handle_level_irq;
> +       girq->parents[0] = ab_parent_irq;
> +       girq->first = irq_base;
> +
> +       return 0;
> +}
> +
> +static int ep93xx_gpio_add_f_irq_chip(struct platform_device *pdev,
> +                                       struct gpio_irq_chip *girq,
> +                                       unsigned int irq_base)
> +{
> +       int gpio_irq;
> +       int i;
> +       struct device *dev = &pdev->dev;
> +
> +       /*
> +        * FIXME: convert this to use hierarchical IRQ support!
> +        * this requires fixing the root irqchip to be hierarchical.
> +        */
> +       girq->parent_handler = ep93xx_gpio_f_irq_handler;
> +       girq->num_parents = 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 = 0; i < ARRAY_SIZE(girq->parents); i++) {
> +               girq->parents[i] = platform_get_irq(pdev, i + 1);
> +               gpio_irq = irq_base + i;
> +               irq_set_chip_data(gpio_irq, &epg->gc[5]);
> +               irq_set_chip_and_handler(gpio_irq,
> +                                        &ep93xx_gpio_irq_chip,
> +                                        handle_level_irq);
> +               irq_clear_status_flags(gpio_irq, IRQ_NOREQUEST);
> +       }
> +       girq->default_type = IRQ_TYPE_NONE;
> +       girq->handler = handle_level_irq;
> +       girq->first = irq_base;
> +
> +       return 0;
> +}
> +
>  static int ep93xx_gpio_add_bank(struct gpio_chip *gc,
>                                 struct platform_device *pdev,
>                                 struct ep93xx_gpio *epg,
> @@ -380,50 +438,16 @@ static int ep93xx_gpio_add_bank(struct
> gpio_chip *gc,
>         }
>  
>         if (bank->has_irq) {
> -               int ab_parent_irq = platform_get_irq(pdev, 0);
> -
> -               girq->parent_handler = ep93xx_gpio_ab_irq_handler;
> -               girq->num_parents = 1;
> -               girq->parents = devm_kcalloc(dev, girq->num_parents,
> -                                            sizeof(*girq->parents),
> -                                            GFP_KERNEL);
> -               if (!girq->parents)
> -                       return -ENOMEM;
> -               girq->default_type = IRQ_TYPE_NONE;
> -               girq->handler = handle_level_irq;
> -               girq->parents[0] = ab_parent_irq;
> -               girq->first = bank->irq_base;
> +               err = ep93xx_gpio_add_ab_irq_chip(pdev, girq, bank-
> >irq_base);
> +               if (err)
> +                       return err;
>         }
>  
>         /* Only bank F has especially funky IRQ handling */
>         if (bank->has_hierarchical_irq) {
> -               int gpio_irq;
> -               int i;
> -
> -               /*
> -                * FIXME: convert this to use hierarchical IRQ
> support!
> -                * this requires fixing the root irqchip to be
> hierarchical.
> -                */
> -               girq->parent_handler = ep93xx_gpio_f_irq_handler;
> -               girq->num_parents = 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 = 0; i < ARRAY_SIZE(girq->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,
> -                                               
> &ep93xx_gpio_irq_chip,
> -                                                handle_level_irq);
> -                       irq_clear_status_flags(gpio_irq,
> IRQ_NOREQUEST);
> -               }
> -               girq->default_type = IRQ_TYPE_NONE;
> -               girq->handler = handle_level_irq;
> -               girq->first = EP93XX_GPIO_F_IRQ_BASE;
> +               err = ep93xx_gpio_add_f_irq_chip(pdev, girq,
> EP93XX_GPIO_F_IRQ_BASE);
> +               if (err)
> +                       return err;
>         }
>  
>         return devm_gpiochip_add_data(dev, gc, epg);

-- 
Alexander Sverdlin.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi gpiochips
  2021-01-27 10:46   ` [PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi gpiochips Nikita Shubin
@ 2021-01-27 21:39     ` Alexander Sverdlin
  2021-01-28 10:17     ` Alexander Sverdlin
  1 sibling, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-27 21:39 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, Maulik Shah,
	linux-gpio, linux-kernel

Hello Nikita,

On Wed, 2021-01-27 at 13:46 +0300, Nikita Shubin 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: a8173820f441 ("gpio: gpiolib: Allow GPIO IRQs to lazy
> disable")

I'd rather say, it fixes commit d2b091961510
("gpio: ep93xx: Pass irqchip when adding gpiochip").
But for sure, not the gpiolib code as above tag claims.

> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>  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 0d0435c07a5a..2eea02c906e0 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];

-- 
Alexander Sverdlin.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F
  2021-01-27 10:46   ` [PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F Nikita Shubin
@ 2021-01-27 21:50     ` Linus Walleij
  2021-01-28 10:15     ` Alexander Sverdlin
  1 sibling, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2021-01-27 21:50 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Bartosz Golaszewski, Alexander Sverdlin,
	open list:GPIO SUBSYSTEM, linux-kernel

On Wed, Jan 27, 2021 at 11:46 AM Nikita Shubin
<nikita.shubin@maquefel.me> 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>

True, to satisfy old board file-type machines we unfortunately
have to do this.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 0/9] gpio: ep93xx: fixes series patch
  2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
                     ` (8 preceding siblings ...)
  2021-01-27 10:46   ` [PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports Nikita Shubin
@ 2021-01-27 21:54   ` Linus Walleij
  2021-01-28 10:19     ` Alexander Sverdlin
  9 siblings, 1 reply; 27+ messages in thread
From: Linus Walleij @ 2021-01-27 21:54 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Bartosz Golaszewski, Alexander Sverdlin,
	open list:GPIO SUBSYSTEM, linux-kernel

On Wed, Jan 27, 2021 at 11:46 AM Nikita Shubin
<nikita.shubin@maquefel.me> wrote:

> Series of patches to fix ep93xx gpio driver to make IRQ's working.
>
> The following are fix patches (these are enough to get gpio-ep93xx
> working with modern kernel):

I see that there is a strange level of attention to patches to this
platform!

Since you fix all my mistakes made in converting this driver
in the past I will just say:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

For all of them.

There are some nitpicks from the reviewers to fix up but
overall this looks very very good.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F
  2021-01-27 10:46   ` [PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F Nikita Shubin
  2021-01-27 21:50     ` Linus Walleij
@ 2021-01-28 10:15     ` Alexander Sverdlin
  1 sibling, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-28 10:15 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, linux-gpio,
	linux-kernel

Hi!

On Wed, 2021-01-27 at 13:46 +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>

I have no code out-of-the-box to test the GPIO interrupts on EP93xx,
so I just did a bootup with this patch. But the change looks fine to
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 2eea02c906e0..9c3d049e5af7 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.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi gpiochips
  2021-01-27 10:46   ` [PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi gpiochips Nikita Shubin
  2021-01-27 21:39     ` Alexander Sverdlin
@ 2021-01-28 10:17     ` Alexander Sverdlin
  1 sibling, 0 replies; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-28 10:17 UTC (permalink / raw)
  To: Nikita Shubin
  Cc: Andy Shevchenko, Linus Walleij, Bartosz Golaszewski, Maulik Shah,
	linux-gpio, linux-kernel

Hi!

On Wed, 2021-01-27 at 13:46 +0300, Nikita Shubin 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: a8173820f441 ("gpio: gpiolib: Allow GPIO IRQs to lazy
> disable")
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

Yes, it indeed fixes the warnigs mentioned above,

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

> ---
>  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 0d0435c07a5a..2eea02c906e0 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];

-- 
Alexander Sverdlin.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 0/9] gpio: ep93xx: fixes series patch
  2021-01-27 21:54   ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Linus Walleij
@ 2021-01-28 10:19     ` Alexander Sverdlin
  2021-01-28 11:57       ` Linus Walleij
  0 siblings, 1 reply; 27+ messages in thread
From: Alexander Sverdlin @ 2021-01-28 10:19 UTC (permalink / raw)
  To: Linus Walleij, Nikita Shubin
  Cc: Andy Shevchenko, Bartosz Golaszewski, open list:GPIO SUBSYSTEM,
	linux-kernel

Hello Linus,

On Wed, 2021-01-27 at 22:54 +0100, Linus Walleij wrote:
> > Series of patches to fix ep93xx gpio driver to make IRQ's working.
> > 
> > The following are fix patches (these are enough to get gpio-ep93xx
> > working with modern kernel):
> 
> I see that there is a strange level of attention to patches to this
> platform!
> 
> Since you fix all my mistakes made in converting this driver
> in the past I will just say:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> For all of them.
> 
> There are some nitpicks from the reviewers to fix up but
> overall this looks very very good.

as we don't have a dedicated EP93xx tree, would you like to take
the series in one of your trees?

-- 
Alexander Sverdlin.



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 0/9] gpio: ep93xx: fixes series patch
  2021-01-28 10:19     ` Alexander Sverdlin
@ 2021-01-28 11:57       ` Linus Walleij
  0 siblings, 0 replies; 27+ messages in thread
From: Linus Walleij @ 2021-01-28 11:57 UTC (permalink / raw)
  To: Alexander Sverdlin, Bartosz Golaszewski
  Cc: Nikita Shubin, Andy Shevchenko, open list:GPIO SUBSYSTEM, linux-kernel

On Thu, Jan 28, 2021 at 11:19 AM Alexander Sverdlin
<alexander.sverdlin@gmail.com> wrote:

> as we don't have a dedicated EP93xx tree, would you like to take
> the series in one of your trees?

Bartosz is managing the GPIO tree right now and I think he will
queue it once all reviews are finished.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2021-01-28 11:59 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-28 15:00 [PATCH] gpiolib: warning on gpiochip->to_irq defined Nikita Shubin
2021-01-04 14:15 ` Bartosz Golaszewski
2021-01-18  9:05 ` [PATCH v2] " Nikita Shubin
2021-01-19 10:51   ` Bartosz Golaszewski
2021-01-27 10:46 ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Nikita Shubin
2021-01-27 10:46   ` [PATCH v2 1/9] gpio: ep93xx: fix BUG_ON port F usage Nikita Shubin
2021-01-27 10:46   ` [PATCH v2 2/9] gpio: ep93xx: Fix single irqchip with multi gpiochips Nikita Shubin
2021-01-27 21:39     ` Alexander Sverdlin
2021-01-28 10:17     ` Alexander Sverdlin
2021-01-27 10:46   ` [PATCH v2 3/9] gpio: ep93xx: Fix wrong irq numbers in port F Nikita Shubin
2021-01-27 21:50     ` Linus Walleij
2021-01-28 10:15     ` Alexander Sverdlin
2021-01-27 10:46   ` [PATCH v2 4/9] gpio: ep93xx: drop to_irq binding Nikita Shubin
2021-01-27 12:21     ` Alexander Sverdlin
2021-01-27 10:46   ` [PATCH v2 5/9] gpio: ep93xx: Fix typo s/hierarchial/hierarchical Nikita Shubin
2021-01-27 13:20     ` Alexander Sverdlin
2021-01-27 10:46   ` [PATCH v2 6/9] gpio: ep93xx: refactor ep93xx_gpio_add_bank Nikita Shubin
2021-01-27 13:24     ` Alexander Sverdlin
2021-01-27 10:46   ` [PATCH v2 7/9] gpio: ep93xx: separate IRQ's setup Nikita Shubin
2021-01-27 20:48     ` Alexander Sverdlin
2021-01-27 10:46   ` [PATCH v2 8/9] gpio: ep93xx: refactor base IRQ number Nikita Shubin
2021-01-27 11:36     ` Alexander Sverdlin
2021-01-27 10:46   ` [PATCH v2 9/9] gpio: ep93xx: replace bools with idx for IRQ ports Nikita Shubin
2021-01-27 11:34     ` Alexander Sverdlin
2021-01-27 21:54   ` [PATCH v2 0/9] gpio: ep93xx: fixes series patch Linus Walleij
2021-01-28 10:19     ` Alexander Sverdlin
2021-01-28 11:57       ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).