pintrl: meson: add support for GPIO IRQs
Subsystems (4)
cover pintrl: meson: add support for GPIO IRQs
This patch series is partially based on a series Jerome Brunet submitted about half a year ago. Due to open questions this series never made it to mainline, see https://patchwork.kernel.org/patch/9384431/
This new attempt uses GPIOLIB_IRQCHIP resulting in less needed code. Included is also support for using two parent IRQs in case of IRQ_TYPE_EDGE_BOTH, like in the vendor driver.
Worth to be mentioned is also that I had to work around a strange issue with spurious interrupts resulting in a deadlock.
The affected critical section in __setup_irq is executed with interrupts disabled, nevertheless spurious GPIO IRQs result in a deadlock what seems to indicate that they are handled on the same CPU.
I didn't find a better explanation than a possible HW flaw so far.
The series was successfully tested on a Odroid-C2, e.g. with removing polling for SD card insertion/removal from the mmc driver.
Heiner Kallweit (5):
pinctrl: meson: add interrupts to pinctrl data
pinctrl: meson: document GPIO IRQ DT binding
pinctrl: meson: add DT node for GPIO IRQ on Meson GX
pinctrl: meson: add DT node for GPIO IRQ on Meson 8 / 8b
pinctrl: meson: add support for GPIO interrupts
.../bindings/gpio/amlogic,meson-gpio-interrupt.txt | 30 ++ arch/arm/boot/dts/meson8.dtsi | 13 + arch/arm/boot/dts/meson8b.dtsi | 13 + arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 13 + drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 22 +- drivers/pinctrl/meson/pinctrl-meson-gxl.c | 20 +- drivers/pinctrl/meson/pinctrl-meson.c | 338 ++++++++++++++++++++- drivers/pinctrl/meson/pinctrl-meson.h | 27 +- drivers/pinctrl/meson/pinctrl-meson8.c | 20 +- drivers/pinctrl/meson/pinctrl-meson8b.c | 32 +- 11 files changed, 482 insertions(+), 47 deletions(-) create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
Signature · 1 line
2.12.2
PATCH 1/5 pintrl: meson: add interrupts to pinctrl data
From: Jerome Brunet <[email protected]>
Add GPIO interrupt information to pinctrl data. Added to the original version from Jerome was data for Meson GXL.
--- drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 22 ++++++++++---------- drivers/pinctrl/meson/pinctrl-meson-gxl.c | 20 +++++++++---------- drivers/pinctrl/meson/pinctrl-meson.h | 15 +++++++++----- drivers/pinctrl/meson/pinctrl-meson8.c | 20 +++++++++---------- drivers/pinctrl/meson/pinctrl-meson8b.c | 32 ++++++++++++++++++++---------- 5 files changed, 63 insertions(+), 46 deletions(-)
drivers/pinctrl/meson/pinctrl-meson-gxbb.c +11 -11
@@ -782,20 +782,20 @@ static struct meson_pmx_func meson_gxbb_aobus_functions[] = { };static struct meson_bank meson_gxbb_periphs_banks[] = { -/* name first last pullen pull dir out in */ -BANK ("X" ,PIN (GPIOX_0 ,EE_OFF ),PIN (GPIOX_22 ,EE_OFF ),4 ,0 ,4 ,0 ,12 ,0 ,13 ,0 ,14 ,0 ), -BANK ("Y" ,PIN (GPIOY_0 ,EE_OFF ),PIN (GPIOY_16 ,EE_OFF ),1 ,0 ,1 ,0 ,3 ,0 ,4 ,0 ,5 ,0 ), -BANK ("DV" ,PIN (GPIODV_0 ,EE_OFF ),PIN (GPIODV_29 ,EE_OFF ),0 ,0 ,0 ,0 ,0 ,0 ,1 ,0 ,2 ,0 ), -BANK ("H" ,PIN (GPIOH_0 ,EE_OFF ),PIN (GPIOH_3 ,EE_OFF ),1 ,20 ,1 ,20 ,3 ,20 ,4 ,20 ,5 ,20 ), -BANK ("Z" ,PIN (GPIOZ_0 ,EE_OFF ),PIN (GPIOZ_15 ,EE_OFF ),3 ,0 ,3 ,0 ,9 ,0 ,10 ,0 ,11 ,0 ), -BANK ("CARD" ,PIN (CARD_0 ,EE_OFF ),PIN (CARD_6 ,EE_OFF ),2 ,20 ,2 ,20 ,6 ,20 ,7 ,20 ,8 ,20 ), -BANK ("BOOT" ,PIN (BOOT_0 ,EE_OFF ),PIN (BOOT_17 ,EE_OFF ),2 ,0 ,2 ,0 ,6 ,0 ,7 ,0 ,8 ,0 ), -BANK ("CLK" ,PIN (GPIOCLK_0 ,EE_OFF ),PIN (GPIOCLK_3 ,EE_OFF ),3 ,28 ,3 ,28 ,9 ,28 ,10 ,28 ,11 ,28 ), +/* name first last irq pullen pull dir out in */ +BANK ("X" ,PIN (GPIOX_0 ,EE_OFF ),PIN (GPIOX_22 ,EE_OFF ),106 ,128 ,4 ,0 ,4 ,0 ,12 ,0 ,13 ,0 ,14 ,0 ), +BANK ("Y" ,PIN (GPIOY_0 ,EE_OFF ),PIN (GPIOY_16 ,EE_OFF ),89 ,105 ,1 ,0 ,1 ,0 ,3 ,0 ,4 ,0 ,5 ,0 ), +BANK ("DV" ,PIN (GPIODV_0 ,EE_OFF ),PIN (GPIODV_29 ,EE_OFF ),59 ,88 ,0 ,0 ,0 ,0 ,0 ,0 ,1 ,0 ,2 ,0 ), +BANK ("H" ,PIN (GPIOH_0 ,EE_OFF ),PIN (GPIOH_3 ,EE_OFF ),30 ,33 ,1 ,20 ,1 ,20 ,3 ,20 ,4 ,20 ,5 ,20 ), +BANK ("Z" ,PIN (GPIOZ_0 ,EE_OFF ),PIN (GPIOZ_15 ,EE_OFF ),14 ,29 ,3 ,0 ,3 ,0 ,9 ,0 ,10 ,0 ,11 ,0 ), +BANK ("CARD" ,PIN (CARD_0 ,EE_OFF ),PIN (CARD_6 ,EE_OFF ),52 ,58 ,2 ,20 ,2 ,20 ,6 ,20 ,7 ,20 ,8 ,20 ), +BANK ("BOOT" ,PIN (BOOT_0 ,EE_OFF ),PIN (BOOT_17 ,EE_OFF ),34 ,51 ,2 ,0 ,2 ,0 ,6 ,0 ,7 ,0 ,8 ,0 ), +BANK ("CLK" ,PIN (GPIOCLK_0 ,EE_OFF ),PIN (GPIOCLK_3 ,EE_OFF ),129 ,132 ,3 ,28 ,3 ,28 ,9 ,28 ,10 ,28 ,11 ,28 ), };static struct meson_bank meson_gxbb_aobus_banks[] = { -/* name first last pullen pull dir out in */ -BANK ("AO" ,PIN (GPIOAO_0 ,0 ),PIN (GPIOAO_13 ,0 ),0 ,0 ,0 ,16 ,0 ,0 ,0 ,16 ,1 ,0 ), +/* name first last irq pullen pull dir out in */ +BANK ("AO" ,PIN (GPIOAO_0 ,0 ),PIN (GPIOAO_13 ,0 ),0 ,13 ,0 ,0 ,0 ,16 ,0 ,0 ,0 ,16 ,1 ,0 ), };struct meson_pinctrl_data meson_gxbb_periphs_pinctrl_data = {
drivers/pinctrl/meson/pinctrl-meson-gxl.c +10 -10
@@ -729,19 +729,19 @@ static struct meson_pmx_func meson_gxl_aobus_functions[] = { };static struct meson_bank meson_gxl_periphs_banks[] = { -/* name first last pullen pull dir out in */ -BANK ("X" ,PIN (GPIOX_0 ,EE_OFF ),PIN (GPIOX_18 ,EE_OFF ),4 ,0 ,4 ,0 ,12 ,0 ,13 ,0 ,14 ,0 ), -BANK ("DV" ,PIN (GPIODV_0 ,EE_OFF ),PIN (GPIODV_29 ,EE_OFF ),0 ,0 ,0 ,0 ,0 ,0 ,1 ,0 ,2 ,0 ), -BANK ("H" ,PIN (GPIOH_0 ,EE_OFF ),PIN (GPIOH_9 ,EE_OFF ),1 ,20 ,1 ,20 ,3 ,20 ,4 ,20 ,5 ,20 ), -BANK ("Z" ,PIN (GPIOZ_0 ,EE_OFF ),PIN (GPIOZ_15 ,EE_OFF ),3 ,0 ,3 ,0 ,9 ,0 ,10 ,0 ,11 ,0 ), -BANK ("CARD" ,PIN (CARD_0 ,EE_OFF ),PIN (CARD_6 ,EE_OFF ),2 ,20 ,2 ,20 ,6 ,20 ,7 ,20 ,8 ,20 ), -BANK ("BOOT" ,PIN (BOOT_0 ,EE_OFF ),PIN (BOOT_15 ,EE_OFF ),2 ,0 ,2 ,0 ,6 ,0 ,7 ,0 ,8 ,0 ), -BANK ("CLK" ,PIN (GPIOCLK_0 ,EE_OFF ),PIN (GPIOCLK_1 ,EE_OFF ),3 ,28 ,3 ,28 ,9 ,28 ,10 ,28 ,11 ,28 ), +/* name first last irq pullen pull dir out in */ +BANK ("X" ,PIN (GPIOX_0 ,EE_OFF ),PIN (GPIOX_18 ,EE_OFF ),89 ,107 ,4 ,0 ,4 ,0 ,12 ,0 ,13 ,0 ,14 ,0 ), +BANK ("DV" ,PIN (GPIODV_0 ,EE_OFF ),PIN (GPIODV_29 ,EE_OFF ),83 ,88 ,0 ,0 ,0 ,0 ,0 ,0 ,1 ,0 ,2 ,0 ), +BANK ("H" ,PIN (GPIOH_0 ,EE_OFF ),PIN (GPIOH_9 ,EE_OFF ),26 ,35 ,1 ,20 ,1 ,20 ,3 ,20 ,4 ,20 ,5 ,20 ), +BANK ("Z" ,PIN (GPIOZ_0 ,EE_OFF ),PIN (GPIOZ_15 ,EE_OFF ),10 ,25 ,3 ,0 ,3 ,0 ,9 ,0 ,10 ,0 ,11 ,0 ), +BANK ("CARD" ,PIN (CARD_0 ,EE_OFF ),PIN (CARD_6 ,EE_OFF ),52 ,58 ,2 ,20 ,2 ,20 ,6 ,20 ,7 ,20 ,8 ,20 ), +BANK ("BOOT" ,PIN (BOOT_0 ,EE_OFF ),PIN (BOOT_15 ,EE_OFF ),36 ,51 ,2 ,0 ,2 ,0 ,6 ,0 ,7 ,0 ,8 ,0 ), +BANK ("CLK" ,PIN (GPIOCLK_0 ,EE_OFF ),PIN (GPIOCLK_1 ,EE_OFF ),108 ,109 ,3 ,28 ,3 ,28 ,9 ,28 ,10 ,28 ,11 ,28 ), };static struct meson_bank meson_gxl_aobus_banks[] = { -/* name first last pullen pull dir out in */ -BANK ("AO" ,PIN (GPIOAO_0 ,0 ),PIN (GPIOAO_9 ,0 ),0 ,0 ,0 ,16 ,0 ,0 ,0 ,16 ,1 ,0 ), +/* name first last irq pullen pull dir out in */ +BANK ("AO" ,PIN (GPIOAO_0 ,0 ),PIN (GPIOAO_9 ,0 ),0 ,9 ,0 ,0 ,0 ,16 ,0 ,0 ,0 ,16 ,1 ,0 ), };struct meson_pinctrl_data meson_gxl_periphs_pinctrl_data = {
drivers/pinctrl/meson/pinctrl-meson.h +10 -5
@@ -81,6 +81,7 @@ enum meson_reg_type {* @name: bank name * @first: first pin of the bank * @last: last pin of the bank +* @irq: hwirq base number of the bank * @regs: array of register descriptors * * A bank represents a set of pins controlled by a contiguous set of
@@ -92,6 +93,8 @@ struct meson_bank {const char *name;unsigned int first;unsigned int last; +int irq_first; +int irq_last;struct meson_reg_desc regs[NUM_REG ]; };
@@ -147,12 +150,14 @@ struct meson_pinctrl { .num_groups = ARRAY_SIZE(fn ## _groups), \ } -#define BANK (n, f, l, per, peb, pr, pb, dr, db, or, ob, ir, ib) \ +#define BANK (n, f, l, fi, li, per, peb, pr, pb, dr, db, or, ob, ir, ib) \ { \ - .name = n, \ - .first = f, \ - .last = l, \ - .regs = { \ + .name = n, \ + .first = f, \ + .last = l, \ + .irq_first = fi, \ + .irq_last = li, \ + .regs = { \ [REG_PULLEN] = { per, peb }, \ [REG_PULL] = { pr, pb }, \ [REG_DIR] = { dr, db }, \
drivers/pinctrl/meson/pinctrl-meson8.c +10 -10
@@ -916,19 +916,19 @@ static struct meson_pmx_func meson8_aobus_functions[] = { };static struct meson_bank meson8_cbus_banks[] = { -/* name first last pullen pull dir out in */ -BANK ("X" ,PIN (GPIOX_0 ,0 ),PIN (GPIOX_21 ,0 ),4 ,0 ,4 ,0 ,0 ,0 ,1 ,0 ,2 ,0 ), -BANK ("Y" ,PIN (GPIOY_0 ,0 ),PIN (GPIOY_16 ,0 ),3 ,0 ,3 ,0 ,3 ,0 ,4 ,0 ,5 ,0 ), -BANK ("DV" ,PIN (GPIODV_0 ,0 ),PIN (GPIODV_29 ,0 ),0 ,0 ,0 ,0 ,7 ,0 ,8 ,0 ,9 ,0 ), -BANK ("H" ,PIN (GPIOH_0 ,0 ),PIN (GPIOH_9 ,0 ),1 ,16 ,1 ,16 ,9 ,19 ,10 ,19 ,11 ,19 ), -BANK ("Z" ,PIN (GPIOZ_0 ,0 ),PIN (GPIOZ_14 ,0 ),1 ,0 ,1 ,0 ,3 ,17 ,4 ,17 ,5 ,17 ), -BANK ("CARD" ,PIN (CARD_0 ,0 ),PIN (CARD_6 ,0 ),2 ,20 ,2 ,20 ,0 ,22 ,1 ,22 ,2 ,22 ), -BANK ("BOOT" ,PIN (BOOT_0 ,0 ),PIN (BOOT_18 ,0 ),2 ,0 ,2 ,0 ,9 ,0 ,10 ,0 ,11 ,0 ), +/* name first last irq pullen pull dir out in */ +BANK ("X" ,PIN (GPIOX_0 ,0 ),PIN (GPIOX_21 ,0 ),112 ,133 ,4 ,0 ,4 ,0 ,0 ,0 ,1 ,0 ,2 ,0 ), +BANK ("Y" ,PIN (GPIOY_0 ,0 ),PIN (GPIOY_16 ,0 ),95 ,111 ,3 ,0 ,3 ,0 ,3 ,0 ,4 ,0 ,5 ,0 ), +BANK ("DV" ,PIN (GPIODV_0 ,0 ),PIN (GPIODV_29 ,0 ),65 ,94 ,0 ,0 ,0 ,0 ,7 ,0 ,8 ,0 ,9 ,0 ), +BANK ("H" ,PIN (GPIOH_0 ,0 ),PIN (GPIOH_9 ,0 ),29 ,38 ,1 ,16 ,1 ,16 ,9 ,19 ,10 ,19 ,11 ,19 ), +BANK ("Z" ,PIN (GPIOZ_0 ,0 ),PIN (GPIOZ_14 ,0 ),14 ,28 ,1 ,0 ,1 ,0 ,3 ,17 ,4 ,17 ,5 ,17 ), +BANK ("CARD" ,PIN (CARD_0 ,0 ),PIN (CARD_6 ,0 ),58 ,64 ,2 ,20 ,2 ,20 ,0 ,22 ,1 ,22 ,2 ,22 ), +BANK ("BOOT" ,PIN (BOOT_0 ,0 ),PIN (BOOT_18 ,0 ),39 ,57 ,2 ,0 ,2 ,0 ,9 ,0 ,10 ,0 ,11 ,0 ), };static struct meson_bank meson8_aobus_banks[] = { -/* name first last pullen pull dir out in */ -BANK ("AO" ,PIN (GPIOAO_0 ,AO_OFF ),PIN (GPIO_TEST_N ,AO_OFF ),0 ,0 ,0 ,16 ,0 ,0 ,0 ,16 ,1 ,0 ), +/* name first last irq pullen pull dir out in */ +BANK ("AO" ,PIN (GPIOAO_0 ,AO_OFF ),PIN (GPIO_TEST_N ,AO_OFF ),0 ,13 ,0 ,0 ,0 ,16 ,0 ,0 ,0 ,16 ,1 ,0 ), };struct meson_pinctrl_data meson8_cbus_pinctrl_data = {
drivers/pinctrl/meson/pinctrl-meson8b.c +22 -10
@@ -124,6 +124,12 @@ static const struct pinctrl_pin_desc meson8b_aobus_pins[] = {MESON_PIN (GPIOAO_11 ,AO_OFF ),MESON_PIN (GPIOAO_12 ,AO_OFF ),MESON_PIN (GPIOAO_13 ,AO_OFF ), + +/* +* The following 2 pins are not mentionned in the public datasheet +* According to this datasheet, they can't be used with the gpio +* interrupt controller +*/ MESON_PIN (GPIO_BSD_EN ,AO_OFF ), MESON_PIN(GPIO_TEST_N, AO_OFF), };
@@ -881,19 +887,25 @@ static struct meson_pmx_func meson8b_aobus_functions[] = { };static struct meson_bank meson8b_cbus_banks[] = { -/* name first last pullen pull dir out in */ -BANK ("X" ,PIN (GPIOX_0 ,0 ),PIN (GPIOX_21 ,0 ),4 ,0 ,4 ,0 ,0 ,0 ,1 ,0 ,2 ,0 ), -BANK ("Y" ,PIN (GPIOY_0 ,0 ),PIN (GPIOY_14 ,0 ),3 ,0 ,3 ,0 ,3 ,0 ,4 ,0 ,5 ,0 ), -BANK ("DV" ,PIN (GPIODV_9 ,0 ),PIN (GPIODV_29 ,0 ),0 ,0 ,0 ,0 ,7 ,0 ,8 ,0 ,9 ,0 ), -BANK ("H" ,PIN (GPIOH_0 ,0 ),PIN (GPIOH_9 ,0 ),1 ,16 ,1 ,16 ,9 ,19 ,10 ,19 ,11 ,19 ), -BANK ("CARD" ,PIN (CARD_0 ,0 ),PIN (CARD_6 ,0 ),2 ,20 ,2 ,20 ,0 ,22 ,1 ,22 ,2 ,22 ), -BANK ("BOOT" ,PIN (BOOT_0 ,0 ),PIN (BOOT_18 ,0 ),2 ,0 ,2 ,0 ,9 ,0 ,10 ,0 ,11 ,0 ), -BANK ("DIF" ,PIN (DIF_0_P ,0 ),PIN (DIF_4_N ,0 ),5 ,8 ,5 ,8 ,12 ,12 ,13 ,12 ,14 ,12 ), +/* name first last irq pullen pull dir out in */ +BANK ("X" ,PIN (GPIOX_0 ,0 ),PIN (GPIOX_21 ,0 ),97 ,118 ,4 ,0 ,4 ,0 ,0 ,0 ,1 ,0 ,2 ,0 ), +BANK ("Y" ,PIN (GPIOY_0 ,0 ),PIN (GPIOY_14 ,0 ),80 ,96 ,3 ,0 ,3 ,0 ,3 ,0 ,4 ,0 ,5 ,0 ), +BANK ("DV" ,PIN (GPIODV_9 ,0 ),PIN (GPIODV_29 ,0 ),59 ,79 ,0 ,0 ,0 ,0 ,7 ,0 ,8 ,0 ,9 ,0 ), +BANK ("H" ,PIN (GPIOH_0 ,0 ),PIN (GPIOH_9 ,0 ),14 ,23 ,1 ,16 ,1 ,16 ,9 ,19 ,10 ,19 ,11 ,19 ), +BANK ("CARD" ,PIN (CARD_0 ,0 ),PIN (CARD_6 ,0 ),43 ,49 ,2 ,20 ,2 ,20 ,0 ,22 ,1 ,22 ,2 ,22 ), +BANK ("BOOT" ,PIN (BOOT_0 ,0 ),PIN (BOOT_18 ,0 ),24 ,42 ,2 ,0 ,2 ,0 ,9 ,0 ,10 ,0 ,11 ,0 ), + +/* +* The following bank is not mentionned in the public datasheet +* There is no information whether it can be used with the gpio +* interrupt controller +*/ +BANK ("DIF" ,PIN (DIF_0_P ,0 ),PIN (DIF_4_N ,0 ),-1 ,-1 ,5 ,8 ,5 ,8 ,12 ,12 ,13 ,12 ,14 ,12 ), };static struct meson_bank meson8b_aobus_banks[] = { -/* name first last pullen pull dir out in */ -BANK ("AO" ,PIN (GPIOAO_0 ,AO_OFF ),PIN (GPIO_TEST_N ,AO_OFF ),0 ,0 ,0 ,16 ,0 ,0 ,0 ,16 ,1 ,0 ), +/* name first last irq pullen pull dir out in */ +BANK ("AO" ,PIN (GPIOAO_0 ,AO_OFF ),PIN (GPIO_TEST_N ,AO_OFF ),0 ,13 ,0 ,0 ,0 ,16 ,0 ,0 ,0 ,16 ,1 ,0 ), };struct meson_pinctrl_data meson8b_cbus_pinctrl_data = {
Signature · 1 line
2.12.2
PATCH 1/5 pintrl: meson: add interrupts to pinctrl data
Quoted 6 lines · Heiner Kallweit
From: Jerome Brunet <[email protected]>
Add GPIO interrupt information to pinctrl data. Added to the original version from Jerome was data for Meson GXL.Signed-off-by Jerome Brunet <[email protected]>Signed-off-by Heiner Kallweit <[email protected]>
So what this does is:
Quoted 13 lines
drivers/pinctrl/meson/pinctrl-meson.h +1
-0@@ -81,6 +81,7 @@ enum meson_reg_type {* @name: bank name * @first: first pin of the bank * @last: last pin of the bank +* @irq: hwirq base number of the bank * @regs: array of register descriptors * * A bank represents a set of pins controlled by a contiguous set of
drivers/pinctrl/meson/pinctrl-meson.h struct meson_bank { · line 99
Quoted 7 lines
drivers/pinctrl/meson/pinctrl-meson.h +2
-0@@ -92,6 +93,8 @@ struct meson_bank {const char *name;unsigned int first;unsigned int last; +int irq_first; +int irq_last;struct meson_reg_desc regs[NUM_REG ]; };
... adds a per-bank parent IRQ.
I am just discussing with Thierry that I would like to see some code in the gpiolib core to deal with this mapping so we don't have to do a whole lot of custom back mapping between parent IRQs and cascaded IRQ in every driver that has a multiple-bank concept.
Please contribute to the discission, see thread subject:
"[PATCH v2] gpio: Add Tegra186 support"
Yours,
Linus Walleij
Jerome Brunet
reviewerPATCH 1/5 pintrl: meson: add interrupts to pinctrl data
Quoted 26 lines · Linus Walleij
Quoted 6 lines · Heiner Kallweit
From: Jerome Brunet <[email protected]>
Add GPIO interrupt information to pinctrl data. Added to the original version from Jerome was data for Meson GXL.Signed-off-by Jerome Brunet <[email protected]>Signed-off-by Heiner Kallweit <[email protected]>So what this does is:
Quoted 15 lines
drivers/pinctrl/meson/pinctrl-meson.h +0
-0@@ -81,6 +81,7 @@ enum meson_reg_type {? * @name:??????bank name
? * @first:?????first pin of the bank
? * @last:??????last pin of the bank
+ * @irq:???????hwirq base number of the bank
? * @regs:??????array of register descriptors
? *
? * A bank represents a set of pins controlled by a contiguous set ofdrivers/pinctrl/meson/pinctrl-meson.h +0
-0@@ -92,6 +93,8 @@ struct meson_bank { ????????const char *name;
Quoted 8 lines
drivers/pinctrl/meson/pinctrl-meson.h struct meson_bank { · line 99
Quoted 6 lines
drivers/pinctrl/meson/pinctrl-meson.h +2
-0????????unsigned int first; ????????unsigned int last; +???????int irq_first; +???????int irq_last; ????????struct meson_reg_desc regs[NUM_REG]; ?};... adds a per-bank parent IRQ.
Hi Linus,
It does not add a per-bank parent IRQ. The meson gpio irq is an IP independent of the gpio/pinctrl subsystem. In a nutshell, we have 8 interrupts on the GIC and we can route the signal of almost any pin of any bank to these parent irqs. The fact that there 1 gpio-irq controller and several gpio controller make the design a bit tricky
We already talked about this IP, here is the discussion :https://patchwork.ozlab s.org/patch/684208/
At the time, the problem was that I was creating the mapping within the gpio_to_irq callback, which is wrong.
irq_first, irq_last were introduced because there is no way to have direct mapping between the gpio number and the hw interrupt number. (details are in our previous discussion). If a more generic solution can be used for this, it would be nice :)
While I think having a irqdomain in the gpio driver and using the request_resources callback to create the mapping in the underlying domain might be a solution to ressouce allocation problem we had, I think the meson-gpio-irq should be implemented has an independent driver because it is an independent device. Eventually, this device should be the parent irq controller of the gpio controller.
I not sure that there is a reason for using syscon here, or if it is a good idea to have the data of this controller as globals for the gpio driver ...
Quoted 11 lines
I am just discussing with Thierry that I would like to see some code in the gpiolib core to deal with this mapping so we don't have to do a whole lot of custom back mapping between parent IRQs and cascaded IRQ in every driver that has a multiple-bank concept.
Please contribute to the discission, see thread subject:
"[PATCH v2] gpio: Add Tegra186 support"Yours,
Linus Walleij
PATCH 1/5 pintrl: meson: add interrupts to pinctrl data
Am 11.05.2017 um 18:08 schrieb Jerome Brunet:
Quoted 55 lines · Linus Walleij
Quoted 26 lines · Heiner Kallweit
Quoted 7 lines
From: Jerome Brunet <[email protected]>
Add GPIO interrupt information to pinctrl data. Added to the original version from Jerome was data for Meson GXL.Signed-off-by Jerome Brunet <[email protected]>Signed-off-by Heiner Kallweit <[email protected]>So what this does is:
Quoted 15 lines
drivers/pinctrl/meson/pinctrl-meson.h +1
-0@@ -81,6 +81,7 @@ enum meson_reg_type {* @name: bank name * @first: first pin of the bank * @last: last pin of the bank +* @irq: hwirq base number of the bank * @regs: array of register descriptors * * A bank represents a set of pins controlled by a contiguous set of @@ -92,6 +93,8 @@ struct meson_bank {const char *name;Quoted 8 lines
drivers/pinctrl/meson/pinctrl-meson.h struct meson_bank { · line 99
Quoted 6 lines
drivers/pinctrl/meson/pinctrl-meson.h +2
-0unsigned int first;unsigned int last; +int irq_first; +int irq_last;struct meson_reg_desc regs[NUM_REG ]; };... adds a per-bank parent IRQ.
Hi Linus,
It does not add a per-bank parent IRQ. The meson gpio irq is an IP independent of the gpio/pinctrl subsystem. In a nutshell, we have 8 interrupts on the GIC and we can route the signal of almost any pin of any bank to these parent irqs. The fact that there 1 gpio-irq controller and several gpio controller make the design a bit tricky
We already talked about this IP, here is the discussion :https://patchwork.ozlab s.org/patch/684208/
At the time, the problem was that I was creating the mapping within the
gpio_to_irqcallback, which is wrong.
irq_first,irq_lastwere introduced because there is no way to have direct mapping between the gpio number and the hw interrupt number. (details are in our previous discussion). If a more generic solution can be used for this, it would be nice :)
Quoted 5 lines
While I think having a irqdomain in the gpio driver and using the
request_resourcescallback to create the mapping in the underlying domain might be a solution to ressouce allocation problem we had, I think the meson-gpio-irq should be implemented has an independent driver because it is an independent device.
This kind of GPIO IRQ multiplexer doesn't really seem to match any use case covered by the GPIO / IRQ subsystems. Especially the needed dynamic mapping from the GPIOs to one of the eight GPIO GIC IRQs is tricky.
After Jerome's attempt from last year (using hierarchical IRQ domains) didn't make it due to concerns of the maintainers, I decided to go another way. I'm not stating at all that this is the right one ..
To re-use existing stuff as far as possible I go with GPIOLIB_IRQCHIP. So far I use one irqchip per gpiochip (we have two of them). I'll check whether I can use just one irqchip for both gpiochips.
Quoted 2 lines
Eventually, this device should be the parent irq controller of the gpio controller.
I think this is one of the central questions here, whether this device can be considered an irq controller.
It just manages routing of IRQs, so I'd tend to say that the gpio controller is the irq controller with the GIC as parent.
Quoted 17 lines
I not sure that there is a reason for using syscon here, or if it is a good idea to have the data of this controller as globals for the gpio driver ...
Quoted 11 lines
I am just discussing with Thierry that I would like to see some code in the gpiolib core to deal with this mapping so we don't have to do a whole lot of custom back mapping between parent IRQs and cascaded IRQ in every driver that has a multiple-bank concept.
Please contribute to the discission, see thread subject:
"[PATCH v2] gpio: Add Tegra186 support"Yours,
Linus Walleij
PATCH 1/5 pintrl: meson: add interrupts to pinctrl data
Am 11.05.2017 um 21:33 schrieb Heiner Kallweit:
Quoted 74 lines
Am 11.05.2017 um 18:08 schrieb Jerome Brunet:
Quoted 55 lines · Linus Walleij
Quoted 26 lines · Heiner Kallweit
Quoted 7 lines
From: Jerome Brunet <[email protected]>
Add GPIO interrupt information to pinctrl data. Added to the original version from Jerome was data for Meson GXL.Signed-off-by Jerome Brunet <[email protected]>Signed-off-by Heiner Kallweit <[email protected]>So what this does is:
Quoted 15 lines
drivers/pinctrl/meson/pinctrl-meson.h +1
-0@@ -81,6 +81,7 @@ enum meson_reg_type {* @name: bank name * @first: first pin of the bank * @last: last pin of the bank +* @irq: hwirq base number of the bank * @regs: array of register descriptors * * A bank represents a set of pins controlled by a contiguous set of @@ -92,6 +93,8 @@ struct meson_bank {const char *name;Quoted 8 lines
drivers/pinctrl/meson/pinctrl-meson.h struct meson_bank { · line 99
Quoted 6 lines
drivers/pinctrl/meson/pinctrl-meson.h +2
-0unsigned int first;unsigned int last; +int irq_first; +int irq_last;struct meson_reg_desc regs[NUM_REG ]; };... adds a per-bank parent IRQ.
Hi Linus,
It does not add a per-bank parent IRQ. The meson gpio irq is an IP independent of the gpio/pinctrl subsystem. In a nutshell, we have 8 interrupts on the GIC and we can route the signal of almost any pin of any bank to these parent irqs. The fact that there 1 gpio-irq controller and several gpio controller make the design a bit tricky
We already talked about this IP, here is the discussion :https://patchwork.ozlab s.org/patch/684208/
At the time, the problem was that I was creating the mapping within the
gpio_to_irqcallback, which is wrong.
irq_first,irq_lastwere introduced because there is no way to have direct mapping between the gpio number and the hw interrupt number. (details are in our previous discussion). If a more generic solution can be used for this, it would be nice :)Quoted 5 lines
While I think having a irqdomain in the gpio driver and using the
request_resourcescallback to create the mapping in the underlying domain might be a solution to ressouce allocation problem we had, I think the meson-gpio-irq should be implemented has an independent driver because it is an independent device.This kind of GPIO IRQ multiplexer doesn't really seem to match any use case covered by the GPIO / IRQ subsystems. Especially the needed dynamic mapping from the GPIOs to one of the eight GPIO GIC IRQs is tricky.
After Jerome's attempt from last year (using hierarchical IRQ domains) didn't make it due to concerns of the maintainers, I decided to go another way. I'm not stating at all that this is the right one ..
To re-use existing stuff as far as possible I go with
GPIOLIB_IRQCHIP. So far I use one irqchip per gpiochip (we have two of them). I'll check whether I can use just one irqchip for both gpiochips.
Quoted 8 lines
Quoted 2 lines
Eventually, this device should be the parent irq controller of the gpio controller.
I think this is one of the central questions here, whether this device can be considered an irq controller.
It just manages routing of IRQs, so I'd tend to say that the gpio controller is the irq controller with the GIC as parent.
Following Jerome's suggestion I prepared a version making the irq multiplexer an own device. Nice here is that with this approach basically only one line of code needs to be changed in the existing pinctrl driver.
As discussion basis I add it here w/o submitting a v2 of patch 5.
--- drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/meson/Makefile | 2 +- drivers/pinctrl/meson/pinctrl-meson-irq.c | 393 ++++++++++++++++++++++++++++++ drivers/pinctrl/meson/pinctrl-meson.c | 8 +- drivers/pinctrl/meson/pinctrl-meson.h | 1 + 5 files changed, 403 insertions(+), 2 deletions(-) create mode 100644 drivers/pinctrl/meson/pinctrl-meson-irq.c
drivers/pinctrl/Kconfig +1 -0
@@ -153,6 +153,7 @@ config PINCTRL_MESON select PINCONF select GENERIC_PINCONF select GPIOLIB + select GPIOLIB_IRQCHIP select OF_GPIO select REGMAP_MMIO
drivers/pinctrl/meson/Makefile +1 -1
@@ -1,3 +1,3 @@ obj-y += pinctrl-meson8.o pinctrl-meson8b.o obj-y += pinctrl-meson-gxbb.o pinctrl-meson-gxl.o -obj-y += pinctrl-meson.o +obj-y += pinctrl-meson.o pinctrl-meson-irq.o
drivers/pinctrl/meson/pinctrl-meson-irq.c new file +393 -0
393 lines not on this page · raw message
drivers/pinctrl/meson/pinctrl-meson.c +7 -1
@@ -62,6 +62,8 @@#include "../pinctrl-utils.h" #include "pinctrl-meson.h" +struct irq_chip *meson_pinctrl_irq_chip; +/** * meson_get_bank() - find the bank containing a given pin *
@@ -558,7 +560,8 @@ static int meson_gpiolib_register(struct meson_pinctrl *pc)return ret; } -return 0 ; +return gpiochip_irqchip_add (&pc->chip, meson_pinctrl_irq_chip,0 , + handle_simple_irq,IRQ_TYPE_NONE ); }static struct regmap_config meson_regmap_config = {
@@ -647,6 +650,9 @@ static int meson_pinctrl_probe(struct platform_device *pdev)struct meson_pinctrl *pc;int ret; +if (!meson_pinctrl_irq_chip) +return -EPROBE_DEFER ; + pc =devm_kzalloc (dev,sizeof (struct meson_pinctrl ),GFP_KERNEL );if (!pc)return -ENOMEM ;
drivers/pinctrl/meson/pinctrl-meson.h +1 -0
@@ -176,3 +176,4 @@ extern struct meson_pinctrl_data meson_gxbb_periphs_pinctrl_data;extern struct meson_pinctrl_data meson_gxbb_aobus_pinctrl_data;extern struct meson_pinctrl_data meson_gxl_periphs_pinctrl_data;extern struct meson_pinctrl_data meson_gxl_aobus_pinctrl_data; +extern struct irq_chip *meson_pinctrl_irq_chip;
Signature · 1 line
2.12.2
PATCH 1/5 pintrl: meson: add interrupts to pinctrl data
Quoted 5 lines · Heiner Kallweit
Following Jerome's suggestion I prepared a version making the irq multiplexer an own device. Nice here is that with this approach basically only one line of code needs to be changed in the existing pinctrl driver.
As discussion basis I add it here w/o submitting a v2 of patch 5.
I think this is the right way to go, but would ideally like some feedback from the irqchip maintainers.
Have you looked at drivers/irqchip/irq-crossbar.c for inspiration? Also read Documentation/devicetree/bindings/arm/omap/crossbar.txt
It uses an hierarchical domain for a similar cross-connection usecase.
Under any circumstances I think decoupling the IRQ multiplexer and dealing with it in separate is easier for maintainance and also for the irq maintainers to review.
Yours,
Linus Walleij
PATCH 1/5 pintrl: meson: add interrupts to pinctrl data
Am 12.05.2017 um 11:51 schrieb Linus Walleij:
Quoted 11 lines · Heiner Kallweit
Quoted 6 lines
Following Jerome's suggestion I prepared a version making the irq multiplexer an own device. Nice here is that with this approach basically only one line of code needs to be changed in the existing pinctrl driver.
As discussion basis I add it here w/o submitting a v2 of patch 5.
I think this is the right way to go, but would ideally like some feedback from the irqchip maintainers.
Quoted 7 lines
Have you looked at
drivers/irqchip/irq-crossbar.cfor inspiration? Also readDocumentation/devicetree/bindings/arm/omap/crossbar.txtIt uses an hierarchical domain for a similar cross-connection usecase.
When looking at the crassbar driver it seems to be quite similar to Jerome's Meson GPIO IRQ driver version. Therefore I think we'd face the same challenges. What is special about the Meson GPIO IRQ hw:
- We have only 8 parent IRQs for 100+ GPIO's.
- If a GPIO requests IRQ_TYPE_EDGE_BOTH (concrete example: mmc sd detection) then we have to use two parent IRQs, one for each edge.
- We must be able to dynamically map a GPIO to a parent IRQ, e.g. if a driver uses gpiod_to_irq().
These specifics are the reason why I do the mapping magic in the irq_set_type callback of the irq_chip. Only in this callback I have all the info needed to do the mapping.
Quoted 4 lines
Under any circumstances I think decoupling the IRQ multiplexer and dealing with it in separate is easier for maintainance and also for the irq maintainers to review.
Then I'll submit the latest version with the separated device as v2 and we'll see which further review comments and suggestions for alternative approaches come in.
Rgds, Heiner
Quoted 3 lines
Yours,
Linus Walleij
PATCH 2/5 pintrl: meson: document GPIO IRQ DT binding
In reply to Heiner KallweitDocument the DT binding for GPIO IRQ support on Amlogic Meson SoC's.
This documentation is intentionally not placed under interrupt-controllers as GPIO IRQ support on these SoC's acts more like an interrupt multiplexer.
--- .../bindings/gpio/amlogic,meson-gpio-interrupt.txt | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +30 -0
@@ -0,0 +1,30 @@ +Amlogic meson GPIO interrupt controller + +Meson SoCs contains an interrupt controller which is able watch the SoC pads +and generate an interrupt on edges or level. The controller is essentially a +256 pads to 8 GIC interrupt multiplexer, with a filter block to select edge +or level and polarity. We don't expose all 256 mux inputs because the +documentation shows that upper part is not mapped to any pad. The actual number +of interrupt exposed depends on the SoC. + +Required properties: + +- compatible : should be "amlogic,meson-gpio-interrupt", "syscon". +- reg : Specifies base physical address and size of the registers. +- interrupts : list of GIC interrupts which can be used with the + GPIO IRQ multiplexer + +Example: + +gpio_irq at 9880 { + compatible = "amlogic,meson-gpio-interrupt", "syscon"; + reg = <0x0 0x09880 0x0 0x10>; + interrupts = <GIC_SPI 64 IRQ_TYPE_NONE>, + <GIC_SPI 65 IRQ_TYPE_NONE>, + <GIC_SPI 66 IRQ_TYPE_NONE>, + <GIC_SPI 67 IRQ_TYPE_NONE>, + <GIC_SPI 68 IRQ_TYPE_NONE>, + <GIC_SPI 69 IRQ_TYPE_NONE>, + <GIC_SPI 70 IRQ_TYPE_NONE>, + <GIC_SPI 71 IRQ_TYPE_NONE>; + };
Signature · 1 line
2.12.2
Rob Herring
maintainerPATCH 2/5 pintrl: meson: document GPIO IRQ DT binding
Quoted 9 lines · Heiner Kallweit
Document the DT binding for GPIO IRQ support on Amlogic Meson SoC's.
This documentation is intentionally not placed under interrupt-controllers as GPIO IRQ support on these SoC's acts more like an interrupt multiplexer.
Signed-off-by Heiner Kallweit <[email protected]>--- .../bindings/gpio/amlogic,meson-gpio-interrupt.txt | 30 ++++++++++++++++++++++
Seems more like an irqchip?
Quoted 11 lines
1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txtDocumentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +2
-0@@ -0,0 +1,30 @@ +Amlogic meson GPIO interrupt controller +
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt line 12
Quoted 10 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +10
-0+Meson SoCs contains an interrupt controller which is able watch the SoC pads +and generate an interrupt on edges or level. The controller is essentially a +256 pads to 8 GIC interrupt multiplexer, with a filter block to select edge +or level and polarity. We don't expose all 256 mux inputs because the +documentation shows that upper part is not mapped to any pad. The actual number +of interrupt exposed depends on the SoC. + +Required properties: + +- compatible : should be "amlogic,meson-gpio-interrupt", "syscon".
Why syscon?
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt line 15
Quoted 3 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +3
-0+- reg : Specifies base physical address and size of the registers. +- interrupts : list of GIC interrupts which can be used with the + GPIO IRQ multiplexer
What about interrupt-controller property?
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt line 19
Quoted 4 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +4
-0+ +Example: + +gpio_irq at 9880 {
interrupt-controller at ...
Quoted 19 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +11
-0+ compatible = "amlogic,meson-gpio-interrupt", "syscon"; + reg = <0x0 0x09880 0x0 0x10>; + interrupts = <GIC_SPI 64 IRQ_TYPE_NONE>, + <GIC_SPI 65 IRQ_TYPE_NONE>, + <GIC_SPI 66 IRQ_TYPE_NONE>, + <GIC_SPI 67 IRQ_TYPE_NONE>, + <GIC_SPI 68 IRQ_TYPE_NONE>, + <GIC_SPI 69 IRQ_TYPE_NONE>, + <GIC_SPI 70 IRQ_TYPE_NONE>, + <GIC_SPI 71 IRQ_TYPE_NONE>; + };Signature · 7 lines
2.12.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 2/5 pintrl: meson: document GPIO IRQ DT binding
Am 12.05.2017 um 21:38 schrieb Rob Herring:
Quoted 7 lines · Heiner Kallweit
Quoted 6 lines
Document the DT binding for GPIO IRQ support on Amlogic Meson SoC's.
This documentation is intentionally not placed under interrupt-controllers as GPIO IRQ support on these SoC's acts more like an interrupt multiplexer.
Quoted 6 lines
Quoted 3 lines
Signed-off-by Heiner Kallweit <[email protected]>--- .../bindings/gpio/amlogic,meson-gpio-interrupt.txt | 30 ++++++++++++++++++++++Seems more like an irqchip?
It is an irq_chip. Would you therefore prefer a different name or another location for this binding documentation?
Quoted 15 lines
Quoted 15 lines
1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txtDocumentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +6
-0@@ -0,0 +1,30 @@ +Amlogic meson GPIO interrupt controller + +Meson SoCs contains an interrupt controller which is able watch the SoC pads +and generate an interrupt on edges or level. The controller is essentially a +256 pads to 8 GIC interrupt multiplexer, with a filter block to select edge +or level and polarity. We don't expose all 256 mux inputs because the
Quoted 9 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt line 12
Quoted 6 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +6
-0+documentation shows that upper part is not mapped to any pad. The actual number +of interrupt exposed depends on the SoC. + +Required properties: + +- compatible : should be "amlogic,meson-gpio-interrupt", "syscon".Why syscon?
Has been removed already in v2 of the patch.
Quoted 6 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt line 15
Quoted 3 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +3
-0+- reg : Specifies base physical address and size of the registers. +- interrupts : list of GIC interrupts which can be used with the + GPIO IRQ multiplexerWhat about interrupt-controller property?
This HW is somewhat special, it's more or less a multiplexer between 100+ GPIO's and 8 GIC IRQ's. And we have two GPIO domains, both having a "gpio-controller".
It doesn't seem that the "interrupt-controller" property is right here. I'd tend to say it should be added to the "gpio-controller" nodes.
Quoted 7 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt line 19
Quoted 4 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +4
-0+ +Example: + +gpio_irq at 9880 {interrupt-controller at ...
This relates to previous comment, as it isn't really an interrupt controller I'm not sure we should use this name.
Quoted 20 lines
Quoted 19 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +11
-0+ compatible = "amlogic,meson-gpio-interrupt", "syscon"; + reg = <0x0 0x09880 0x0 0x10>; + interrupts = <GIC_SPI 64 IRQ_TYPE_NONE>, + <GIC_SPI 65 IRQ_TYPE_NONE>, + <GIC_SPI 66 IRQ_TYPE_NONE>, + <GIC_SPI 67 IRQ_TYPE_NONE>, + <GIC_SPI 68 IRQ_TYPE_NONE>, + <GIC_SPI 69 IRQ_TYPE_NONE>, + <GIC_SPI 70 IRQ_TYPE_NONE>, + <GIC_SPI 71 IRQ_TYPE_NONE>; + };Signature · 7 lines
2.12.2 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Rob Herring
maintainerPATCH 2/5 pintrl: meson: document GPIO IRQ DT binding
Quoted 8 lines · Heiner Kallweit
Am 12.05.2017 um 21:38 schrieb Rob Herring:
Quoted 7 lines · Heiner Kallweit
Quoted 6 lines
Document the DT binding for GPIO IRQ support on Amlogic Meson SoC's.
This documentation is intentionally not placed under interrupt-controllers as GPIO IRQ support on these SoC's acts more like an interrupt multiplexer.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Signed-off-by Heiner Kallweit <[email protected]>--- .../bindings/gpio/amlogic,meson-gpio-interrupt.txt | 30 ++++++++++++++++++++++Seems more like an irqchip?
It is an
irq_chip. Would you therefore prefer a different name or another location for this binding documentation?
bindings/interrupt-controller
Quoted 31 lines
Quoted 16 lines
Quoted 16 lines
1 file changed, 30 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txtDocumentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +6
-0@@ -0,0 +1,30 @@ +Amlogic meson GPIO interrupt controller + +Meson SoCs contains an interrupt controller which is able watch the SoC pads +and generate an interrupt on edges or level. The controller is essentially a +256 pads to 8 GIC interrupt multiplexer, with a filter block to select edge +or level and polarity. We don't expose all 256 mux inputs because theQuoted 9 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt line 12
Quoted 6 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +6
-0+documentation shows that upper part is not mapped to any pad. The actual number +of interrupt exposed depends on the SoC. + +Required properties: + +- compatible : should be "amlogic,meson-gpio-interrupt", "syscon".Why syscon?
Has been removed already in v2 of the patch.
Quoted 4 lines
Quoted 3 lines
Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt new file +3
-0+- reg : Specifies base physical address and size of the registers. +- interrupts : list of GIC interrupts which can be used with the + GPIO IRQ multiplexer
Quoted 7 lines
Quoted 2 lines
What about interrupt-controller property?
This HW is somewhat special, it's more or less a multiplexer between 100+ GPIO's and 8 GIC IRQ's. And we have two GPIO domains, both having a "gpio-controller".
It doesn't seem that the "interrupt-controller" property is right here. I'd tend to say it should be added to the "gpio-controller" nodes.
I would say an interrupt controller is anything that controls routing/delivery of interrupts from source to cpu. This certainly meets that criteria.
You could even use interrupt-map property to statically assign the sources to parent interrupts.
Rob
PATCH 3/5 pintrl: meson: add DT node for GPIO IRQ on Meson GX
In reply to Heiner KallweitAdd the DT node for GPIO IRQ support on AMlogic Meson GX.
--- arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 13 +++++++++++++ 1 file changed, 13 insertions(+)
arch/arm64/boot/dts/amlogic/meson-gx.dtsi +13 -0
@@ -312,6 +312,19 @@ status = "disabled"; }; + gpio_irq at 9880 { + compatible = "amlogic,meson-gpio-interrupt", "syscon"; + reg = <0x0 0x09880 0x0 0x10>; + interrupts = <GIC_SPI 64 IRQ_TYPE_NONE>, + <GIC_SPI 65 IRQ_TYPE_NONE>, + <GIC_SPI 66 IRQ_TYPE_NONE>, + <GIC_SPI 67 IRQ_TYPE_NONE>, + <GIC_SPI 68 IRQ_TYPE_NONE>, + <GIC_SPI 69 IRQ_TYPE_NONE>, + <GIC_SPI 70 IRQ_TYPE_NONE>, + <GIC_SPI 71 IRQ_TYPE_NONE>; + }; + watchdog at 98d0 { compatible = "amlogic,meson-gx-wdt", "amlogic,meson-gxbb-wdt"; reg = <0x0 0x098d0 0x0 0x10>;
Signature · 1 line
2.12.2
PATCH 4/5 pintrl: meson: add DT node for GPIO IRQ on Meson 8 / 8b
In reply to Heiner KallweitAdd the DT node for GPIO IRQ support on AMlogic Meson 8 and 8b.
--- arch/arm/boot/dts/meson8.dtsi | 13 +++++++++++++ arch/arm/boot/dts/meson8b.dtsi | 13 +++++++++++++ 2 files changed, 26 insertions(+)
arch/arm/boot/dts/meson8.dtsi +13 -0
@@ -91,6 +91,19 @@ clock-frequency = <141666666>; }; + gpio_irq at c1109880 { + compatible = "amlogic,meson-gpio-interrupt", "syscon"; + reg = <0xc1109880 0x10>; + interrupts = <GIC_SPI 64 IRQ_TYPE_NONE>, + <GIC_SPI 65 IRQ_TYPE_NONE>, + <GIC_SPI 66 IRQ_TYPE_NONE>, + <GIC_SPI 67 IRQ_TYPE_NONE>, + <GIC_SPI 68 IRQ_TYPE_NONE>, + <GIC_SPI 69 IRQ_TYPE_NONE>, + <GIC_SPI 70 IRQ_TYPE_NONE>, + <GIC_SPI 71 IRQ_TYPE_NONE>; + }; + pinctrl_cbus: pinctrl at c1109880 { compatible = "amlogic,meson8-cbus-pinctrl"; reg = <0xc1109880 0x10>;
arch/arm/boot/dts/meson8b.dtsi +13 -0
@@ -183,6 +183,19 @@ status = "disabled"; }; + gpio_irq at c1109880 { + compatible = "amlogic,meson-gpio-interrupt", "syscon"; + reg = <0xc1109880 0x10>; + interrupts = <GIC_SPI 64 IRQ_TYPE_NONE>, + <GIC_SPI 65 IRQ_TYPE_NONE>, + <GIC_SPI 66 IRQ_TYPE_NONE>, + <GIC_SPI 67 IRQ_TYPE_NONE>, + <GIC_SPI 68 IRQ_TYPE_NONE>, + <GIC_SPI 69 IRQ_TYPE_NONE>, + <GIC_SPI 70 IRQ_TYPE_NONE>, + <GIC_SPI 71 IRQ_TYPE_NONE>; + }; + pinctrl_cbus: pinctrl at c1109880 { compatible = "amlogic,meson8b-cbus-pinctrl"; reg = <0xc1109880 0x10>;
Signature · 1 line
2.12.2
PATCH 5/5 pintrl: meson: add support for GPIO interrupts
In reply to Heiner KallweitAdd support for GPIO interrupts on Amlogic Meson SoC's.
There's a limit of 8 parent interupts which can be used in total. Note that IRQ_TYPE_EDGE_BOTH interrupts reserve two parent IRQ's, one for each edge.
--- drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/meson/pinctrl-meson.c | 338 +++++++++++++++++++++++++++++++++- drivers/pinctrl/meson/pinctrl-meson.h | 12 ++ 3 files changed, 350 insertions(+), 1 deletion(-)
drivers/pinctrl/Kconfig +1 -0
@@ -153,6 +153,7 @@ config PINCTRL_MESON select PINCONF select GENERIC_PINCONF select GPIOLIB + select GPIOLIB_IRQCHIP select OF_GPIO select REGMAP_MMIO
drivers/pinctrl/meson/pinctrl-meson.c +337 -1
388 lines not on this page · raw message
drivers/pinctrl/meson/pinctrl-meson.h +12 -0
@@ -17,6 +17,17 @@#include <linux/types.h> /** +* struct meson_gpio_irq_slot - a gpio irq slot descriptor +* +* @irq: virq of the parent irq +* @owner: owning gpio hwirq +*/ +struct meson_gpio_irq_slot { +int irq; +int owner; +}; + +/** * struct meson_pmx_group - a pinmux group * * @name: group name
@@ -121,6 +132,7 @@ struct meson_pinctrl {struct regmap *reg_pull;struct regmap *reg_gpio;struct gpio_chip chip; +struct irq_chip irq_chip;struct device_node *of_node; };
Signature · 1 line
2.12.2
PATCH 5/5 pintrl: meson: add support for GPIO interrupts
Quoted 7 lines · Heiner Kallweit
Add support for GPIO interrupts on Amlogic Meson SoC's.
There's a limit of 8 parent interupts which can be used in total. Note that
IRQ_TYPE_EDGE_BOTHinterrupts reserve two parent IRQ's, one for each edge.Signed-off-by Heiner Kallweit <[email protected]>
(..)
Quoted 2 lines
select GPIOLIB
+ selectGPIOLIB_IRQCHIP
This is nice.
Quoted 6 lines
drivers/pinctrl/meson/pinctrl-meson.c +6
-0+static int meson_gpio_to_hwirq (struct meson_bank *bank,unsigned int offset) +{ +int hwirq; + +if (bank->irq_first <0 ) +/* this bank cannot generate irqs */
drivers/pinctrl/meson/pinctrl-meson.c static const struct of_device_id meson_pinctrl_dt_match[] = { · line 580
Quoted 10 lines
drivers/pinctrl/meson/pinctrl-meson.c +10
-0+return -EINVAL ; + + hwirq = offset - bank->first + bank->irq_first; + +if (hwirq > bank->irq_last) +/* this pin cannot generate irqs */ +return -EINVAL ; + +return hwirq; +}
So this is the kind of stuff I want the gpiolib core to handle. We cant just proliferate
hundreds of drivers to bank-to-irq mappings of cascaded IRQs.
Quoted 13 lines
drivers/pinctrl/meson/pinctrl-meson.c +13
-0+static int meson_gpio_alloc_irq_slot (struct irq_data *data,int num_slots, +int *slots) +{ +int i, cnt =0 ; + +mutex_lock (&meson_gpio_irq_slot_mutex); + +for (i =0 ; i < meson_gpio_num_irq_slots; i++) +if (!meson_gpio_irq_slots[i].owner) { + meson_gpio_irq_slots[i].owner = data->irq; + slots[cnt++] = i; +if (cnt == num_slots) +break ;
drivers/pinctrl/meson/pinctrl-meson.c static const struct of_device_id meson_pinctrl_dt_match[] = { · line 658
Quoted 10 lines
drivers/pinctrl/meson/pinctrl-meson.c +10
-0+ } + +if (cnt < num_slots) +for (i =0 ; i < cnt; i++) + meson_gpio_irq_slots[i].owner =0 ; + +mutex_unlock (&meson_gpio_irq_slot_mutex); + +return cnt == num_slots ?0 : -ENOSPC ; +}
I don't understand what is happening here but it looks like a hierarchical IRQ domain.
Quoted 6 lines
drivers/pinctrl/meson/pinctrl-meson.c +6
-0+static int meson_gpio_irq_request_resources (struct irq_data *data) +{ +struct gpio_chip *chip =irq_data_get_irq_chip_data (data); +unsigned gpio =irqd_to_hwirq (data); + +return gpiochip_lock_as_irq (chip, gpio);
drivers/pinctrl/meson/pinctrl-meson.c static const struct of_device_id meson_pinctrl_dt_match[] = { · line 705
Quoted 10 lines
drivers/pinctrl/meson/pinctrl-meson.c +10
-0+} + +static void meson_gpio_irq_release_resources (struct irq_data *data) +{ +struct gpio_chip *chip =irq_data_get_irq_chip_data (data); +unsigned gpio =irqd_to_hwirq (data); + +meson_gpio_free_irq_slot (data); +gpiochip_unlock_as_irq (chip, gpio); +}
Nice that you implemented this though.
drivers/pinctrl/meson/pinctrl-meson.c static const struct of_device_id meson_pinctrl_dt_match[] = { · line 714
Quoted 8 lines
drivers/pinctrl/meson/pinctrl-meson.c +8
-0+static void meson_gpio_set_hwirq (int idx,int hwirq) +{ +int reg = idx >3 ?REG_PIN_47_SEL :REG_PIN_03_SEL ; +int shift =8 * (idx %4 ); + +regmap_update_bits (meson_gpio_irq_regmap, reg,0xff << shift, + hwirq << shift); +}
So you program the hardware to route the IRQ, OK.
Quoted 2 lines
(quoted diff) +2
-0+static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type) +{
(...)
drivers/pinctrl/meson/pinctrl-meson.c static const struct of_device_id meson_pinctrl_dt_match[] = { · line 788
Quoted 9 lines
drivers/pinctrl/meson/pinctrl-meson.c +9
-0+for (i =0 ; i < num_slots; i++) { + irq = meson_gpio_irq_slots[slots[i]].irq; + ret =irq_set_irq_type (irq, val); +if (ret) +break ; + ret =request_irq (irq, meson_gpio_irq_handler,0 , +"GPIO parent" , data); +if (ret) +break ;
Issueing request_irq() inside .set_type() is definately wrong.
You want to do chained or nested IRQs for this, possible hierarchical as well.
drivers/pinctrl/meson/pinctrl-meson.c static int meson_gpiolib_register(struct meson_pinctrl *pc) · line 853
Quoted 2 lines
drivers/pinctrl/meson/pinctrl-meson.c +2
-0+return gpiochip_irqchip_add (&pc->chip, &pc->irq_chip,0 , + handle_simple_irq,IRQ_TYPE_NONE );
gpiochip_irqchip_add() should be followed by gpiochip_set_chained_irqchip() or gpiochip_set_nested_irqchip().
Else we are not providing the helpers you need.
Which I guess is the case.
I'm a bit confused, I think we need a better understanding of how the hardware works.
Is it a chained interrupt handler where you can select a number of GPIOs in each bank to cascade IRQs off?
Yours,
Linus Walleij
cover pintrl: meson: add support for GPIO IRQs
In reply to Heiner KallweitQuoted 18 lines · Heiner Kallweit
This patch series is partially based on a series Jerome Brunet submitted about half a year ago. Due to open questions this series never made it to mainline, see https://patchwork.kernel.org/patch/9384431/
This new attempt uses
GPIOLIB_IRQCHIPresulting in less needed code. Included is also support for using two parent IRQs in case ofIRQ_TYPE_EDGE_BOTH, like in the vendor driver.Worth to be mentioned is also that I had to work around a strange issue with spurious interrupts resulting in a deadlock.
The affected critical section in__setup_irqis executed with interrupts disabled, nevertheless spurious GPIO IRQs result in a deadlock what seems to indicate that they are handled on the same CPU.
I didn't find a better explanation than a possible HW flaw so far.The series was successfully tested on a Odroid-C2, e.g. with removing polling for SD card insertion/removal from the mmc driver.
Quoted 6 lines
Heiner Kallweit (5):
pinctrl: meson: add interrupts to pinctrl data
pinctrl: meson: document GPIO IRQ DT binding
pinctrl: meson: add DT node for GPIO IRQ on Meson GX
pinctrl: meson: add DT node for GPIO IRQ on Meson 8 / 8b
pinctrl: meson: add support for GPIO interrupts
I just merged a bunch of patches from Martin Blumenstigl for this driver.
Martin can you help out reviewing these patches?
Yours,
Linus Walleij
Martin Blumenstingl
reviewercover pintrl: meson: add support for GPIO IRQs
Hi Linus,
Quoted 20 lines · Linus Walleij
Quoted 18 lines · Heiner Kallweit
This patch series is partially based on a series Jerome Brunet submitted about half a year ago. Due to open questions this series never made it to mainline, see https://patchwork.kernel.org/patch/9384431/
This new attempt uses
GPIOLIB_IRQCHIPresulting in less needed code. Included is also support for using two parent IRQs in case ofIRQ_TYPE_EDGE_BOTH, like in the vendor driver.Worth to be mentioned is also that I had to work around a strange issue with spurious interrupts resulting in a deadlock.
The affected critical section in__setup_irqis executed with interrupts disabled, nevertheless spurious GPIO IRQs result in a deadlock what seems to indicate that they are handled on the same CPU.
I didn't find a better explanation than a possible HW flaw so far.The series was successfully tested on a Odroid-C2, e.g. with removing polling for SD card insertion/removal from the mmc driver.
Quoted 8 lines
Quoted 6 lines
Heiner Kallweit (5):
pinctrl: meson: add interrupts to pinctrl data
pinctrl: meson: document GPIO IRQ DT binding
pinctrl: meson: add DT node for GPIO IRQ on Meson GX
pinctrl: meson: add DT node for GPIO IRQ on Meson 8 / 8b
pinctrl: meson: add support for GPIO interruptsI just merged a bunch of patches from Martin Blumenstigl for this driver.
thank you!
Quoted 1 line
Martin can you help out reviewing these patches?
I think it would be nice to have GPIO interrupt support and I can certainly help testing this. however, I am neither a pinctrl nor an interrupt expert. I can have a look at the patches and give (limited) feedback, but these should certainly be reviewed by people with more knowledge of these (pinctrl and irq) subsystems.
Jerome has already spent (probably quite some...) time with the Meson GPIO interrupt support, he has already replied to patch #1.
apart from that: thanks to anybody who dedicates time to this topic - this is probably one of the most exotic bits (and thus hard to implement) in the Meson SoCs!
Regards,
Martin