[PATCH v3 0/6] pinctrl: meson: enable support for external GPIO interrupts

[PATCH v3 0/6] pinctrl: meson: enable support for external GPIO interrupts

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

In Meson SoCs we have 8 independent GPIO interrupts that can be programmed to
use any of the GPIOs in the chip as interrupt source.

These GPIOs are managed by GIC but they can be conditioned (and enabled) by
some registers external to the GIC.

GPIOs |--[mux1 or mux2]--[polarity]--[filter]--[edge_select]--> GIC

Changelog:

* V2:
    - Introduced .irq_request_resources() and .irq_release_resources()
    - s/virq/irq/ and s/pin/hwirq/
    - Moved to the new irq_fwspec

* V3:
    - EXPORT_SYMBOL_GPL for of_irq_find_parent()
    - split GPIO management and irqchip side
    - the GIC IRQs list is not kept as set of fwspecs. it's now a regular
      integer array.
    - irq_of_phandle_args_to_fwspec discarded


Carlo Caione (6):
  of/irq: Export of_irq_find_parent again
  pinctrl: meson: Update pinctrl data with GPIO IRQ info
  pinctrl: meson: Make helper functions public
  pinctrl: meson: Enable GPIO IRQs
  pinctrl: dt-binding: Extend meson documentation with GPIO IRQs support
  ARM: meson: DTS: Enable GPIO IRQs

 .../devicetree/bindings/pinctrl/meson,pinctrl.txt  |  11 +
 arch/arm/boot/dts/meson8b.dtsi                     |   5 +
 drivers/of/irq.c                                   |   3 +-
 drivers/pinctrl/Kconfig                            |   1 +
 drivers/pinctrl/meson/Makefile                     |   2 +-
 drivers/pinctrl/meson/irqchip-gpio-meson.c         | 321 +++++++++++++++++++++
 drivers/pinctrl/meson/pinctrl-meson.c              |  40 ++-
 drivers/pinctrl/meson/pinctrl-meson.h              |  28 +-
 drivers/pinctrl/meson/pinctrl-meson8.c             |  36 ++-
 drivers/pinctrl/meson/pinctrl-meson8b.c            |  36 ++-
 include/linux/of_irq.h                             |   6 +
 11 files changed, 451 insertions(+), 38 deletions(-)
 create mode 100644 drivers/pinctrl/meson/irqchip-gpio-meson.c

-- 
2.5.0

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

[PATCH v3 1/6] of/irq: Export of_irq_find_parent again

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

of_irq_find_parent was made static since it had no users outside of
of_irq.c. Export it again since we are going to use it again.

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 drivers/of/irq.c       | 3 ++-
 include/linux/of_irq.h | 6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 902b89b..4fa916d 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -53,7 +53,7 @@ EXPORT_SYMBOL_GPL(irq_of_parse_and_map);
  * Returns a pointer to the interrupt parent node, or NULL if the interrupt
  * parent could not be determined.
  */
-static struct device_node *of_irq_find_parent(struct device_node *child)
+struct device_node *of_irq_find_parent(struct device_node *child)
 {
 	struct device_node *p;
 	const __be32 *parp;
@@ -77,6 +77,7 @@ static struct device_node *of_irq_find_parent(struct device_node *child)
 
 	return p;
 }
+EXPORT_SYMBOL_GPL(of_irq_find_parent);
 
 /**
  * of_irq_parse_raw - Low level interrupt tree parsing
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index 039f2ee..6f879c6 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -94,6 +94,7 @@ static inline void of_msi_configure(struct device *dev, struct device_node *np)
  */
 extern unsigned int irq_of_parse_and_map(struct device_node *node, int index);
 u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in);
+extern struct device_node *of_irq_find_parent(struct device_node *child);
 
 #else /* !CONFIG_OF && !CONFIG_SPARC */
 static inline unsigned int irq_of_parse_and_map(struct device_node *dev,
@@ -107,6 +108,11 @@ static inline u32 of_msi_map_rid(struct device *dev,
 {
 	return rid_in;
 }
+
+static inline void *of_irq_find_parent(struct device_node *child)
+{
+	return NULL;
+}
 #endif /* !CONFIG_OF */
 
 #endif /* __OF_IRQ_H */
-- 
2.5.0

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

Re: [PATCH v3 1/6] of/irq: Export of_irq_find_parent again

[Rob Herring]

On Tue, Dec 1, 2015 at 10:24 AM, Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org> wrote:
> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
>
> of_irq_find_parent was made static since it had no users outside of
> of_irq.c. Export it again since we are going to use it again.
>
> Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/of/irq.c       | 3 ++-
>  include/linux/of_irq.h | 6 ++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 902b89b..4fa916d 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -53,7 +53,7 @@ EXPORT_SYMBOL_GPL(irq_of_parse_and_map);
>   * Returns a pointer to the interrupt parent node, or NULL if the interrupt
>   * parent could not be determined.
>   */
> -static struct device_node *of_irq_find_parent(struct device_node *child)
> +struct device_node *of_irq_find_parent(struct device_node *child)
>  {
>         struct device_node *p;
>         const __be32 *parp;
> @@ -77,6 +77,7 @@ static struct device_node *of_irq_find_parent(struct device_node *child)
>
>         return p;
>  }
> +EXPORT_SYMBOL_GPL(of_irq_find_parent);
>
>  /**
>   * of_irq_parse_raw - Low level interrupt tree parsing
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index 039f2ee..6f879c6 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -94,6 +94,7 @@ static inline void of_msi_configure(struct device *dev, struct device_node *np)
>   */
>  extern unsigned int irq_of_parse_and_map(struct device_node *node, int index);
>  u32 of_msi_map_rid(struct device *dev, struct device_node *msi_np, u32 rid_in);
> +extern struct device_node *of_irq_find_parent(struct device_node *child);

The whole reason this got removed started with there was a problem
with building for OF=y and OF_IRQ=n, so you need to move the
declarations to the OF_IRQ if/else section.

Given multiple people needing this, I'm going to apply this for 4.4 if
we can get this sorted out.

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

[PATCH v3 2/6] pinctrl: meson: Update pinctrl data with GPIO IRQ info

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

This patch extends the pinctrl SoC specific data adding two new
information: IRQ register and last pin number in the SoC.
These two numbers are used in the GPIO IRQ controller.

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 drivers/pinctrl/meson/pinctrl-meson.h   |  6 +++++-
 drivers/pinctrl/meson/pinctrl-meson8.c  | 36 +++++++++++++++++++--------------
 drivers/pinctrl/meson/pinctrl-meson8b.c | 36 +++++++++++++++++++--------------
 3 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index 0fe7d53..374f4b6 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -83,6 +83,7 @@ enum meson_reg_type {
  * @first:	first pin of the bank
  * @last:	last pin of the bank
  * @regs:	array of register descriptors
+ * @irq:	input mux location for IRQs
  *
  * A bank represents a set of pins controlled by a contiguous set of
  * bits in the domain registers. The structure specifies which bits in
@@ -94,6 +95,7 @@ struct meson_bank {
 	unsigned int first;
 	unsigned int last;
 	struct meson_reg_desc regs[NUM_REG];
+	unsigned int irq;
 };
 
 /**
@@ -145,6 +147,7 @@ struct meson_pinctrl_data {
 	unsigned int num_groups;
 	unsigned int num_funcs;
 	unsigned int num_domains;
+	unsigned int last_pin;
 };
 
 struct meson_pinctrl {
@@ -192,11 +195,12 @@ 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, per, peb, pr, pb, dr, db, or, ob, ir, ib, i)	\
 	{								\
 		.name	= n,						\
 		.first	= f,						\
 		.last	= l,						\
+		.irq	= i,						\
 		.regs	= {						\
 			[REG_PULLEN]	= { per, peb },			\
 			[REG_PULL]	= { pr, pb },			\
diff --git a/drivers/pinctrl/meson/pinctrl-meson8.c b/drivers/pinctrl/meson/pinctrl-meson8.c
index 7b1cc91..d941568 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8.c
@@ -14,7 +14,12 @@
 #include <dt-bindings/gpio/meson8-gpio.h>
 #include "pinctrl-meson.h"
 
-#define AO_OFF	120
+#define EE_BASE		0
+#define EE_NPINS	120
+#define AO_BASE		120
+#define AO_NPINS	16
+
+#define AO_OFF		AO_BASE
 
 static const struct pinctrl_pin_desc meson8_pins[] = {
 	MESON_PIN(GPIOX_0, 0),
@@ -907,19 +912,19 @@ static struct meson_pmx_func meson8_functions[] = {
 };
 
 static struct meson_bank meson8_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                 pullen  pull    dir     out     in     irq */
+	BANK("X",    PIN(GPIOX_0, 0),  PIN(GPIOX_21, 0),    4,  0,  4,  0,  0,  0,  1,  0,  2,  0, 112),
+	BANK("Y",    PIN(GPIOY_0, 0),  PIN(GPIOY_16, 0),    3,  0,  3,  0,  3,  0,  4,  0,  5,  0,  95),
+	BANK("DV",   PIN(GPIODV_0, 0), PIN(GPIODV_29, 0),   0,  0,  0,  0,  7,  0,  8,  0,  9,  0,  65),
+	BANK("H",    PIN(GPIOH_0, 0),  PIN(GPIOH_9, 0),     1, 16,  1, 16,  9, 19, 10, 19, 11, 19,  29),
+	BANK("Z",    PIN(GPIOZ_0, 0),  PIN(GPIOZ_14, 0),    1,  0,  1,  0,  3, 17,  4, 17,  5, 17,  14),
+	BANK("CARD", PIN(CARD_0, 0),   PIN(CARD_6, 0),      2, 20,  2, 20,  0, 22,  1, 22,  2, 22,  58),
+	BANK("BOOT", PIN(BOOT_0, 0),   PIN(BOOT_18, 0),     2,  0,  2,  0,  9,  0, 10,  0, 11,  0,  39),
 };
 
 static struct meson_bank meson8_ao_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                      pullen  pull    dir     out     in     irq */
+	BANK("AO",   PIN(GPIOAO_0, AO_OFF), PIN(GPIO_TEST_N, AO_OFF), 0,  0,  0, 16,  0,  0,  0, 16,  1,  0,  0),
 };
 
 static struct meson_domain_data meson8_domain_data[] = {
@@ -927,15 +932,15 @@ static struct meson_domain_data meson8_domain_data[] = {
 		.name		= "banks",
 		.banks		= meson8_banks,
 		.num_banks	= ARRAY_SIZE(meson8_banks),
-		.pin_base	= 0,
-		.num_pins	= 120,
+		.pin_base	= EE_BASE,
+		.num_pins	= EE_NPINS,
 	},
 	{
 		.name		= "ao-bank",
 		.banks		= meson8_ao_banks,
 		.num_banks	= ARRAY_SIZE(meson8_ao_banks),
-		.pin_base	= 120,
-		.num_pins	= 16,
+		.pin_base	= AO_BASE,
+		.num_pins	= AO_NPINS,
 	},
 };
 
@@ -948,4 +953,5 @@ struct meson_pinctrl_data meson8_pinctrl_data = {
 	.num_groups	= ARRAY_SIZE(meson8_groups),
 	.num_funcs	= ARRAY_SIZE(meson8_functions),
 	.num_domains	= ARRAY_SIZE(meson8_domain_data),
+	.last_pin	= EE_NPINS + AO_NPINS,
 };
diff --git a/drivers/pinctrl/meson/pinctrl-meson8b.c b/drivers/pinctrl/meson/pinctrl-meson8b.c
index 9677807..c921ae3 100644
--- a/drivers/pinctrl/meson/pinctrl-meson8b.c
+++ b/drivers/pinctrl/meson/pinctrl-meson8b.c
@@ -15,7 +15,12 @@
 #include <dt-bindings/gpio/meson8b-gpio.h>
 #include "pinctrl-meson.h"
 
-#define AO_OFF	130
+#define EE_BASE		0
+#define EE_NPINS	130
+#define AO_BASE		130
+#define AO_NPINS	16
+
+#define AO_OFF		AO_BASE
 
 static const struct pinctrl_pin_desc meson8b_pins[] = {
 	MESON_PIN(GPIOX_0, 0),
@@ -855,19 +860,19 @@ static struct meson_pmx_func meson8b_functions[] = {
 };
 
 static struct meson_bank meson8b_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                   pullen  pull    dir     out     in     irq */
+	BANK("X",    PIN(GPIOX_0, 0),		PIN(GPIOX_21, 0),      4,  0,  4,  0,  0,  0,  1,  0,  2,  0,  97),
+	BANK("Y",    PIN(GPIOY_0, 0),		PIN(GPIOY_14, 0),      3,  0,  3,  0,  3,  0,  4,  0,  5,  0,  80),
+	BANK("DV",   PIN(GPIODV_9, 0),		PIN(GPIODV_29, 0),     0,  0,  0,  0,  7,  0,  8,  0,  9,  0,  59),
+	BANK("H",    PIN(GPIOH_0, 0),		PIN(GPIOH_9, 0),       1, 16,  1, 16,  9, 19, 10, 19, 11, 19,  14),
+	BANK("CARD", PIN(CARD_0, 0),		PIN(CARD_6, 0),        2, 20,  2, 20,  0, 22,  1, 22,  2, 22,  43),
+	BANK("BOOT", PIN(BOOT_0, 0),		PIN(BOOT_18, 0),       2,  0,  2,  0,  9,  0, 10,  0, 11,  0,  24),
+	BANK("DIF",  PIN(DIF_0_P, 0),		PIN(DIF_4_N, 0),       5,  8,  5,  8, 12, 12, 13, 12, 14, 12, 119),
 };
 
 static struct meson_bank meson8b_ao_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                      pullen  pull    dir     out     in    irq */
+	BANK("AO",   PIN(GPIOAO_0, AO_OFF), PIN(GPIO_TEST_N, AO_OFF), 0,  0,  0, 16,  0,  0,  0, 16,  1,  0,  0),
 };
 
 static struct meson_domain_data meson8b_domain_data[] = {
@@ -875,15 +880,15 @@ static struct meson_domain_data meson8b_domain_data[] = {
 		.name		= "banks",
 		.banks		= meson8b_banks,
 		.num_banks	= ARRAY_SIZE(meson8b_banks),
-		.pin_base	= 0,
-		.num_pins	= 130,
+		.pin_base	= EE_BASE,
+		.num_pins	= EE_NPINS,
 	},
 	{
 		.name		= "ao-bank",
 		.banks		= meson8b_ao_banks,
 		.num_banks	= ARRAY_SIZE(meson8b_ao_banks),
-		.pin_base	= 130,
-		.num_pins	= 16,
+		.pin_base	= AO_BASE,
+		.num_pins	= AO_NPINS,
 	},
 };
 
@@ -896,4 +901,5 @@ struct meson_pinctrl_data meson8b_pinctrl_data = {
 	.num_groups	= ARRAY_SIZE(meson8b_groups),
 	.num_funcs	= ARRAY_SIZE(meson8b_functions),
 	.num_domains	= ARRAY_SIZE(meson8b_domain_data),
+	.last_pin	= EE_NPINS + AO_NPINS,
 };
-- 
2.5.0

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

[PATCH v3 3/6] pinctrl: meson: Make helper functions public

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

meson_get_domain_and_bank() and meson_map_resource() are not static
anymore.

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 drivers/pinctrl/meson/pinctrl-meson.c | 10 +++++-----
 drivers/pinctrl/meson/pinctrl-meson.h |  7 +++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 84943e4..0c5655b 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -99,9 +99,9 @@ static int meson_get_bank(struct meson_domain *domain, unsigned int pin,
  *
  * Return:	0 on success, a negative value on error
  */
-static int meson_get_domain_and_bank(struct meson_pinctrl *pc, unsigned int pin,
-				     struct meson_domain **domain,
-				     struct meson_bank **bank)
+int meson_get_domain_and_bank(struct meson_pinctrl *pc, unsigned int pin,
+			      struct meson_domain **domain,
+			      struct meson_bank **bank)
 {
 	struct meson_domain *d;
 	int i;
@@ -618,8 +618,8 @@ static struct regmap_config meson_regmap_config = {
 	.reg_stride = 4,
 };
 
-static struct regmap *meson_map_resource(struct meson_pinctrl *pc,
-					 struct device_node *node, char *name)
+struct regmap *meson_map_resource(struct meson_pinctrl *pc,
+				  struct device_node *node, char *name)
 {
 	struct resource res;
 	void __iomem *base;
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index 374f4b6..a0bf705 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -214,3 +214,10 @@ struct meson_pinctrl {
 
 extern struct meson_pinctrl_data meson8_pinctrl_data;
 extern struct meson_pinctrl_data meson8b_pinctrl_data;
+
+int meson_get_domain_and_bank(struct meson_pinctrl *pc, unsigned int pin,
+			      struct meson_domain **domain,
+			      struct meson_bank **bank);
+
+struct regmap *meson_map_resource(struct meson_pinctrl *pc,
+				  struct device_node *node, char *name);
-- 
2.5.0

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

[PATCH v3 4/6] pinctrl: meson: Enable GPIO IRQs

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

On Meson8 and Meson8b SoCs there are 8 independent filtered GPIO
interrupt modules that can be programmed to use any of the GPIOs in the
chip as an interrupt source.

For each GPIO IRQ we have:

GPIOs --> [mux]--> [polarity]--> [filter]--> [edge select]--> GIC

The eight GPIO interrupts respond to mask/unmask/clear/etc.. just like
any other interrupt in the chip. The difference for the GPIO interrupts
is that they can be filtered and conditioned.

This patch adds support for the external GPIOs interrupts and enables
them for Meson8 and Meson8b SoCs.

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
Signed-off-by: Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/pinctrl/Kconfig                    |   1 +
 drivers/pinctrl/meson/Makefile             |   2 +-
 drivers/pinctrl/meson/irqchip-gpio-meson.c | 321 +++++++++++++++++++++++++++++
 drivers/pinctrl/meson/pinctrl-meson.c      |  30 +++
 drivers/pinctrl/meson/pinctrl-meson.h      |  15 ++
 5 files changed, 368 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pinctrl/meson/irqchip-gpio-meson.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index b422e4e..07097d3 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -130,6 +130,7 @@ config PINCTRL_MESON
 	select GPIOLIB
 	select OF_GPIO
 	select REGMAP_MMIO
+	select IRQ_DOMAIN_HIERARCHY
 
 config PINCTRL_ROCKCHIP
 	bool
diff --git a/drivers/pinctrl/meson/Makefile b/drivers/pinctrl/meson/Makefile
index c751d22..8e83780 100644
--- a/drivers/pinctrl/meson/Makefile
+++ b/drivers/pinctrl/meson/Makefile
@@ -1,2 +1,2 @@
 obj-y	+= pinctrl-meson8.o pinctrl-meson8b.o
-obj-y	+= pinctrl-meson.o
+obj-y	+= pinctrl-meson.o irqchip-gpio-meson.o
diff --git a/drivers/pinctrl/meson/irqchip-gpio-meson.c b/drivers/pinctrl/meson/irqchip-gpio-meson.c
new file mode 100644
index 0000000..0a4b250
--- /dev/null
+++ b/drivers/pinctrl/meson/irqchip-gpio-meson.c
@@ -0,0 +1,321 @@
+/*
+ * GPIO IRQ driver for Amlogic Meson SoCs
+ *
+ * Copyright (C) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/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
+ * version 2 as published by the Free Software Foundation.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Amlogic Meson SoCs have only a limited number of IRQs on the GIC side that
+ * can be used for the GPIOs.
+ *
+ * GPIO# -> [mux] -> [polarity] -> [filter] -> [edge select] -> GIC IRQ#
+ *
+ * The GPIO used to trigger the IRQ is chosen by filling a bitmask in the 'mux'
+ * registers.
+ *
+ * The bitmask position determines the IRQ
+ *
+ * GPIO -> [mux1 [7:0]]   -> ... -> GIC / GPIO IRQ0
+ * GPIO -> [mux1 [15:8]]  -> ... -> GIC / GPIO IRQ1
+ * ...
+ * GPIO -> [mux2 [23:16]] -> ... -> GIC / GPIO IRQ6
+ * ...
+ *
+ * The bitmask value determines the GPIO used to trigger the IRQ
+ *
+ * GPIOX_21 -> 118 in the mux# bitmask register
+ * ...
+ * GPIOH_9  -> 23 in the mux# bitmask register
+ * ...
+ *
+ */
+
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/of_irq.h>
+
+#include "pinctrl-meson.h"
+
+#define REG_EDGE_POL		0x00
+#define REG_GPIO_SEL0		0x04
+#define REG_GPIO_SEL1		0x08
+#define REG_FILTER		0x0c
+
+#define IRQ_FREE		(-1)
+
+#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))
+
+static int meson_get_gic_irq(struct meson_pinctrl *pc, int hwirq)
+{
+	int i = 0;
+
+	for (i = 0; i < pc->num_gic_irqs; i++) {
+		if (pc->irq_map[i] == hwirq)
+			return i;
+	}
+
+	return -1;
+}
+
+static int meson_irq_set_type(struct irq_data *data, unsigned int type)
+{
+	struct meson_pinctrl *pc = irq_data_get_irq_chip_data(data);
+	u32 val = 0;
+	int index;
+
+	dev_dbg(pc->dev, "set type of hwirq %lu to %u\n", data->hwirq, type);
+	spin_lock(&pc->lock);
+	index = meson_get_gic_irq(pc, data->hwirq);
+
+	if (index < 0) {
+		spin_unlock(&pc->lock);
+		dev_err(pc->dev, "hwirq %lu not allocated\n", data->hwirq);
+		return -EINVAL;
+	}
+
+	if (type == IRQ_TYPE_EDGE_FALLING || type == IRQ_TYPE_EDGE_RISING)
+		val |= REG_EDGE_POL_EDGE(index);
+	if (type == IRQ_TYPE_EDGE_FALLING || type == IRQ_TYPE_LEVEL_LOW)
+		val |= REG_EDGE_POL_LOW(index);
+
+	regmap_update_bits(pc->reg_irq, REG_EDGE_POL, REG_EDGE_POL_MASK(index),
+			   val);
+	spin_unlock(&pc->lock);
+
+	if (type == IRQ_TYPE_LEVEL_LOW)
+		type = IRQ_TYPE_LEVEL_HIGH;
+	else if (type == IRQ_TYPE_EDGE_FALLING)
+		type = IRQ_TYPE_EDGE_RISING;
+
+	return irq_chip_set_type_parent(data, type);
+}
+
+int meson_irq_request_resources(struct irq_data *data)
+{
+	struct meson_pinctrl *pc = irq_data_get_irq_chip_data(data);
+	struct meson_domain *domain;
+	struct meson_bank *bank;
+
+	if (meson_get_domain_and_bank(pc, data->hwirq, &domain, &bank))
+		return -EINVAL;
+
+	if (gpiochip_lock_as_irq(&domain->chip, data->hwirq))
+		return -EINVAL;
+
+	return 0;
+}
+
+void meson_irq_release_resources(struct irq_data *data)
+{
+	struct meson_pinctrl *pc = irq_data_get_irq_chip_data(data);
+	struct meson_domain *domain;
+	struct meson_bank *bank;
+
+	if (meson_get_domain_and_bank(pc, data->hwirq, &domain, &bank))
+		return;
+
+	gpiochip_unlock_as_irq(&domain->chip, data->hwirq);
+}
+
+static struct irq_chip meson_irq_chip = {
+	.name			= "meson-gpio-irqchip",
+	.irq_mask		= irq_chip_mask_parent,
+	.irq_unmask		= irq_chip_unmask_parent,
+	.irq_eoi		= irq_chip_eoi_parent,
+	.irq_set_type		= meson_irq_set_type,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+	.irq_request_resources	= meson_irq_request_resources,
+	.irq_release_resources	= meson_irq_release_resources,
+};
+
+static int meson_map_gic_irq(struct irq_domain *irq_domain,
+			     irq_hw_number_t hwirq)
+{
+	struct meson_pinctrl *pc = irq_domain->host_data;
+	struct meson_domain *domain;
+	struct meson_bank *bank;
+	int index, reg, ret;
+
+	ret = meson_get_domain_and_bank(pc, hwirq, &domain, &bank);
+	if (ret)
+		return ret;
+
+	spin_lock(&pc->lock);
+
+	index = meson_get_gic_irq(pc, IRQ_FREE);
+	if (index < 0) {
+		spin_unlock(&pc->lock);
+		dev_err(pc->dev, "no free GIC interrupt found");
+		return -ENOSPC;
+	}
+
+	dev_dbg(pc->dev, "found free GIC interrupt %d\n", index);
+	pc->irq_map[index] = hwirq;
+
+	/* Setup IRQ mapping */
+	reg = index < 4 ? REG_GPIO_SEL0 : REG_GPIO_SEL1;
+	regmap_update_bits(pc->reg_irq, reg, 0xff << (index % 4) * 8,
+			  (bank->irq + hwirq - bank->first) << (index % 4) * 8);
+
+	/* Set filter to the default, undocumented value of 7 */
+	regmap_update_bits(pc->reg_irq, REG_FILTER, 0xf << index * 4,
+			   7 << index * 4);
+
+	spin_unlock(&pc->lock);
+
+	return index;
+}
+
+static int meson_irq_domain_alloc(struct irq_domain *domain, unsigned int irq,
+				  unsigned int nr_irqs, void *arg)
+{
+	struct meson_pinctrl *pc = domain->host_data;
+	struct irq_fwspec *irq_data = arg;
+	struct irq_fwspec gic_data;
+	irq_hw_number_t hwirq;
+	int index, ret, i;
+
+	if (irq_data->param_count != 2)
+		return -EINVAL;
+
+	hwirq = irq_data->param[0];
+	dev_dbg(pc->dev, "%s irq %d, nr %d, hwirq %lu\n",
+			__func__, irq, nr_irqs, hwirq);
+
+	for (i = 0; i < nr_irqs; i++) {
+		index = meson_map_gic_irq(domain, hwirq + i);
+		if (index < 0)
+			return index;
+
+		irq_domain_set_hwirq_and_chip(domain, irq + i,
+					      hwirq + i,
+					      &meson_irq_chip,
+					      pc);
+
+		gic_data.param_count = 3;
+		gic_data.fwnode = domain->parent->fwnode;
+		gic_data.param[0] = 0; /* SPI */
+		gic_data.param[1] = pc->gic_irqs[index];
+		gic_data.param[1] = IRQ_TYPE_EDGE_RISING;
+
+		ret = irq_domain_alloc_irqs_parent(domain, irq + i, nr_irqs,
+						   &gic_data);
+	}
+
+	return 0;
+}
+
+static void meson_irq_domain_free(struct irq_domain *domain, unsigned int irq,
+				  unsigned int nr_irqs)
+{
+	struct meson_pinctrl *pc = domain->host_data;
+	struct irq_data *irq_data;
+	int index, i;
+
+	spin_lock(&pc->lock);
+	for (i = 0; i < nr_irqs; i++) {
+		irq_data = irq_domain_get_irq_data(domain, irq + i);
+		index = meson_get_gic_irq(pc, irq_data->hwirq);
+		if (index < 0)
+			continue;
+		pc->irq_map[index] = IRQ_FREE;
+	}
+	spin_unlock(&pc->lock);
+
+	irq_domain_free_irqs_parent(domain, irq, nr_irqs);
+}
+
+static int meson_irq_domain_translate(struct irq_domain *d,
+				      struct irq_fwspec *fwspec,
+				      unsigned long *hwirq,
+				      unsigned int *type)
+{
+	if (is_of_node(fwspec->fwnode)) {
+		if (fwspec->param_count != 2)
+			return -EINVAL;
+
+		*hwirq = fwspec->param[0];
+		*type = fwspec->param[1];
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static struct irq_domain_ops meson_irq_domain_ops = {
+	.alloc		= meson_irq_domain_alloc,
+	.free		= meson_irq_domain_free,
+	.translate	= meson_irq_domain_translate,
+};
+
+int meson_gpio_irq_init(struct meson_pinctrl *pc)
+{
+	struct device_node *node = pc->dev->of_node;
+	struct device_node *parent_node;
+	struct irq_domain *parent_domain;
+	const __be32 *irqs;
+	int i, size;
+
+	parent_node = of_irq_find_parent(node);
+	if (!parent_node) {
+		dev_err(pc->dev, "can't find parent interrupt controller\n");
+		return -EINVAL;
+	}
+
+	parent_domain = irq_find_host(parent_node);
+	if (!parent_domain) {
+		dev_err(pc->dev, "can't find parent IRQ domain\n");
+		return -EINVAL;
+	}
+
+	pc->reg_irq = meson_map_resource(pc, node, "irq");
+	if (!pc->reg_irq) {
+		dev_err(pc->dev, "can't find irq registers\n");
+		return -EINVAL;
+	}
+
+	irqs = of_get_property(node, "amlogic,irqs-gpio", &size);
+	if (!irqs) {
+		dev_err(pc->dev, "no parent interrupts specified\n");
+		return -EINVAL;
+	}
+	pc->num_gic_irqs = size / sizeof(__be32);
+
+	pc->irq_map = devm_kmalloc(pc->dev, sizeof(int) * pc->num_gic_irqs,
+				   GFP_KERNEL);
+	if (!pc->irq_map)
+		return -ENOMEM;
+
+	pc->gic_irqs = devm_kzalloc(pc->dev, sizeof(int) * pc->num_gic_irqs,
+				    GFP_KERNEL);
+	if (!pc->gic_irqs)
+		return -ENOMEM;
+
+	for (i = 0; i < pc->num_gic_irqs; i++) {
+		pc->irq_map[i] = IRQ_FREE;
+		of_property_read_u32_index(node, "amlogic,irqs-gpio", i,
+					   &pc->gic_irqs[i]);
+	}
+
+	pc->irq_domain = irq_domain_add_hierarchy(parent_domain, 0,
+						  pc->data->last_pin,
+						  node, &meson_irq_domain_ops,
+						  pc);
+	if (!pc->irq_domain)
+		return -EINVAL;
+
+	return 0;
+}
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 0c5655b..894b9ad 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -540,6 +540,30 @@ static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio)
 	return !!(val & BIT(bit));
 }
 
+static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+	struct meson_domain *domain = to_meson_domain(chip);
+	struct meson_pinctrl *pc = domain->pinctrl;
+	struct meson_bank *bank;
+	struct irq_fwspec irq_data;
+	unsigned int hwirq, irq;
+
+	hwirq = domain->data->pin_base + offset;
+
+	if (meson_get_bank(domain, hwirq, &bank))
+		return -ENXIO;
+
+	irq_data.param_count = 2;
+	irq_data.param[0] = hwirq;
+
+	/* dummy. It will be changed later in meson_irq_set_type */
+	irq_data.param[1] = IRQ_TYPE_EDGE_RISING;
+
+	irq = irq_domain_alloc_irqs(pc->irq_domain, 1, NUMA_NO_NODE, &irq_data);
+
+	return irq ? irq : -ENXIO;
+}
+
 static const struct of_device_id meson_pinctrl_dt_match[] = {
 	{
 		.compatible = "amlogic,meson8-pinctrl",
@@ -569,6 +593,7 @@ static int meson_gpiolib_register(struct meson_pinctrl *pc)
 		domain->chip.direction_output = meson_gpio_direction_output;
 		domain->chip.get = meson_gpio_get;
 		domain->chip.set = meson_gpio_set;
+		domain->chip.to_irq = meson_gpio_to_irq;
 		domain->chip.base = domain->data->pin_base;
 		domain->chip.ngpio = domain->data->num_pins;
 		domain->chip.can_sleep = false;
@@ -680,6 +705,7 @@ static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
 		}
 
 		domain->of_node = np;
+		domain->pinctrl = pc;
 
 		domain->reg_mux = meson_map_resource(pc, np, "mux");
 		if (IS_ERR(domain->reg_mux)) {
@@ -749,6 +775,10 @@ static int meson_pinctrl_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	ret = meson_gpio_irq_init(pc);
+	if (ret)
+		dev_err(pc->dev, "can't setup gpio interrupts\n");
+
 	return 0;
 }
 
diff --git a/drivers/pinctrl/meson/pinctrl-meson.h b/drivers/pinctrl/meson/pinctrl-meson.h
index a0bf705..c597bf1 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.h
+++ b/drivers/pinctrl/meson/pinctrl-meson.h
@@ -16,6 +16,8 @@
 #include <linux/regmap.h>
 #include <linux/types.h>
 
+struct meson_pinctrl;
+
 /**
  * struct meson_pmx_group - a pinmux group
  *
@@ -111,6 +113,7 @@ struct meson_domain_data {
 	unsigned int num_banks;
 	unsigned int pin_base;
 	unsigned int num_pins;
+	struct meson_pinctrl *pinctrl;
 };
 
 /**
@@ -123,6 +126,7 @@ struct meson_domain_data {
  * @chip:	gpio chip associated with the domain
  * @data;	platform data for the domain
  * @node:	device tree node for the domain
+ * @pinctrl:	pinctrl struct associated with the domain
  *
  * A domain represents a set of banks controlled by the same set of
  * registers.
@@ -136,6 +140,7 @@ struct meson_domain {
 	struct gpio_chip chip;
 	struct meson_domain_data *data;
 	struct device_node *of_node;
+	struct meson_pinctrl *pinctrl;
 };
 
 struct meson_pinctrl_data {
@@ -156,6 +161,14 @@ struct meson_pinctrl {
 	struct pinctrl_desc desc;
 	struct meson_pinctrl_data *data;
 	struct meson_domain *domains;
+
+	struct irq_domain *irq_domain;
+	struct regmap *reg_irq;
+	spinlock_t lock;
+
+	int num_gic_irqs;
+	int *irq_map;
+	int *gic_irqs;
 };
 
 #define PIN(x, b)	(b + x)
@@ -221,3 +234,5 @@ int meson_get_domain_and_bank(struct meson_pinctrl *pc, unsigned int pin,
 
 struct regmap *meson_map_resource(struct meson_pinctrl *pc,
 				  struct device_node *node, char *name);
+
+int meson_gpio_irq_init(struct meson_pinctrl *pc);
-- 
2.5.0

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

Re: [PATCH v3 4/6] pinctrl: meson: Enable GPIO IRQs

[Marc Zyngier]

On 01/12/15 16:24, Carlo Caione wrote:
> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> 
> On Meson8 and Meson8b SoCs there are 8 independent filtered GPIO
> interrupt modules that can be programmed to use any of the GPIOs in the
> chip as an interrupt source.
> 
> For each GPIO IRQ we have:
> 
> GPIOs --> [mux]--> [polarity]--> [filter]--> [edge select]--> GIC
> 
> The eight GPIO interrupts respond to mask/unmask/clear/etc.. just like
> any other interrupt in the chip. The difference for the GPIO interrupts
> is that they can be filtered and conditioned.
> 
> This patch adds support for the external GPIOs interrupts and enables
> them for Meson8 and Meson8b SoCs.
> 
> Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/pinctrl/Kconfig                    |   1 +
>  drivers/pinctrl/meson/Makefile             |   2 +-
>  drivers/pinctrl/meson/irqchip-gpio-meson.c | 321 +++++++++++++++++++++++++++++
>  drivers/pinctrl/meson/pinctrl-meson.c      |  30 +++
>  drivers/pinctrl/meson/pinctrl-meson.h      |  15 ++
>  5 files changed, 368 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/pinctrl/meson/irqchip-gpio-meson.c
> 

[...]

> +static int meson_irq_domain_alloc(struct irq_domain *domain, unsigned int irq,
> +				  unsigned int nr_irqs, void *arg)
> +{
> +	struct meson_pinctrl *pc = domain->host_data;
> +	struct irq_fwspec *irq_data = arg;
> +	struct irq_fwspec gic_data;
> +	irq_hw_number_t hwirq;
> +	int index, ret, i;
> +
> +	if (irq_data->param_count != 2)
> +		return -EINVAL;
> +
> +	hwirq = irq_data->param[0];
> +	dev_dbg(pc->dev, "%s irq %d, nr %d, hwirq %lu\n",
> +			__func__, irq, nr_irqs, hwirq);
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		index = meson_map_gic_irq(domain, hwirq + i);
> +		if (index < 0)
> +			return index;
> +
> +		irq_domain_set_hwirq_and_chip(domain, irq + i,
> +					      hwirq + i,
> +					      &meson_irq_chip,
> +					      pc);
> +
> +		gic_data.param_count = 3;
> +		gic_data.fwnode = domain->parent->fwnode;
> +		gic_data.param[0] = 0; /* SPI */
> +		gic_data.param[1] = pc->gic_irqs[index];
> +		gic_data.param[1] = IRQ_TYPE_EDGE_RISING;

That feels quite wrong. Hardcoding the trigger like this and hoping for
a set_type to set it right at a later time is just asking for trouble.
Why can't you use the trigger type that has been provided by the
interrupt descriptor?

> +
> +		ret = irq_domain_alloc_irqs_parent(domain, irq + i, nr_irqs,
> +						   &gic_data);
> +	}
> +
> +	return 0;
> +}
> +
> +static void meson_irq_domain_free(struct irq_domain *domain, unsigned int irq,
> +				  unsigned int nr_irqs)
> +{
> +	struct meson_pinctrl *pc = domain->host_data;
> +	struct irq_data *irq_data;
> +	int index, i;
> +
> +	spin_lock(&pc->lock);
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_data = irq_domain_get_irq_data(domain, irq + i);
> +		index = meson_get_gic_irq(pc, irq_data->hwirq);
> +		if (index < 0)
> +			continue;
> +		pc->irq_map[index] = IRQ_FREE;
> +	}
> +	spin_unlock(&pc->lock);
> +
> +	irq_domain_free_irqs_parent(domain, irq, nr_irqs);
> +}
> +
> +static int meson_irq_domain_translate(struct irq_domain *d,
> +				      struct irq_fwspec *fwspec,
> +				      unsigned long *hwirq,
> +				      unsigned int *type)
> +{
> +	if (is_of_node(fwspec->fwnode)) {
> +		if (fwspec->param_count != 2)
> +			return -EINVAL;
> +
> +		*hwirq = fwspec->param[0];
> +		*type = fwspec->param[1];
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static struct irq_domain_ops meson_irq_domain_ops = {
> +	.alloc		= meson_irq_domain_alloc,
> +	.free		= meson_irq_domain_free,
> +	.translate	= meson_irq_domain_translate,
> +};
> +
> +int meson_gpio_irq_init(struct meson_pinctrl *pc)
> +{
> +	struct device_node *node = pc->dev->of_node;
> +	struct device_node *parent_node;
> +	struct irq_domain *parent_domain;
> +	const __be32 *irqs;
> +	int i, size;
> +
> +	parent_node = of_irq_find_parent(node);
> +	if (!parent_node) {
> +		dev_err(pc->dev, "can't find parent interrupt controller\n");
> +		return -EINVAL;
> +	}
> +
> +	parent_domain = irq_find_host(parent_node);
> +	if (!parent_domain) {
> +		dev_err(pc->dev, "can't find parent IRQ domain\n");
> +		return -EINVAL;
> +	}
> +
> +	pc->reg_irq = meson_map_resource(pc, node, "irq");
> +	if (!pc->reg_irq) {
> +		dev_err(pc->dev, "can't find irq registers\n");
> +		return -EINVAL;
> +	}
> +
> +	irqs = of_get_property(node, "amlogic,irqs-gpio", &size);
> +	if (!irqs) {
> +		dev_err(pc->dev, "no parent interrupts specified\n");
> +		return -EINVAL;
> +	}
> +	pc->num_gic_irqs = size / sizeof(__be32);
> +
> +	pc->irq_map = devm_kmalloc(pc->dev, sizeof(int) * pc->num_gic_irqs,
> +				   GFP_KERNEL);
> +	if (!pc->irq_map)
> +		return -ENOMEM;
> +
> +	pc->gic_irqs = devm_kzalloc(pc->dev, sizeof(int) * pc->num_gic_irqs,
> +				    GFP_KERNEL);
> +	if (!pc->gic_irqs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < pc->num_gic_irqs; i++) {
> +		pc->irq_map[i] = IRQ_FREE;
> +		of_property_read_u32_index(node, "amlogic,irqs-gpio", i,
> +					   &pc->gic_irqs[i]);
> +	}
> +
> +	pc->irq_domain = irq_domain_add_hierarchy(parent_domain, 0,
> +						  pc->data->last_pin,
> +						  node, &meson_irq_domain_ops,
> +						  pc);
> +	if (!pc->irq_domain)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
> index 0c5655b..894b9ad 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
> @@ -540,6 +540,30 @@ static int meson_gpio_get(struct gpio_chip *chip, unsigned gpio)
>  	return !!(val & BIT(bit));
>  }
>  
> +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> +	struct meson_domain *domain = to_meson_domain(chip);
> +	struct meson_pinctrl *pc = domain->pinctrl;
> +	struct meson_bank *bank;
> +	struct irq_fwspec irq_data;
> +	unsigned int hwirq, irq;
> +
> +	hwirq = domain->data->pin_base + offset;
> +
> +	if (meson_get_bank(domain, hwirq, &bank))
> +		return -ENXIO;
> +
> +	irq_data.param_count = 2;
> +	irq_data.param[0] = hwirq;
> +
> +	/* dummy. It will be changed later in meson_irq_set_type */
> +	irq_data.param[1] = IRQ_TYPE_EDGE_RISING;

Blah. Worse than I though... How do you end-up here? Why can't you
obtain the corresponding of_phandle_args instead of making things up?
This looks mad. Do you really need this?

	M.
-- 
Jazz is not dead. It just smells funny...
--
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

Re: [PATCH v3 4/6] pinctrl: meson: Enable GPIO IRQs

[Carlo Caione]

On Tue, Dec 1, 2015 at 8:16 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote:
> On 01/12/15 16:24, Carlo Caione wrote:
>> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

>> +static int meson_irq_domain_alloc(struct irq_domain *domain, unsigned int irq,
>> +                               unsigned int nr_irqs, void *arg)
>> +{
>> +     struct meson_pinctrl *pc = domain->host_data;
>> +     struct irq_fwspec *irq_data = arg;
>> +     struct irq_fwspec gic_data;
>> +     irq_hw_number_t hwirq;
>> +     int index, ret, i;
>> +
>> +     if (irq_data->param_count != 2)
>> +             return -EINVAL;
>> +
>> +     hwirq = irq_data->param[0];
>> +     dev_dbg(pc->dev, "%s irq %d, nr %d, hwirq %lu\n",
>> +                     __func__, irq, nr_irqs, hwirq);
>> +
>> +     for (i = 0; i < nr_irqs; i++) {
>> +             index = meson_map_gic_irq(domain, hwirq + i);
>> +             if (index < 0)
>> +                     return index;
>> +
>> +             irq_domain_set_hwirq_and_chip(domain, irq + i,
>> +                                           hwirq + i,
>> +                                           &meson_irq_chip,
>> +                                           pc);
>> +
>> +             gic_data.param_count = 3;
>> +             gic_data.fwnode = domain->parent->fwnode;
>> +             gic_data.param[0] = 0; /* SPI */
>> +             gic_data.param[1] = pc->gic_irqs[index];
>> +             gic_data.param[1] = IRQ_TYPE_EDGE_RISING;

Oh, this should be gic_data.param[2]. Just noticed.

> That feels quite wrong. Hardcoding the trigger like this and hoping for
> a set_type to set it right at a later time is just asking for trouble.
> Why can't you use the trigger type that has been provided by the
> interrupt descriptor?

In v2 I had the set of fwspec to track number and trigger type of the
IRQ, so it was straightforward. With this patch I have moved away from
that solution (as you suggested) and I'm using the 'amlogic,irqs-gpio'
parameter to track down the IRQ numbers (but not the trigger type).
It's the same solution we have in drivers/irqchip/irq-crossbar.c where
the trigger type is hardcoded in allocate_gic_irq().
If I need to save both the IRQ and the trigger type at this point I
wonder if it's better to go back to the set of fwspec or convert the
fwspec to of_phandle_args and save that.

>> +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>> +{
>> +     struct meson_domain *domain = to_meson_domain(chip);
>> +     struct meson_pinctrl *pc = domain->pinctrl;
>> +     struct meson_bank *bank;
>> +     struct irq_fwspec irq_data;
>> +     unsigned int hwirq, irq;
>> +
>> +     hwirq = domain->data->pin_base + offset;
>> +
>> +     if (meson_get_bank(domain, hwirq, &bank))
>> +             return -ENXIO;
>> +
>> +     irq_data.param_count = 2;
>> +     irq_data.param[0] = hwirq;
>> +
>> +     /* dummy. It will be changed later in meson_irq_set_type */
>> +     irq_data.param[1] = IRQ_TYPE_EDGE_RISING;
>
> Blah. Worse than I though... How do you end-up here? Why can't you
> obtain the corresponding of_phandle_args instead of making things up?

because I do not have a of_phandle. This is basically the .to_irq hook
of the gpio_chip. This code is being called programmatically from the
gpiolib. No DTS/OF involved here.

> This looks mad. Do you really need this?

Well, I'm open to any suggestion on how improve this mess.

-- 
Carlo Caione
--
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

Re: [PATCH v3 4/6] pinctrl: meson: Enable GPIO IRQs

[Marc Zyngier]

On 01/12/15 19:41, Carlo Caione wrote:
> On Tue, Dec 1, 2015 at 8:16 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 01/12/15 16:24, Carlo Caione wrote:
>>> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> 
>>> +static int meson_irq_domain_alloc(struct irq_domain *domain, unsigned int irq,
>>> +                               unsigned int nr_irqs, void *arg)
>>> +{
>>> +     struct meson_pinctrl *pc = domain->host_data;
>>> +     struct irq_fwspec *irq_data = arg;
>>> +     struct irq_fwspec gic_data;
>>> +     irq_hw_number_t hwirq;
>>> +     int index, ret, i;
>>> +
>>> +     if (irq_data->param_count != 2)
>>> +             return -EINVAL;
>>> +
>>> +     hwirq = irq_data->param[0];
>>> +     dev_dbg(pc->dev, "%s irq %d, nr %d, hwirq %lu\n",
>>> +                     __func__, irq, nr_irqs, hwirq);
>>> +
>>> +     for (i = 0; i < nr_irqs; i++) {
>>> +             index = meson_map_gic_irq(domain, hwirq + i);
>>> +             if (index < 0)
>>> +                     return index;
>>> +
>>> +             irq_domain_set_hwirq_and_chip(domain, irq + i,
>>> +                                           hwirq + i,
>>> +                                           &meson_irq_chip,
>>> +                                           pc);
>>> +
>>> +             gic_data.param_count = 3;
>>> +             gic_data.fwnode = domain->parent->fwnode;
>>> +             gic_data.param[0] = 0; /* SPI */
>>> +             gic_data.param[1] = pc->gic_irqs[index];
>>> +             gic_data.param[1] = IRQ_TYPE_EDGE_RISING;
> 
> Oh, this should be gic_data.param[2]. Just noticed.
> 
>> That feels quite wrong. Hardcoding the trigger like this and hoping for
>> a set_type to set it right at a later time is just asking for trouble.
>> Why can't you use the trigger type that has been provided by the
>> interrupt descriptor?
> 
> In v2 I had the set of fwspec to track number and trigger type of the
> IRQ, so it was straightforward. With this patch I have moved away from
> that solution (as you suggested) and I'm using the 'amlogic,irqs-gpio'
> parameter to track down the IRQ numbers (but not the trigger type).
> It's the same solution we have in drivers/irqchip/irq-crossbar.c where
> the trigger type is hardcoded in allocate_gic_irq().
> If I need to save both the IRQ and the trigger type at this point I
> wonder if it's better to go back to the set of fwspec or convert the
> fwspec to of_phandle_args and save that.

No. This should come from the interrupt specifier you are getting from
the device. You should never make up that information.

Your amlogic,irqs-gpio property gives you a list of downstream
interrupts. The device connected to your pinctrl HW provides you with
the upstream interrupt number (which you will map to one of your
downstream IRQ) and crucially the trigger type. Please look at how the
TI cross bar works (again).

>>> +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>>> +{
>>> +     struct meson_domain *domain = to_meson_domain(chip);
>>> +     struct meson_pinctrl *pc = domain->pinctrl;
>>> +     struct meson_bank *bank;
>>> +     struct irq_fwspec irq_data;
>>> +     unsigned int hwirq, irq;
>>> +
>>> +     hwirq = domain->data->pin_base + offset;
>>> +
>>> +     if (meson_get_bank(domain, hwirq, &bank))
>>> +             return -ENXIO;
>>> +
>>> +     irq_data.param_count = 2;
>>> +     irq_data.param[0] = hwirq;
>>> +
>>> +     /* dummy. It will be changed later in meson_irq_set_type */
>>> +     irq_data.param[1] = IRQ_TYPE_EDGE_RISING;
>>
>> Blah. Worse than I though... How do you end-up here? Why can't you
>> obtain the corresponding of_phandle_args instead of making things up?
> 
> because I do not have a of_phandle. This is basically the .to_irq hook
> of the gpio_chip. This code is being called programmatically from the
> gpiolib. No DTS/OF involved here.
> 
>> This looks mad. Do you really need this?
> 
> Well, I'm open to any suggestion on how improve this mess.

The question to answer is: in what circumstances do you have to convert
a GPIO into an IRQ at runtime? The only case should be "when you
discover a device having an interrupt pointing to your pinctrl". And in
this case, you have all the information to reconfigure the HW and assign
the interrupt.

I really don't get why you want or need to involve gpiolib in this.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
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

Re: [PATCH v3 4/6] pinctrl: meson: Enable GPIO IRQs

[Carlo Caione]

On Wed, Dec 2, 2015 at 10:03 AM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote:
> On 01/12/15 19:41, Carlo Caione wrote:
>> On Tue, Dec 1, 2015 at 8:16 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote:
>>> On 01/12/15 16:24, Carlo Caione wrote:
>>>> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

[...]

>> In v2 I had the set of fwspec to track number and trigger type of the
>> IRQ, so it was straightforward. With this patch I have moved away from
>> that solution (as you suggested) and I'm using the 'amlogic,irqs-gpio'
>> parameter to track down the IRQ numbers (but not the trigger type).
>> It's the same solution we have in drivers/irqchip/irq-crossbar.c where
>> the trigger type is hardcoded in allocate_gic_irq().
>> If I need to save both the IRQ and the trigger type at this point I
>> wonder if it's better to go back to the set of fwspec or convert the
>> fwspec to of_phandle_args and save that.
>
> No. This should come from the interrupt specifier you are getting from
> the device. You should never make up that information.
>
> Your amlogic,irqs-gpio property gives you a list of downstream
> interrupts. The device connected to your pinctrl HW provides you with
> the upstream interrupt number (which you will map to one of your
> downstream IRQ) and crucially the trigger type. Please look at how the
> TI cross bar works (again).

Ok, this definitely makes sense and I'm going to fix it in the next
revision, thanks for the explanation. Still I fail to see how the TI
cross bar driver is actually doing what you are suggesting here. If
I'm correctly reading the code here
http://lxr.free-electrons.com/source/drivers/irqchip/irq-crossbar.c#L101
the driver is hardcoding the trigger type for the downstream IRQ to be
passed to the GIC code. But probably I'm missing something obvious.

>>>> +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>>>> +{
>>>> +     struct meson_domain *domain = to_meson_domain(chip);
>>>> +     struct meson_pinctrl *pc = domain->pinctrl;
>>>> +     struct meson_bank *bank;
>>>> +     struct irq_fwspec irq_data;
>>>> +     unsigned int hwirq, irq;
>>>> +
>>>> +     hwirq = domain->data->pin_base + offset;
>>>> +
>>>> +     if (meson_get_bank(domain, hwirq, &bank))
>>>> +             return -ENXIO;
>>>> +
>>>> +     irq_data.param_count = 2;
>>>> +     irq_data.param[0] = hwirq;
>>>> +
>>>> +     /* dummy. It will be changed later in meson_irq_set_type */
>>>> +     irq_data.param[1] = IRQ_TYPE_EDGE_RISING;
>>>
>>> Blah. Worse than I though... How do you end-up here? Why can't you
>>> obtain the corresponding of_phandle_args instead of making things up?
>>
>> because I do not have a of_phandle. This is basically the .to_irq hook
>> of the gpio_chip. This code is being called programmatically from the
>> gpiolib. No DTS/OF involved here.
>>
>>> This looks mad. Do you really need this?
>>
>> Well, I'm open to any suggestion on how improve this mess.
>
> The question to answer is: in what circumstances do you have to convert
> a GPIO into an IRQ at runtime? The only case should be "when you
> discover a device having an interrupt pointing to your pinctrl". And in
> this case, you have all the information to reconfigure the HW and assign
> the interrupt.
>
> I really don't get why you want or need to involve gpiolib in this.

Again probably I'm missing something (and Linus probably could help
here) but the only place I see the .to_irq hook (that is
meson_gpio_to_irq() in the driver code) being called is from
gpiod_to_irq() function in the gpiolib code.

One practical case in which that code path is involved is when (for
example) I have something like 'cd-gpios = <&gpio GPIOX 0>;' for the
card detection IRQ in the MMC node and in this case I fail to see an
easy way to get the trigger type without touching the MMC / gpiolib
code (any idea?).
This function is not called at all when in the DTS I explicitly have
the interrupt specifier defined using the 'interrupts = <...>'
property and in that case I have all the information I need to map the
downstream IRQ.

Thanks,

-- 
Carlo Caione
--
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

Re: [PATCH v3 4/6] pinctrl: meson: Enable GPIO IRQs

[Marc Zyngier]

On 02/12/15 11:37, Carlo Caione wrote:
> On Wed, Dec 2, 2015 at 10:03 AM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote:
>> On 01/12/15 19:41, Carlo Caione wrote:
>>> On Tue, Dec 1, 2015 at 8:16 PM, Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org> wrote:
>>>> On 01/12/15 16:24, Carlo Caione wrote:
>>>>> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> 
> [...]
> 
>>> In v2 I had the set of fwspec to track number and trigger type of the
>>> IRQ, so it was straightforward. With this patch I have moved away from
>>> that solution (as you suggested) and I'm using the 'amlogic,irqs-gpio'
>>> parameter to track down the IRQ numbers (but not the trigger type).
>>> It's the same solution we have in drivers/irqchip/irq-crossbar.c where
>>> the trigger type is hardcoded in allocate_gic_irq().
>>> If I need to save both the IRQ and the trigger type at this point I
>>> wonder if it's better to go back to the set of fwspec or convert the
>>> fwspec to of_phandle_args and save that.
>>
>> No. This should come from the interrupt specifier you are getting from
>> the device. You should never make up that information.
>>
>> Your amlogic,irqs-gpio property gives you a list of downstream
>> interrupts. The device connected to your pinctrl HW provides you with
>> the upstream interrupt number (which you will map to one of your
>> downstream IRQ) and crucially the trigger type. Please look at how the
>> TI cross bar works (again).
> 
> Ok, this definitely makes sense and I'm going to fix it in the next
> revision, thanks for the explanation. Still I fail to see how the TI
> cross bar driver is actually doing what you are suggesting here. If
> I'm correctly reading the code here
> http://lxr.free-electrons.com/source/drivers/irqchip/irq-crossbar.c#L101
> the driver is hardcoding the trigger type for the downstream IRQ to be
> passed to the GIC code. But probably I'm missing something obvious.

Damn. No, you're right. I'll fix that. Thanks for the heads up, and
apologies for the shouting! ;-)

>>>>> +static int meson_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>>>>> +{
>>>>> +     struct meson_domain *domain = to_meson_domain(chip);
>>>>> +     struct meson_pinctrl *pc = domain->pinctrl;
>>>>> +     struct meson_bank *bank;
>>>>> +     struct irq_fwspec irq_data;
>>>>> +     unsigned int hwirq, irq;
>>>>> +
>>>>> +     hwirq = domain->data->pin_base + offset;
>>>>> +
>>>>> +     if (meson_get_bank(domain, hwirq, &bank))
>>>>> +             return -ENXIO;
>>>>> +
>>>>> +     irq_data.param_count = 2;
>>>>> +     irq_data.param[0] = hwirq;
>>>>> +
>>>>> +     /* dummy. It will be changed later in meson_irq_set_type */
>>>>> +     irq_data.param[1] = IRQ_TYPE_EDGE_RISING;
>>>>
>>>> Blah. Worse than I though... How do you end-up here? Why can't you
>>>> obtain the corresponding of_phandle_args instead of making things up?
>>>
>>> because I do not have a of_phandle. This is basically the .to_irq hook
>>> of the gpio_chip. This code is being called programmatically from the
>>> gpiolib. No DTS/OF involved here.
>>>
>>>> This looks mad. Do you really need this?
>>>
>>> Well, I'm open to any suggestion on how improve this mess.
>>
>> The question to answer is: in what circumstances do you have to convert
>> a GPIO into an IRQ at runtime? The only case should be "when you
>> discover a device having an interrupt pointing to your pinctrl". And in
>> this case, you have all the information to reconfigure the HW and assign
>> the interrupt.
>>
>> I really don't get why you want or need to involve gpiolib in this.
> 
> Again probably I'm missing something (and Linus probably could help
> here) but the only place I see the .to_irq hook (that is
> meson_gpio_to_irq() in the driver code) being called is from
> gpiod_to_irq() function in the gpiolib code.
> 
> One practical case in which that code path is involved is when (for
> example) I have something like 'cd-gpios = <&gpio GPIOX 0>;' for the
> card detection IRQ in the MMC node and in this case I fail to see an
> easy way to get the trigger type without touching the MMC / gpiolib
> code (any idea?).
> This function is not called at all when in the DTS I explicitly have
> the interrupt specifier defined using the 'interrupts = <...>'
> property and in that case I have all the information I need to map the
> downstream IRQ.

I do think that the moment you want to describe an interrupt (and have
the HW that can trigger one), you should end up describing this as a
real interrupt, and not as a "shake this pin" kind of thing.

The former gives you strong guarantees as to how this is going to be
processed, while the later looks like a hack to paper over missing
functionalities in past kernels...

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
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

[PATCH v3 5/6] pinctrl: dt-binding: Extend meson documentation with GPIO IRQs support

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

Extend the pinctrl binding documentation with the support for external
GPIO interrupts.

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
Signed-off-by: Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
index 3f6a524..de669ce 100644
--- a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
@@ -3,6 +3,12 @@
 Required properties for the root node:
  - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
  - reg: address and size of registers controlling irq functionality
+ - reg-names: should be "irq"
+ - interrupt-controller: marks the device node as an interrupt controller
+ - #interrupt-cells: should be 2. The first cell is the GPIO number. The
+		     second cell is used to specify trigger type.
+ - amlogic,irqs-gpio: specifies the GPIO IRQ numbers on the GIC (GPIO_IRQ#)
+ - interrupt-parent: specifies the parent interrupt controller.
 
 === GPIO sub-nodes ===
 
@@ -46,7 +52,12 @@ pinctrl-bindings.txt
 
 	pinctrl: pinctrl@c1109880 {
 		compatible = "amlogic,meson8-pinctrl";
+		reg-names = "irq";
 		reg = <0xc1109880 0x10>;
+		interrupt-parent = <&gic>;
+		amlogic,irqs-gpio = <64 65 66 67 68 69 70 71>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
 		#address-cells = <1>;
 		#size-cells = <1>;
 		ranges;
-- 
2.5.0

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

Re: [PATCH v3 5/6] pinctrl: dt-binding: Extend meson documentation with GPIO IRQs support

[Rob Herring]

On Tue, Dec 01, 2015 at 05:24:21PM +0100, Carlo Caione wrote:
> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> 
> Extend the pinctrl binding documentation with the support for external
> GPIO interrupts.
> 
> Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
> index 3f6a524..de669ce 100644
> --- a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
> @@ -3,6 +3,12 @@
>  Required properties for the root node:
>   - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
>   - reg: address and size of registers controlling irq functionality
> + - reg-names: should be "irq"
> + - interrupt-controller: marks the device node as an interrupt controller
> + - #interrupt-cells: should be 2. The first cell is the GPIO number. The
> +		     second cell is used to specify trigger type.
> + - amlogic,irqs-gpio: specifies the GPIO IRQ numbers on the GIC (GPIO_IRQ#)
> + - interrupt-parent: specifies the parent interrupt controller.
>  
>  === GPIO sub-nodes ===
>  
> @@ -46,7 +52,12 @@ pinctrl-bindings.txt
>  
>  	pinctrl: pinctrl@c1109880 {
>  		compatible = "amlogic,meson8-pinctrl";
> +		reg-names = "irq";
>  		reg = <0xc1109880 0x10>;
> +		interrupt-parent = <&gic>;
> +		amlogic,irqs-gpio = <64 65 66 67 68 69 70 71>;

Can't you just use "interrupts" here?

Rob

> +		interrupt-controller;
> +		#interrupt-cells = <2>;
>  		#address-cells = <1>;
>  		#size-cells = <1>;
>  		ranges;
> -- 
> 2.5.0
> 
--
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

Re: [PATCH v3 5/6] pinctrl: dt-binding: Extend meson documentation with GPIO IRQs support

[Carlo Caione]

On Wed, Dec 2, 2015 at 4:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Dec 01, 2015 at 05:24:21PM +0100, Carlo Caione wrote:
>> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
>>
>> Extend the pinctrl binding documentation with the support for external
>> GPIO interrupts.
>>
>> Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Beniamino Galvani <b.galvani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
>> index 3f6a524..de669ce 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
>> +++ b/Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt
>> @@ -3,6 +3,12 @@
>>  Required properties for the root node:
>>   - compatible: "amlogic,meson8-pinctrl" or "amlogic,meson8b-pinctrl"
>>   - reg: address and size of registers controlling irq functionality
>> + - reg-names: should be "irq"
>> + - interrupt-controller: marks the device node as an interrupt controller
>> + - #interrupt-cells: should be 2. The first cell is the GPIO number. The
>> +                  second cell is used to specify trigger type.
>> + - amlogic,irqs-gpio: specifies the GPIO IRQ numbers on the GIC (GPIO_IRQ#)
>> + - interrupt-parent: specifies the parent interrupt controller.
>>
>>  === GPIO sub-nodes ===
>>
>> @@ -46,7 +52,12 @@ pinctrl-bindings.txt
>>
>>       pinctrl: pinctrl@c1109880 {
>>               compatible = "amlogic,meson8-pinctrl";
>> +             reg-names = "irq";
>>               reg = <0xc1109880 0x10>;
>> +             interrupt-parent = <&gic>;
>> +             amlogic,irqs-gpio = <64 65 66 67 68 69 70 71>;
>
> Can't you just use "interrupts" here?

Following this discussion for v2 with Marc
http://www.spinics.net/lists/devicetree/msg104385.html I decided to
move away from using the "interrupts" property since these interrupts
are not actually the pinctrl's interrupts (they are owned by the
devices using them). I guess that using "interrupts" would be a bit
misleading.

-- 
Carlo Caione
--
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

[PATCH v3 6/6] ARM: meson: DTS: Enable GPIO IRQs

[Carlo Caione]

From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

Signed-off-by: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 arch/arm/boot/dts/meson8b.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/meson8b.dtsi b/arch/arm/boot/dts/meson8b.dtsi
index ee352bf..dd7ccd3 100644
--- a/arch/arm/boot/dts/meson8b.dtsi
+++ b/arch/arm/boot/dts/meson8b.dtsi
@@ -152,6 +152,11 @@
 		pinctrl: pinctrl@c1109880 {
 			compatible = "amlogic,meson8b-pinctrl";
 			reg = <0xc1109880 0x10>;
+			reg-names = "irq";
+			interrupt-parent = <&gic>;
+			amlogic,irqs-gpio = <64 65 66 67 68 69 70 71>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges;
-- 
2.5.0

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

Re: [PATCH v3 0/6] pinctrl: meson: enable support for external GPIO interrupts

[Linus Walleij]

On Tue, Dec 1, 2015 at 5:24 PM, Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org> wrote:

> From: Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
>
> In Meson SoCs we have 8 independent GPIO interrupts that can be programmed to
> use any of the GPIOs in the chip as interrupt source.
>
> These GPIOs are managed by GIC but they can be conditioned (and enabled) by
> some registers external to the GIC.
>
> GPIOs |--[mux1 or mux2]--[polarity]--[filter]--[edge_select]--> GIC
>
> Changelog:
>
> * V2:
>     - Introduced .irq_request_resources() and .irq_release_resources()
>     - s/virq/irq/ and s/pin/hwirq/
>     - Moved to the new irq_fwspec
>
> * V3:
>     - EXPORT_SYMBOL_GPL for of_irq_find_parent()
>     - split GPIO management and irqchip side
>     - the GIC IRQs list is not kept as set of fwspecs. it's now a regular
>       integer array.
>     - irq_of_phandle_args_to_fwspec discarded

I don't know where this patch set is standing but I get the impression that it
still needs revising so waiting for a v4.

Given the complex nature of the IRQs I will probably want Marc to provide
his Review/ACK before applying.

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