[PATCH v1 0/5] pinctrl: st: Add interrupt controller support.

[PATCH v1 0/5] pinctrl: st: Add interrupt controller support.

[srinivas.kandagatla-qxv4g6HH51o]

From: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>

Hi Linus W, 

This series of patches add interrupt controller support to ST pinctrl driver.
ST pin controller GPIO bank can have one of the two possible types of
interrupt-wirings.

First type is via irqmux, single interrupt is used by multiple gpio
banks. This reduces number of interrupts numbers required by pin controller.
All these gpio banks belong to a single pin controller.

Second type has a dedicated interrupt per gpio bank. Interrupt number usage is
very high in this case.

This patch adds support to both these wirings.

Also, ST pin controller hardware only supports level triggered interrupts.
However for drivers like gpio keypad requires edge trigger interrupt support,
so one of the patch adds edge trigger interrupt support in software by using
the existing level trigger support in hardware.

Patch 01: Is a minor typo fix.
Patch 02: Adds level interrupt support to pin controller.
Patch 03: Adds software edge trigger support.
Patch 04,05 : Updates STiH415, STiH416 dts files for interrupt support.

As Patch-01 only has a typo fix I did not move it out of this series.

Comments?

Thanks,
srini

Srinivas Kandagatla (5):
  pinctrl: st: Fix a typo in probe
  pinctrl: st: Add Interrupt support.
  pinctrl: st: Add software edge trigger interrupt support.
  ARM:STi: STiH416: Add interrupt support for pin controller
  ARM:STi: STiH415: Add interrupt support for pin controller

 .../devicetree/bindings/pinctrl/pinctrl-st.txt     |   60 ++++-
 arch/arm/boot/dts/stih415-pinctrl.dtsi             |   75 +++++
 arch/arm/boot/dts/stih416-pinctrl.dtsi             |   81 ++++++
 drivers/pinctrl/pinctrl-st.c                       |  287 +++++++++++++++++++-
 4 files changed, 492 insertions(+), 11 deletions(-)

-- 
1.7.6.5

--
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 v1 1/5] pinctrl: st: Fix a typo in probe

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

Probe function had commas instead of semi-colons on some of the lines.
This patch just fixes those lines. No functional chagnes done in this
patch.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 drivers/pinctrl/pinctrl-st.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 9cadc68..320c273 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -1370,10 +1370,10 @@ static int st_pctl_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	pctl_desc->owner	= THIS_MODULE,
-	pctl_desc->pctlops	= &st_pctlops,
-	pctl_desc->pmxops	= &st_pmxops,
-	pctl_desc->confops	= &st_confops,
+	pctl_desc->owner	= THIS_MODULE;
+	pctl_desc->pctlops	= &st_pctlops;
+	pctl_desc->pmxops	= &st_pmxops;
+	pctl_desc->confops	= &st_confops;
 	pctl_desc->name		= dev_name(&pdev->dev);
 
 	info->pctl = pinctrl_register(pctl_desc, &pdev->dev, info);
-- 
1.7.6.5

Re: [PATCH v1 1/5] pinctrl: st: Fix a typo in probe

[Linus Walleij]

On Tue, Jan 14, 2014 at 3:52 PM,  <srinivas.kandagatla@st.com> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> Probe function had commas instead of semi-colons on some of the lines.
> This patch just fixes those lines. No functional chagnes done in this
> patch.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>

Patch applied!

Yours,
Linus Walleij

[PATCH v1 2/5] pinctrl: st: Add Interrupt support.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

This patch add interrupt support to the pincontroller driver.

ST Pincontroller GPIO bank can have one of the two possible types of
interrupt-wirings.

First type is via irqmux, single interrupt is used by multiple gpio
banks. This reduces number of overall interrupts numbers required. All
these banks belong to a single pincontroller.
		  _________
		 |	   |----> [gpio-bank (n)    ]
		 |	   |----> [gpio-bank (n + 1)]
	[irqN]-- | irq-mux |----> [gpio-bank (n + 2)]
		 |	   |----> [gpio-bank (...  )]
		 |_________|----> [gpio-bank (n + 7)]

Second type has a dedicated interrupt per gpio bank.

	[irqN]----> [gpio-bank (n)]

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 .../devicetree/bindings/pinctrl/pinctrl-st.txt     |   60 +++++-
 drivers/pinctrl/pinctrl-st.c                       |  217 +++++++++++++++++++-
 2 files changed, 270 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-st.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-st.txt
index 05bf82a..4077845 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-st.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-st.txt
@@ -11,18 +11,66 @@ Pull Up (PU) are driven by the related PIO block.
 ST pinctrl driver controls PIO multiplexing block and also interacts with
 gpio driver to configure a pin.
 
-Required properties: (PIO multiplexing block)
+GPIO bank can have one of the two possible types of interrupt-wirings.
+
+First type is via irqmux, single interrupt is used by multiple gpio banks. This
+reduces number of overall interrupts numbers required. All these banks belong to
+a single pincontroller.
+		  _________
+		 |	   |----> [gpio-bank (n)    ]
+		 |	   |----> [gpio-bank (n + 1)]
+	[irqN]-- | irq-mux |----> [gpio-bank (n + 2)]
+		 |	   |----> [gpio-bank (...  )]
+		 |_________|----> [gpio-bank (n + 7)]
+
+Second type has a dedicated interrupt per gpio bank.
+
+	[irqN]----> [gpio-bank (n)]
+
+
+Pin controller node:
+Required properties:
 - compatible	: should be "st,<SOC>-<pio-block>-pinctrl"
 	like st,stih415-sbc-pinctrl, st,stih415-front-pinctrl and so on.
-- gpio-controller : Indicates this device is a GPIO controller
-- #gpio-cells	  : Should be one. The first cell is the pin number.
+- st,syscfg		: Should be a phandle of the syscfg node.
 - st,retime-pin-mask	: Should be mask to specify which pins can be retimed.
 	If the property is not present, it is assumed that all the pins in the
 	bank are capable of retiming. Retiming is mainly used to improve the
 	IO timing margins of external synchronous interfaces.
-- st,bank-name		: Should be a name string for this bank as
-			specified in datasheet.
-- st,syscfg		: Should be a phandle of the syscfg node.
+- ranges	: specifies the ranges for the pin controller memory.
+
+Optional properties:
+- interrupts	: Interrupt number of the irqmux. If the interrupt is shared
+  with other gpio banks via irqmux.
+  a irqline and gpio banks.
+- reg		: irqmux memory resource. If irqmux is present.
+- reg-names	: irqmux resource should be named as "irqmux".
+
+GPIO controller node.
+Required properties:
+- gpio-controller : Indicates this device is a GPIO controller
+- #gpio-cells	  : Should be one. The first cell is the pin number.
+- st,bank-name	  : Should be a name string for this bank as specified in
+  datasheet.
+
+Optional properties:
+- interrupts	: Interrupt number for this gpio bank. If there is a dedicated
+  interrupt wired up for this gpio bank.
+
+- interrupt-controller : Indicates this device is a interrupt controller. GPIO
+  bank can be an interrupt controller iff one of the interrupt type either via
+irqmux or a dedicated interrupt per bank is specified.
+
+- #interrupt-cells: the value of this property should be 2.
+     - First Cell: represents the external gpio interrupt number local to the
+       external gpio interrupt space of the controller.
+     - Second Cell: flags to identify the type of the interrupt
+       - 1 = rising edge triggered
+       - 2 = falling edge triggered
+       - 3 = rising and falling edge triggered
+       - 4 = high level triggered
+       - 8 = low level triggered
+
 
 Example:
 	pin-controller-sbc {
diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 320c273..03c0cfd 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -13,7 +13,12 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_gpio.h>
 #include <linux/of_address.h>
 #include <linux/regmap.h>
@@ -271,6 +276,8 @@ struct st_gpio_bank {
 	struct pinctrl_gpio_range	range;
 	void __iomem			*base;
 	struct st_pio_control		pc;
+	struct	irq_domain		*domain;
+	int				gpio_irq;
 };
 
 struct st_pinctrl {
@@ -284,6 +291,8 @@ struct st_pinctrl {
 	int				ngroups;
 	struct regmap			*regmap;
 	const struct st_pctl_data	*data;
+	void __iomem			*irqmux_base;
+	int				irqmux_irq;
 };
 
 /* SOC specific data */
@@ -1200,6 +1209,104 @@ static int st_pctl_parse_functions(struct device_node *np,
 	return 0;
 }
 
+static int st_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+	struct st_gpio_bank *bank = gpio_chip_to_bank(chip);
+	int virq;
+
+	if (bank->domain && (offset < chip->ngpio))
+		virq = irq_create_mapping(bank->domain, offset);
+	else
+		virq = -ENXIO;
+
+	dev_info(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
+				chip->label, offset + chip->base, virq);
+	return virq;
+}
+
+static void st_gpio_irq_disable(struct irq_data *d)
+{
+	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+
+	writel(BIT(d->hwirq), bank->base + REG_PIO_CLR_PMASK);
+}
+
+static void st_gpio_irq_enable(struct irq_data *d)
+{
+	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+
+	writel(BIT(d->hwirq), bank->base + REG_PIO_SET_PMASK);
+}
+
+static int st_gpio_irq_set_type(struct irq_data *d, unsigned type)
+{
+	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	int comp, pin = d->hwirq;
+	u32 val;
+
+	switch (type) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		comp = 0;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		comp = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	val = readl(bank->base + REG_PIO_PCOMP);
+	val &= ~BIT(pin);
+	val |= (comp << pin);
+	writel(val, bank->base + REG_PIO_PCOMP);
+
+	return 0;
+}
+
+static void __gpio_irq_handler(struct st_gpio_bank *bank)
+{
+	unsigned long port_in, port_mask, port_comp, port_active;
+	int n;
+
+	port_in = readl(bank->base + REG_PIO_PIN);
+	port_comp = readl(bank->base + REG_PIO_PCOMP);
+	port_mask = readl(bank->base + REG_PIO_PMASK);
+
+	port_active = (port_in ^ port_comp) & port_mask;
+
+	for_each_set_bit(n, &port_active, BITS_PER_LONG) {
+		generic_handle_irq(irq_find_mapping(bank->domain, n));
+	}
+}
+
+static void st_gpio_irq_handler(unsigned irq, struct irq_desc *desc)
+{
+	/* interrupt dedicated per bank */
+	struct irq_chip *chip = irq_get_chip(irq);
+	struct st_gpio_bank *bank = irq_get_handler_data(irq);
+
+	chained_irq_enter(chip, desc);
+	__gpio_irq_handler(bank);
+	chained_irq_exit(chip, desc);
+}
+
+static void st_gpio_irqmux_handler(unsigned irq, struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_get_chip(irq);
+	struct st_pinctrl *info = irq_get_handler_data(irq);
+	unsigned long status;
+	int n;
+
+	chained_irq_enter(chip, desc);
+
+	status = readl(info->irqmux_base);
+
+	for_each_set_bit(n, &status, ST_GPIO_PINS_PER_BANK)
+		__gpio_irq_handler(&info->banks[n]);
+
+	chained_irq_exit(chip, desc);
+}
+
 static struct gpio_chip st_gpio_template = {
 	.request		= st_gpio_request,
 	.free			= st_gpio_free,
@@ -1210,6 +1317,58 @@ static struct gpio_chip st_gpio_template = {
 	.ngpio			= ST_GPIO_PINS_PER_BANK,
 	.of_gpio_n_cells	= 1,
 	.of_xlate		= st_gpio_xlate,
+	.to_irq			= st_gpio_to_irq,
+};
+
+static struct irq_chip st_gpio_irqchip = {
+	.name		= "GPIO",
+	.irq_disable	= st_gpio_irq_disable,
+	.irq_mask	= st_gpio_irq_disable,
+	.irq_unmask	= st_gpio_irq_enable,
+	.irq_set_type	= st_gpio_irq_set_type,
+};
+
+static int st_gpio_irq_domain_map(struct irq_domain *h,
+			unsigned int virq, irq_hw_number_t hw)
+{
+	struct st_gpio_bank *bank = h->host_data;
+
+	irq_set_chip(virq, &st_gpio_irqchip);
+	irq_set_handler(virq, handle_level_irq);
+	set_irq_flags(virq, IRQF_VALID);
+	irq_set_chip_data(virq, bank);
+
+	return 0;
+}
+
+static int st_gpio_irq_domain_xlate(struct irq_domain *d,
+	struct device_node *ctrlr, const u32 *intspec, unsigned int intsize,
+	irq_hw_number_t *out_hwirq, unsigned int *out_type)
+{
+	struct st_gpio_bank *bank = d->host_data;
+	int ret;
+	int pin = bank->gpio_chip.base + intspec[0];
+
+	if (WARN_ON(intsize < 2))
+		return -EINVAL;
+
+	*out_hwirq = intspec[0];
+	*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
+
+	ret = gpio_request(pin, ctrlr->full_name);
+	if (ret)
+		return ret;
+
+	ret = gpio_direction_input(pin);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static struct irq_domain_ops st_gpio_irq_ops = {
+	.map	= st_gpio_irq_domain_map,
+	.xlate	= st_gpio_irq_domain_xlate,
 };
 
 static int st_gpiolib_register_bank(struct st_pinctrl *info,
@@ -1219,7 +1378,7 @@ static int st_gpiolib_register_bank(struct st_pinctrl *info,
 	struct pinctrl_gpio_range *range = &bank->range;
 	struct device *dev = info->dev;
 	int bank_num = of_alias_get_id(np, "gpio");
-	struct resource res;
+	struct resource res, irq_res;
 	int err;
 
 	if (of_address_to_resource(np, 0, &res))
@@ -1248,6 +1407,43 @@ static int st_gpiolib_register_bank(struct st_pinctrl *info,
 	}
 	dev_info(dev, "%s bank added.\n", range->name);
 
+	/**
+	 * GPIO bank can have one of the two possible types of
+	 * interrupt-wirings.
+	 *
+	 * First type is via irqmux, single interrupt is used by multiple
+	 * gpio banks. This reduces number of overall interrupts numbers
+	 * required. All these banks belong to a single pincontroller.
+	 *		  _________
+	 *		 |	   |----> [gpio-bank (n)    ]
+	 *		 |	   |----> [gpio-bank (n + 1)]
+	 *	[irqN]-- | irq-mux |----> [gpio-bank (n + 2)]
+	 *		 |	   |----> [gpio-bank (...  )]
+	 *		 |_________|----> [gpio-bank (n + 7)]
+	 *
+	 * Second type has a dedicated interrupt per each gpio bank.
+	 *
+	 *	[irqN]----> [gpio-bank (n)]
+	 */
+
+	if (!of_irq_to_resource(np, 0, &irq_res)) {
+		bank->gpio_irq = irq_res.start;
+		irq_set_chained_handler(bank->gpio_irq, st_gpio_irq_handler);
+		irq_set_handler_data(bank->gpio_irq, bank);
+	}
+
+	if (info->irqmux_irq > 0 || bank->gpio_irq > 0) {
+		/* Setup IRQ domain */
+		bank->domain  = irq_domain_add_linear(np,
+						ST_GPIO_PINS_PER_BANK,
+						&st_gpio_irq_ops, bank);
+		if (!bank->domain)
+			dev_err(dev, "Failed to add irq domain for [%s]\n",
+				np->full_name);
+	} else {
+		dev_info(dev, "No IRQ support for [%s] bank\n", np->full_name);
+	}
+
 	return 0;
 }
 
@@ -1276,6 +1472,8 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *child;
 	int grp_index = 0;
+	int irq;
+	struct resource *res;
 
 	st_pctl_dt_child_count(info, np);
 	if (!info->nbanks) {
@@ -1306,6 +1504,23 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
 	}
 	info->data = of_match_node(st_pctl_of_match, np)->data;
 
+	irq = platform_get_irq(pdev, 0);
+
+	if (irq > 0) {
+		info->irqmux_irq = irq;
+		res = platform_get_resource_byname(pdev,
+					IORESOURCE_MEM, "irqmux");
+		info->irqmux_base = devm_ioremap_resource(&pdev->dev, res);
+
+		if (IS_ERR(info->irqmux_base))
+			return PTR_ERR(info->irqmux_base);
+
+		irq_set_chained_handler(info->irqmux_irq,
+						st_gpio_irqmux_handler);
+		irq_set_handler_data(info->irqmux_irq, info);
+
+	}
+
 	pctl_desc->npins = info->nbanks * ST_GPIO_PINS_PER_BANK;
 	pdesc =	devm_kzalloc(&pdev->dev,
 			sizeof(*pdesc) * pctl_desc->npins, GFP_KERNEL);
-- 
1.7.6.5

Re: [PATCH v1 2/5] pinctrl: st: Add Interrupt support.

[Linus Walleij]

On Tue, Jan 14, 2014 at 3:52 PM,  <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org> wrote:

> ST Pincontroller GPIO bank can have one of the two possible types of
> interrupt-wirings.

Interesting :-)

> +Pin controller node:
> +Required properties:
>  - compatible   : should be "st,<SOC>-<pio-block>-pinctrl"
>         like st,stih415-sbc-pinctrl, st,stih415-front-pinctrl and so on.
> -- gpio-controller : Indicates this device is a GPIO controller
> -- #gpio-cells    : Should be one. The first cell is the pin number.
> +- st,syscfg            : Should be a phandle of the syscfg node.

So why do you add this? This is totally unused by your driver.

>  - st,retime-pin-mask   : Should be mask to specify which pins can be retimed.
>         If the property is not present, it is assumed that all the pins in the
>         bank are capable of retiming. Retiming is mainly used to improve the
>         IO timing margins of external synchronous interfaces.
> -- st,bank-name         : Should be a name string for this bank as
> -                       specified in datasheet.

Why do you have this property? The driver is not using it.

And what is wrong with just using that name for the node?

> -- st,syscfg            : Should be a phandle of the syscfg node.
> +- ranges       : specifies the ranges for the pin controller memory.

And what are they used for? I've never seen this before.

> +Optional properties:
> +- interrupts   : Interrupt number of the irqmux. If the interrupt is shared
> +  with other gpio banks via irqmux.
> +  a irqline and gpio banks.
> +- reg          : irqmux memory resource. If irqmux is present.
> +- reg-names    : irqmux resource should be named as "irqmux".
> +
> +GPIO controller node.
> +Required properties:
> +- gpio-controller : Indicates this device is a GPIO controller
> +- #gpio-cells    : Should be one. The first cell is the pin number.
> +- st,bank-name   : Should be a name string for this bank as specified in
> +  datasheet.

Again, why?

> +Optional properties:
> +- interrupts   : Interrupt number for this gpio bank. If there is a dedicated
> +  interrupt wired up for this gpio bank.
> +
> +- interrupt-controller : Indicates this device is a interrupt controller. GPIO
> +  bank can be an interrupt controller iff one of the interrupt type either via
> +irqmux or a dedicated interrupt per bank is specified.
> +
> +- #interrupt-cells: the value of this property should be 2.
> +     - First Cell: represents the external gpio interrupt number local to the
> +       external gpio interrupt space of the controller.
> +     - Second Cell: flags to identify the type of the interrupt
> +       - 1 = rising edge triggered
> +       - 2 = falling edge triggered
> +       - 3 = rising and falling edge triggered
> +       - 4 = high level triggered
> +       - 8 = low level triggered

Correct, but reference symbols from:
include/dt-bindings/interrupt-controller/irq.h
in example.


>
>  Example:
>         pin-controller-sbc {

Please put in an updated example making use of all the
new props.


(...)
> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c

> @@ -271,6 +276,8 @@ struct st_gpio_bank {
>         struct pinctrl_gpio_range       range;
>         void __iomem                    *base;
>         struct st_pio_control           pc;
> +       struct  irq_domain              *domain;
> +       int                             gpio_irq;

Why are you putting this IRQ into the state container when it can be
a function local variable in probe()?

>  struct st_pinctrl {
> @@ -284,6 +291,8 @@ struct st_pinctrl {
>         int                             ngroups;
>         struct regmap                   *regmap;
>         const struct st_pctl_data       *data;
> +       void __iomem                    *irqmux_base;
> +       int                             irqmux_irq;

Dito. I think.

> +static int st_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> +       struct st_gpio_bank *bank = gpio_chip_to_bank(chip);
> +       int virq;

Just name this variable "irq". It is no more virtual than any other
IRQ.

> +
> +       if (bank->domain && (offset < chip->ngpio))
> +               virq = irq_create_mapping(bank->domain, offset);

No, don't do the create_mapping() call in the .to_irq() function.
Call irq_create_mapping() for *all* valid hwirqs in probe() and use
irq_find_mapping() here.

> +static void st_gpio_irq_disable(struct irq_data *d)
> +{
> +       struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> +
> +       writel(BIT(d->hwirq), bank->base + REG_PIO_CLR_PMASK);
> +}
> +
> +static void st_gpio_irq_enable(struct irq_data *d)
> +{
> +       struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
> +
> +       writel(BIT(d->hwirq), bank->base + REG_PIO_SET_PMASK);
> +}

I *strongly* suspect that these two should be replaced with
_mask()/_unmask() rather than using disable/enable.

Because that seems to be what they are doing.

(...)
> +static void __gpio_irq_handler(struct st_gpio_bank *bank)
> +{
> +       unsigned long port_in, port_mask, port_comp, port_active;
> +       int n;
> +
> +       port_in = readl(bank->base + REG_PIO_PIN);
> +       port_comp = readl(bank->base + REG_PIO_PCOMP);
> +       port_mask = readl(bank->base + REG_PIO_PMASK);
> +
> +       port_active = (port_in ^ port_comp) & port_mask;
> +
> +       for_each_set_bit(n, &port_active, BITS_PER_LONG) {
> +               generic_handle_irq(irq_find_mapping(bank->domain, n));

So what happens if new IRQs appear in the register while you are
inside this loop?

Check this recent patch:
http://marc.info/?l=linux-arm-kernel&m=138979164119464&w=2

Especially this:

+ for (;;) {
+     mask = ioread8(host->base + CLRHILVINT) & 0xff;
+     mask |= (ioread8(host->base + SECOINT) & SECOINT_MASK) << 8;
+     mask |= (ioread8(host->base + PRIMINT) & PRIMINT_MASK) << 8;
+     mask &= host->irq_high_enabled | (host->irq_sys_enabled << 8);
+     if (mask == 0)
+         break;
+     for_each_set_bit(n, &mask, BITS_PER_LONG)
+     generic_handle_irq(irq_find_mapping(host->domain, n));
+ }


> +static struct irq_chip st_gpio_irqchip = {
> +       .name           = "GPIO",
> +       .irq_disable    = st_gpio_irq_disable,
> +       .irq_mask       = st_gpio_irq_disable,
> +       .irq_unmask     = st_gpio_irq_enable,

Just implement mask/unmask as indicated earlier.

> +       .irq_set_type   = st_gpio_irq_set_type,
> +};

You need to mark IRQ GPIO lines as used for IRQ.

Add startup() and shutdown() hooks similar to those I add
in this patch:

http://marc.info/?l=linux-gpio&m=138977709114785&w=2

> +static int st_gpio_irq_domain_xlate(struct irq_domain *d,
> +       struct device_node *ctrlr, const u32 *intspec, unsigned int intsize,
> +       irq_hw_number_t *out_hwirq, unsigned int *out_type)
> +{
> +       struct st_gpio_bank *bank = d->host_data;
> +       int ret;
> +       int pin = bank->gpio_chip.base + intspec[0];
> +
> +       if (WARN_ON(intsize < 2))
> +               return -EINVAL;
> +
> +       *out_hwirq = intspec[0];
> +       *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> +
> +       ret = gpio_request(pin, ctrlr->full_name);
> +       if (ret)
> +               return ret;
> +
> +       ret = gpio_direction_input(pin);
> +       if (ret)
> +               return ret;

We recently concluded that you should *NOT* call
gpio_request() and gpio_direction_input() from xlate or similar.

Instead: set up the hardware directly in mask/unmask callbacks
so that the irqchip can trigger IRQs directly without any
interaction with the GPIO subsystem.

By implementing the startup/shutdown hooks as indicated
above you can still indicate to the GPIO subsystem what is
going on and it will enforce that e.g. the line is not set to
output.

>  static int st_gpiolib_register_bank(struct st_pinctrl *info,
> @@ -1219,7 +1378,7 @@ static int st_gpiolib_register_bank(struct st_pinctrl *info,
>         struct pinctrl_gpio_range *range = &bank->range;
>         struct device *dev = info->dev;
>         int bank_num = of_alias_get_id(np, "gpio");
> -       struct resource res;
> +       struct resource res, irq_res;
>         int err;
>
>         if (of_address_to_resource(np, 0, &res))
> @@ -1248,6 +1407,43 @@ static int st_gpiolib_register_bank(struct st_pinctrl *info,
>         }
>         dev_info(dev, "%s bank added.\n", range->name);
>
> +       /**
> +        * GPIO bank can have one of the two possible types of
> +        * interrupt-wirings.
> +        *
> +        * First type is via irqmux, single interrupt is used by multiple
> +        * gpio banks. This reduces number of overall interrupts numbers
> +        * required. All these banks belong to a single pincontroller.
> +        *                _________
> +        *               |         |----> [gpio-bank (n)    ]
> +        *               |         |----> [gpio-bank (n + 1)]
> +        *      [irqN]-- | irq-mux |----> [gpio-bank (n + 2)]
> +        *               |         |----> [gpio-bank (...  )]
> +        *               |_________|----> [gpio-bank (n + 7)]
> +        *
> +        * Second type has a dedicated interrupt per each gpio bank.
> +        *
> +        *      [irqN]----> [gpio-bank (n)]
> +        */
> +
> +       if (!of_irq_to_resource(np, 0, &irq_res)) {
> +               bank->gpio_irq = irq_res.start;
> +               irq_set_chained_handler(bank->gpio_irq, st_gpio_irq_handler);
> +               irq_set_handler_data(bank->gpio_irq, bank);
> +       }
> +
> +       if (info->irqmux_irq > 0 || bank->gpio_irq > 0) {
> +               /* Setup IRQ domain */
> +               bank->domain  = irq_domain_add_linear(np,
> +                                               ST_GPIO_PINS_PER_BANK,
> +                                               &st_gpio_irq_ops, bank);
> +               if (!bank->domain)
> +                       dev_err(dev, "Failed to add irq domain for [%s]\n",
> +                               np->full_name);
> +       } else {
> +               dev_info(dev, "No IRQ support for [%s] bank\n", np->full_name);

Why is the [bank name] inside [brackets]?

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

Re: [PATCH v1 2/5] pinctrl: st: Add Interrupt support.

[srinivas kandagatla]

Thankyou for the detailed comments.

On 15/01/14 14:15, Linus Walleij wrote:
> On Tue, Jan 14, 2014 at 3:52 PM,  <srinivas.kandagatla@st.com> wrote:
> 
>> ST Pincontroller GPIO bank can have one of the two possible types of
>> interrupt-wirings.
> 
> Interesting :-)
> 
>> +Pin controller node:
>> +Required properties:
>>  - compatible   : should be "st,<SOC>-<pio-block>-pinctrl"
>>         like st,stih415-sbc-pinctrl, st,stih415-front-pinctrl and so on.
>> -- gpio-controller : Indicates this device is a GPIO controller
>> -- #gpio-cells    : Should be one. The first cell is the pin number.
>> +- st,syscfg            : Should be a phandle of the syscfg node.

> 
> So why do you add this? This is totally unused by your driver.
The document got re-ordered by adding new information.
st,syscfg is not a new field, we use it for getting hold of regmap instance.

I will re-check if this addition was due to some white spaces.
> 
>>  - st,retime-pin-mask   : Should be mask to specify which pins can be retimed.
>>         If the property is not present, it is assumed that all the pins in the
>>         bank are capable of retiming. Retiming is mainly used to improve the
>>         IO timing margins of external synchronous interfaces.
>> -- st,bank-name         : Should be a name string for this bank as
>> -                       specified in datasheet.
> 
> Why do you have this property? The driver is not using it.
Its used for bank label.
> 
> And what is wrong with just using that name for the node?
On ST chips the gpio banks are named irregularly in some instances.
They are named like PIO0, PIO1 , PIO100, PIO101 and so on. Same naming
is used across board schematics too, so Its easy for debugging if we
have this consistency in the driver as well.

As node names are in pio@address format, which are not same as
bank-names in the datasheet.
> 
>> -- st,syscfg            : Should be a phandle of the syscfg node.
>> +- ranges       : specifies the ranges for the pin controller memory.
> 
> And what are they used for? I've never seen this before.
This was suggested when I first submitted the driver.
The intention here was to describe the mapping between pinctrl (parent)
node and the gpio bank(children) nodes.

I will fix the document for this too in next version.

> 
>> +Optional properties:
>> +- interrupts   : Interrupt number of the irqmux. If the interrupt is shared
>> +  with other gpio banks via irqmux.
>> +  a irqline and gpio banks.
>> +- reg          : irqmux memory resource. If irqmux is present.
>> +- reg-names    : irqmux resource should be named as "irqmux".
>> +
>> +GPIO controller node.
>> +Required properties:
>> +- gpio-controller : Indicates this device is a GPIO controller
>> +- #gpio-cells    : Should be one. The first cell is the pin number.
>> +- st,bank-name   : Should be a name string for this bank as specified in
>> +  datasheet.
> 
> Again, why?
> 
>> +Optional properties:
>> +- interrupts   : Interrupt number for this gpio bank. If there is a dedicated
>> +  interrupt wired up for this gpio bank.
>> +
>> +- interrupt-controller : Indicates this device is a interrupt controller. GPIO
>> +  bank can be an interrupt controller iff one of the interrupt type either via
>> +irqmux or a dedicated interrupt per bank is specified.
>> +
>> +- #interrupt-cells: the value of this property should be 2.
>> +     - First Cell: represents the external gpio interrupt number local to the
>> +       external gpio interrupt space of the controller.
>> +     - Second Cell: flags to identify the type of the interrupt
>> +       - 1 = rising edge triggered
>> +       - 2 = falling edge triggered
>> +       - 3 = rising and falling edge triggered
>> +       - 4 = high level triggered
>> +       - 8 = low level triggered
> 
> Correct, but reference symbols from:
> include/dt-bindings/interrupt-controller/irq.h
> in example.
Ok.
> 
> 
>>
>>  Example:
>>         pin-controller-sbc {
> 
> Please put in an updated example making use of all the
> new props.
Will do it in next version.
> 
> 
> (...)
>> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
> 
>> @@ -271,6 +276,8 @@ struct st_gpio_bank {
>>         struct pinctrl_gpio_range       range;
>>         void __iomem                    *base;
>>         struct st_pio_control           pc;
>> +       struct  irq_domain              *domain;
>> +       int                             gpio_irq;
> 
> Why are you putting this IRQ into the state container when it can be
> a function local variable in probe()?
I agree, this should be a local variable.
> 
>>  struct st_pinctrl {
>> @@ -284,6 +291,8 @@ struct st_pinctrl {
>>         int                             ngroups;
>>         struct regmap                   *regmap;
>>         const struct st_pctl_data       *data;
>> +       void __iomem                    *irqmux_base;
>> +       int                             irqmux_irq;
> 
> Dito. I think.
I agree, this should be a local variable too.
> 
>> +static int st_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
>> +{
>> +       struct st_gpio_bank *bank = gpio_chip_to_bank(chip);
>> +       int virq;
> 
> Just name this variable "irq". It is no more virtual than any other
> IRQ.
Ok.

> 
>> +
>> +       if (bank->domain && (offset < chip->ngpio))
>> +               virq = irq_create_mapping(bank->domain, offset);
> 
> No, don't do the create_mapping() call in the .to_irq() function.
> Call irq_create_mapping() for *all* valid hwirqs in probe() and use
> irq_find_mapping() here.
Ok.

> 
>> +static void st_gpio_irq_disable(struct irq_data *d)
>> +{
>> +       struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
>> +
>> +       writel(BIT(d->hwirq), bank->base + REG_PIO_CLR_PMASK);
>> +}
>> +
>> +static void st_gpio_irq_enable(struct irq_data *d)
>> +{
>> +       struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
>> +
>> +       writel(BIT(d->hwirq), bank->base + REG_PIO_SET_PMASK);
>> +}
> 
> I *strongly* suspect that these two should be replaced with
> _mask()/_unmask() rather than using disable/enable.
> 
> Because that seems to be what they are doing.
I will do this change in next version.
> 
> (...)
>> +static void __gpio_irq_handler(struct st_gpio_bank *bank)
>> +{
>> +       unsigned long port_in, port_mask, port_comp, port_active;
>> +       int n;
>> +
>> +       port_in = readl(bank->base + REG_PIO_PIN);
>> +       port_comp = readl(bank->base + REG_PIO_PCOMP);
>> +       port_mask = readl(bank->base + REG_PIO_PMASK);
>> +
>> +       port_active = (port_in ^ port_comp) & port_mask;
>> +
>> +       for_each_set_bit(n, &port_active, BITS_PER_LONG) {
>> +               generic_handle_irq(irq_find_mapping(bank->domain, n));
> 
> So what happens if new IRQs appear in the register while you are
> inside this loop?
I agree, there is a possibility of missing interrupt here, doing similar
to what you suggested makes sense.
> 
> Check this recent patch:
> http://marc.info/?l=linux-arm-kernel&m=138979164119464&w=2
> 
> Especially this:
> 
> + for (;;) {
> +     mask = ioread8(host->base + CLRHILVINT) & 0xff;
> +     mask |= (ioread8(host->base + SECOINT) & SECOINT_MASK) << 8;
> +     mask |= (ioread8(host->base + PRIMINT) & PRIMINT_MASK) << 8;
> +     mask &= host->irq_high_enabled | (host->irq_sys_enabled << 8);
> +     if (mask == 0)
> +         break;
> +     for_each_set_bit(n, &mask, BITS_PER_LONG)
> +     generic_handle_irq(irq_find_mapping(host->domain, n));
> + }
> 
> 
>> +static struct irq_chip st_gpio_irqchip = {
>> +       .name           = "GPIO",
>> +       .irq_disable    = st_gpio_irq_disable,
>> +       .irq_mask       = st_gpio_irq_disable,
>> +       .irq_unmask     = st_gpio_irq_enable,
> 
> Just implement mask/unmask as indicated earlier.
> 
>> +       .irq_set_type   = st_gpio_irq_set_type,
>> +};
> 
> You need to mark IRQ GPIO lines as used for IRQ.
> 
> Add startup() and shutdown() hooks similar to those I add
> in this patch:
> 
> http://marc.info/?l=linux-gpio&m=138977709114785&w=2

Will add this in next version.

> 
>> +static int st_gpio_irq_domain_xlate(struct irq_domain *d,
>> +       struct device_node *ctrlr, const u32 *intspec, unsigned int intsize,
>> +       irq_hw_number_t *out_hwirq, unsigned int *out_type)
>> +{
>> +       struct st_gpio_bank *bank = d->host_data;
>> +       int ret;
>> +       int pin = bank->gpio_chip.base + intspec[0];
>> +
>> +       if (WARN_ON(intsize < 2))
>> +               return -EINVAL;
>> +
>> +       *out_hwirq = intspec[0];
>> +       *out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
>> +
>> +       ret = gpio_request(pin, ctrlr->full_name);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = gpio_direction_input(pin);
>> +       if (ret)
>> +               return ret;
> 
> We recently concluded that you should *NOT* call
> gpio_request() and gpio_direction_input() from xlate or similar.

Great, I can get rid of this function and use generic xlate function
instead.
> 
> Instead: set up the hardware directly in mask/unmask callbacks
> so that the irqchip can trigger IRQs directly without any
> interaction with the GPIO subsystem.
Ok, got it.
> 
> By implementing the startup/shutdown hooks as indicated
> above you can still indicate to the GPIO subsystem what is
> going on and it will enforce that e.g. the line is not set to
> output.
> 
Ok.

>> +               bank->domain  = irq_domain_add_linear(np,
>> +                                               ST_GPIO_PINS_PER_BANK,
>> +                                               &st_gpio_irq_ops, bank);
>> +               if (!bank->domain)
>> +                       dev_err(dev, "Failed to add irq domain for [%s]\n",
>> +                               np->full_name);
>> +       } else {
>> +               dev_info(dev, "No IRQ support for [%s] bank\n", np->full_name);
> 
> Why is the [bank name] inside [brackets]?
There is no strong reason, I will remove it.

> 
> Yours,
> Linus Walleij
> 
> 

[PATCH v1 3/5] pinctrl: st: Add software edge trigger interrupt support.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

ST pin controller does not have hardware support for detecting edge
triggered interrupts, It only has level triggering support.
This patch attempts to fake up edge triggers from hw level trigger
support in software. With this facility now the gpios can be easily used
for keypads, otherwise it would be difficult for drivers like keypads to
work with level trigger interrupts.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 drivers/pinctrl/pinctrl-st.c |   66 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 03c0cfd..7d2780e 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -271,6 +271,24 @@ struct st_pctl_group {
 	struct st_pinconf	*pin_conf;
 };
 
+#define ST_IRQ_EDGE_CONF_BITS_PER_PIN	4
+#define ST_IRQ_EDGE_MASK		0xf
+#define ST_IRQ_EDGE_FALLING		BIT(0)
+#define ST_IRQ_EDGE_RISING		BIT(1)
+#define ST_IRQ_EDGE_BOTH		(BIT(0) | BIT(1))
+
+#define ST_IRQ_RISING_EDGE_CONF(pin) \
+	(ST_IRQ_EDGE_RISING << (pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN))
+
+#define ST_IRQ_FALLING_EDGE_CONF(pin) \
+	(ST_IRQ_EDGE_FALLING << (pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN))
+
+#define ST_IRQ_BOTH_EDGE_CONF(pin) \
+	(ST_IRQ_EDGE_BOTH << (pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN))
+
+#define ST_IRQ_EDGE_CONF(conf, pin) \
+	(conf >> (pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN) & ST_IRQ_EDGE_MASK)
+
 struct st_gpio_bank {
 	struct gpio_chip		gpio_chip;
 	struct pinctrl_gpio_range	range;
@@ -278,6 +296,8 @@ struct st_gpio_bank {
 	struct st_pio_control		pc;
 	struct	irq_domain		*domain;
 	int				gpio_irq;
+	unsigned long			irq_edge_conf;
+	spinlock_t                      lock;
 };
 
 struct st_pinctrl {
@@ -1241,20 +1261,40 @@ static void st_gpio_irq_enable(struct irq_data *d)
 static int st_gpio_irq_set_type(struct irq_data *d, unsigned type)
 {
 	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
 	int comp, pin = d->hwirq;
 	u32 val;
+	u32 pin_edge_conf = 0;
 
 	switch (type) {
 	case IRQ_TYPE_LEVEL_HIGH:
 		comp = 0;
 		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		comp = 0;
+		pin_edge_conf = ST_IRQ_FALLING_EDGE_CONF(pin);
+		break;
 	case IRQ_TYPE_LEVEL_LOW:
 		comp = 1;
 		break;
+	case IRQ_TYPE_EDGE_RISING:
+		comp = 1;
+		pin_edge_conf = ST_IRQ_RISING_EDGE_CONF(pin);
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		comp = st_gpio_get(&bank->gpio_chip, pin);
+		pin_edge_conf = ST_IRQ_BOTH_EDGE_CONF(pin);
+		break;
 	default:
 		return -EINVAL;
 	}
 
+	spin_lock_irqsave(&bank->lock, flags);
+	bank->irq_edge_conf &=  ~(ST_IRQ_EDGE_MASK << (
+				pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN));
+	bank->irq_edge_conf |= pin_edge_conf;
+	spin_unlock_irqrestore(&bank->lock, flags);
+
 	val = readl(bank->base + REG_PIO_PCOMP);
 	val &= ~BIT(pin);
 	val |= (comp << pin);
@@ -1266,7 +1306,12 @@ static int st_gpio_irq_set_type(struct irq_data *d, unsigned type)
 static void __gpio_irq_handler(struct st_gpio_bank *bank)
 {
 	unsigned long port_in, port_mask, port_comp, port_active;
-	int n;
+	unsigned long bank_edge_mask, flags;
+	int n, val, pin_edge_cfg;
+
+	spin_lock_irqsave(&bank->lock, flags);
+	bank_edge_mask = bank->irq_edge_conf;
+	spin_unlock_irqrestore(&bank->lock, flags);
 
 	port_in = readl(bank->base + REG_PIO_PIN);
 	port_comp = readl(bank->base + REG_PIO_PCOMP);
@@ -1275,6 +1320,22 @@ static void __gpio_irq_handler(struct st_gpio_bank *bank)
 	port_active = (port_in ^ port_comp) & port_mask;
 
 	for_each_set_bit(n, &port_active, BITS_PER_LONG) {
+		/* check if we are detecting fake edges ... */
+		pin_edge_cfg = ST_IRQ_EDGE_CONF(bank_edge_mask, n);
+
+		if (pin_edge_cfg) {
+			/* edge detection. */
+			val = st_gpio_get(&bank->gpio_chip, n);
+			if (val)
+				writel(BIT(n), bank->base + REG_PIO_SET_PCOMP);
+			else
+				writel(BIT(n), bank->base + REG_PIO_CLR_PCOMP);
+
+			if (pin_edge_cfg != ST_IRQ_EDGE_BOTH &&
+				!((pin_edge_cfg & ST_IRQ_EDGE_FALLING) ^ val))
+					continue;
+		}
+
 		generic_handle_irq(irq_find_mapping(bank->domain, n));
 	}
 }
@@ -1334,7 +1395,7 @@ static int st_gpio_irq_domain_map(struct irq_domain *h,
 	struct st_gpio_bank *bank = h->host_data;
 
 	irq_set_chip(virq, &st_gpio_irqchip);
-	irq_set_handler(virq, handle_level_irq);
+	irq_set_handler(virq, handle_simple_irq);
 	set_irq_flags(virq, IRQF_VALID);
 	irq_set_chip_data(virq, bank);
 
@@ -1392,6 +1453,7 @@ static int st_gpiolib_register_bank(struct st_pinctrl *info,
 	bank->gpio_chip.base = bank_num * ST_GPIO_PINS_PER_BANK;
 	bank->gpio_chip.ngpio = ST_GPIO_PINS_PER_BANK;
 	bank->gpio_chip.of_node = np;
+	spin_lock_init(&bank->lock);
 
 	of_property_read_string(np, "st,bank-name", &range->name);
 	bank->gpio_chip.label = range->name;
-- 
1.7.6.5

Re: [PATCH v1 3/5] pinctrl: st: Add software edge trigger interrupt support.

[Linus Walleij]

On Tue, Jan 14, 2014 at 3:52 PM,  <srinivas.kandagatla@st.com> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> ST pin controller does not have hardware support for detecting edge
> triggered interrupts, It only has level triggering support.
> This patch attempts to fake up edge triggers from hw level trigger
> support in software. With this facility now the gpios can be easily used
> for keypads, otherwise it would be difficult for drivers like keypads to
> work with level trigger interrupts.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>

Clever! Mostly I like the patch as it is but:

>         for_each_set_bit(n, &port_active, BITS_PER_LONG) {
> +               /* check if we are detecting fake edges ... */
> +               pin_edge_cfg = ST_IRQ_EDGE_CONF(bank_edge_mask, n);
> +
> +               if (pin_edge_cfg) {
> +                       /* edge detection. */
> +                       val = st_gpio_get(&bank->gpio_chip, n);
> +                       if (val)
> +                               writel(BIT(n), bank->base + REG_PIO_SET_PCOMP);
> +                       else
> +                               writel(BIT(n), bank->base + REG_PIO_CLR_PCOMP);
> +
> +                       if (pin_edge_cfg != ST_IRQ_EDGE_BOTH &&
> +                               !((pin_edge_cfg & ST_IRQ_EDGE_FALLING) ^ val))
> +                                       continue;
> +               }
> +

Please insert comments here to explain what you are actually doing
because I sure as hell do not understand this code without comments
describing the trick used.

Yours,
Linus Walleij

Re: [PATCH v1 3/5] pinctrl: st: Add software edge trigger interrupt support.

[srinivas kandagatla]

Thankyou for reviewing the patch.

On 15/01/14 14:27, Linus Walleij wrote:
> On Tue, Jan 14, 2014 at 3:52 PM,  <srinivas.kandagatla@st.com> wrote:
> 
>> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>>
>> ST pin controller does not have hardware support for detecting edge
>> triggered interrupts, It only has level triggering support.
>> This patch attempts to fake up edge triggers from hw level trigger
>> support in software. With this facility now the gpios can be easily used
>> for keypads, otherwise it would be difficult for drivers like keypads to
>> work with level trigger interrupts.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> 
> Clever! Mostly I like the patch as it is but:
> 
>>         for_each_set_bit(n, &port_active, BITS_PER_LONG) {
>> +               /* check if we are detecting fake edges ... */
>> +               pin_edge_cfg = ST_IRQ_EDGE_CONF(bank_edge_mask, n);
>> +
>> +               if (pin_edge_cfg) {
>> +                       /* edge detection. */
>> +                       val = st_gpio_get(&bank->gpio_chip, n);
>> +                       if (val)
>> +                               writel(BIT(n), bank->base + REG_PIO_SET_PCOMP);
>> +                       else
>> +                               writel(BIT(n), bank->base + REG_PIO_CLR_PCOMP);
>> +
>> +                       if (pin_edge_cfg != ST_IRQ_EDGE_BOTH &&
>> +                               !((pin_edge_cfg & ST_IRQ_EDGE_FALLING) ^ val))
>> +                                       continue;
>> +               }
>> +
> 
> Please insert comments here to explain what you are actually doing
> because I sure as hell do not understand this code without comments
> describing the trick used.

I agree, I will document this logic in next version.

Thanks,
srini
> 
> Yours,
> Linus Walleij
> 
> 

[PATCH v1 4/5] ARM:STi: STiH416: Add interrupt support for pin controller

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

This patch adds interrupt support for STiH416 pin controllers.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 arch/arm/boot/dts/stih416-pinctrl.dtsi |   81 ++++++++++++++++++++++++++++++++
 1 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/stih416-pinctrl.dtsi b/arch/arm/boot/dts/stih416-pinctrl.dtsi
index 74f8cfc..3adbd98 100644
--- a/arch/arm/boot/dts/stih416-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih416-pinctrl.dtsi
@@ -8,6 +8,7 @@
  * publishhed by the Free Software Foundation.
  */
 #include "st-pincfg.h"
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 / {
 
 	aliases {
@@ -49,41 +50,57 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-sbc-pinctrl";
 			st,syscfg	= <&syscfg_sbc>;
+			reg 		= <0xfe61f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfe610000 0x6000>;
 
 			PIO0: gpio@fe610000 {
 				gpio-controller;
 				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO0";
 			};
 			PIO1: gpio@fe611000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO1";
 			};
 			PIO2: gpio@fe612000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO2";
 			};
 			PIO3: gpio@fe613000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO3";
 			};
 			PIO4: gpio@fe614000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO4";
 			};
 			PIO40: gpio@fe615000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO40";
 				st,retime-pin-mask = <0x7f>;
@@ -181,65 +198,89 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-front-pinctrl";
 			st,syscfg	= <&syscfg_front>;
+			reg 		= <0xfee0f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfee00000 0x10000>;
 
 			PIO5: gpio@fee00000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO5";
 			};
 			PIO6: gpio@fee01000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO6";
 			};
 			PIO7: gpio@fee02000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO7";
 			};
 			PIO8: gpio@fee03000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO8";
 			};
 			PIO9: gpio@fee04000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO9";
 			};
 			PIO10: gpio@fee05000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO10";
 			};
 			PIO11: gpio@fee06000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x6000 0x100>;
 				st,bank-name	= "PIO11";
 			};
 			PIO12: gpio@fee07000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x7000 0x100>;
 				st,bank-name	= "PIO12";
 			};
 			PIO30: gpio@fee08000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x8000 0x100>;
 				st,bank-name	= "PIO30";
 			};
 			PIO31: gpio@fee09000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x9000 0x100>;
 				st,bank-name	= "PIO31";
 			};
@@ -276,41 +317,57 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-rear-pinctrl";
 			st,syscfg	= <&syscfg_rear>;
+			reg 		= <0xfe82f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges 		= <0 0xfe820000 0x6000>;
 
 			PIO13: gpio@fe820000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO13";
 			};
 			PIO14: gpio@fe821000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO14";
 			};
 			PIO15: gpio@fe822000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO15";
 			};
 			PIO16: gpio@fe823000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO16";
 			};
 			PIO17: gpio@fe824000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO17";
 			};
 			PIO18: gpio@fe825000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO18";
 				st,retime-pin-mask = <0xf>;
@@ -384,23 +441,33 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-fvdp-fe-pinctrl";
 			st,syscfg	= <&syscfg_fvdp_fe>;
+			reg 		= <0xfd6bf080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfd6b0000 0x3000>;
 
 			PIO100: gpio@fd6b0000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO100";
 			};
 			PIO101: gpio@fd6b1000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO101";
 			};
 			PIO102: gpio@fd6b2000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO102";
 			};
@@ -411,29 +478,41 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-fvdp-lite-pinctrl";
 			st,syscfg		= <&syscfg_fvdp_lite>;
+			reg 		= <0xfd33f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges			= <0 0xfd330000 0x5000>;
 
 			PIO103: gpio@fd330000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO103";
 			};
 			PIO104: gpio@fd331000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO104";
 			};
 			PIO105: gpio@fd332000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO105";
 			};
 			PIO106: gpio@fd333000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO106";
 			};
@@ -441,6 +520,8 @@
 			PIO107: gpio@fd334000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO107";
 				st,retime-pin-mask = <0xf>;
-- 
1.7.6.5

[PATCH v1 5/5] ARM:STi: STiH415: Add interrupt support for pin controller

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

This patch adds interrupt support for STiH415 pin controllers.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 arch/arm/boot/dts/stih415-pinctrl.dtsi |   75 ++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/stih415-pinctrl.dtsi b/arch/arm/boot/dts/stih415-pinctrl.dtsi
index a9324fa..21b3b53 100644
--- a/arch/arm/boot/dts/stih415-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih415-pinctrl.dtsi
@@ -7,6 +7,7 @@
  * publishhed by the Free Software Foundation.
  */
 #include "st-pincfg.h"
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 / {
 
 	aliases {
@@ -45,35 +46,49 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-sbc-pinctrl";
 			st,syscfg	= <&syscfg_sbc>;
+			reg 		= <0xfe61f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges 		= <0 0xfe610000 0x5000>;
 
 			PIO0: gpio@fe610000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO0";
 			};
 			PIO1: gpio@fe611000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO1";
 			};
 			PIO2: gpio@fe612000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO2";
 			};
 			PIO3: gpio@fe613000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO3";
 			};
 			PIO4: gpio@fe614000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO4";
 			};
@@ -169,53 +184,73 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-front-pinctrl";
 			st,syscfg	= <&syscfg_front>;
+			reg 		= <0xfee0f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfee00000 0x8000>;
 
 			PIO5: gpio@fee00000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO5";
 			};
 			PIO6: gpio@fee01000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO6";
 			};
 			PIO7: gpio@fee02000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO7";
 			};
 			PIO8: gpio@fee03000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO8";
 			};
 			PIO9: gpio@fee04000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO9";
 			};
 			PIO10: gpio@fee05000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO10";
 			};
 			PIO11: gpio@fee06000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x6000 0x100>;
 				st,bank-name	= "PIO11";
 			};
 			PIO12: gpio@fee07000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x7000 0x100>;
 				st,bank-name	= "PIO12";
 			};
@@ -244,41 +279,57 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-rear-pinctrl";
 			st,syscfg	= <&syscfg_rear>;
+			reg 		= <0xfe82f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfe820000 0x8000>;
 
 			PIO13: gpio@fe820000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO13";
 			};
 			PIO14: gpio@fe821000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO14";
 			};
 			PIO15: gpio@fe822000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO15";
 			};
 			PIO16: gpio@fe823000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO16";
 			};
 			PIO17: gpio@fe824000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO17";
 			};
 			PIO18: gpio@fe825000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO18";
 			};
@@ -329,23 +380,33 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-left-pinctrl";
 			st,syscfg	= <&syscfg_left>;
+			reg 		= <0xfd6bf080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfd6b0000 0x3000>;
 
 			PIO100: gpio@fd6b0000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO100";
 			};
 			PIO101: gpio@fd6b1000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO101";
 			};
 			PIO102: gpio@fd6b2000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO102";
 			};
@@ -356,35 +417,49 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-right-pinctrl";
 			st,syscfg	= <&syscfg_right>;
+			reg 		= <0xfd33f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfd330000 0x5000>;
 
 			PIO103: gpio@fd330000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO103";
 			};
 			PIO104: gpio@fd331000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO104";
 			};
 			PIO105: gpio@fd332000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO105";
 			};
 			PIO106: gpio@fd333000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO106";
 			};
 			PIO107: gpio@fd334000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO107";
 			};
-- 
1.7.6.5

[PATCH v2 0/4] pinctrl: st: Add interrupt controller support.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

Hi Linus W, 

Thankyou for reviewing v1 of the patch-set the comments were very useful.

This series of v2 patches add interrupt controller support to ST pinctrl
driver incorporating all the review comments.

ST pin controller GPIO bank can have one of the two possible types of
interrupt-wirings.

First type is via irqmux, single interrupt is used by multiple gpio
banks. This reduces number of interrupts numbers required by pin controller.
All these gpio banks belong to a single pin controller.

Second type has a dedicated interrupt per gpio bank. Interrupt number usage is
very high in this case.

This patch adds support to both these wirings.

Also, ST pin controller hardware only supports level triggered interrupts.
However for drivers like gpio keypad requires edge trigger interrupt support,
so one of the patch adds edge trigger interrupt support in software by using
the existing level trigger support in hardware.

Patch 01: Adds level interrupt support to pin controller.
Patch 02 Adds software edge trigger support.
Patch 03,04 : Updates STiH415, STiH416 dts files for interrupt support.


Changes since v1:
	Updated below changes suggest by Linus W.
	- Added updated example in binding document.
	- removed un-necessary varibles in st_gpio_bank and st_pinctrl struct.
	- renamed irq disabled/enabled functions with masked/unmasked
	  functions.
	- Fixed a issue in interrupt handler which could miss interrupts.
	- Added new hooks startup and shutdown which can mark gpio lines.
	- create irq mapping at probe stage instead of doing it in to_irq.
	- removed driver specfic irq_domain xlate function with generic one.
	- Added extra documention for software edge trigger support.
	- reformated few debug messages.

I did skip the first patch from my last v1 patchset as you have already
applied it.

Thanks,
srini


Srinivas Kandagatla (4):
  pinctrl: st: Add Interrupt support.
  pinctrl: st: Add software edge trigger interrupt support.
  ARM:STi: STiH416: Add interrupt support for pin controller
  ARM:STi: STiH415: Add interrupt support for pin controller

 .../devicetree/bindings/pinctrl/pinctrl-st.txt     |   73 ++++-
 arch/arm/boot/dts/stih415-pinctrl.dtsi             |   75 +++++
 arch/arm/boot/dts/stih416-pinctrl.dtsi             |   81 +++++
 drivers/pinctrl/pinctrl-st.c                       |  337 +++++++++++++++++++-
 4 files changed, 558 insertions(+), 8 deletions(-)

-- 
1.7.6.5

[PATCH v2 1/4] pinctrl: st: Add Interrupt support.

[srinivas.kandagatla-qxv4g6HH51o]

From: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>

This patch add interrupt support to the pincontroller driver.

ST Pincontroller GPIO bank can have one of the two possible types of
interrupt-wirings.

First type is via irqmux, single interrupt is used by multiple gpio
banks. This reduces number of overall interrupts numbers required. All
these banks belong to a single pincontroller.
		  _________
		 |	   |----> [gpio-bank (n)    ]
		 |	   |----> [gpio-bank (n + 1)]
	[irqN]-- | irq-mux |----> [gpio-bank (n + 2)]
		 |	   |----> [gpio-bank (...  )]
		 |_________|----> [gpio-bank (n + 7)]

Second type has a dedicated interrupt per gpio bank.

	[irqN]----> [gpio-bank (n)]

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>
---
 .../devicetree/bindings/pinctrl/pinctrl-st.txt     |   73 ++++++-
 drivers/pinctrl/pinctrl-st.c                       |  225 +++++++++++++++++++-
 2 files changed, 290 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-st.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-st.txt
index 05bf82a..4bd5be0 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-st.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-st.txt
@@ -11,18 +11,68 @@ Pull Up (PU) are driven by the related PIO block.
 ST pinctrl driver controls PIO multiplexing block and also interacts with
 gpio driver to configure a pin.
 
-Required properties: (PIO multiplexing block)
+GPIO bank can have one of the two possible types of interrupt-wirings.
+
+First type is via irqmux, single interrupt is used by multiple gpio banks. This
+reduces number of overall interrupts numbers required. All these banks belong to
+a single pincontroller.
+		  _________
+		 |	   |----> [gpio-bank (n)    ]
+		 |	   |----> [gpio-bank (n + 1)]
+	[irqN]-- | irq-mux |----> [gpio-bank (n + 2)]
+		 |	   |----> [gpio-bank (...  )]
+		 |_________|----> [gpio-bank (n + 7)]
+
+Second type has a dedicated interrupt per gpio bank.
+
+	[irqN]----> [gpio-bank (n)]
+
+
+Pin controller node:
+Required properties:
 - compatible	: should be "st,<SOC>-<pio-block>-pinctrl"
 	like st,stih415-sbc-pinctrl, st,stih415-front-pinctrl and so on.
-- gpio-controller : Indicates this device is a GPIO controller
-- #gpio-cells	  : Should be one. The first cell is the pin number.
+- st,syscfg		: Should be a phandle of the syscfg node.
 - st,retime-pin-mask	: Should be mask to specify which pins can be retimed.
 	If the property is not present, it is assumed that all the pins in the
 	bank are capable of retiming. Retiming is mainly used to improve the
 	IO timing margins of external synchronous interfaces.
-- st,bank-name		: Should be a name string for this bank as
-			specified in datasheet.
-- st,syscfg		: Should be a phandle of the syscfg node.
+- ranges : defines mapping between pin controller node (parent) to gpio-bank
+  node (children).
+
+Optional properties:
+- interrupts	: Interrupt number of the irqmux. If the interrupt is shared
+  with other gpio banks via irqmux.
+  a irqline and gpio banks.
+- reg		: irqmux memory resource. If irqmux is present.
+- reg-names	: irqmux resource should be named as "irqmux".
+
+GPIO controller/bank node.
+Required properties:
+- gpio-controller : Indicates this device is a GPIO controller
+- #gpio-cells	  : Should be one. The first cell is the pin number.
+- st,bank-name	  : Should be a name string for this bank as specified in
+  datasheet.
+
+Optional properties:
+- interrupts	: Interrupt number for this gpio bank. If there is a dedicated
+  interrupt wired up for this gpio bank.
+
+- interrupt-controller : Indicates this device is a interrupt controller. GPIO
+  bank can be an interrupt controller iff one of the interrupt type either via
+irqmux or a dedicated interrupt per bank is specified.
+
+- #interrupt-cells: the value of this property should be 2.
+     - First Cell: represents the external gpio interrupt number local to the
+       gpio interrupt space of the controller.
+     - Second Cell: flags to identify the type of the interrupt
+       - 1 = rising edge triggered
+       - 2 = falling edge triggered
+       - 3 = rising and falling edge triggered
+       - 4 = high level triggered
+       - 8 = low level triggered
+for related macros look in:
+include/dt-bindings/interrupt-controller/irq.h
 
 Example:
 	pin-controller-sbc {
@@ -30,10 +80,17 @@ Example:
 		#size-cells	= <1>;
 		compatible	= "st,stih415-sbc-pinctrl";
 		st,syscfg	= <&syscfg_sbc>;
+		reg 		= <0xfe61f080 0x4>;
+		reg-names	= "irqmux";
+		interrupts 	= <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
+		interrupts-names = "irqmux";
 		ranges 		= <0 0xfe610000 0x5000>;
+
 		PIO0: gpio@fe610000 {
 			gpio-controller;
 			#gpio-cells	= <1>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
 			reg		= <0 0x100>;
 			st,bank-name	= "PIO0";
 		};
@@ -105,6 +162,10 @@ pin-controller {
 
 sdhci0:sdhci@fe810000{
 	...
+	interrupt-parent = <&PIO3>;
+	#interrupt-cells = <2>;
+	interrupts = <3 IRQ_TYPE_LEVEL_HIGH>; /* Interrupt line via PIO3-3 */
+	interrupts-names = "card-detect";
 	pinctrl-names = "default";
 	pinctrl-0	= <&pinctrl_mmc>;
 };
diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 320c273..51e4f3a 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -13,7 +13,12 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/of_gpio.h>
 #include <linux/of_address.h>
 #include <linux/regmap.h>
@@ -271,6 +276,7 @@ struct st_gpio_bank {
 	struct pinctrl_gpio_range	range;
 	void __iomem			*base;
 	struct st_pio_control		pc;
+	struct	irq_domain		*domain;
 };
 
 struct st_pinctrl {
@@ -284,6 +290,7 @@ struct st_pinctrl {
 	int				ngroups;
 	struct regmap			*regmap;
 	const struct st_pctl_data	*data;
+	void __iomem			*irqmux_base;
 };
 
 /* SOC specific data */
@@ -1200,6 +1207,130 @@ static int st_pctl_parse_functions(struct device_node *np,
 	return 0;
 }
 
+static int st_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+	struct st_gpio_bank *bank = gpio_chip_to_bank(chip);
+	int irq = -ENXIO;
+
+	if (offset < chip->ngpio)
+		irq = irq_find_mapping(bank->domain, offset);
+
+	dev_info(chip->dev, "%s: request IRQ for GPIO %d, return %d\n",
+				chip->label, offset + chip->base, irq);
+	return irq;
+}
+
+static void st_gpio_irq_mask(struct irq_data *d)
+{
+	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+
+	writel(BIT(d->hwirq), bank->base + REG_PIO_CLR_PMASK);
+}
+
+static void st_gpio_irq_unmask(struct irq_data *d)
+{
+	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+
+	writel(BIT(d->hwirq), bank->base + REG_PIO_SET_PMASK);
+}
+
+static unsigned int st_gpio_irq_startup(struct irq_data *d)
+{
+	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+
+	if (gpio_lock_as_irq(&bank->gpio_chip, d->hwirq))
+		dev_err(bank->gpio_chip.dev,
+			"unable to lock HW IRQ %lu for IRQ\n",
+			d->hwirq);
+
+	st_gpio_irq_unmask(d);
+
+	return 0;
+}
+
+static void st_gpio_irq_shutdown(struct irq_data *d)
+{
+	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+
+	st_gpio_irq_mask(d);
+	gpio_unlock_as_irq(&bank->gpio_chip, d->hwirq);
+}
+
+static int st_gpio_irq_set_type(struct irq_data *d, unsigned type)
+{
+	struct st_gpio_bank *bank = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
+	int comp, pin = d->hwirq;
+	u32 val;
+
+	switch (type) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		comp = 0;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		comp = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	val = readl(bank->base + REG_PIO_PCOMP);
+	val &= ~BIT(pin);
+	val |= (comp << pin);
+	writel(val, bank->base + REG_PIO_PCOMP);
+
+	return 0;
+}
+
+static void __gpio_irq_handler(struct st_gpio_bank *bank)
+{
+	unsigned long port_in, port_mask, port_comp, active_irqs;
+	int n;
+
+	for (;;) {
+		port_in = readl(bank->base + REG_PIO_PIN);
+		port_comp = readl(bank->base + REG_PIO_PCOMP);
+		port_mask = readl(bank->base + REG_PIO_PMASK);
+
+		active_irqs = (port_in ^ port_comp) & port_mask;
+
+		if (active_irqs == 0)
+			break;
+
+		for_each_set_bit(n, &active_irqs, BITS_PER_LONG) {
+			generic_handle_irq(irq_find_mapping(bank->domain, n));
+		}
+	}
+}
+
+static void st_gpio_irq_handler(unsigned irq, struct irq_desc *desc)
+{
+	/* interrupt dedicated per bank */
+	struct irq_chip *chip = irq_get_chip(irq);
+	struct st_gpio_bank *bank = irq_get_handler_data(irq);
+
+	chained_irq_enter(chip, desc);
+	__gpio_irq_handler(bank);
+	chained_irq_exit(chip, desc);
+}
+
+static void st_gpio_irqmux_handler(unsigned irq, struct irq_desc *desc)
+{
+	struct irq_chip *chip = irq_get_chip(irq);
+	struct st_pinctrl *info = irq_get_handler_data(irq);
+	unsigned long status;
+	int n;
+
+	chained_irq_enter(chip, desc);
+
+	status = readl(info->irqmux_base);
+
+	for_each_set_bit(n, &status, ST_GPIO_PINS_PER_BANK)
+		__gpio_irq_handler(&info->banks[n]);
+
+	chained_irq_exit(chip, desc);
+}
+
 static struct gpio_chip st_gpio_template = {
 	.request		= st_gpio_request,
 	.free			= st_gpio_free,
@@ -1210,6 +1341,34 @@ static struct gpio_chip st_gpio_template = {
 	.ngpio			= ST_GPIO_PINS_PER_BANK,
 	.of_gpio_n_cells	= 1,
 	.of_xlate		= st_gpio_xlate,
+	.to_irq			= st_gpio_to_irq,
+};
+
+static struct irq_chip st_gpio_irqchip = {
+	.name		= "GPIO",
+	.irq_mask	= st_gpio_irq_mask,
+	.irq_unmask	= st_gpio_irq_unmask,
+	.irq_set_type	= st_gpio_irq_set_type,
+	.irq_startup	= st_gpio_irq_startup,
+	.irq_shutdown	= st_gpio_irq_shutdown,
+};
+
+static int st_gpio_irq_domain_map(struct irq_domain *h,
+			unsigned int virq, irq_hw_number_t hw)
+{
+	struct st_gpio_bank *bank = h->host_data;
+
+	irq_set_chip(virq, &st_gpio_irqchip);
+	irq_set_handler(virq, handle_level_irq);
+	set_irq_flags(virq, IRQF_VALID);
+	irq_set_chip_data(virq, bank);
+
+	return 0;
+}
+
+static struct irq_domain_ops st_gpio_irq_ops = {
+	.map	= st_gpio_irq_domain_map,
+	.xlate	= irq_domain_xlate_twocell,
 };
 
 static int st_gpiolib_register_bank(struct st_pinctrl *info,
@@ -1219,8 +1378,8 @@ static int st_gpiolib_register_bank(struct st_pinctrl *info,
 	struct pinctrl_gpio_range *range = &bank->range;
 	struct device *dev = info->dev;
 	int bank_num = of_alias_get_id(np, "gpio");
-	struct resource res;
-	int err;
+	struct resource res, irq_res;
+	int gpio_irq = 0, err, i;
 
 	if (of_address_to_resource(np, 0, &res))
 		return -ENODEV;
@@ -1248,6 +1407,51 @@ static int st_gpiolib_register_bank(struct st_pinctrl *info,
 	}
 	dev_info(dev, "%s bank added.\n", range->name);
 
+	/**
+	 * GPIO bank can have one of the two possible types of
+	 * interrupt-wirings.
+	 *
+	 * First type is via irqmux, single interrupt is used by multiple
+	 * gpio banks. This reduces number of overall interrupts numbers
+	 * required. All these banks belong to a single pincontroller.
+	 *		  _________
+	 *		 |	   |----> [gpio-bank (n)    ]
+	 *		 |	   |----> [gpio-bank (n + 1)]
+	 *	[irqN]-- | irq-mux |----> [gpio-bank (n + 2)]
+	 *		 |	   |----> [gpio-bank (...  )]
+	 *		 |_________|----> [gpio-bank (n + 7)]
+	 *
+	 * Second type has a dedicated interrupt per each gpio bank.
+	 *
+	 *	[irqN]----> [gpio-bank (n)]
+	 */
+
+	if (!of_irq_to_resource(np, 0, &irq_res)) {
+		gpio_irq = irq_res.start;
+		irq_set_chained_handler(gpio_irq, st_gpio_irq_handler);
+		irq_set_handler_data(gpio_irq, bank);
+	}
+
+	if (info->irqmux_base > 0 || gpio_irq > 0) {
+		/* Setup IRQ domain */
+		bank->domain  = irq_domain_add_linear(np,
+						ST_GPIO_PINS_PER_BANK,
+						&st_gpio_irq_ops, bank);
+		if (!bank->domain) {
+			dev_err(dev, "Failed to add irq domain for %s\n",
+				np->full_name);
+		} else  {
+			for (i = 0; i < ST_GPIO_PINS_PER_BANK; i++) {
+				if (irq_create_mapping(bank->domain, i) < 0)
+					dev_err(dev,
+						"Failed to map IRQ %i\n", i);
+			}
+		}
+
+	} else {
+		dev_info(dev, "No IRQ support for %s bank\n", np->full_name);
+	}
+
 	return 0;
 }
 
@@ -1276,6 +1480,8 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
 	struct device_node *np = pdev->dev.of_node;
 	struct device_node *child;
 	int grp_index = 0;
+	int irq = 0;
+	struct resource *res;
 
 	st_pctl_dt_child_count(info, np);
 	if (!info->nbanks) {
@@ -1306,6 +1512,21 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
 	}
 	info->data = of_match_node(st_pctl_of_match, np)->data;
 
+	irq = platform_get_irq(pdev, 0);
+
+	if (irq > 0) {
+		res = platform_get_resource_byname(pdev,
+					IORESOURCE_MEM, "irqmux");
+		info->irqmux_base = devm_ioremap_resource(&pdev->dev, res);
+
+		if (IS_ERR(info->irqmux_base))
+			return PTR_ERR(info->irqmux_base);
+
+		irq_set_chained_handler(irq, st_gpio_irqmux_handler);
+		irq_set_handler_data(irq, info);
+
+	}
+
 	pctl_desc->npins = info->nbanks * ST_GPIO_PINS_PER_BANK;
 	pdesc =	devm_kzalloc(&pdev->dev,
 			sizeof(*pdesc) * pctl_desc->npins, GFP_KERNEL);
-- 
1.7.6.5

--
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 v2 1/4] pinctrl: st: Add Interrupt support.

[Linus Walleij]

On Thu, Jan 16, 2014 at 4:36 PM,  <srinivas.kandagatla@st.com> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> This patch add interrupt support to the pincontroller driver.
>
> ST Pincontroller GPIO bank can have one of the two possible types of
> interrupt-wirings.
>
> First type is via irqmux, single interrupt is used by multiple gpio
> banks. This reduces number of overall interrupts numbers required. All
> these banks belong to a single pincontroller.
>                   _________
>                  |         |----> [gpio-bank (n)    ]
>                  |         |----> [gpio-bank (n + 1)]
>         [irqN]-- | irq-mux |----> [gpio-bank (n + 2)]
>                  |         |----> [gpio-bank (...  )]
>                  |_________|----> [gpio-bank (n + 7)]
>
> Second type has a dedicated interrupt per gpio bank.
>
>         [irqN]----> [gpio-bank (n)]
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>

I've applied this patch so that it won't just sit idle.
Besides you've done a real good job on it Srinivas.

It'll be for v3.15 though and I'm still waiting to see if
the device tree people say something about it.

Yours,
Linus Walleij

Re: [PATCH v2 1/4] pinctrl: st: Add Interrupt support.

[srinivas kandagatla]

On 23/01/14 07:22, Linus Walleij wrote:
> I've applied this patch so that it won't just sit idle.
> Besides you've done a real good job on it Srinivas.
Thanks Linus W.
> 
> It'll be for v3.15 though and I'm still waiting to see if
> the device tree people say something about it.
> 
> Yours,
> Linus Walleij

[PATCH v2 2/4] pinctrl: st: Add software edge trigger interrupt support.

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

ST pin controller does not have hardware support for detecting edge
triggered interrupts, It only has level triggering support.
This patch attempts to fake up edge triggers from hw level trigger
support in software. With this facility now the gpios can be easily used
for keypads, otherwise it would be difficult for drivers like keypads to
work with level trigger interrupts.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 drivers/pinctrl/pinctrl-st.c |  116 +++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 51e4f3a..9fb66aa 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -271,12 +271,59 @@ struct st_pctl_group {
 	struct st_pinconf	*pin_conf;
 };
 
+/*
+ * Edge triggers are not supported at hardware level, it is supported by
+ * software by exploiting the level trigger support in hardware.
+ * Software uses a virtual register (EDGE_CONF) for edge trigger configuration
+ * of each gpio pin in a GPIO bank.
+ *
+ * Each bank has a 32 bit EDGE_CONF register which is divided in to 8 parts of
+ * 4-bits. Each 4-bit space is allocated for each pin in a gpio bank.
+ *
+ * bit allocation per pin is:
+ * Bits:  [0 - 3] | [4 - 7]  [8 - 11] ... ... ... ...  [ 28 - 31]
+ *       --------------------------------------------------------
+ *       |  pin-0  |  pin-2 | pin-3  | ... ... ... ... | pin -7 |
+ *       --------------------------------------------------------
+ *
+ *  A pin can have one of following the values in its edge configuration field.
+ *
+ *	-------   ----------------------------
+ *	[0-3]	- Description
+ *	-------   ----------------------------
+ *	0000	- No edge IRQ.
+ *	0001	- Falling edge IRQ.
+ *	0010	- Rising edge IRQ.
+ *	0011	- Rising and Falling edge IRQ.
+ *	-------   ----------------------------
+ */
+
+#define ST_IRQ_EDGE_CONF_BITS_PER_PIN	4
+#define ST_IRQ_EDGE_MASK		0xf
+#define ST_IRQ_EDGE_FALLING		BIT(0)
+#define ST_IRQ_EDGE_RISING		BIT(1)
+#define ST_IRQ_EDGE_BOTH		(BIT(0) | BIT(1))
+
+#define ST_IRQ_RISING_EDGE_CONF(pin) \
+	(ST_IRQ_EDGE_RISING << (pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN))
+
+#define ST_IRQ_FALLING_EDGE_CONF(pin) \
+	(ST_IRQ_EDGE_FALLING << (pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN))
+
+#define ST_IRQ_BOTH_EDGE_CONF(pin) \
+	(ST_IRQ_EDGE_BOTH << (pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN))
+
+#define ST_IRQ_EDGE_CONF(conf, pin) \
+	(conf >> (pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN) & ST_IRQ_EDGE_MASK)
+
 struct st_gpio_bank {
 	struct gpio_chip		gpio_chip;
 	struct pinctrl_gpio_range	range;
 	void __iomem			*base;
 	struct st_pio_control		pc;
 	struct	irq_domain		*domain;
+	unsigned long			irq_edge_conf;
+	spinlock_t                      lock;
 };
 
 struct st_pinctrl {
@@ -1262,18 +1309,37 @@ static int st_gpio_irq_set_type(struct irq_data *d, unsigned type)
 	unsigned long flags;
 	int comp, pin = d->hwirq;
 	u32 val;
+	u32 pin_edge_conf = 0;
 
 	switch (type) {
 	case IRQ_TYPE_LEVEL_HIGH:
 		comp = 0;
 		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		comp = 0;
+		pin_edge_conf = ST_IRQ_FALLING_EDGE_CONF(pin);
+		break;
 	case IRQ_TYPE_LEVEL_LOW:
 		comp = 1;
 		break;
+	case IRQ_TYPE_EDGE_RISING:
+		comp = 1;
+		pin_edge_conf = ST_IRQ_RISING_EDGE_CONF(pin);
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		comp = st_gpio_get(&bank->gpio_chip, pin);
+		pin_edge_conf = ST_IRQ_BOTH_EDGE_CONF(pin);
+		break;
 	default:
 		return -EINVAL;
 	}
 
+	spin_lock_irqsave(&bank->lock, flags);
+	bank->irq_edge_conf &=  ~(ST_IRQ_EDGE_MASK << (
+				pin * ST_IRQ_EDGE_CONF_BITS_PER_PIN));
+	bank->irq_edge_conf |= pin_edge_conf;
+	spin_unlock_irqrestore(&bank->lock, flags);
+
 	val = readl(bank->base + REG_PIO_PCOMP);
 	val &= ~BIT(pin);
 	val |= (comp << pin);
@@ -1282,10 +1348,39 @@ static int st_gpio_irq_set_type(struct irq_data *d, unsigned type)
 	return 0;
 }
 
+/*
+ * As edge triggers are not supported at hardware level, it is supported by
+ * software by exploiting the level trigger support in hardware.
+ *
+ * Steps for detection raising edge interrupt in software.
+ *
+ * Step 1: CONFIGURE pin to detect level LOW interrupts.
+ *
+ * Step 2: DETECT level LOW interrupt and in irqmux/gpio bank interrupt handler,
+ * if the value of pin is low, then CONFIGURE pin for level HIGH interrupt.
+ * IGNORE calling the actual interrupt handler for the pin at this stage.
+ *
+ * Step 3: DETECT level HIGH interrupt and in irqmux/gpio-bank interrupt handler
+ * if the value of pin is HIGH, CONFIGURE pin for level LOW interrupt and then
+ * DISPATCH the interrupt to the interrupt handler of the pin.
+ *
+ *		 step-1  ________     __________
+ *				|     | step - 3
+ *			        |     |
+ *			step -2 |_____|
+ *
+ * falling edge is also detected int the same way.
+ *
+ */
 static void __gpio_irq_handler(struct st_gpio_bank *bank)
 {
 	unsigned long port_in, port_mask, port_comp, active_irqs;
-	int n;
+	unsigned long bank_edge_mask, flags;
+	int n, val, ecfg;
+
+	spin_lock_irqsave(&bank->lock, flags);
+	bank_edge_mask = bank->irq_edge_conf;
+	spin_unlock_irqrestore(&bank->lock, flags);
 
 	for (;;) {
 		port_in = readl(bank->base + REG_PIO_PIN);
@@ -1298,6 +1393,22 @@ static void __gpio_irq_handler(struct st_gpio_bank *bank)
 			break;
 
 		for_each_set_bit(n, &active_irqs, BITS_PER_LONG) {
+			/* check if we are detecting fake edges ... */
+			ecfg = ST_IRQ_EDGE_CONF(bank_edge_mask, n);
+
+			if (ecfg) {
+				/* edge detection. */
+				val = st_gpio_get(&bank->gpio_chip, n);
+
+				writel(BIT(n),
+					val ? bank->base + REG_PIO_SET_PCOMP :
+					bank->base + REG_PIO_CLR_PCOMP);
+
+				if (ecfg != ST_IRQ_EDGE_BOTH &&
+					!((ecfg & ST_IRQ_EDGE_FALLING) ^ val))
+					continue;
+			}
+
 			generic_handle_irq(irq_find_mapping(bank->domain, n));
 		}
 	}
@@ -1359,7 +1470,7 @@ static int st_gpio_irq_domain_map(struct irq_domain *h,
 	struct st_gpio_bank *bank = h->host_data;
 
 	irq_set_chip(virq, &st_gpio_irqchip);
-	irq_set_handler(virq, handle_level_irq);
+	irq_set_handler(virq, handle_simple_irq);
 	set_irq_flags(virq, IRQF_VALID);
 	irq_set_chip_data(virq, bank);
 
@@ -1392,6 +1503,7 @@ static int st_gpiolib_register_bank(struct st_pinctrl *info,
 	bank->gpio_chip.base = bank_num * ST_GPIO_PINS_PER_BANK;
 	bank->gpio_chip.ngpio = ST_GPIO_PINS_PER_BANK;
 	bank->gpio_chip.of_node = np;
+	spin_lock_init(&bank->lock);
 
 	of_property_read_string(np, "st,bank-name", &range->name);
 	bank->gpio_chip.label = range->name;
-- 
1.7.6.5

Re: [PATCH v2 2/4] pinctrl: st: Add software edge trigger interrupt support.

[Linus Walleij]

On Thu, Jan 16, 2014 at 4:37 PM,  <srinivas.kandagatla-qxv4g6HH51o@public.gmane.org> wrote:

> ST pin controller does not have hardware support for detecting edge
> triggered interrupts, It only has level triggering support.
> This patch attempts to fake up edge triggers from hw level trigger
> support in software.

(...)


> +/*
> + * Edge triggers are not supported at hardware level, it is supported by
> + * software by exploiting the level trigger support in hardware.
> + * Software uses a virtual register (EDGE_CONF) for edge trigger configuration
> + * of each gpio pin in a GPIO bank.

(...)

> +/*
> + * As edge triggers are not supported at hardware level, it is supported by
> + * software by exploiting the level trigger support in hardware.

(...)

All this is quite hard to understand. Maybe it's just because
it's hard overall. Edge triggers are not supported by hardware
so we use the hardware edge trigger support?

That is a bit oxymoronic...

> + * Steps for detection raising edge interrupt in software.
> + *
> + * Step 1: CONFIGURE pin to detect level LOW interrupts.
> + *
> + * Step 2: DETECT level LOW interrupt and in irqmux/gpio bank interrupt handler,
> + * if the value of pin is low, then CONFIGURE pin for level HIGH interrupt.
> + * IGNORE calling the actual interrupt handler for the pin at this stage.
> + *
> + * Step 3: DETECT level HIGH interrupt and in irqmux/gpio-bank interrupt handler
> + * if the value of pin is HIGH, CONFIGURE pin for level LOW interrupt and then
> + * DISPATCH the interrupt to the interrupt handler of the pin.

But I do understand this, that's VERY clever and may be something
that can be exploited on other hardware as well some day.

So patch applied.

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

Re: [PATCH v2 2/4] pinctrl: st: Add software edge trigger interrupt support.

[Lee Jones]

> > +/*
> > + * As edge triggers are not supported at hardware level, it is supported by
> > + * software by exploiting the level trigger support in hardware.
> 
> (...)
> 
> All this is quite hard to understand. Maybe it's just because
> it's hard overall. Edge triggers are not supported by hardware
> so we use the hardware edge trigger support?
> 
> That is a bit oxymoronic...

That's not what is says. Read it again. :)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v2 2/4] pinctrl: st: Add software edge trigger interrupt support.

[Linus Walleij]

On Fri, Jan 24, 2014 at 9:28 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> > +/*
>> > + * As edge triggers are not supported at hardware level, it is supported by
>> > + * software by exploiting the level trigger support in hardware.
>>
>> (...)
>>
>> All this is quite hard to understand. Maybe it's just because
>> it's hard overall. Edge triggers are not supported by hardware
>> so we use the hardware edge trigger support?
>>
>> That is a bit oxymoronic...
>
> That's not what is says. Read it again. :)

Argh yeah I was wrong ... it's perfectly consistent with the
implementation. Sorry for being such a slow brain this last
friday.

Yours,
Linus Walleij

[PATCH v2 3/4] ARM:STi: STiH416: Add interrupt support for pin controller

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

This patch adds interrupt support for STiH416 pin controllers.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 arch/arm/boot/dts/stih416-pinctrl.dtsi |   81 ++++++++++++++++++++++++++++++++
 1 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/stih416-pinctrl.dtsi b/arch/arm/boot/dts/stih416-pinctrl.dtsi
index 74f8cfc..3adbd98 100644
--- a/arch/arm/boot/dts/stih416-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih416-pinctrl.dtsi
@@ -8,6 +8,7 @@
  * publishhed by the Free Software Foundation.
  */
 #include "st-pincfg.h"
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 / {
 
 	aliases {
@@ -49,41 +50,57 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-sbc-pinctrl";
 			st,syscfg	= <&syscfg_sbc>;
+			reg 		= <0xfe61f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfe610000 0x6000>;
 
 			PIO0: gpio@fe610000 {
 				gpio-controller;
 				#gpio-cells = <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO0";
 			};
 			PIO1: gpio@fe611000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO1";
 			};
 			PIO2: gpio@fe612000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO2";
 			};
 			PIO3: gpio@fe613000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO3";
 			};
 			PIO4: gpio@fe614000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO4";
 			};
 			PIO40: gpio@fe615000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO40";
 				st,retime-pin-mask = <0x7f>;
@@ -181,65 +198,89 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-front-pinctrl";
 			st,syscfg	= <&syscfg_front>;
+			reg 		= <0xfee0f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 183 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfee00000 0x10000>;
 
 			PIO5: gpio@fee00000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO5";
 			};
 			PIO6: gpio@fee01000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO6";
 			};
 			PIO7: gpio@fee02000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO7";
 			};
 			PIO8: gpio@fee03000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO8";
 			};
 			PIO9: gpio@fee04000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO9";
 			};
 			PIO10: gpio@fee05000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO10";
 			};
 			PIO11: gpio@fee06000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x6000 0x100>;
 				st,bank-name	= "PIO11";
 			};
 			PIO12: gpio@fee07000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x7000 0x100>;
 				st,bank-name	= "PIO12";
 			};
 			PIO30: gpio@fee08000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x8000 0x100>;
 				st,bank-name	= "PIO30";
 			};
 			PIO31: gpio@fee09000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x9000 0x100>;
 				st,bank-name	= "PIO31";
 			};
@@ -276,41 +317,57 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-rear-pinctrl";
 			st,syscfg	= <&syscfg_rear>;
+			reg 		= <0xfe82f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges 		= <0 0xfe820000 0x6000>;
 
 			PIO13: gpio@fe820000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO13";
 			};
 			PIO14: gpio@fe821000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO14";
 			};
 			PIO15: gpio@fe822000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO15";
 			};
 			PIO16: gpio@fe823000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO16";
 			};
 			PIO17: gpio@fe824000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO17";
 			};
 			PIO18: gpio@fe825000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO18";
 				st,retime-pin-mask = <0xf>;
@@ -384,23 +441,33 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-fvdp-fe-pinctrl";
 			st,syscfg	= <&syscfg_fvdp_fe>;
+			reg 		= <0xfd6bf080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfd6b0000 0x3000>;
 
 			PIO100: gpio@fd6b0000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO100";
 			};
 			PIO101: gpio@fd6b1000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO101";
 			};
 			PIO102: gpio@fd6b2000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO102";
 			};
@@ -411,29 +478,41 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih416-fvdp-lite-pinctrl";
 			st,syscfg		= <&syscfg_fvdp_lite>;
+			reg 		= <0xfd33f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges			= <0 0xfd330000 0x5000>;
 
 			PIO103: gpio@fd330000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO103";
 			};
 			PIO104: gpio@fd331000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO104";
 			};
 			PIO105: gpio@fd332000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO105";
 			};
 			PIO106: gpio@fd333000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO106";
 			};
@@ -441,6 +520,8 @@
 			PIO107: gpio@fd334000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO107";
 				st,retime-pin-mask = <0xf>;
-- 
1.7.6.5

Re: [PATCH v2 3/4] ARM:STi: STiH416: Add interrupt support for pin controller

[Linus Walleij]

On Thu, Jan 16, 2014 at 4:37 PM,  <srinivas.kandagatla@st.com> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> This patch adds interrupt support for STiH416 pin controllers.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>

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

Please merge this through the ARM SoC tree with the rest of
your device tree stuff.

Yours,
Linus Walleij

Re: [PATCH v2 3/4] ARM:STi: STiH416: Add interrupt support for pin controller

[srinivas kandagatla]

On 23/01/14 07:30, Linus Walleij wrote:
> On Thu, Jan 16, 2014 at 4:37 PM,  <srinivas.kandagatla@st.com> wrote:
> 
>> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>>
>> This patch adds interrupt support for STiH416 pin controllers.
>>
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
> 
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
Thankyou Linus W.
> Please merge this through the ARM SoC tree with the rest of
> your device tree stuff.
> 
I will include these in my next DT pull request.

Thanks,
srini

> Yours,
> Linus Walleij
> 
> 

[PATCH v2 4/4] ARM:STi: STiH415: Add interrupt support for pin controller

[srinivas.kandagatla]

From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

This patch adds interrupt support for STiH415 pin controllers.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
 arch/arm/boot/dts/stih415-pinctrl.dtsi |   75 ++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/stih415-pinctrl.dtsi b/arch/arm/boot/dts/stih415-pinctrl.dtsi
index a9324fa..21b3b53 100644
--- a/arch/arm/boot/dts/stih415-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih415-pinctrl.dtsi
@@ -7,6 +7,7 @@
  * publishhed by the Free Software Foundation.
  */
 #include "st-pincfg.h"
+#include <dt-bindings/interrupt-controller/arm-gic.h>
 / {
 
 	aliases {
@@ -45,35 +46,49 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-sbc-pinctrl";
 			st,syscfg	= <&syscfg_sbc>;
+			reg 		= <0xfe61f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 180 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges 		= <0 0xfe610000 0x5000>;
 
 			PIO0: gpio@fe610000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO0";
 			};
 			PIO1: gpio@fe611000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO1";
 			};
 			PIO2: gpio@fe612000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO2";
 			};
 			PIO3: gpio@fe613000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO3";
 			};
 			PIO4: gpio@fe614000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO4";
 			};
@@ -169,53 +184,73 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-front-pinctrl";
 			st,syscfg	= <&syscfg_front>;
+			reg 		= <0xfee0f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 181 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfee00000 0x8000>;
 
 			PIO5: gpio@fee00000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO5";
 			};
 			PIO6: gpio@fee01000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO6";
 			};
 			PIO7: gpio@fee02000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO7";
 			};
 			PIO8: gpio@fee03000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO8";
 			};
 			PIO9: gpio@fee04000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO9";
 			};
 			PIO10: gpio@fee05000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO10";
 			};
 			PIO11: gpio@fee06000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x6000 0x100>;
 				st,bank-name	= "PIO11";
 			};
 			PIO12: gpio@fee07000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x7000 0x100>;
 				st,bank-name	= "PIO12";
 			};
@@ -244,41 +279,57 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-rear-pinctrl";
 			st,syscfg	= <&syscfg_rear>;
+			reg 		= <0xfe82f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 182 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfe820000 0x8000>;
 
 			PIO13: gpio@fe820000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO13";
 			};
 			PIO14: gpio@fe821000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO14";
 			};
 			PIO15: gpio@fe822000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO15";
 			};
 			PIO16: gpio@fe823000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO16";
 			};
 			PIO17: gpio@fe824000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO17";
 			};
 			PIO18: gpio@fe825000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x5000 0x100>;
 				st,bank-name	= "PIO18";
 			};
@@ -329,23 +380,33 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-left-pinctrl";
 			st,syscfg	= <&syscfg_left>;
+			reg 		= <0xfd6bf080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfd6b0000 0x3000>;
 
 			PIO100: gpio@fd6b0000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO100";
 			};
 			PIO101: gpio@fd6b1000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO101";
 			};
 			PIO102: gpio@fd6b2000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO102";
 			};
@@ -356,35 +417,49 @@
 			#size-cells	= <1>;
 			compatible	= "st,stih415-right-pinctrl";
 			st,syscfg	= <&syscfg_right>;
+			reg 		= <0xfd33f080 0x4>;
+			reg-names	= "irqmux";
+			interrupts 	= <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts-names = "irqmux";
 			ranges		= <0 0xfd330000 0x5000>;
 
 			PIO103: gpio@fd330000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0 0x100>;
 				st,bank-name	= "PIO103";
 			};
 			PIO104: gpio@fd331000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x1000 0x100>;
 				st,bank-name	= "PIO104";
 			};
 			PIO105: gpio@fd332000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x2000 0x100>;
 				st,bank-name	= "PIO105";
 			};
 			PIO106: gpio@fd333000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x3000 0x100>;
 				st,bank-name	= "PIO106";
 			};
 			PIO107: gpio@fd334000 {
 				gpio-controller;
 				#gpio-cells	= <1>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
 				reg		= <0x4000 0x100>;
 				st,bank-name	= "PIO107";
 			};
-- 
1.7.6.5

Re: [PATCH v2 4/4] ARM:STi: STiH415: Add interrupt support for pin controller

[Linus Walleij]

On Thu, Jan 16, 2014 at 4:37 PM,  <srinivas.kandagatla@st.com> wrote:

> From: Srinivas Kandagatla <srinivas.kandagatla@st.com>
>
> This patch adds interrupt support for STiH415 pin controllers.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>

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

Merge this through ARM SoC.

> +                       reg             = <0xfe61f080 0x4>;
> +                       reg-names       = "irqmux";

Aha so that is how it looks when you handle the separate
mux reg, well I can't think of anything better and this works
fine for me.

The alternative would be to have a syscon thing using regmap
such as drivers/mfd/syscon.c but I really cannot decide
between these two design patterns for odd registers.

Yours,
Linus Walleij