* [PATCH 0/5 RfC] pintrl: meson: add support for GPIO IRQs
@ 2017-05-07 16:19 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:19 UTC (permalink / raw)
To: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij
Cc: devicetree, linux-amlogic, linux-gpio
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
--
2.12.2
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 0/5 RfC] pintrl: meson: add support for GPIO IRQs
@ 2017-05-07 16:19 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:19 UTC (permalink / raw)
To: linus-amlogic
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
--
2.12.2
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
2017-05-07 16:19 ` Heiner Kallweit
@ 2017-05-07 16:34 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij
Cc: devicetree, linux-amlogic, linux-gpio
From: Jerome Brunet <jbrunet@baylibre.com>
Add GPIO interrupt information to pinctrl data. Added to the original
version from Jerome was data for Meson GXL.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
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(-)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
index 9b00be15..e54dbeaa 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
@@ -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 = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxl.c b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
index 998210ea..bcd8da10 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxl.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
@@ -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 = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index 1aa871d5..890f296f 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -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 }, \
diff --git a/drivers/pinctrl/meson/pinctrl-meson8.c b/drivers/pinctrl/meson/pinctrl-meson8.c
index 07f1cb21..32449820 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8.c
@@ -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 = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson8b.c b/drivers/pinctrl/meson/pinctrl-meson8b.c
index bf747eb1..71f216b5 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8b.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8b.c
@@ -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 = {
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
@ 2017-05-07 16:34 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: linus-amlogic
From: Jerome Brunet <jbrunet@baylibre.com>
Add GPIO interrupt information to pinctrl data. Added to the original
version from Jerome was data for Meson GXL.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
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(-)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
index 9b00be15..e54dbeaa 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
@@ -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 = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxl.c b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
index 998210ea..bcd8da10 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxl.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxl.c
@@ -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 = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index 1aa871d5..890f296f 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -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 }, \
diff --git a/drivers/pinctrl/meson/pinctrl-meson8.c b/drivers/pinctrl/meson/pinctrl-meson8.c
index 07f1cb21..32449820 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8.c
@@ -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 = {
diff --git a/drivers/pinctrl/meson/pinctrl-meson8b.c b/drivers/pinctrl/meson/pinctrl-meson8b.c
index bf747eb1..71f216b5 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8b.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8b.c
@@ -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 = {
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding
2017-05-07 16:19 ` Heiner Kallweit
@ 2017-05-07 16:34 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij
Cc: devicetree, linux-amlogic, linux-gpio
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 <hkallweit1@gmail.com>
---
.../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
diff --git a/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
new file mode 100644
index 00000000..35a052b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
@@ -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@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>;
+ };
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding
@ 2017-05-07 16:34 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: linus-amlogic
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 <hkallweit1@gmail.com>
---
.../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
diff --git a/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
new file mode 100644
index 00000000..35a052b8
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
@@ -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>;
+ };
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 3/5] pintrl: meson: add DT node for GPIO IRQ on Meson GX
2017-05-07 16:19 ` Heiner Kallweit
@ 2017-05-07 16:34 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij
Cc: devicetree, linux-amlogic, linux-gpio
Add the DT node for GPIO IRQ support on AMlogic Meson GX.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 436b8750..4d8219f2 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -312,6 +312,19 @@
status = "disabled";
};
+ gpio_irq@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@98d0 {
compatible = "amlogic,meson-gx-wdt", "amlogic,meson-gxbb-wdt";
reg = <0x0 0x098d0 0x0 0x10>;
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 3/5] pintrl: meson: add DT node for GPIO IRQ on Meson GX
@ 2017-05-07 16:34 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: linus-amlogic
Add the DT node for GPIO IRQ support on AMlogic Meson GX.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
index 436b8750..4d8219f2 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
@@ -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>;
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 4/5] pintrl: meson: add DT node for GPIO IRQ on Meson 8 / 8b
2017-05-07 16:19 ` Heiner Kallweit
@ 2017-05-07 16:34 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij
Cc: devicetree, linux-amlogic, linux-gpio
Add the DT node for GPIO IRQ support on AMlogic Meson 8 and 8b.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
arch/arm/boot/dts/meson8.dtsi | 13 +++++++++++++
arch/arm/boot/dts/meson8b.dtsi | 13 +++++++++++++
2 files changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index ebc763ea..bf76a120 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -91,6 +91,19 @@
clock-frequency = <141666666>;
};
+ gpio_irq@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@c1109880 {
compatible = "amlogic,meson8-cbus-pinctrl";
reg = <0xc1109880 0x10>;
diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index 828aa49c..b5a92461 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -183,6 +183,19 @@
status = "disabled";
};
+ gpio_irq@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@c1109880 {
compatible = "amlogic,meson8b-cbus-pinctrl";
reg = <0xc1109880 0x10>;
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 4/5] pintrl: meson: add DT node for GPIO IRQ on Meson 8 / 8b
@ 2017-05-07 16:34 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: linus-amlogic
Add the DT node for GPIO IRQ support on AMlogic Meson 8 and 8b.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
arch/arm/boot/dts/meson8.dtsi | 13 +++++++++++++
arch/arm/boot/dts/meson8b.dtsi | 13 +++++++++++++
2 files changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/meson8.dtsi b/arch/arm/boot/dts/meson8.dtsi
index ebc763ea..bf76a120 100644
--- a/arch/arm/boot/dts/meson8.dtsi
+++ b/arch/arm/boot/dts/meson8.dtsi
@@ -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>;
diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index 828aa49c..b5a92461 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -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>;
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 5/5] pintrl: meson: add support for GPIO interrupts
2017-05-07 16:19 ` Heiner Kallweit
@ 2017-05-07 16:34 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA
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_BOTH interrupts reserve two parent IRQ's,
one for each edge.
Signed-off-by: Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
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(-)
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 37af5e30..f8f401a0 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -153,6 +153,7 @@ config PINCTRL_MESON
select PINCONF
select GENERIC_PINCONF
select GPIOLIB
+ select GPIOLIB_IRQCHIP
select OF_GPIO
select REGMAP_MMIO
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 66ed70c1..525879f2 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -46,9 +46,12 @@
#include <linux/device.h>
#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/interrupt.h>
#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/of_address.h>
#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinconf.h>
@@ -57,11 +60,30 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/seq_file.h>
+#include <linux/mfd/syscon.h>
#include "../core.h"
#include "../pinctrl-utils.h"
#include "pinctrl-meson.h"
+#define REG_EDGE_POL 0x00
+#define REG_PIN_03_SEL 0x04
+#define REG_PIN_47_SEL 0x08
+#define REG_FILTER_SEL 0x0c
+
+#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x)))
+#define REG_EDGE_POL_EDGE(x) BIT(x)
+#define REG_EDGE_POL_LOW(x) BIT(16 + (x))
+
+#define MESON_GPIO_MAX_PARENT_IRQ_NUM 8
+
+static struct regmap *meson_gpio_irq_regmap;
+static struct meson_gpio_irq_slot
+ meson_gpio_irq_slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+static int meson_gpio_num_irq_slots;
+static DEFINE_MUTEX(meson_gpio_irq_slot_mutex);
+static DECLARE_BITMAP(meson_gpio_irq_locked, 256);
+
/**
* meson_get_bank() - find the bank containing a given pin
*
@@ -533,6 +555,275 @@ static const struct of_device_id meson_pinctrl_dt_match[] = {
{ },
};
+static struct meson_pinctrl *meson_gpio_data_to_pc(struct irq_data *data)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+
+ return gpiochip_get_data(chip);
+}
+
+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 */
+ return -EINVAL;
+
+ hwirq = offset - bank->first + bank->irq_first;
+
+ if (hwirq > bank->irq_last)
+ /* this pin cannot generate irqs */
+ return -EINVAL;
+
+ return hwirq;
+}
+
+static int meson_gpio_to_irq(struct meson_pinctrl *pc, unsigned int offset)
+{
+ struct meson_bank *bank;
+ int ret;
+
+ offset += pc->data->pin_base;
+
+ ret = meson_get_bank(pc, offset, &bank);
+ if (ret)
+ return ret;
+
+ ret = meson_gpio_to_hwirq(bank, offset);
+ if (ret < 0) {
+ dev_dbg(pc->dev, "no interrupt for pin %u\n", offset);
+ return 0;
+ }
+
+ return ret;
+}
+
+static int meson_gpio_data_to_hwirq(struct irq_data *data)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ unsigned gpio = irqd_to_hwirq(data);
+
+ return meson_gpio_to_irq(pc, gpio);
+}
+
+static irqreturn_t meson_gpio_irq_handler(int irq, void *data)
+{
+ struct irq_data *gpio_irqdata = data;
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ int hwirq = meson_gpio_data_to_hwirq(gpio_irqdata);
+
+ /*
+ * For some strange reason spurious interrupts created by the chip when
+ * the interrupt source registers are written cause a deadlock here.
+ * generic_handle_irq calls handle_simple_irq which tries to get
+ * spinlock desc->lock. This interrupt handler is called whilst
+ * __setup_irq holds desc->lock.
+ * The deadlock means that both are running on the same CPU what should
+ * not happen as __setup_irq called raw_spin_lock_irqsave thus disabling
+ * interrupts on this CPU.
+ * Work around this by ignoring interrupts in code protected by
+ * chip_bus_lock (__setup_irq/__free_irq for the respective GPIO hwirq).
+ */
+ if (test_bit(hwirq, meson_gpio_irq_locked))
+ dev_dbg(pc->dev, "spurious interrupt detected!\n");
+ else
+ generic_handle_irq(gpio_irqdata->irq);
+
+ return IRQ_HANDLED;
+}
+
+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;
+ }
+
+ 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;
+}
+
+static void meson_gpio_free_irq_slot(struct irq_data *data)
+{
+ int i;
+
+ mutex_lock(&meson_gpio_irq_slot_mutex);
+
+ for (i = 0; i < meson_gpio_num_irq_slots; i++)
+ if (meson_gpio_irq_slots[i].owner == data->irq) {
+ free_irq(meson_gpio_irq_slots[i].irq, data);
+ meson_gpio_irq_slots[i].owner = 0;
+ }
+
+ mutex_unlock(&meson_gpio_irq_slot_mutex);
+}
+
+static int meson_gpio_find_irq_slot(struct irq_data *data, 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 == data->irq)
+ slots[cnt++] = i;
+
+ mutex_unlock(&meson_gpio_irq_slot_mutex);
+
+ return cnt ?: -EINVAL;
+}
+
+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);
+}
+
+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);
+}
+
+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);
+}
+
+static void meson_gpio_irq_set_hwirq(struct irq_data *data, int hwirq)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ int i, cnt, slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+
+ cnt = meson_gpio_find_irq_slot(data, slots);
+ if (cnt < 0) {
+ dev_err(pc->dev, "didn't find gpio irq slot\n");
+ return;
+ }
+
+ for (i = 0; i < cnt; i++)
+ meson_gpio_set_hwirq(slots[i], hwirq);
+}
+
+static void meson_gpio_irq_unmask(struct irq_data *data)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ unsigned gpio = irqd_to_hwirq(data);
+ int hwirq = meson_gpio_to_irq(pc, gpio);
+
+ meson_gpio_irq_set_hwirq(data, hwirq);
+}
+
+static void meson_gpio_irq_mask(struct irq_data *data)
+{
+ meson_gpio_irq_set_hwirq(data, 0xff);
+}
+
+static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ int i, ret, irq, num_slots, slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+ unsigned int val = 0;
+
+ num_slots = (type == IRQ_TYPE_EDGE_BOTH) ? 2 : 1;
+ ret = meson_gpio_alloc_irq_slot(data, num_slots, slots);
+ if (ret)
+ return ret;
+
+ if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_EDGE(slots[0]);
+
+ if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_LOW(slots[0]);
+
+ regmap_update_bits(meson_gpio_irq_regmap, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(slots[0]), val);
+
+ /*
+ * The chip can create an interrupt for either rising or falling edge
+ * only. Therefore use two interrupts in case of IRQ_TYPE_EDGE_BOTH,
+ * first for falling edge and second one for rising edge.
+ */
+ if (num_slots > 1) {
+ val = REG_EDGE_POL_EDGE(slots[1]);
+ regmap_update_bits(meson_gpio_irq_regmap, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(slots[1]), val);
+ }
+
+ if (type & IRQ_TYPE_EDGE_BOTH)
+ val = IRQ_TYPE_EDGE_RISING;
+ else
+ val = IRQ_TYPE_LEVEL_HIGH;
+
+ 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;
+ }
+
+ if (ret)
+ while (--i >= 0)
+ free_irq(meson_gpio_irq_slots[slots[i]].irq, data);
+
+ return ret;
+}
+
+static void meson_gpio_irq_bus_lock(struct irq_data *data)
+{
+ int hwirq = meson_gpio_data_to_hwirq(data);
+
+ set_bit(hwirq, meson_gpio_irq_locked);
+}
+
+static void meson_gpio_irq_bus_sync_unlock(struct irq_data *data)
+{
+ int hwirq = meson_gpio_data_to_hwirq(data);
+
+ clear_bit(hwirq, meson_gpio_irq_locked);
+}
+
+static void meson_gpiolib_init_irq_chip(struct meson_pinctrl *pc)
+{
+ static atomic_t irqchip_idx = ATOMIC_INIT(-1);
+ int idx = atomic_inc_return(&irqchip_idx);
+
+ pc->irq_chip.name = devm_kasprintf(pc->dev, GFP_KERNEL, "GPIO%d", idx);
+ pc->irq_chip.irq_request_resources = meson_gpio_irq_request_resources;
+ pc->irq_chip.irq_release_resources = meson_gpio_irq_release_resources;
+ pc->irq_chip.irq_set_type = meson_gpio_irq_set_type;
+ pc->irq_chip.irq_mask = meson_gpio_irq_mask;
+ pc->irq_chip.irq_unmask = meson_gpio_irq_unmask;
+ pc->irq_chip.irq_bus_lock = meson_gpio_irq_bus_lock;
+ pc->irq_chip.irq_bus_sync_unlock = meson_gpio_irq_bus_sync_unlock;
+}
+
static int meson_gpiolib_register(struct meson_pinctrl *pc)
{
int ret;
@@ -558,7 +849,8 @@ static int meson_gpiolib_register(struct meson_pinctrl *pc)
return ret;
}
- return 0;
+ return gpiochip_irqchip_add(&pc->chip, &pc->irq_chip, 0,
+ handle_simple_irq, IRQ_TYPE_NONE);
}
static struct regmap_config meson_regmap_config = {
@@ -640,6 +932,29 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
return 0;
}
+static int meson_gpio_get_parent_irqs(void)
+{
+ struct device_node *np;
+ int irq, i;
+
+ np = of_find_compatible_node(NULL, NULL,
+ "amlogic,meson-gpio-interrupt");
+ if (!np)
+ return -EINVAL;
+
+ for (i = 0; i < MESON_GPIO_MAX_PARENT_IRQ_NUM; i++) {
+ irq = irq_of_parse_and_map(np, i);
+ if (!irq)
+ break;
+ meson_gpio_irq_slots[i].irq = irq;
+ }
+
+ of_node_put(np);
+ meson_gpio_num_irq_slots = i;
+
+ return meson_gpio_num_irq_slots ? 0 : -EINVAL;
+}
+
static int meson_pinctrl_probe(struct platform_device *pdev)
{
const struct of_device_id *match;
@@ -647,6 +962,25 @@ static int meson_pinctrl_probe(struct platform_device *pdev)
struct meson_pinctrl *pc;
int ret;
+ if (!meson_gpio_irq_regmap) {
+ meson_gpio_irq_regmap = syscon_regmap_lookup_by_compatible(
+ "amlogic,meson-gpio-interrupt");
+ if (IS_ERR(meson_gpio_irq_regmap))
+ return PTR_ERR(meson_gpio_irq_regmap);
+
+ /* initialize to IRQ_TYPE_LEVEL_HIGH */
+ regmap_write(meson_gpio_irq_regmap, REG_EDGE_POL, 0);
+ /* disable all GPIO interrupt sources */
+ regmap_write(meson_gpio_irq_regmap, REG_PIN_03_SEL, 0xffffffff);
+ regmap_write(meson_gpio_irq_regmap, REG_PIN_47_SEL, 0xffffffff);
+ /* disable filtering */
+ regmap_write(meson_gpio_irq_regmap, REG_FILTER_SEL, 0);
+
+ ret = meson_gpio_get_parent_irqs();
+ if(ret)
+ return ret;
+ }
+
pc = devm_kzalloc(dev, sizeof(struct meson_pinctrl), GFP_KERNEL);
if (!pc)
return -ENOMEM;
@@ -673,6 +1007,8 @@ static int meson_pinctrl_probe(struct platform_device *pdev)
return PTR_ERR(pc->pcdev);
}
+ meson_gpiolib_init_irq_chip(pc);
+
return meson_gpiolib_register(pc);
}
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index 890f296f..4792e769 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -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;
};
--
2.12.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 5/5] pintrl: meson: add support for GPIO interrupts
@ 2017-05-07 16:34 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-07 16:34 UTC (permalink / raw)
To: linus-amlogic
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_BOTH interrupts reserve two parent IRQ's,
one for each edge.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
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(-)
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 37af5e30..f8f401a0 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -153,6 +153,7 @@ config PINCTRL_MESON
select PINCONF
select GENERIC_PINCONF
select GPIOLIB
+ select GPIOLIB_IRQCHIP
select OF_GPIO
select REGMAP_MMIO
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 66ed70c1..525879f2 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -46,9 +46,12 @@
#include <linux/device.h>
#include <linux/gpio.h>
+#include <linux/gpio/driver.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/interrupt.h>
#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/of_address.h>
#include <linux/pinctrl/pinconf-generic.h>
#include <linux/pinctrl/pinconf.h>
@@ -57,11 +60,30 @@
#include <linux/platform_device.h>
#include <linux/regmap.h>
#include <linux/seq_file.h>
+#include <linux/mfd/syscon.h>
#include "../core.h"
#include "../pinctrl-utils.h"
#include "pinctrl-meson.h"
+#define REG_EDGE_POL 0x00
+#define REG_PIN_03_SEL 0x04
+#define REG_PIN_47_SEL 0x08
+#define REG_FILTER_SEL 0x0c
+
+#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x)))
+#define REG_EDGE_POL_EDGE(x) BIT(x)
+#define REG_EDGE_POL_LOW(x) BIT(16 + (x))
+
+#define MESON_GPIO_MAX_PARENT_IRQ_NUM 8
+
+static struct regmap *meson_gpio_irq_regmap;
+static struct meson_gpio_irq_slot
+ meson_gpio_irq_slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+static int meson_gpio_num_irq_slots;
+static DEFINE_MUTEX(meson_gpio_irq_slot_mutex);
+static DECLARE_BITMAP(meson_gpio_irq_locked, 256);
+
/**
* meson_get_bank() - find the bank containing a given pin
*
@@ -533,6 +555,275 @@ static const struct of_device_id meson_pinctrl_dt_match[] = {
{ },
};
+static struct meson_pinctrl *meson_gpio_data_to_pc(struct irq_data *data)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+
+ return gpiochip_get_data(chip);
+}
+
+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 */
+ return -EINVAL;
+
+ hwirq = offset - bank->first + bank->irq_first;
+
+ if (hwirq > bank->irq_last)
+ /* this pin cannot generate irqs */
+ return -EINVAL;
+
+ return hwirq;
+}
+
+static int meson_gpio_to_irq(struct meson_pinctrl *pc, unsigned int offset)
+{
+ struct meson_bank *bank;
+ int ret;
+
+ offset += pc->data->pin_base;
+
+ ret = meson_get_bank(pc, offset, &bank);
+ if (ret)
+ return ret;
+
+ ret = meson_gpio_to_hwirq(bank, offset);
+ if (ret < 0) {
+ dev_dbg(pc->dev, "no interrupt for pin %u\n", offset);
+ return 0;
+ }
+
+ return ret;
+}
+
+static int meson_gpio_data_to_hwirq(struct irq_data *data)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ unsigned gpio = irqd_to_hwirq(data);
+
+ return meson_gpio_to_irq(pc, gpio);
+}
+
+static irqreturn_t meson_gpio_irq_handler(int irq, void *data)
+{
+ struct irq_data *gpio_irqdata = data;
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ int hwirq = meson_gpio_data_to_hwirq(gpio_irqdata);
+
+ /*
+ * For some strange reason spurious interrupts created by the chip when
+ * the interrupt source registers are written cause a deadlock here.
+ * generic_handle_irq calls handle_simple_irq which tries to get
+ * spinlock desc->lock. This interrupt handler is called whilst
+ * __setup_irq holds desc->lock.
+ * The deadlock means that both are running on the same CPU what should
+ * not happen as __setup_irq called raw_spin_lock_irqsave thus disabling
+ * interrupts on this CPU.
+ * Work around this by ignoring interrupts in code protected by
+ * chip_bus_lock (__setup_irq/__free_irq for the respective GPIO hwirq).
+ */
+ if (test_bit(hwirq, meson_gpio_irq_locked))
+ dev_dbg(pc->dev, "spurious interrupt detected!\n");
+ else
+ generic_handle_irq(gpio_irqdata->irq);
+
+ return IRQ_HANDLED;
+}
+
+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;
+ }
+
+ 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;
+}
+
+static void meson_gpio_free_irq_slot(struct irq_data *data)
+{
+ int i;
+
+ mutex_lock(&meson_gpio_irq_slot_mutex);
+
+ for (i = 0; i < meson_gpio_num_irq_slots; i++)
+ if (meson_gpio_irq_slots[i].owner == data->irq) {
+ free_irq(meson_gpio_irq_slots[i].irq, data);
+ meson_gpio_irq_slots[i].owner = 0;
+ }
+
+ mutex_unlock(&meson_gpio_irq_slot_mutex);
+}
+
+static int meson_gpio_find_irq_slot(struct irq_data *data, 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 == data->irq)
+ slots[cnt++] = i;
+
+ mutex_unlock(&meson_gpio_irq_slot_mutex);
+
+ return cnt ?: -EINVAL;
+}
+
+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);
+}
+
+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);
+}
+
+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);
+}
+
+static void meson_gpio_irq_set_hwirq(struct irq_data *data, int hwirq)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ int i, cnt, slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+
+ cnt = meson_gpio_find_irq_slot(data, slots);
+ if (cnt < 0) {
+ dev_err(pc->dev, "didn't find gpio irq slot\n");
+ return;
+ }
+
+ for (i = 0; i < cnt; i++)
+ meson_gpio_set_hwirq(slots[i], hwirq);
+}
+
+static void meson_gpio_irq_unmask(struct irq_data *data)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ unsigned gpio = irqd_to_hwirq(data);
+ int hwirq = meson_gpio_to_irq(pc, gpio);
+
+ meson_gpio_irq_set_hwirq(data, hwirq);
+}
+
+static void meson_gpio_irq_mask(struct irq_data *data)
+{
+ meson_gpio_irq_set_hwirq(data, 0xff);
+}
+
+static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ int i, ret, irq, num_slots, slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+ unsigned int val = 0;
+
+ num_slots = (type == IRQ_TYPE_EDGE_BOTH) ? 2 : 1;
+ ret = meson_gpio_alloc_irq_slot(data, num_slots, slots);
+ if (ret)
+ return ret;
+
+ if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_EDGE(slots[0]);
+
+ if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_LOW(slots[0]);
+
+ regmap_update_bits(meson_gpio_irq_regmap, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(slots[0]), val);
+
+ /*
+ * The chip can create an interrupt for either rising or falling edge
+ * only. Therefore use two interrupts in case of IRQ_TYPE_EDGE_BOTH,
+ * first for falling edge and second one for rising edge.
+ */
+ if (num_slots > 1) {
+ val = REG_EDGE_POL_EDGE(slots[1]);
+ regmap_update_bits(meson_gpio_irq_regmap, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(slots[1]), val);
+ }
+
+ if (type & IRQ_TYPE_EDGE_BOTH)
+ val = IRQ_TYPE_EDGE_RISING;
+ else
+ val = IRQ_TYPE_LEVEL_HIGH;
+
+ 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;
+ }
+
+ if (ret)
+ while (--i >= 0)
+ free_irq(meson_gpio_irq_slots[slots[i]].irq, data);
+
+ return ret;
+}
+
+static void meson_gpio_irq_bus_lock(struct irq_data *data)
+{
+ int hwirq = meson_gpio_data_to_hwirq(data);
+
+ set_bit(hwirq, meson_gpio_irq_locked);
+}
+
+static void meson_gpio_irq_bus_sync_unlock(struct irq_data *data)
+{
+ int hwirq = meson_gpio_data_to_hwirq(data);
+
+ clear_bit(hwirq, meson_gpio_irq_locked);
+}
+
+static void meson_gpiolib_init_irq_chip(struct meson_pinctrl *pc)
+{
+ static atomic_t irqchip_idx = ATOMIC_INIT(-1);
+ int idx = atomic_inc_return(&irqchip_idx);
+
+ pc->irq_chip.name = devm_kasprintf(pc->dev, GFP_KERNEL, "GPIO%d", idx);
+ pc->irq_chip.irq_request_resources = meson_gpio_irq_request_resources;
+ pc->irq_chip.irq_release_resources = meson_gpio_irq_release_resources;
+ pc->irq_chip.irq_set_type = meson_gpio_irq_set_type;
+ pc->irq_chip.irq_mask = meson_gpio_irq_mask;
+ pc->irq_chip.irq_unmask = meson_gpio_irq_unmask;
+ pc->irq_chip.irq_bus_lock = meson_gpio_irq_bus_lock;
+ pc->irq_chip.irq_bus_sync_unlock = meson_gpio_irq_bus_sync_unlock;
+}
+
static int meson_gpiolib_register(struct meson_pinctrl *pc)
{
int ret;
@@ -558,7 +849,8 @@ static int meson_gpiolib_register(struct meson_pinctrl *pc)
return ret;
}
- return 0;
+ return gpiochip_irqchip_add(&pc->chip, &pc->irq_chip, 0,
+ handle_simple_irq, IRQ_TYPE_NONE);
}
static struct regmap_config meson_regmap_config = {
@@ -640,6 +932,29 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
return 0;
}
+static int meson_gpio_get_parent_irqs(void)
+{
+ struct device_node *np;
+ int irq, i;
+
+ np = of_find_compatible_node(NULL, NULL,
+ "amlogic,meson-gpio-interrupt");
+ if (!np)
+ return -EINVAL;
+
+ for (i = 0; i < MESON_GPIO_MAX_PARENT_IRQ_NUM; i++) {
+ irq = irq_of_parse_and_map(np, i);
+ if (!irq)
+ break;
+ meson_gpio_irq_slots[i].irq = irq;
+ }
+
+ of_node_put(np);
+ meson_gpio_num_irq_slots = i;
+
+ return meson_gpio_num_irq_slots ? 0 : -EINVAL;
+}
+
static int meson_pinctrl_probe(struct platform_device *pdev)
{
const struct of_device_id *match;
@@ -647,6 +962,25 @@ static int meson_pinctrl_probe(struct platform_device *pdev)
struct meson_pinctrl *pc;
int ret;
+ if (!meson_gpio_irq_regmap) {
+ meson_gpio_irq_regmap = syscon_regmap_lookup_by_compatible(
+ "amlogic,meson-gpio-interrupt");
+ if (IS_ERR(meson_gpio_irq_regmap))
+ return PTR_ERR(meson_gpio_irq_regmap);
+
+ /* initialize to IRQ_TYPE_LEVEL_HIGH */
+ regmap_write(meson_gpio_irq_regmap, REG_EDGE_POL, 0);
+ /* disable all GPIO interrupt sources */
+ regmap_write(meson_gpio_irq_regmap, REG_PIN_03_SEL, 0xffffffff);
+ regmap_write(meson_gpio_irq_regmap, REG_PIN_47_SEL, 0xffffffff);
+ /* disable filtering */
+ regmap_write(meson_gpio_irq_regmap, REG_FILTER_SEL, 0);
+
+ ret = meson_gpio_get_parent_irqs();
+ if(ret)
+ return ret;
+ }
+
pc = devm_kzalloc(dev, sizeof(struct meson_pinctrl), GFP_KERNEL);
if (!pc)
return -ENOMEM;
@@ -673,6 +1007,8 @@ static int meson_pinctrl_probe(struct platform_device *pdev)
return PTR_ERR(pc->pcdev);
}
+ meson_gpiolib_init_irq_chip(pc);
+
return meson_gpiolib_register(pc);
}
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index 890f296f..4792e769 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -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;
};
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5 RfC] pintrl: meson: add support for GPIO IRQs
2017-05-07 16:19 ` Heiner Kallweit
@ 2017-05-11 14:34 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2017-05-11 14:34 UTC (permalink / raw)
To: Heiner Kallweit, Martin Blumenstingl
Cc: Jerome Brunet, Mark Rutland, Marc Zyngier,
devicetree-u79uwXL29TY76Z2rM5mHXA, open list:ARM/Amlogic Meson...,
linux-gpio-u79uwXL29TY76Z2rM5mHXA
On Sun, May 7, 2017 at 6:19 PM, Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 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
I just merged a bunch of patches from Martin Blumenstigl for this driver.
Martin can you help out reviewing these patches?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 0/5 RfC] pintrl: meson: add support for GPIO IRQs
@ 2017-05-11 14:34 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2017-05-11 14:34 UTC (permalink / raw)
To: linus-amlogic
On Sun, May 7, 2017 at 6:19 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> 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
I just merged a bunch of patches from Martin Blumenstigl for this driver.
Martin can you help out reviewing these patches?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
2017-05-07 16:34 ` Heiner Kallweit
@ 2017-05-11 14:50 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2017-05-11 14:50 UTC (permalink / raw)
To: Heiner Kallweit, thierry.reding, Thierry Reding
Cc: Jerome Brunet, Mark Rutland, Marc Zyngier, devicetree,
open list:ARM/Amlogic Meson...,
linux-gpio
On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> From: Jerome Brunet <jbrunet@baylibre.com>
> Add GPIO interrupt information to pinctrl data. Added to the original
> version from Jerome was data for Meson GXL.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
So what this does is:
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
> index 1aa871d5..890f296f 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.h
> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
> @@ -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];
> };
... 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
@ 2017-05-11 14:50 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2017-05-11 14:50 UTC (permalink / raw)
To: linus-amlogic
On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> From: Jerome Brunet <jbrunet@baylibre.com>
> Add GPIO interrupt information to pinctrl data. Added to the original
> version from Jerome was data for Meson GXL.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
So what this does is:
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
> index 1aa871d5..890f296f 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.h
> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
> @@ -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];
> };
... 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 5/5] pintrl: meson: add support for GPIO interrupts
2017-05-07 16:34 ` Heiner Kallweit
@ 2017-05-11 15:10 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2017-05-11 15:10 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Jerome Brunet, Mark Rutland, Marc Zyngier, devicetree,
open list:ARM/Amlogic Meson...,
linux-gpio, thierry.reding
On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> 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_BOTH interrupts reserve two parent IRQ's,
> one for each edge.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
(..)
> select GPIOLIB
> + select GPIOLIB_IRQCHIP
This is nice.
> +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 */
> + 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.
> +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;
> + }
> +
> + 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.
> +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);
> +}
> +
> +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.
> +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.
> +static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
> +{
(...)
> + 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.
> + 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 5/5] pintrl: meson: add support for GPIO interrupts
@ 2017-05-11 15:10 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2017-05-11 15:10 UTC (permalink / raw)
To: linus-amlogic
On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> 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_BOTH interrupts reserve two parent IRQ's,
> one for each edge.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
(..)
> select GPIOLIB
> + select GPIOLIB_IRQCHIP
This is nice.
> +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 */
> + 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.
> +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;
> + }
> +
> + 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.
> +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);
> +}
> +
> +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.
> +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.
> +static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
> +{
(...)
> + 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.
> + 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
2017-05-11 14:50 ` Linus Walleij
@ 2017-05-11 16:08 ` Jerome Brunet
-1 siblings, 0 replies; 36+ messages in thread
From: Jerome Brunet @ 2017-05-11 16:08 UTC (permalink / raw)
To: Linus Walleij, Heiner Kallweit, thierry.reding, Thierry Reding
Cc: Mark Rutland, Marc Zyngier, devicetree,
open list:ARM/Amlogic Meson...,
linux-gpio
On Thu, 2017-05-11 at 16:50 +0200, Linus Walleij wrote:
> On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> > From: Jerome Brunet <jbrunet@baylibre.com>
> > Add GPIO interrupt information to pinctrl data. Added to the original
> > version from Jerome was data for Meson GXL.
> >
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> So what this does is:
>
> > diff --git a/drivers/pinctrl/meson/pinctrl-meson.h
> > b/drivers/pinctrl/meson/pinctrl-meson.h
> > index 1aa871d5..890f296f 100644
> > --- a/drivers/pinctrl/meson/pinctrl-meson.h
> > +++ b/drivers/pinctrl/meson/pinctrl-meson.h
> > @@ -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];
> > };
>
> ... 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 ...
>
> 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
@ 2017-05-11 16:08 ` Jerome Brunet
0 siblings, 0 replies; 36+ messages in thread
From: Jerome Brunet @ 2017-05-11 16:08 UTC (permalink / raw)
To: linus-amlogic
On Thu, 2017-05-11 at 16:50 +0200, Linus Walleij wrote:
> On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> > From: Jerome Brunet <jbrunet@baylibre.com>
> > Add GPIO interrupt information to pinctrl data. Added to the original
> > version from Jerome was data for Meson GXL.
> >
> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> So what this does is:
>
> > diff --git a/drivers/pinctrl/meson/pinctrl-meson.h
> > b/drivers/pinctrl/meson/pinctrl-meson.h
> > index 1aa871d5..890f296f 100644
> > --- a/drivers/pinctrl/meson/pinctrl-meson.h
> > +++ b/drivers/pinctrl/meson/pinctrl-meson.h
> > @@ -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];
> > ?};
>
> ... 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 ...
>
> 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
2017-05-11 16:08 ` Jerome Brunet
@ 2017-05-11 19:33 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-11 19:33 UTC (permalink / raw)
To: Jerome Brunet, Linus Walleij, thierry.reding, Thierry Reding
Cc: Mark Rutland, Marc Zyngier, devicetree,
open list:ARM/Amlogic Meson...,
linux-gpio
Am 11.05.2017 um 18:08 schrieb Jerome Brunet:
> On Thu, 2017-05-11 at 16:50 +0200, Linus Walleij wrote:
>> On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>>> From: Jerome Brunet <jbrunet@baylibre.com>
>>> Add GPIO interrupt information to pinctrl data. Added to the original
>>> version from Jerome was data for Meson GXL.
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> So what this does is:
>>
>>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h
>>> b/drivers/pinctrl/meson/pinctrl-meson.h
>>> index 1aa871d5..890f296f 100644
>>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>>> @@ -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];
>>> };
>>
>> ... 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.
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.
> 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.
>
> 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 ...
>
>
>>
>> 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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
@ 2017-05-11 19:33 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-11 19:33 UTC (permalink / raw)
To: linus-amlogic
Am 11.05.2017 um 18:08 schrieb Jerome Brunet:
> On Thu, 2017-05-11 at 16:50 +0200, Linus Walleij wrote:
>> On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>>> From: Jerome Brunet <jbrunet@baylibre.com>
>>> Add GPIO interrupt information to pinctrl data. Added to the original
>>> version from Jerome was data for Meson GXL.
>>>
>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> So what this does is:
>>
>>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h
>>> b/drivers/pinctrl/meson/pinctrl-meson.h
>>> index 1aa871d5..890f296f 100644
>>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>>> @@ -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];
>>> };
>>
>> ... 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.
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.
> 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.
>
> 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 ...
>
>
>>
>> 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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 0/5 RfC] pintrl: meson: add support for GPIO IRQs
2017-05-11 14:34 ` Linus Walleij
@ 2017-05-11 20:52 ` Martin Blumenstingl
-1 siblings, 0 replies; 36+ messages in thread
From: Martin Blumenstingl @ 2017-05-11 20:52 UTC (permalink / raw)
To: Linus Walleij
Cc: Heiner Kallweit, Jerome Brunet, Mark Rutland, Marc Zyngier,
devicetree-u79uwXL29TY76Z2rM5mHXA, open list:ARM/Amlogic Meson...,
linux-gpio-u79uwXL29TY76Z2rM5mHXA
Hi Linus,
On Thu, May 11, 2017 at 4:34 PM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Sun, May 7, 2017 at 6:19 PM, Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> 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
>
> I just merged a bunch of patches from Martin Blumenstigl for this driver.
thank you!
> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 0/5 RfC] pintrl: meson: add support for GPIO IRQs
@ 2017-05-11 20:52 ` Martin Blumenstingl
0 siblings, 0 replies; 36+ messages in thread
From: Martin Blumenstingl @ 2017-05-11 20:52 UTC (permalink / raw)
To: linus-amlogic
Hi Linus,
On Thu, May 11, 2017 at 4:34 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, May 7, 2017 at 6:19 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
>> 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
>
> I just merged a bunch of patches from Martin Blumenstigl for this driver.
thank you!
> 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
2017-05-11 19:33 ` Heiner Kallweit
@ 2017-05-12 6:08 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-12 6:08 UTC (permalink / raw)
To: Jerome Brunet, Linus Walleij,
thierry.reding-Re5JQEeQqe8AvxtiuMwx3w, Thierry Reding
Cc: Mark Rutland, Marc Zyngier, devicetree-u79uwXL29TY76Z2rM5mHXA,
open list:ARM/Amlogic Meson...,
linux-gpio-u79uwXL29TY76Z2rM5mHXA
Am 11.05.2017 um 21:33 schrieb Heiner Kallweit:
> Am 11.05.2017 um 18:08 schrieb Jerome Brunet:
>> On Thu, 2017-05-11 at 16:50 +0200, Linus Walleij wrote:
>>> On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>>> From: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>> Add GPIO interrupt information to pinctrl data. Added to the original
>>>> version from Jerome was data for Meson GXL.
>>>>
>>>> Signed-off-by: Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>
>>> So what this does is:
>>>
>>>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h
>>>> b/drivers/pinctrl/meson/pinctrl-meson.h
>>>> index 1aa871d5..890f296f 100644
>>>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>>>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>>>> @@ -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];
>>>> };
>>>
>>> ... 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.
>
> 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.
>
>> 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
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 37af5e30..f8f401a0 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -153,6 +153,7 @@ config PINCTRL_MESON
select PINCONF
select GENERIC_PINCONF
select GPIOLIB
+ select GPIOLIB_IRQCHIP
select OF_GPIO
select REGMAP_MMIO
diff --git a/drivers/pinctrl/meson/Makefile b/drivers/pinctrl/meson/Makefile
index 27c5b512..827e416d 100644
--- a/drivers/pinctrl/meson/Makefile
+++ b/drivers/pinctrl/meson/Makefile
@@ -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
diff --git a/drivers/pinctrl/meson/pinctrl-meson-irq.c b/drivers/pinctrl/meson/pinctrl-meson-irq.c
new file mode 100644
index 00000000..dfc3af77
--- /dev/null
+++ b/drivers/pinctrl/meson/pinctrl-meson-irq.c
@@ -0,0 +1,393 @@
+/*
+ * Amlogic Meson GPIO IRQ driver
+ *
+ * Copyright 2017 Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ * Based on a first version by Jerome Brunet <jbrunet-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include "pinctrl-meson.h"
+
+#define REG_EDGE_POL 0x00
+#define REG_PIN_03_SEL 0x04
+#define REG_PIN_47_SEL 0x08
+#define REG_FILTER_SEL 0x0c
+
+#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x)))
+#define REG_EDGE_POL_EDGE(x) BIT(x)
+#define REG_EDGE_POL_LOW(x) BIT(16 + (x))
+
+#define MESON_GPIO_MAX_PARENT_IRQ_NUM 8
+
+struct meson_gpio_irq_slot {
+ int irq;
+ int owner;
+};
+
+static struct regmap *meson_gpio_irq_regmap;
+static struct meson_gpio_irq_slot
+ meson_gpio_irq_slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+static int meson_gpio_num_irq_slots;
+static DEFINE_MUTEX(meson_gpio_irq_slot_mutex);
+static DECLARE_BITMAP(meson_gpio_irq_locked, 256);
+
+/**
+ * meson_get_bank() - find the bank containing a given pin
+ *
+ * @pc: the pinctrl instance
+ * @pin: the pin number
+ * @bank: the found bank
+ *
+ * Return: 0 on success, a negative value on error
+ */
+static int meson_get_bank(struct meson_pinctrl *pc, unsigned int pin,
+ struct meson_bank **bank)
+{
+ int i;
+
+ for (i = 0; i < pc->data->num_banks; i++) {
+ if (pin >= pc->data->banks[i].first &&
+ pin <= pc->data->banks[i].last) {
+ *bank = &pc->data->banks[i];
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static struct meson_pinctrl *meson_gpio_data_to_pc(struct irq_data *data)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+
+ return gpiochip_get_data(chip);
+}
+
+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 */
+ return -EINVAL;
+
+ hwirq = offset - bank->first + bank->irq_first;
+
+ if (hwirq > bank->irq_last)
+ /* this pin cannot generate irqs */
+ return -EINVAL;
+
+ return hwirq;
+}
+
+static int meson_gpio_to_irq(struct meson_pinctrl *pc, unsigned int offset)
+{
+ struct meson_bank *bank;
+ int ret;
+
+ offset += pc->data->pin_base;
+
+ ret = meson_get_bank(pc, offset, &bank);
+ if (ret)
+ return ret;
+
+ ret = meson_gpio_to_hwirq(bank, offset);
+ if (ret < 0) {
+ dev_dbg(pc->dev, "no interrupt for pin %u\n", offset);
+ return 0;
+ }
+
+ return ret;
+}
+
+static int meson_gpio_data_to_hwirq(struct irq_data *data)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ unsigned gpio = irqd_to_hwirq(data);
+
+ return meson_gpio_to_irq(pc, gpio);
+}
+
+static irqreturn_t meson_gpio_irq_handler(int irq, void *data)
+{
+ struct irq_data *gpio_irqdata = data;
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ int hwirq = meson_gpio_data_to_hwirq(gpio_irqdata);
+
+ /*
+ * For some strange reason spurious interrupts created by the chip when
+ * the interrupt source registers are written cause a deadlock here.
+ * generic_handle_irq calls handle_simple_irq which tries to get
+ * spinlock desc->lock. This interrupt handler is called whilst
+ * __setup_irq holds desc->lock.
+ * The deadlock means that both are running on the same CPU what should
+ * not happen as __setup_irq called raw_spin_lock_irqsave thus disabling
+ * interrupts on this CPU.
+ * Work around this by ignoring interrupts in code protected by
+ * chip_bus_lock (__setup_irq/__free_irq for the respective GPIO hwirq).
+ */
+ if (test_bit(hwirq, meson_gpio_irq_locked))
+ dev_dbg(pc->dev, "spurious interrupt detected!\n");
+ else
+ generic_handle_irq(gpio_irqdata->irq);
+
+ return IRQ_HANDLED;
+}
+
+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;
+ }
+
+ 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;
+}
+
+static void meson_gpio_free_irq_slot(struct irq_data *data)
+{
+ int i;
+
+ mutex_lock(&meson_gpio_irq_slot_mutex);
+
+ for (i = 0; i < meson_gpio_num_irq_slots; i++)
+ if (meson_gpio_irq_slots[i].owner == data->irq) {
+ free_irq(meson_gpio_irq_slots[i].irq, data);
+ meson_gpio_irq_slots[i].owner = 0;
+ }
+
+ mutex_unlock(&meson_gpio_irq_slot_mutex);
+}
+
+static int meson_gpio_find_irq_slot(struct irq_data *data, 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 == data->irq)
+ slots[cnt++] = i;
+
+ mutex_unlock(&meson_gpio_irq_slot_mutex);
+
+ return cnt ?: -EINVAL;
+}
+
+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);
+}
+
+static void meson_gpio_irq_set_hwirq(struct irq_data *data, int hwirq)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ int i, cnt, slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+
+ cnt = meson_gpio_find_irq_slot(data, slots);
+ if (cnt < 0) {
+ dev_err(pc->dev, "didn't find gpio irq slot\n");
+ return;
+ }
+
+ for (i = 0; i < cnt; i++)
+ meson_gpio_set_hwirq(slots[i], hwirq);
+}
+
+static void meson_gpio_irq_unmask(struct irq_data *data)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ unsigned gpio = irqd_to_hwirq(data);
+ int hwirq = meson_gpio_to_irq(pc, gpio);
+
+ meson_gpio_irq_set_hwirq(data, hwirq);
+}
+
+static void meson_gpio_irq_mask(struct irq_data *data)
+{
+ meson_gpio_irq_set_hwirq(data, 0xff);
+}
+
+static void meson_gpio_irq_shutdown(struct irq_data *data)
+{
+ meson_gpio_irq_mask(data);
+ meson_gpio_free_irq_slot(data);
+}
+
+static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ int i, ret, irq, num_slots, slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+ unsigned int val = 0;
+
+ num_slots = (type == IRQ_TYPE_EDGE_BOTH) ? 2 : 1;
+ ret = meson_gpio_alloc_irq_slot(data, num_slots, slots);
+ if (ret)
+ return ret;
+
+ if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_EDGE(slots[0]);
+
+ if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_LOW(slots[0]);
+
+ regmap_update_bits(meson_gpio_irq_regmap, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(slots[0]), val);
+
+ /*
+ * The chip can create an interrupt for either rising or falling edge
+ * only. Therefore use two interrupts in case of IRQ_TYPE_EDGE_BOTH,
+ * first for falling edge and second one for rising edge.
+ */
+ if (num_slots > 1) {
+ val = REG_EDGE_POL_EDGE(slots[1]);
+ regmap_update_bits(meson_gpio_irq_regmap, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(slots[1]), val);
+ }
+
+ if (type & IRQ_TYPE_EDGE_BOTH)
+ val = IRQ_TYPE_EDGE_RISING;
+ else
+ val = IRQ_TYPE_LEVEL_HIGH;
+
+ 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;
+ }
+
+ if (ret)
+ while (--i >= 0)
+ free_irq(meson_gpio_irq_slots[slots[i]].irq, data);
+
+ return ret;
+}
+
+static void meson_gpio_irq_bus_lock(struct irq_data *data)
+{
+ int hwirq = meson_gpio_data_to_hwirq(data);
+
+ set_bit(hwirq, meson_gpio_irq_locked);
+}
+
+static void meson_gpio_irq_bus_sync_unlock(struct irq_data *data)
+{
+ int hwirq = meson_gpio_data_to_hwirq(data);
+
+ clear_bit(hwirq, meson_gpio_irq_locked);
+}
+
+static struct irq_chip meson_gpio_irq_chip = {
+ .name = "GPIO",
+ .irq_set_type = meson_gpio_irq_set_type,
+ .irq_mask = meson_gpio_irq_mask,
+ .irq_unmask = meson_gpio_irq_unmask,
+ .irq_shutdown = meson_gpio_irq_shutdown,
+ .irq_bus_lock = meson_gpio_irq_bus_lock,
+ .irq_bus_sync_unlock = meson_gpio_irq_bus_sync_unlock,
+};
+
+static int meson_gpio_get_irqs(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ int irq, i;
+
+ for (i = 0; i < MESON_GPIO_MAX_PARENT_IRQ_NUM; i++) {
+ irq = irq_of_parse_and_map(np, i);
+ if (!irq)
+ break;
+ meson_gpio_irq_slots[i].irq = irq;
+ }
+
+ meson_gpio_num_irq_slots = i;
+
+ return meson_gpio_num_irq_slots ? 0 : -EINVAL;
+}
+
+static const struct regmap_config meson_gpio_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = REG_FILTER_SEL,
+};
+
+static int meson_gpio_irq_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ void __iomem *io_base;
+ int ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ io_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(io_base))
+ return PTR_ERR(io_base);
+
+ meson_gpio_irq_regmap = devm_regmap_init_mmio(&pdev->dev, io_base, &meson_gpio_regmap_config);
+ if (IS_ERR(meson_gpio_irq_regmap))
+ return PTR_ERR(meson_gpio_irq_regmap);
+
+ /* initialize to IRQ_TYPE_LEVEL_HIGH */
+ regmap_write(meson_gpio_irq_regmap, REG_EDGE_POL, 0);
+ /* disable all GPIO interrupt sources */
+ regmap_write(meson_gpio_irq_regmap, REG_PIN_03_SEL, 0xffffffff);
+ regmap_write(meson_gpio_irq_regmap, REG_PIN_47_SEL, 0xffffffff);
+ /* disable filtering */
+ regmap_write(meson_gpio_irq_regmap, REG_FILTER_SEL, 0);
+
+ ret = meson_gpio_get_irqs(pdev);
+ if (ret)
+ return ret;
+
+ meson_pinctrl_irq_chip = &meson_gpio_irq_chip;
+
+ return 0;
+}
+
+static const struct of_device_id meson_gpio_irq_dt_match[] = {
+ { .compatible = "amlogic,meson-gpio-interrupt" },
+ { },
+};
+
+static struct platform_driver meson_gpio_irq_driver = {
+ .probe = meson_gpio_irq_probe,
+ .driver = {
+ .name = "meson-gpio-interrupt",
+ .of_match_table = meson_gpio_irq_dt_match,
+ },
+};
+builtin_platform_driver(meson_gpio_irq_driver);
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 66ed70c1..3907e032 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -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;
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index 890f296f..35a5da09 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -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;
--
2.12.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
@ 2017-05-12 6:08 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-12 6:08 UTC (permalink / raw)
To: linus-amlogic
Am 11.05.2017 um 21:33 schrieb Heiner Kallweit:
> Am 11.05.2017 um 18:08 schrieb Jerome Brunet:
>> On Thu, 2017-05-11 at 16:50 +0200, Linus Walleij wrote:
>>> On Sun, May 7, 2017 at 6:34 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>>
>>>> From: Jerome Brunet <jbrunet@baylibre.com>
>>>> Add GPIO interrupt information to pinctrl data. Added to the original
>>>> version from Jerome was data for Meson GXL.
>>>>
>>>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>
>>> So what this does is:
>>>
>>>> diff --git a/drivers/pinctrl/meson/pinctrl-meson.h
>>>> b/drivers/pinctrl/meson/pinctrl-meson.h
>>>> index 1aa871d5..890f296f 100644
>>>> --- a/drivers/pinctrl/meson/pinctrl-meson.h
>>>> +++ b/drivers/pinctrl/meson/pinctrl-meson.h
>>>> @@ -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];
>>>> };
>>>
>>> ... 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.
>
> 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.
>
>> 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
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 37af5e30..f8f401a0 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -153,6 +153,7 @@ config PINCTRL_MESON
select PINCONF
select GENERIC_PINCONF
select GPIOLIB
+ select GPIOLIB_IRQCHIP
select OF_GPIO
select REGMAP_MMIO
diff --git a/drivers/pinctrl/meson/Makefile b/drivers/pinctrl/meson/Makefile
index 27c5b512..827e416d 100644
--- a/drivers/pinctrl/meson/Makefile
+++ b/drivers/pinctrl/meson/Makefile
@@ -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
diff --git a/drivers/pinctrl/meson/pinctrl-meson-irq.c b/drivers/pinctrl/meson/pinctrl-meson-irq.c
new file mode 100644
index 00000000..dfc3af77
--- /dev/null
+++ b/drivers/pinctrl/meson/pinctrl-meson-irq.c
@@ -0,0 +1,393 @@
+/*
+ * Amlogic Meson GPIO IRQ driver
+ *
+ * Copyright 2017 Heiner Kallweit <hkallweit1@gmail.com>
+ * Based on a first version by Jerome Brunet <jbrunet@baylibre.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include "pinctrl-meson.h"
+
+#define REG_EDGE_POL 0x00
+#define REG_PIN_03_SEL 0x04
+#define REG_PIN_47_SEL 0x08
+#define REG_FILTER_SEL 0x0c
+
+#define REG_EDGE_POL_MASK(x) (BIT(x) | BIT(16 + (x)))
+#define REG_EDGE_POL_EDGE(x) BIT(x)
+#define REG_EDGE_POL_LOW(x) BIT(16 + (x))
+
+#define MESON_GPIO_MAX_PARENT_IRQ_NUM 8
+
+struct meson_gpio_irq_slot {
+ int irq;
+ int owner;
+};
+
+static struct regmap *meson_gpio_irq_regmap;
+static struct meson_gpio_irq_slot
+ meson_gpio_irq_slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+static int meson_gpio_num_irq_slots;
+static DEFINE_MUTEX(meson_gpio_irq_slot_mutex);
+static DECLARE_BITMAP(meson_gpio_irq_locked, 256);
+
+/**
+ * meson_get_bank() - find the bank containing a given pin
+ *
+ * @pc: the pinctrl instance
+ * @pin: the pin number
+ * @bank: the found bank
+ *
+ * Return: 0 on success, a negative value on error
+ */
+static int meson_get_bank(struct meson_pinctrl *pc, unsigned int pin,
+ struct meson_bank **bank)
+{
+ int i;
+
+ for (i = 0; i < pc->data->num_banks; i++) {
+ if (pin >= pc->data->banks[i].first &&
+ pin <= pc->data->banks[i].last) {
+ *bank = &pc->data->banks[i];
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static struct meson_pinctrl *meson_gpio_data_to_pc(struct irq_data *data)
+{
+ struct gpio_chip *chip = irq_data_get_irq_chip_data(data);
+
+ return gpiochip_get_data(chip);
+}
+
+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 */
+ return -EINVAL;
+
+ hwirq = offset - bank->first + bank->irq_first;
+
+ if (hwirq > bank->irq_last)
+ /* this pin cannot generate irqs */
+ return -EINVAL;
+
+ return hwirq;
+}
+
+static int meson_gpio_to_irq(struct meson_pinctrl *pc, unsigned int offset)
+{
+ struct meson_bank *bank;
+ int ret;
+
+ offset += pc->data->pin_base;
+
+ ret = meson_get_bank(pc, offset, &bank);
+ if (ret)
+ return ret;
+
+ ret = meson_gpio_to_hwirq(bank, offset);
+ if (ret < 0) {
+ dev_dbg(pc->dev, "no interrupt for pin %u\n", offset);
+ return 0;
+ }
+
+ return ret;
+}
+
+static int meson_gpio_data_to_hwirq(struct irq_data *data)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ unsigned gpio = irqd_to_hwirq(data);
+
+ return meson_gpio_to_irq(pc, gpio);
+}
+
+static irqreturn_t meson_gpio_irq_handler(int irq, void *data)
+{
+ struct irq_data *gpio_irqdata = data;
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ int hwirq = meson_gpio_data_to_hwirq(gpio_irqdata);
+
+ /*
+ * For some strange reason spurious interrupts created by the chip when
+ * the interrupt source registers are written cause a deadlock here.
+ * generic_handle_irq calls handle_simple_irq which tries to get
+ * spinlock desc->lock. This interrupt handler is called whilst
+ * __setup_irq holds desc->lock.
+ * The deadlock means that both are running on the same CPU what should
+ * not happen as __setup_irq called raw_spin_lock_irqsave thus disabling
+ * interrupts on this CPU.
+ * Work around this by ignoring interrupts in code protected by
+ * chip_bus_lock (__setup_irq/__free_irq for the respective GPIO hwirq).
+ */
+ if (test_bit(hwirq, meson_gpio_irq_locked))
+ dev_dbg(pc->dev, "spurious interrupt detected!\n");
+ else
+ generic_handle_irq(gpio_irqdata->irq);
+
+ return IRQ_HANDLED;
+}
+
+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;
+ }
+
+ 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;
+}
+
+static void meson_gpio_free_irq_slot(struct irq_data *data)
+{
+ int i;
+
+ mutex_lock(&meson_gpio_irq_slot_mutex);
+
+ for (i = 0; i < meson_gpio_num_irq_slots; i++)
+ if (meson_gpio_irq_slots[i].owner == data->irq) {
+ free_irq(meson_gpio_irq_slots[i].irq, data);
+ meson_gpio_irq_slots[i].owner = 0;
+ }
+
+ mutex_unlock(&meson_gpio_irq_slot_mutex);
+}
+
+static int meson_gpio_find_irq_slot(struct irq_data *data, 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 == data->irq)
+ slots[cnt++] = i;
+
+ mutex_unlock(&meson_gpio_irq_slot_mutex);
+
+ return cnt ?: -EINVAL;
+}
+
+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);
+}
+
+static void meson_gpio_irq_set_hwirq(struct irq_data *data, int hwirq)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ int i, cnt, slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+
+ cnt = meson_gpio_find_irq_slot(data, slots);
+ if (cnt < 0) {
+ dev_err(pc->dev, "didn't find gpio irq slot\n");
+ return;
+ }
+
+ for (i = 0; i < cnt; i++)
+ meson_gpio_set_hwirq(slots[i], hwirq);
+}
+
+static void meson_gpio_irq_unmask(struct irq_data *data)
+{
+ struct meson_pinctrl *pc = meson_gpio_data_to_pc(data);
+ unsigned gpio = irqd_to_hwirq(data);
+ int hwirq = meson_gpio_to_irq(pc, gpio);
+
+ meson_gpio_irq_set_hwirq(data, hwirq);
+}
+
+static void meson_gpio_irq_mask(struct irq_data *data)
+{
+ meson_gpio_irq_set_hwirq(data, 0xff);
+}
+
+static void meson_gpio_irq_shutdown(struct irq_data *data)
+{
+ meson_gpio_irq_mask(data);
+ meson_gpio_free_irq_slot(data);
+}
+
+static int meson_gpio_irq_set_type(struct irq_data *data, unsigned int type)
+{
+ int i, ret, irq, num_slots, slots[MESON_GPIO_MAX_PARENT_IRQ_NUM];
+ unsigned int val = 0;
+
+ num_slots = (type == IRQ_TYPE_EDGE_BOTH) ? 2 : 1;
+ ret = meson_gpio_alloc_irq_slot(data, num_slots, slots);
+ if (ret)
+ return ret;
+
+ if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_EDGE(slots[0]);
+
+ if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING))
+ val |= REG_EDGE_POL_LOW(slots[0]);
+
+ regmap_update_bits(meson_gpio_irq_regmap, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(slots[0]), val);
+
+ /*
+ * The chip can create an interrupt for either rising or falling edge
+ * only. Therefore use two interrupts in case of IRQ_TYPE_EDGE_BOTH,
+ * first for falling edge and second one for rising edge.
+ */
+ if (num_slots > 1) {
+ val = REG_EDGE_POL_EDGE(slots[1]);
+ regmap_update_bits(meson_gpio_irq_regmap, REG_EDGE_POL,
+ REG_EDGE_POL_MASK(slots[1]), val);
+ }
+
+ if (type & IRQ_TYPE_EDGE_BOTH)
+ val = IRQ_TYPE_EDGE_RISING;
+ else
+ val = IRQ_TYPE_LEVEL_HIGH;
+
+ 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;
+ }
+
+ if (ret)
+ while (--i >= 0)
+ free_irq(meson_gpio_irq_slots[slots[i]].irq, data);
+
+ return ret;
+}
+
+static void meson_gpio_irq_bus_lock(struct irq_data *data)
+{
+ int hwirq = meson_gpio_data_to_hwirq(data);
+
+ set_bit(hwirq, meson_gpio_irq_locked);
+}
+
+static void meson_gpio_irq_bus_sync_unlock(struct irq_data *data)
+{
+ int hwirq = meson_gpio_data_to_hwirq(data);
+
+ clear_bit(hwirq, meson_gpio_irq_locked);
+}
+
+static struct irq_chip meson_gpio_irq_chip = {
+ .name = "GPIO",
+ .irq_set_type = meson_gpio_irq_set_type,
+ .irq_mask = meson_gpio_irq_mask,
+ .irq_unmask = meson_gpio_irq_unmask,
+ .irq_shutdown = meson_gpio_irq_shutdown,
+ .irq_bus_lock = meson_gpio_irq_bus_lock,
+ .irq_bus_sync_unlock = meson_gpio_irq_bus_sync_unlock,
+};
+
+static int meson_gpio_get_irqs(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ int irq, i;
+
+ for (i = 0; i < MESON_GPIO_MAX_PARENT_IRQ_NUM; i++) {
+ irq = irq_of_parse_and_map(np, i);
+ if (!irq)
+ break;
+ meson_gpio_irq_slots[i].irq = irq;
+ }
+
+ meson_gpio_num_irq_slots = i;
+
+ return meson_gpio_num_irq_slots ? 0 : -EINVAL;
+}
+
+static const struct regmap_config meson_gpio_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = REG_FILTER_SEL,
+};
+
+static int meson_gpio_irq_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ void __iomem *io_base;
+ int ret;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ io_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(io_base))
+ return PTR_ERR(io_base);
+
+ meson_gpio_irq_regmap = devm_regmap_init_mmio(&pdev->dev, io_base, &meson_gpio_regmap_config);
+ if (IS_ERR(meson_gpio_irq_regmap))
+ return PTR_ERR(meson_gpio_irq_regmap);
+
+ /* initialize to IRQ_TYPE_LEVEL_HIGH */
+ regmap_write(meson_gpio_irq_regmap, REG_EDGE_POL, 0);
+ /* disable all GPIO interrupt sources */
+ regmap_write(meson_gpio_irq_regmap, REG_PIN_03_SEL, 0xffffffff);
+ regmap_write(meson_gpio_irq_regmap, REG_PIN_47_SEL, 0xffffffff);
+ /* disable filtering */
+ regmap_write(meson_gpio_irq_regmap, REG_FILTER_SEL, 0);
+
+ ret = meson_gpio_get_irqs(pdev);
+ if (ret)
+ return ret;
+
+ meson_pinctrl_irq_chip = &meson_gpio_irq_chip;
+
+ return 0;
+}
+
+static const struct of_device_id meson_gpio_irq_dt_match[] = {
+ { .compatible = "amlogic,meson-gpio-interrupt" },
+ { },
+};
+
+static struct platform_driver meson_gpio_irq_driver = {
+ .probe = meson_gpio_irq_probe,
+ .driver = {
+ .name = "meson-gpio-interrupt",
+ .of_match_table = meson_gpio_irq_dt_match,
+ },
+};
+builtin_platform_driver(meson_gpio_irq_driver);
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 66ed70c1..3907e032 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -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;
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index 890f296f..35a5da09 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -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;
--
2.12.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
2017-05-12 6:08 ` Heiner Kallweit
@ 2017-05-12 9:51 ` Linus Walleij
-1 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2017-05-12 9:51 UTC (permalink / raw)
To: Heiner Kallweit, Marc Zyngier, Thomas Gleixner
Cc: Jerome Brunet, thierry.reding, Thierry Reding, Mark Rutland,
devicetree, open list:ARM/Amlogic Meson...,
linux-gpio
On Fri, May 12, 2017 at 8:08 AM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
@ 2017-05-12 9:51 ` Linus Walleij
0 siblings, 0 replies; 36+ messages in thread
From: Linus Walleij @ 2017-05-12 9:51 UTC (permalink / raw)
To: linus-amlogic
On Fri, May 12, 2017 at 8:08 AM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
2017-05-12 9:51 ` Linus Walleij
@ 2017-05-12 17:23 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-12 17:23 UTC (permalink / raw)
To: Linus Walleij, Marc Zyngier, Thomas Gleixner
Cc: Jerome Brunet, thierry.reding, Thierry Reding, Mark Rutland,
devicetree, open list:ARM/Amlogic Meson...,
linux-gpio
Am 12.05.2017 um 11:51 schrieb Linus Walleij:
> On Fri, May 12, 2017 at 8:08 AM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
>> 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.
>
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.
> 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
> Yours,
> Linus Walleij
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data
@ 2017-05-12 17:23 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-12 17:23 UTC (permalink / raw)
To: linus-amlogic
Am 12.05.2017 um 11:51 schrieb Linus Walleij:
> On Fri, May 12, 2017 at 8:08 AM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
>> 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.
>
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.
> 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
> Yours,
> Linus Walleij
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding
2017-05-07 16:34 ` Heiner Kallweit
@ 2017-05-12 19:38 ` Rob Herring
-1 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2017-05-12 19:38 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA
On Sun, May 07, 2017 at 06:34:09PM +0200, Heiner Kallweit wrote:
> 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 <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../bindings/gpio/amlogic,meson-gpio-interrupt.txt | 30 ++++++++++++++++++++++
Seems more like an irqchip?
> 1 file changed, 30 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
> new file mode 100644
> index 00000000..35a052b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
> @@ -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".
Why 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
What about interrupt-controller property?
> +
> +Example:
> +
> +gpio_irq@9880 {
interrupt-controller@...
> + 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>;
> + };
> --
> 2.12.2
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding
@ 2017-05-12 19:38 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2017-05-12 19:38 UTC (permalink / raw)
To: linus-amlogic
On Sun, May 07, 2017 at 06:34:09PM +0200, Heiner Kallweit wrote:
> 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 <hkallweit1@gmail.com>
> ---
> .../bindings/gpio/amlogic,meson-gpio-interrupt.txt | 30 ++++++++++++++++++++++
Seems more like an irqchip?
> 1 file changed, 30 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
> new file mode 100644
> index 00000000..35a052b8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
> @@ -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".
Why 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
What about interrupt-controller property?
> +
> +Example:
> +
> +gpio_irq at 9880 {
interrupt-controller at ...
> + 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>;
> + };
> --
> 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
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding
2017-05-12 19:38 ` Rob Herring
@ 2017-05-12 21:41 ` Heiner Kallweit
-1 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-12 21:41 UTC (permalink / raw)
To: Rob Herring
Cc: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij,
devicetree, linux-amlogic, linux-gpio
Am 12.05.2017 um 21:38 schrieb Rob Herring:
> On Sun, May 07, 2017 at 06:34:09PM +0200, Heiner Kallweit wrote:
>> 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 <hkallweit1@gmail.com>
>> ---
>> .../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?
>> 1 file changed, 30 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>> new file mode 100644
>> index 00000000..35a052b8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>> @@ -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".
>
> Why syscon?
>
Has been removed already in v2 of the patch.
>> +- 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?
>
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.
>> +
>> +Example:
>> +
>> +gpio_irq@9880 {
>
> interrupt-controller@...
>
This relates to previous comment, as it isn't really an interrupt
controller I'm not sure we should use this name.
>> + 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>;
>> + };
>> --
>> 2.12.2
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding
@ 2017-05-12 21:41 ` Heiner Kallweit
0 siblings, 0 replies; 36+ messages in thread
From: Heiner Kallweit @ 2017-05-12 21:41 UTC (permalink / raw)
To: linus-amlogic
Am 12.05.2017 um 21:38 schrieb Rob Herring:
> On Sun, May 07, 2017 at 06:34:09PM +0200, Heiner Kallweit wrote:
>> 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 <hkallweit1@gmail.com>
>> ---
>> .../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?
>> 1 file changed, 30 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>> new file mode 100644
>> index 00000000..35a052b8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>> @@ -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".
>
> Why syscon?
>
Has been removed already in v2 of the patch.
>> +- 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?
>
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.
>> +
>> +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.
>> + 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>;
>> + };
>> --
>> 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
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding
2017-05-12 21:41 ` Heiner Kallweit
@ 2017-05-16 0:31 ` Rob Herring
-1 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2017-05-16 0:31 UTC (permalink / raw)
To: Heiner Kallweit
Cc: Jerome Brunet, Mark Rutland, Marc Zyngier, Linus Walleij,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-gpio-u79uwXL29TY76Z2rM5mHXA
On Fri, May 12, 2017 at 4:41 PM, Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Am 12.05.2017 um 21:38 schrieb Rob Herring:
>> On Sun, May 07, 2017 at 06:34:09PM +0200, Heiner Kallweit wrote:
>>> 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 <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>> .../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
>
>>> 1 file changed, 30 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>>> new file mode 100644
>>> index 00000000..35a052b8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>>> @@ -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".
>>
>> Why syscon?
>>
> Has been removed already in v2 of the patch.
>
>>> +- 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?
>>
> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding
@ 2017-05-16 0:31 ` Rob Herring
0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2017-05-16 0:31 UTC (permalink / raw)
To: linus-amlogic
On Fri, May 12, 2017 at 4:41 PM, Heiner Kallweit <hkallweit1@gmail.com> wrote:
> Am 12.05.2017 um 21:38 schrieb Rob Herring:
>> On Sun, May 07, 2017 at 06:34:09PM +0200, Heiner Kallweit wrote:
>>> 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 <hkallweit1@gmail.com>
>>> ---
>>> .../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
>
>>> 1 file changed, 30 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>>> new file mode 100644
>>> index 00000000..35a052b8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/gpio/amlogic,meson-gpio-interrupt.txt
>>> @@ -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".
>>
>> Why syscon?
>>
> Has been removed already in v2 of the patch.
>
>>> +- 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?
>>
> 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
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2017-05-16 0:31 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-07 16:19 [PATCH 0/5 RfC] pintrl: meson: add support for GPIO IRQs Heiner Kallweit
2017-05-07 16:19 ` Heiner Kallweit
2017-05-07 16:34 ` [PATCH 1/5] pintrl: meson: add interrupts to pinctrl data Heiner Kallweit
2017-05-07 16:34 ` Heiner Kallweit
2017-05-11 14:50 ` Linus Walleij
2017-05-11 14:50 ` Linus Walleij
2017-05-11 16:08 ` Jerome Brunet
2017-05-11 16:08 ` Jerome Brunet
2017-05-11 19:33 ` Heiner Kallweit
2017-05-11 19:33 ` Heiner Kallweit
[not found] ` <7ed5a88a-43dd-f7ab-2794-48ceb5a62a93-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-05-12 6:08 ` Heiner Kallweit
2017-05-12 6:08 ` Heiner Kallweit
2017-05-12 9:51 ` Linus Walleij
2017-05-12 9:51 ` Linus Walleij
2017-05-12 17:23 ` Heiner Kallweit
2017-05-12 17:23 ` Heiner Kallweit
2017-05-07 16:34 ` [PATCH 2/5] pintrl: meson: document GPIO IRQ DT binding Heiner Kallweit
2017-05-07 16:34 ` Heiner Kallweit
[not found] ` <0d835130-7c6c-751c-af15-c2ab69edcb42-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-05-12 19:38 ` Rob Herring
2017-05-12 19:38 ` Rob Herring
2017-05-12 21:41 ` Heiner Kallweit
2017-05-12 21:41 ` Heiner Kallweit
[not found] ` <b39f1742-0558-beaa-1e06-eed80d644424-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-05-16 0:31 ` Rob Herring
2017-05-16 0:31 ` Rob Herring
2017-05-07 16:34 ` [PATCH 3/5] pintrl: meson: add DT node for GPIO IRQ on Meson GX Heiner Kallweit
2017-05-07 16:34 ` Heiner Kallweit
2017-05-07 16:34 ` [PATCH 4/5] pintrl: meson: add DT node for GPIO IRQ on Meson 8 / 8b Heiner Kallweit
2017-05-07 16:34 ` Heiner Kallweit
[not found] ` <a4ca587b-d3e8-a22d-4d25-289871d2d095-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-05-07 16:34 ` [PATCH 5/5] pintrl: meson: add support for GPIO interrupts Heiner Kallweit
2017-05-07 16:34 ` Heiner Kallweit
2017-05-11 15:10 ` Linus Walleij
2017-05-11 15:10 ` Linus Walleij
2017-05-11 14:34 ` [PATCH 0/5 RfC] pintrl: meson: add support for GPIO IRQs Linus Walleij
2017-05-11 14:34 ` Linus Walleij
[not found] ` <CACRpkdbeA9pwnOFjdryKjeyZewLWA7Q6ZRbxc=MreT0zJMbgQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-05-11 20:52 ` Martin Blumenstingl
2017-05-11 20:52 ` Martin Blumenstingl
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.