Re: [PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios

2016-09-08 Thread Alexandre Torgue 
Hi Thomas,

On 09/02/2016 09:10 PM, Thomas Gleixner wrote:
> On Fri, 2 Sep 2016, Alexandre TORGUE wrote:
>> +static int stm32_gpio_domain_translate(struct irq_domain *d,
>> +   struct irq_fwspec *fwspec,
>> +   unsigned long *hwirq,
>> +   unsigned int *type)
>> +{
>> +if ((fwspec->param_count != 2) ||
>> +(fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
>> +return -EINVAL;
>
> Just a nitpick. This is unnecessarily hard to parse because you indented
> the line break like a conditional statement

I agree. I will modify it as the one below.
>
>> +if ((fwspec->param_count != 2) ||
>> +(fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
>> +return -EINVAL;
>
> Makes it immediately obvious that the second line belongs to the if.
>
>> +static void stm32_gpio_domain_activate(struct irq_domain *d,
>> +   struct irq_data *irq_data)
>> +{
>> +struct stm32_gpio_bank *bank = d->host_data;
>> +struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
>> +
>> +if (gpiochip_lock_as_irq(>gpio_chip, irq_data->hwirq)) {
>> +dev_err(pctl->dev,
>> +"Unable to configure STM32 %s%ld as IRQ\n",
>> +bank->gpio_chip.label, irq_data->hwirq);
>> +return;
>
> Hmm, that's nasty. When an interrupt is mapped then we don't expect the
> activate function to fail. You really should lock that interrupt when it's
> mapped.

Ok. I will remove it from here.

>
>> +}
>> +regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
>> +}
>
>> +static int stm32_gpio_domain_alloc(struct irq_domain *domain,
>> +   unsigned int virq,
>> +   unsigned int nr_irqs, void *data)
>> +{
>> +struct irq_fwspec *fwspec = data;
>> +struct irq_fwspec parent_fwspec;
>> +struct stm32_pinctrl *pctl = domain->host_data;
>> +irq_hw_number_t hwirq;
>> +unsigned int i;
>> +
>> +hwirq = fwspec->param[0];
>> +for (i = 0; i < nr_irqs; i++)
>> +irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
>> +  _gpio_irq_chip, pctl);
>> +
>> +parent_fwspec.fwnode = domain->parent->fwnode;
>> +parent_fwspec.param_count = 2;
>> +parent_fwspec.param[0] = fwspec->param[0];
>> +parent_fwspec.param[1] = fwspec->param[1];
>> +
>> +return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
>> +_fwspec);
>
> So doing it here would be probably the right thing to do:
>
>
>   ret = gpiochip_lock_as_irq();
>   if (ret)
>   return ret;
>
>   ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
>  _fwspec);
>   if (ret)
>   gpiochip_unlock_as_irq();
>
>   return ret;
>
> So of course you need your own free() function which undoes that lock
> thingy.

Ok thanks for proposal.

Best regards.

Alex


>
> Thanks,
>
>   tglx
>

Re: [PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios

2016-09-08 Thread Alexandre Torgue 
Hi Thomas,

On 09/02/2016 09:10 PM, Thomas Gleixner wrote:
> On Fri, 2 Sep 2016, Alexandre TORGUE wrote:
>> +static int stm32_gpio_domain_translate(struct irq_domain *d,
>> +   struct irq_fwspec *fwspec,
>> +   unsigned long *hwirq,
>> +   unsigned int *type)
>> +{
>> +if ((fwspec->param_count != 2) ||
>> +(fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
>> +return -EINVAL;
>
> Just a nitpick. This is unnecessarily hard to parse because you indented
> the line break like a conditional statement

I agree. I will modify it as the one below.
>
>> +if ((fwspec->param_count != 2) ||
>> +(fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
>> +return -EINVAL;
>
> Makes it immediately obvious that the second line belongs to the if.
>
>> +static void stm32_gpio_domain_activate(struct irq_domain *d,
>> +   struct irq_data *irq_data)
>> +{
>> +struct stm32_gpio_bank *bank = d->host_data;
>> +struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
>> +
>> +if (gpiochip_lock_as_irq(>gpio_chip, irq_data->hwirq)) {
>> +dev_err(pctl->dev,
>> +"Unable to configure STM32 %s%ld as IRQ\n",
>> +bank->gpio_chip.label, irq_data->hwirq);
>> +return;
>
> Hmm, that's nasty. When an interrupt is mapped then we don't expect the
> activate function to fail. You really should lock that interrupt when it's
> mapped.

Ok. I will remove it from here.

>
>> +}
>> +regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
>> +}
>
>> +static int stm32_gpio_domain_alloc(struct irq_domain *domain,
>> +   unsigned int virq,
>> +   unsigned int nr_irqs, void *data)
>> +{
>> +struct irq_fwspec *fwspec = data;
>> +struct irq_fwspec parent_fwspec;
>> +struct stm32_pinctrl *pctl = domain->host_data;
>> +irq_hw_number_t hwirq;
>> +unsigned int i;
>> +
>> +hwirq = fwspec->param[0];
>> +for (i = 0; i < nr_irqs; i++)
>> +irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
>> +  _gpio_irq_chip, pctl);
>> +
>> +parent_fwspec.fwnode = domain->parent->fwnode;
>> +parent_fwspec.param_count = 2;
>> +parent_fwspec.param[0] = fwspec->param[0];
>> +parent_fwspec.param[1] = fwspec->param[1];
>> +
>> +return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
>> +_fwspec);
>
> So doing it here would be probably the right thing to do:
>
>
>   ret = gpiochip_lock_as_irq();
>   if (ret)
>   return ret;
>
>   ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
>  _fwspec);
>   if (ret)
>   gpiochip_unlock_as_irq();
>
>   return ret;
>
> So of course you need your own free() function which undoes that lock
> thingy.

Ok thanks for proposal.

Best regards.

Alex


>
> Thanks,
>
>   tglx
>

Re: [PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios

2016-09-05 Thread Alexandre Torgue 

Hi Thomas,

On 09/02/2016 09:10 PM, Thomas Gleixner wrote:

On Fri, 2 Sep 2016, Alexandre TORGUE wrote:

+static int stm32_gpio_domain_translate(struct irq_domain *d,
+  struct irq_fwspec *fwspec,
+  unsigned long *hwirq,
+  unsigned int *type)
+{
+   if ((fwspec->param_count != 2) ||
+   (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
+   return -EINVAL;


Just a nitpick. This is unnecessarily hard to parse because you indented
the line break like a conditional statement


I agree. I will modify it as the one below.



+   if ((fwspec->param_count != 2) ||
+   (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
+   return -EINVAL;


Makes it immediately obvious that the second line belongs to the if.


+static void stm32_gpio_domain_activate(struct irq_domain *d,
+  struct irq_data *irq_data)
+{
+   struct stm32_gpio_bank *bank = d->host_data;
+   struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
+
+   if (gpiochip_lock_as_irq(>gpio_chip, irq_data->hwirq)) {
+   dev_err(pctl->dev,
+   "Unable to configure STM32 %s%ld as IRQ\n",
+   bank->gpio_chip.label, irq_data->hwirq);
+   return;


Hmm, that's nasty. When an interrupt is mapped then we don't expect the
activate function to fail. You really should lock that interrupt when it's
mapped.


Ok. I will remove it from here.




+   }
+   regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
+}



+static int stm32_gpio_domain_alloc(struct irq_domain *domain,
+  unsigned int virq,
+  unsigned int nr_irqs, void *data)
+{
+   struct irq_fwspec *fwspec = data;
+   struct irq_fwspec parent_fwspec;
+   struct stm32_pinctrl *pctl = domain->host_data;
+   irq_hw_number_t hwirq;
+   unsigned int i;
+
+   hwirq = fwspec->param[0];
+   for (i = 0; i < nr_irqs; i++)
+   irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+ _gpio_irq_chip, pctl);
+
+   parent_fwspec.fwnode = domain->parent->fwnode;
+   parent_fwspec.param_count = 2;
+   parent_fwspec.param[0] = fwspec->param[0];
+   parent_fwspec.param[1] = fwspec->param[1];
+
+   return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
+   _fwspec);


So doing it here would be probably the right thing to do:


ret = gpiochip_lock_as_irq();
if (ret)
return ret;

ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
   _fwspec);
if (ret)
gpiochip_unlock_as_irq();

return ret;

So of course you need your own free() function which undoes that lock
thingy.


Ok thanks for proposal.

Best regards.

Alex




Thanks,

tglx

Re: [PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios

2016-09-05 Thread Alexandre Torgue 

Hi Thomas,

On 09/02/2016 09:10 PM, Thomas Gleixner wrote:

On Fri, 2 Sep 2016, Alexandre TORGUE wrote:

+static int stm32_gpio_domain_translate(struct irq_domain *d,
+  struct irq_fwspec *fwspec,
+  unsigned long *hwirq,
+  unsigned int *type)
+{
+   if ((fwspec->param_count != 2) ||
+   (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
+   return -EINVAL;


Just a nitpick. This is unnecessarily hard to parse because you indented
the line break like a conditional statement


I agree. I will modify it as the one below.



+   if ((fwspec->param_count != 2) ||
+   (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
+   return -EINVAL;


Makes it immediately obvious that the second line belongs to the if.


+static void stm32_gpio_domain_activate(struct irq_domain *d,
+  struct irq_data *irq_data)
+{
+   struct stm32_gpio_bank *bank = d->host_data;
+   struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
+
+   if (gpiochip_lock_as_irq(>gpio_chip, irq_data->hwirq)) {
+   dev_err(pctl->dev,
+   "Unable to configure STM32 %s%ld as IRQ\n",
+   bank->gpio_chip.label, irq_data->hwirq);
+   return;


Hmm, that's nasty. When an interrupt is mapped then we don't expect the
activate function to fail. You really should lock that interrupt when it's
mapped.


Ok. I will remove it from here.




+   }
+   regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
+}



+static int stm32_gpio_domain_alloc(struct irq_domain *domain,
+  unsigned int virq,
+  unsigned int nr_irqs, void *data)
+{
+   struct irq_fwspec *fwspec = data;
+   struct irq_fwspec parent_fwspec;
+   struct stm32_pinctrl *pctl = domain->host_data;
+   irq_hw_number_t hwirq;
+   unsigned int i;
+
+   hwirq = fwspec->param[0];
+   for (i = 0; i < nr_irqs; i++)
+   irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+ _gpio_irq_chip, pctl);
+
+   parent_fwspec.fwnode = domain->parent->fwnode;
+   parent_fwspec.param_count = 2;
+   parent_fwspec.param[0] = fwspec->param[0];
+   parent_fwspec.param[1] = fwspec->param[1];
+
+   return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
+   _fwspec);


So doing it here would be probably the right thing to do:


ret = gpiochip_lock_as_irq();
if (ret)
return ret;

ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
   _fwspec);
if (ret)
gpiochip_unlock_as_irq();

return ret;

So of course you need your own free() function which undoes that lock
thingy.


Ok thanks for proposal.

Best regards.

Alex




Thanks,

tglx

Re: [PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios

2016-09-02 Thread Thomas Gleixner 
On Fri, 2 Sep 2016, Alexandre TORGUE wrote:
> +static int stm32_gpio_domain_translate(struct irq_domain *d,
> +struct irq_fwspec *fwspec,
> +unsigned long *hwirq,
> +unsigned int *type)
> +{
> + if ((fwspec->param_count != 2) ||
> + (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
> + return -EINVAL;

Just a nitpick. This is unnecessarily hard to parse because you indented
the line break like a conditional statement

> + if ((fwspec->param_count != 2) ||
> + (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
> + return -EINVAL;

Makes it immediately obvious that the second line belongs to the if.

> +static void stm32_gpio_domain_activate(struct irq_domain *d,
> +struct irq_data *irq_data)
> +{
> + struct stm32_gpio_bank *bank = d->host_data;
> + struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
> +
> + if (gpiochip_lock_as_irq(>gpio_chip, irq_data->hwirq)) {
> + dev_err(pctl->dev,
> + "Unable to configure STM32 %s%ld as IRQ\n",
> + bank->gpio_chip.label, irq_data->hwirq);
> + return;

Hmm, that's nasty. When an interrupt is mapped then we don't expect the
activate function to fail. You really should lock that interrupt when it's
mapped.

> + }
> + regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
> +}

> +static int stm32_gpio_domain_alloc(struct irq_domain *domain,
> +unsigned int virq,
> +unsigned int nr_irqs, void *data)
> +{
> + struct irq_fwspec *fwspec = data;
> + struct irq_fwspec parent_fwspec;
> + struct stm32_pinctrl *pctl = domain->host_data;
> + irq_hw_number_t hwirq;
> + unsigned int i;
> +
> + hwirq = fwspec->param[0];
> + for (i = 0; i < nr_irqs; i++)
> + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> +   _gpio_irq_chip, pctl);
> +
> + parent_fwspec.fwnode = domain->parent->fwnode;
> + parent_fwspec.param_count = 2;
> + parent_fwspec.param[0] = fwspec->param[0];
> + parent_fwspec.param[1] = fwspec->param[1];
> +
> + return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> + _fwspec);

So doing it here would be probably the right thing to do:


ret = gpiochip_lock_as_irq();
if (ret)
return ret;

ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
   _fwspec);
if (ret)
gpiochip_unlock_as_irq();

return ret;

So of course you need your own free() function which undoes that lock
thingy.

Thanks,

tglx

Re: [PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios

2016-09-02 Thread Thomas Gleixner 
On Fri, 2 Sep 2016, Alexandre TORGUE wrote:
> +static int stm32_gpio_domain_translate(struct irq_domain *d,
> +struct irq_fwspec *fwspec,
> +unsigned long *hwirq,
> +unsigned int *type)
> +{
> + if ((fwspec->param_count != 2) ||
> + (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
> + return -EINVAL;

Just a nitpick. This is unnecessarily hard to parse because you indented
the line break like a conditional statement

> + if ((fwspec->param_count != 2) ||
> + (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
> + return -EINVAL;

Makes it immediately obvious that the second line belongs to the if.

> +static void stm32_gpio_domain_activate(struct irq_domain *d,
> +struct irq_data *irq_data)
> +{
> + struct stm32_gpio_bank *bank = d->host_data;
> + struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
> +
> + if (gpiochip_lock_as_irq(>gpio_chip, irq_data->hwirq)) {
> + dev_err(pctl->dev,
> + "Unable to configure STM32 %s%ld as IRQ\n",
> + bank->gpio_chip.label, irq_data->hwirq);
> + return;

Hmm, that's nasty. When an interrupt is mapped then we don't expect the
activate function to fail. You really should lock that interrupt when it's
mapped.

> + }
> + regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
> +}

> +static int stm32_gpio_domain_alloc(struct irq_domain *domain,
> +unsigned int virq,
> +unsigned int nr_irqs, void *data)
> +{
> + struct irq_fwspec *fwspec = data;
> + struct irq_fwspec parent_fwspec;
> + struct stm32_pinctrl *pctl = domain->host_data;
> + irq_hw_number_t hwirq;
> + unsigned int i;
> +
> + hwirq = fwspec->param[0];
> + for (i = 0; i < nr_irqs; i++)
> + irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> +   _gpio_irq_chip, pctl);
> +
> + parent_fwspec.fwnode = domain->parent->fwnode;
> + parent_fwspec.param_count = 2;
> + parent_fwspec.param[0] = fwspec->param[0];
> + parent_fwspec.param[1] = fwspec->param[1];
> +
> + return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
> + _fwspec);

So doing it here would be probably the right thing to do:


ret = gpiochip_lock_as_irq();
if (ret)
return ret;

ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs,
   _fwspec);
if (ret)
gpiochip_unlock_as_irq();

return ret;

So of course you need your own free() function which undoes that lock
thingy.

Thanks,

tglx

[PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios

2016-09-02 Thread Alexandre TORGUE 
This patch adds IRQ support to STM32 gpios.

The EXTI controller has 16 lines dedicated to GPIOs.
EXTI line n can be connected to only line n of one of the GPIO ports, for
example EXTI0 can be connected to either PA0, or PB0, or PC0...
This port selection is done by specifying the port number into System
Config registers.

Signed-off-by: Maxime Coquelin 
Signed-off-by: Alexandre TORGUE 

diff --git a/drivers/pinctrl/stm32/Kconfig b/drivers/pinctrl/stm32/Kconfig
index 4c40dae..40d5abc 100644
--- a/drivers/pinctrl/stm32/Kconfig
+++ b/drivers/pinctrl/stm32/Kconfig
@@ -6,6 +6,8 @@ config PINCTRL_STM32
select PINMUX
select GENERIC_PINCONF
select GPIOLIB
+   select GPIOLIB_IRQCHIP
+   select MFD_SYSCON
 
 config PINCTRL_STM32F429
bool "STMicroelectronics STM32F429 pin control" if COMPILE_TEST && 
!MACH_STM32F429
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c 
b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 4ae596b..8fd25e2 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -8,6 +8,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -20,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -40,6 +43,7 @@
 #define STM32_GPIO_AFRH0x24
 
 #define STM32_GPIO_PINS_PER_BANK 16
+#define STM32_GPIO_IRQ_LINE 16
 
 #define gpio_range_to_bank(chip) \
container_of(chip, struct stm32_gpio_bank, range)
@@ -65,6 +69,7 @@ struct stm32_gpio_bank {
spinlock_t lock;
struct gpio_chip gpio_chip;
struct pinctrl_gpio_range range;
+   struct fwnode_handle *fwnode;
 };
 
 struct stm32_pinctrl {
@@ -77,6 +82,9 @@ struct stm32_pinctrl {
struct stm32_gpio_bank *banks;
unsigned nbanks;
const struct stm32_pinctrl_match_data *match_data;
+   struct irq_domain   *domain;
+   struct regmap   *regmap;
+   struct regmap_field *irqmux[STM32_GPIO_PINS_PER_BANK];
 };
 
 static inline int stm32_gpio_pin(int gpio)
@@ -174,6 +182,20 @@ static int stm32_gpio_direction_output(struct gpio_chip 
*chip,
return 0;
 }
 
+
+static int stm32_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+   struct irq_fwspec fwspec;
+   struct stm32_gpio_bank *bank = gpiochip_get_data(chip);
+
+   fwspec.fwnode = bank->fwnode;
+   fwspec.param_count = 2;
+   fwspec.param[0] = offset;
+   fwspec.param[1] = IRQ_TYPE_NONE;
+
+   return irq_create_fwspec_mapping();
+}
+
 static struct gpio_chip stm32_gpio_template = {
.request= stm32_gpio_request,
.free   = stm32_gpio_free,
@@ -181,10 +203,87 @@ static struct gpio_chip stm32_gpio_template = {
.set= stm32_gpio_set,
.direction_input= stm32_gpio_direction_input,
.direction_output   = stm32_gpio_direction_output,
+   .to_irq = stm32_gpio_to_irq,
 };
 
-/* Pinctrl functions */
+static struct irq_chip stm32_gpio_irq_chip = {
+   .name   = "stm32gpio",
+   .irq_eoi= irq_chip_eoi_parent,
+   .irq_mask   = irq_chip_mask_parent,
+   .irq_unmask = irq_chip_unmask_parent,
+   .irq_set_type   = irq_chip_set_type_parent,
+};
+
+static int stm32_gpio_domain_translate(struct irq_domain *d,
+  struct irq_fwspec *fwspec,
+  unsigned long *hwirq,
+  unsigned int *type)
+{
+   if ((fwspec->param_count != 2) ||
+   (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
+   return -EINVAL;
+
+   *hwirq = fwspec->param[0];
+   *type = fwspec->param[1];
+   return 0;
+}
+
+static void stm32_gpio_domain_activate(struct irq_domain *d,
+  struct irq_data *irq_data)
+{
+   struct stm32_gpio_bank *bank = d->host_data;
+   struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
+
+   if (gpiochip_lock_as_irq(>gpio_chip, irq_data->hwirq)) {
+   dev_err(pctl->dev,
+   "Unable to configure STM32 %s%ld as IRQ\n",
+   bank->gpio_chip.label, irq_data->hwirq);
+   return;
+   }
+   regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
+}
+
+static void stm32_gpio_domain_deactivate(struct irq_domain *d,
+struct irq_data *irq_data)
+{
+   struct stm32_gpio_bank *bank = d->host_data;
+
+   gpiochip_unlock_as_irq(>gpio_chip, irq_data->hwirq);
+}
+
+static int stm32_gpio_domain_alloc(struct irq_domain *domain,
+  unsigned int virq,
+  unsigned int nr_irqs, void *data)
+{
+   struct irq_fwspec *fwspec = data;
+   struct irq_fwspec

[PATCH v3 6/9] pinctrl: Add IRQ support to STM32 gpios

2016-09-02 Thread Alexandre TORGUE 
This patch adds IRQ support to STM32 gpios.

The EXTI controller has 16 lines dedicated to GPIOs.
EXTI line n can be connected to only line n of one of the GPIO ports, for
example EXTI0 can be connected to either PA0, or PB0, or PC0...
This port selection is done by specifying the port number into System
Config registers.

Signed-off-by: Maxime Coquelin 
Signed-off-by: Alexandre TORGUE 

diff --git a/drivers/pinctrl/stm32/Kconfig b/drivers/pinctrl/stm32/Kconfig
index 4c40dae..40d5abc 100644
--- a/drivers/pinctrl/stm32/Kconfig
+++ b/drivers/pinctrl/stm32/Kconfig
@@ -6,6 +6,8 @@ config PINCTRL_STM32
select PINMUX
select GENERIC_PINCONF
select GPIOLIB
+   select GPIOLIB_IRQCHIP
+   select MFD_SYSCON
 
 config PINCTRL_STM32F429
bool "STMicroelectronics STM32F429 pin control" if COMPILE_TEST && 
!MACH_STM32F429
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c 
b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 4ae596b..8fd25e2 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -8,6 +8,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -20,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -40,6 +43,7 @@
 #define STM32_GPIO_AFRH0x24
 
 #define STM32_GPIO_PINS_PER_BANK 16
+#define STM32_GPIO_IRQ_LINE 16
 
 #define gpio_range_to_bank(chip) \
container_of(chip, struct stm32_gpio_bank, range)
@@ -65,6 +69,7 @@ struct stm32_gpio_bank {
spinlock_t lock;
struct gpio_chip gpio_chip;
struct pinctrl_gpio_range range;
+   struct fwnode_handle *fwnode;
 };
 
 struct stm32_pinctrl {
@@ -77,6 +82,9 @@ struct stm32_pinctrl {
struct stm32_gpio_bank *banks;
unsigned nbanks;
const struct stm32_pinctrl_match_data *match_data;
+   struct irq_domain   *domain;
+   struct regmap   *regmap;
+   struct regmap_field *irqmux[STM32_GPIO_PINS_PER_BANK];
 };
 
 static inline int stm32_gpio_pin(int gpio)
@@ -174,6 +182,20 @@ static int stm32_gpio_direction_output(struct gpio_chip 
*chip,
return 0;
 }
 
+
+static int stm32_gpio_to_irq(struct gpio_chip *chip, unsigned int offset)
+{
+   struct irq_fwspec fwspec;
+   struct stm32_gpio_bank *bank = gpiochip_get_data(chip);
+
+   fwspec.fwnode = bank->fwnode;
+   fwspec.param_count = 2;
+   fwspec.param[0] = offset;
+   fwspec.param[1] = IRQ_TYPE_NONE;
+
+   return irq_create_fwspec_mapping();
+}
+
 static struct gpio_chip stm32_gpio_template = {
.request= stm32_gpio_request,
.free   = stm32_gpio_free,
@@ -181,10 +203,87 @@ static struct gpio_chip stm32_gpio_template = {
.set= stm32_gpio_set,
.direction_input= stm32_gpio_direction_input,
.direction_output   = stm32_gpio_direction_output,
+   .to_irq = stm32_gpio_to_irq,
 };
 
-/* Pinctrl functions */
+static struct irq_chip stm32_gpio_irq_chip = {
+   .name   = "stm32gpio",
+   .irq_eoi= irq_chip_eoi_parent,
+   .irq_mask   = irq_chip_mask_parent,
+   .irq_unmask = irq_chip_unmask_parent,
+   .irq_set_type   = irq_chip_set_type_parent,
+};
+
+static int stm32_gpio_domain_translate(struct irq_domain *d,
+  struct irq_fwspec *fwspec,
+  unsigned long *hwirq,
+  unsigned int *type)
+{
+   if ((fwspec->param_count != 2) ||
+   (fwspec->param[0] >= STM32_GPIO_IRQ_LINE))
+   return -EINVAL;
+
+   *hwirq = fwspec->param[0];
+   *type = fwspec->param[1];
+   return 0;
+}
+
+static void stm32_gpio_domain_activate(struct irq_domain *d,
+  struct irq_data *irq_data)
+{
+   struct stm32_gpio_bank *bank = d->host_data;
+   struct stm32_pinctrl *pctl = dev_get_drvdata(bank->gpio_chip.parent);
+
+   if (gpiochip_lock_as_irq(>gpio_chip, irq_data->hwirq)) {
+   dev_err(pctl->dev,
+   "Unable to configure STM32 %s%ld as IRQ\n",
+   bank->gpio_chip.label, irq_data->hwirq);
+   return;
+   }
+   regmap_field_write(pctl->irqmux[irq_data->hwirq], bank->range.id);
+}
+
+static void stm32_gpio_domain_deactivate(struct irq_domain *d,
+struct irq_data *irq_data)
+{
+   struct stm32_gpio_bank *bank = d->host_data;
+
+   gpiochip_unlock_as_irq(>gpio_chip, irq_data->hwirq);
+}
+
+static int stm32_gpio_domain_alloc(struct irq_domain *domain,
+  unsigned int virq,
+  unsigned int nr_irqs, void *data)
+{
+   struct irq_fwspec *fwspec = data;
+   struct irq_fwspec parent_fwspec;
+   struct stm32_pinctrl *pctl =