device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

Hello Nicolas, Jean-Christophe,

As I was trying to enable the touchscreen on the at91sam9261ek with
device-tree support, I ran into an issue. The touchscreen driver needs
to know the state of the pendown gpio and also needs it as an
interrupt source.

The problem is that when a gpio is used as an interrupt, it's
requested by the pinctrl driver during the xlate stage, marking it
unavaliable for the other driver.
It looks like the at91 pinctrl driver is the only one to use
gpio_request() in the xlate stage. Maybe we should remove this:

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a7549c4..cf91a35 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
irq_domain *d,
        *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;
 }

Jean-Jacques

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[boris brezillon]

On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
> Hello Nicolas, Jean-Christophe,
>
> As I was trying to enable the touchscreen on the at91sam9261ek with
> device-tree support, I ran into an issue. The touchscreen driver needs
> to know the state of the pendown gpio and also needs it as an
> interrupt source.
>
> The problem is that when a gpio is used as an interrupt, it's
> requested by the pinctrl driver during the xlate stage, marking it
> unavaliable for the other driver.
> It looks like the at91 pinctrl driver is the only one to use
> gpio_request() in the xlate stage. Maybe we should remove this:

You should only request it as a GPIO and then use gpio_to_irq to get the
related IRQ.
Because what is done here, is to solve the case where only the irq
is request, and in this specific case we need to request the pin as a
GPIO.


Best Regards,

Boris
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a7549c4..cf91a35 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
> irq_domain *d,
>          *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;
>   }
>
> Jean-Jacques

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

Hi Boris,

2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
> On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>>
>> Hello Nicolas, Jean-Christophe,
>>
>> As I was trying to enable the touchscreen on the at91sam9261ek with
>> device-tree support, I ran into an issue. The touchscreen driver needs
>> to know the state of the pendown gpio and also needs it as an
>> interrupt source.
>>
>> The problem is that when a gpio is used as an interrupt, it's
>> requested by the pinctrl driver during the xlate stage, marking it
>> unavaliable for the other driver.
>> It looks like the at91 pinctrl driver is the only one to use
>> gpio_request() in the xlate stage. Maybe we should remove this:
>
>
> You should only request it as a GPIO and then use gpio_to_irq to get the
> related IRQ.
> Because what is done here, is to solve the case where only the irq
> is request, and in this specific case we need to request the pin as a
> GPIO.
>

That's what I did first, and was about to submit the patch for the
touchscreen driver.
However it doesn't feel right. Being able to get the state of a gpio
that is also an interrupt seems very useful to me, not only for a
touchscreen controller.

I understand why it's being done here. It's a matter of being sure
that the GPIO is an input and that it'll not be configured otherwise
latter.
But:
1) I'm wondering why the atmel pinctrl is the only one to do that.
2) I believe that configuration of the direction can be done by
describing the GPIO in the DT. (pinctrl-at91.c line 592).
3) If all the GPIOs are described in the DT with a proper pinmux
description, i believe the exclusion is also handled. It's probably
not the case today though.

>
> Best Regards,
>
> Boris
>
>>
>> diff --git a/drivers/pinctrl/pinctrl-at91.c
>> b/drivers/pinctrl/pinctrl-at91.c
>> index a7549c4..cf91a35 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
>> irq_domain *d,
>>          *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;
>>   }
>>
>> Jean-Jacques
>
>

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
> Hi Boris,
> 
> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
> >>
> >> Hello Nicolas, Jean-Christophe,
> >>
> >> As I was trying to enable the touchscreen on the at91sam9261ek with
> >> device-tree support, I ran into an issue. The touchscreen driver needs
> >> to know the state of the pendown gpio and also needs it as an
> >> interrupt source.
> >>
> >> The problem is that when a gpio is used as an interrupt, it's
> >> requested by the pinctrl driver during the xlate stage, marking it
> >> unavaliable for the other driver.
> >> It looks like the at91 pinctrl driver is the only one to use
> >> gpio_request() in the xlate stage. Maybe we should remove this:
> >
> >
> > You should only request it as a GPIO and then use gpio_to_irq to get the
> > related IRQ.
> > Because what is done here, is to solve the case where only the irq
> > is request, and in this specific case we need to request the pin as a
> > GPIO.
> >
> 
> That's what I did first, and was about to submit the patch for the
> touchscreen driver.
> However it doesn't feel right. Being able to get the state of a gpio
> that is also an interrupt seems very useful to me, not only for a
> touchscreen controller.
> 
> I understand why it's being done here. It's a matter of being sure
> that the GPIO is an input and that it'll not be configured otherwise
> latter.
> But:
> 1) I'm wondering why the atmel pinctrl is the only one to do that.

because this the only to start to do it right
I had a very long discussion woth LinusW and Grant the Gpio need to stop to
use gpio_to_irq & co for irq.

I even send a proposition to do this work across the kernel t the CE-Linux for
funding
> 2) I believe that configuration of the direction can be done by
> describing the GPIO in the DT. (pinctrl-at91.c line 592).
No pinctrl is for pinmux ONLY not gpio direction & co

Best Regards,
J.

> 3) If all the GPIOs are described in the DT with a proper pinmux
> description, i believe the exclusion is also handled. It's probably
> not the case today though.

> 
> >
> > Best Regards,
> >
> > Boris
> >
> >>
> >> diff --git a/drivers/pinctrl/pinctrl-at91.c
> >> b/drivers/pinctrl/pinctrl-at91.c
> >> index a7549c4..cf91a35 100644
> >> --- a/drivers/pinctrl/pinctrl-at91.c
> >> +++ b/drivers/pinctrl/pinctrl-at91.c
> >> @@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
> >> irq_domain *d,
> >>          *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;
> >>   }
> >>
> >> Jean-Jacques
> >
> >
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
>> Hi Boris,
>>
>> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
>> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>> >>
>> >> Hello Nicolas, Jean-Christophe,
>> >>
>> >> As I was trying to enable the touchscreen on the at91sam9261ek with
>> >> device-tree support, I ran into an issue. The touchscreen driver needs
>> >> to know the state of the pendown gpio and also needs it as an
>> >> interrupt source.
>> >>
>> >> The problem is that when a gpio is used as an interrupt, it's
>> >> requested by the pinctrl driver during the xlate stage, marking it
>> >> unavaliable for the other driver.
>> >> It looks like the at91 pinctrl driver is the only one to use
>> >> gpio_request() in the xlate stage. Maybe we should remove this:
>> >
>> >
>> > You should only request it as a GPIO and then use gpio_to_irq to get the
>> > related IRQ.
>> > Because what is done here, is to solve the case where only the irq
>> > is request, and in this specific case we need to request the pin as a
>> > GPIO.
>> >
>>
>> That's what I did first, and was about to submit the patch for the
>> touchscreen driver.
>> However it doesn't feel right. Being able to get the state of a gpio
>> that is also an interrupt seems very useful to me, not only for a
>> touchscreen controller.
>>
>> I understand why it's being done here. It's a matter of being sure
>> that the GPIO is an input and that it'll not be configured otherwise
>> latter.
>> But:
>> 1) I'm wondering why the atmel pinctrl is the only one to do that.
>
> because this the only to start to do it right
> I had a very long discussion woth LinusW and Grant the Gpio need to stop to
> use gpio_to_irq & co for irq.
How can you get the value of the gpio that is also an interrupt source then ?
Can you give a short example?

>
> I even send a proposition to do this work across the kernel t the CE-Linux for
> funding
>> 2) I believe that configuration of the direction can be done by
>> describing the GPIO in the DT. (pinctrl-at91.c line 592).
> No pinctrl is for pinmux ONLY not gpio direction & co
>
> Best Regards,
> J.
>
>> 3) If all the GPIOs are described in the DT with a proper pinmux
>> description, i believe the exclusion is also handled. It's probably
>> not the case today though.
>
>>
>> >
>> > Best Regards,
>> >
>> > Boris
>> >
>> >>
>> >> diff --git a/drivers/pinctrl/pinctrl-at91.c
>> >> b/drivers/pinctrl/pinctrl-at91.c
>> >> index a7549c4..cf91a35 100644
>> >> --- a/drivers/pinctrl/pinctrl-at91.c
>> >> +++ b/drivers/pinctrl/pinctrl-at91.c
>> >> @@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
>> >> irq_domain *d,
>> >>          *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;
>> >>   }
>> >>
>> >> Jean-Jacques
>> >
>> >
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Christophe PLAGNIOL-VILLARD]

On 14:04 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> > On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
> >> Hi Boris,
> >>
> >> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
> >> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
> >> >>
> >> >> Hello Nicolas, Jean-Christophe,
> >> >>
> >> >> As I was trying to enable the touchscreen on the at91sam9261ek with
> >> >> device-tree support, I ran into an issue. The touchscreen driver needs
> >> >> to know the state of the pendown gpio and also needs it as an
> >> >> interrupt source.
> >> >>
> >> >> The problem is that when a gpio is used as an interrupt, it's
> >> >> requested by the pinctrl driver during the xlate stage, marking it
> >> >> unavaliable for the other driver.
> >> >> It looks like the at91 pinctrl driver is the only one to use
> >> >> gpio_request() in the xlate stage. Maybe we should remove this:
> >> >
> >> >
> >> > You should only request it as a GPIO and then use gpio_to_irq to get the
> >> > related IRQ.
> >> > Because what is done here, is to solve the case where only the irq
> >> > is request, and in this specific case we need to request the pin as a
> >> > GPIO.
> >> >
> >>
> >> That's what I did first, and was about to submit the patch for the
> >> touchscreen driver.
> >> However it doesn't feel right. Being able to get the state of a gpio
> >> that is also an interrupt seems very useful to me, not only for a
> >> touchscreen controller.
> >>
> >> I understand why it's being done here. It's a matter of being sure
> >> that the GPIO is an input and that it'll not be configured otherwise
> >> latter.
> >> But:
> >> 1) I'm wondering why the atmel pinctrl is the only one to do that.
> >
> > because this the only to start to do it right
> > I had a very long discussion woth LinusW and Grant the Gpio need to stop to
> > use gpio_to_irq & co for irq.
> How can you get the value of the gpio that is also an interrupt source then ?
> Can you give a short example?

you just have to check the irq source

failing or raising

but on 9261 impossible the gpio does not have such detail

but anyway this is the invert you need to get the information from the IRQ no the way
arround

so the gpio driver may need to be updated to provide such information to the
touch driver

Best Regards,
J.
> 
> >
> > I even send a proposition to do this work across the kernel t the CE-Linux for
> > funding
> >> 2) I believe that configuration of the direction can be done by
> >> describing the GPIO in the DT. (pinctrl-at91.c line 592).
> > No pinctrl is for pinmux ONLY not gpio direction & co
> >
> > Best Regards,
> > J.
> >
> >> 3) If all the GPIOs are described in the DT with a proper pinmux
> >> description, i believe the exclusion is also handled. It's probably
> >> not the case today though.
> >
> >>
> >> >
> >> > Best Regards,
> >> >
> >> > Boris
> >> >
> >> >>
> >> >> diff --git a/drivers/pinctrl/pinctrl-at91.c
> >> >> b/drivers/pinctrl/pinctrl-at91.c
> >> >> index a7549c4..cf91a35 100644
> >> >> --- a/drivers/pinctrl/pinctrl-at91.c
> >> >> +++ b/drivers/pinctrl/pinctrl-at91.c
> >> >> @@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
> >> >> irq_domain *d,
> >> >>          *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;
> >> >>   }
> >> >>
> >> >> Jean-Jacques
> >> >
> >> >
> >>
> >> _______________________________________________
> >> linux-arm-kernel mailing list
> >> linux-arm-kernel at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> On 14:04 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
>> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> > On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
>> >> Hi Boris,
>> >>
>> >> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
>> >> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>> >> >>
>> >> >> Hello Nicolas, Jean-Christophe,
>> >> >>
>> >> >> As I was trying to enable the touchscreen on the at91sam9261ek with
>> >> >> device-tree support, I ran into an issue. The touchscreen driver needs
>> >> >> to know the state of the pendown gpio and also needs it as an
>> >> >> interrupt source.
>> >> >>
>> >> >> The problem is that when a gpio is used as an interrupt, it's
>> >> >> requested by the pinctrl driver during the xlate stage, marking it
>> >> >> unavaliable for the other driver.
>> >> >> It looks like the at91 pinctrl driver is the only one to use
>> >> >> gpio_request() in the xlate stage. Maybe we should remove this:
>> >> >
>> >> >
>> >> > You should only request it as a GPIO and then use gpio_to_irq to get the
>> >> > related IRQ.
>> >> > Because what is done here, is to solve the case where only the irq
>> >> > is request, and in this specific case we need to request the pin as a
>> >> > GPIO.
>> >> >
>> >>
>> >> That's what I did first, and was about to submit the patch for the
>> >> touchscreen driver.
>> >> However it doesn't feel right. Being able to get the state of a gpio
>> >> that is also an interrupt seems very useful to me, not only for a
>> >> touchscreen controller.
>> >>
>> >> I understand why it's being done here. It's a matter of being sure
>> >> that the GPIO is an input and that it'll not be configured otherwise
>> >> latter.
>> >> But:
>> >> 1) I'm wondering why the atmel pinctrl is the only one to do that.
>> >
>> > because this the only to start to do it right
>> > I had a very long discussion woth LinusW and Grant the Gpio need to stop to
>> > use gpio_to_irq & co for irq.
>> How can you get the value of the gpio that is also an interrupt source then ?
>> Can you give a short example?
>
> you just have to check the irq source
>
> failing or raising
>
> but on 9261 impossible the gpio does not have such detail
>
> but anyway this is the invert you need to get the information from the IRQ no the way
> arround

Should I modify the touchscreen driver to use irq_to_gpio() in this
case then ? or is this also not proper ?

>
> so the gpio driver may need to be updated to provide such information to the
> touch driver
>
> Best Regards,
> J.
>>
>> >
>> > I even send a proposition to do this work across the kernel t the CE-Linux for
>> > funding
>> >> 2) I believe that configuration of the direction can be done by
>> >> describing the GPIO in the DT. (pinctrl-at91.c line 592).
>> > No pinctrl is for pinmux ONLY not gpio direction & co
>> >
>> > Best Regards,
>> > J.
>> >
>> >> 3) If all the GPIOs are described in the DT with a proper pinmux
>> >> description, i believe the exclusion is also handled. It's probably
>> >> not the case today though.
>> >
>> >>
>> >> >
>> >> > Best Regards,
>> >> >
>> >> > Boris
>> >> >
>> >> >>
>> >> >> diff --git a/drivers/pinctrl/pinctrl-at91.c
>> >> >> b/drivers/pinctrl/pinctrl-at91.c
>> >> >> index a7549c4..cf91a35 100644
>> >> >> --- a/drivers/pinctrl/pinctrl-at91.c
>> >> >> +++ b/drivers/pinctrl/pinctrl-at91.c
>> >> >> @@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
>> >> >> irq_domain *d,
>> >> >>          *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;
>> >> >>   }
>> >> >>
>> >> >> Jean-Jacques
>> >> >
>> >> >
>> >>
>> >> _______________________________________________
>> >> linux-arm-kernel mailing list
>> >> linux-arm-kernel at lists.infradead.org
>> >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Arnd Bergmann]

On Wednesday 15 January 2014 14:44:24 Jean-Jacques Hiblot wrote:
> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> > On 14:04 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >> > because this the only to start to do it right
> >> > I had a very long discussion woth LinusW and Grant the Gpio need to stop to
> >> > use gpio_to_irq & co for irq.
> >> How can you get the value of the gpio that is also an interrupt source then ?
> >> Can you give a short example?
> >
> > you just have to check the irq source
> >
> > failing or raising
> >
> > but on 9261 impossible the gpio does not have such detail
> >
> > but anyway this is the invert you need to get the information from the IRQ no the way
> > arround
> 
> Should I modify the touchscreen driver to use irq_to_gpio() in this
> case then ? or is this also not proper ?

irq_to_gpio no longer exists.

	Arnd

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Christophe PLAGNIOL-VILLARD]

On 14:44 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> > On 14:04 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >> > On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
> >> >> Hi Boris,
> >> >>
> >> >> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
> >> >> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
> >> >> >>
> >> >> >> Hello Nicolas, Jean-Christophe,
> >> >> >>
> >> >> >> As I was trying to enable the touchscreen on the at91sam9261ek with
> >> >> >> device-tree support, I ran into an issue. The touchscreen driver needs
> >> >> >> to know the state of the pendown gpio and also needs it as an
> >> >> >> interrupt source.
> >> >> >>
> >> >> >> The problem is that when a gpio is used as an interrupt, it's
> >> >> >> requested by the pinctrl driver during the xlate stage, marking it
> >> >> >> unavaliable for the other driver.
> >> >> >> It looks like the at91 pinctrl driver is the only one to use
> >> >> >> gpio_request() in the xlate stage. Maybe we should remove this:
> >> >> >
> >> >> >
> >> >> > You should only request it as a GPIO and then use gpio_to_irq to get the
> >> >> > related IRQ.
> >> >> > Because what is done here, is to solve the case where only the irq
> >> >> > is request, and in this specific case we need to request the pin as a
> >> >> > GPIO.
> >> >> >
> >> >>
> >> >> That's what I did first, and was about to submit the patch for the
> >> >> touchscreen driver.
> >> >> However it doesn't feel right. Being able to get the state of a gpio
> >> >> that is also an interrupt seems very useful to me, not only for a
> >> >> touchscreen controller.
> >> >>
> >> >> I understand why it's being done here. It's a matter of being sure
> >> >> that the GPIO is an input and that it'll not be configured otherwise
> >> >> latter.
> >> >> But:
> >> >> 1) I'm wondering why the atmel pinctrl is the only one to do that.
> >> >
> >> > because this the only to start to do it right
> >> > I had a very long discussion woth LinusW and Grant the Gpio need to stop to
> >> > use gpio_to_irq & co for irq.
> >> How can you get the value of the gpio that is also an interrupt source then ?
> >> Can you give a short example?
> >
> > you just have to check the irq source
> >
> > failing or raising
> >
> > but on 9261 impossible the gpio does not have such detail
> >
> > but anyway this is the invert you need to get the information from the IRQ no the way
> > arround
> 
> Should I modify the touchscreen driver to use irq_to_gpio() in this
> case then ? or is this also not proper ?
> 
no as said by arnd irq_to_gpio does not exsit

that's why I said the irq need to provide you the information as it's a
raising or failing irq

Best Regards,
J.

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> On 14:44 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
>> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> > On 14:04 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
>> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> >> > On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
>> >> >> Hi Boris,
>> >> >>
>> >> >> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
>> >> >> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>> >> >> >>
>> >> >> >> Hello Nicolas, Jean-Christophe,
>> >> >> >>
>> >> >> >> As I was trying to enable the touchscreen on the at91sam9261ek with
>> >> >> >> device-tree support, I ran into an issue. The touchscreen driver needs
>> >> >> >> to know the state of the pendown gpio and also needs it as an
>> >> >> >> interrupt source.
>> >> >> >>
>> >> >> >> The problem is that when a gpio is used as an interrupt, it's
>> >> >> >> requested by the pinctrl driver during the xlate stage, marking it
>> >> >> >> unavaliable for the other driver.
>> >> >> >> It looks like the at91 pinctrl driver is the only one to use
>> >> >> >> gpio_request() in the xlate stage. Maybe we should remove this:
>> >> >> >
>> >> >> >
>> >> >> > You should only request it as a GPIO and then use gpio_to_irq to get the
>> >> >> > related IRQ.
>> >> >> > Because what is done here, is to solve the case where only the irq
>> >> >> > is request, and in this specific case we need to request the pin as a
>> >> >> > GPIO.
>> >> >> >
>> >> >>
>> >> >> That's what I did first, and was about to submit the patch for the
>> >> >> touchscreen driver.
>> >> >> However it doesn't feel right. Being able to get the state of a gpio
>> >> >> that is also an interrupt seems very useful to me, not only for a
>> >> >> touchscreen controller.
>> >> >>
>> >> >> I understand why it's being done here. It's a matter of being sure
>> >> >> that the GPIO is an input and that it'll not be configured otherwise
>> >> >> latter.
>> >> >> But:
>> >> >> 1) I'm wondering why the atmel pinctrl is the only one to do that.
>> >> >
>> >> > because this the only to start to do it right
>> >> > I had a very long discussion woth LinusW and Grant the Gpio need to stop to
>> >> > use gpio_to_irq & co for irq.
>> >> How can you get the value of the gpio that is also an interrupt source then ?
>> >> Can you give a short example?
>> >
>> > you just have to check the irq source
>> >
>> > failing or raising
>> >
>> > but on 9261 impossible the gpio does not have such detail
>> >
>> > but anyway this is the invert you need to get the information from the IRQ no the way
>> > arround
>>
>> Should I modify the touchscreen driver to use irq_to_gpio() in this
>> case then ? or is this also not proper ?
>>
> no as said by arnd irq_to_gpio does not exsit
>
> that's why I said the irq need to provide you the information as it's a
> raising or failing irq

Even when this kind of information is available it's not enough to
know for sure the state of the gpio. Think of short pulses such as the
glitches you have when you press a button.

>
> Best Regards,
> J.

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Christophe PLAGNIOL-VILLARD]

On 15:41 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> > On 14:44 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >> > On 14:04 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> >> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >> >> > On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
> >> >> >> Hi Boris,
> >> >> >>
> >> >> >> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
> >> >> >> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
> >> >> >> >>
> >> >> >> >> Hello Nicolas, Jean-Christophe,
> >> >> >> >>
> >> >> >> >> As I was trying to enable the touchscreen on the at91sam9261ek with
> >> >> >> >> device-tree support, I ran into an issue. The touchscreen driver needs
> >> >> >> >> to know the state of the pendown gpio and also needs it as an
> >> >> >> >> interrupt source.
> >> >> >> >>
> >> >> >> >> The problem is that when a gpio is used as an interrupt, it's
> >> >> >> >> requested by the pinctrl driver during the xlate stage, marking it
> >> >> >> >> unavaliable for the other driver.
> >> >> >> >> It looks like the at91 pinctrl driver is the only one to use
> >> >> >> >> gpio_request() in the xlate stage. Maybe we should remove this:
> >> >> >> >
> >> >> >> >
> >> >> >> > You should only request it as a GPIO and then use gpio_to_irq to get the
> >> >> >> > related IRQ.
> >> >> >> > Because what is done here, is to solve the case where only the irq
> >> >> >> > is request, and in this specific case we need to request the pin as a
> >> >> >> > GPIO.
> >> >> >> >
> >> >> >>
> >> >> >> That's what I did first, and was about to submit the patch for the
> >> >> >> touchscreen driver.
> >> >> >> However it doesn't feel right. Being able to get the state of a gpio
> >> >> >> that is also an interrupt seems very useful to me, not only for a
> >> >> >> touchscreen controller.
> >> >> >>
> >> >> >> I understand why it's being done here. It's a matter of being sure
> >> >> >> that the GPIO is an input and that it'll not be configured otherwise
> >> >> >> latter.
> >> >> >> But:
> >> >> >> 1) I'm wondering why the atmel pinctrl is the only one to do that.
> >> >> >
> >> >> > because this the only to start to do it right
> >> >> > I had a very long discussion woth LinusW and Grant the Gpio need to stop to
> >> >> > use gpio_to_irq & co for irq.
> >> >> How can you get the value of the gpio that is also an interrupt source then ?
> >> >> Can you give a short example?
> >> >
> >> > you just have to check the irq source
> >> >
> >> > failing or raising
> >> >
> >> > but on 9261 impossible the gpio does not have such detail
> >> >
> >> > but anyway this is the invert you need to get the information from the IRQ no the way
> >> > arround
> >>
> >> Should I modify the touchscreen driver to use irq_to_gpio() in this
> >> case then ? or is this also not proper ?
> >>
> > no as said by arnd irq_to_gpio does not exsit
> >
> > that's why I said the irq need to provide you the information as it's a
> > raising or failing irq
> 
> Even when this kind of information is available it's not enough to
> know for sure the state of the gpio. Think of short pulses such as the
> glitches you have when you press a button.

this why you have debounce in the gpio IP to solve this

Best Regards,
J.
> 
> >
> > Best Regards,
> > J.

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> On 15:41 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
>> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> > On 14:44 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
>> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> >> > On 14:04 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
>> >> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
>> >> >> > On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
>> >> >> >> Hi Boris,
>> >> >> >>
>> >> >> >> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
>> >> >> >> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>> >> >> >> >>
>> >> >> >> >> Hello Nicolas, Jean-Christophe,
>> >> >> >> >>
>> >> >> >> >> As I was trying to enable the touchscreen on the at91sam9261ek with
>> >> >> >> >> device-tree support, I ran into an issue. The touchscreen driver needs
>> >> >> >> >> to know the state of the pendown gpio and also needs it as an
>> >> >> >> >> interrupt source.
>> >> >> >> >>
>> >> >> >> >> The problem is that when a gpio is used as an interrupt, it's
>> >> >> >> >> requested by the pinctrl driver during the xlate stage, marking it
>> >> >> >> >> unavaliable for the other driver.
>> >> >> >> >> It looks like the at91 pinctrl driver is the only one to use
>> >> >> >> >> gpio_request() in the xlate stage. Maybe we should remove this:
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > You should only request it as a GPIO and then use gpio_to_irq to get the
>> >> >> >> > related IRQ.
>> >> >> >> > Because what is done here, is to solve the case where only the irq
>> >> >> >> > is request, and in this specific case we need to request the pin as a
>> >> >> >> > GPIO.
>> >> >> >> >
>> >> >> >>
>> >> >> >> That's what I did first, and was about to submit the patch for the
>> >> >> >> touchscreen driver.
>> >> >> >> However it doesn't feel right. Being able to get the state of a gpio
>> >> >> >> that is also an interrupt seems very useful to me, not only for a
>> >> >> >> touchscreen controller.
>> >> >> >>
>> >> >> >> I understand why it's being done here. It's a matter of being sure
>> >> >> >> that the GPIO is an input and that it'll not be configured otherwise
>> >> >> >> latter.
>> >> >> >> But:
>> >> >> >> 1) I'm wondering why the atmel pinctrl is the only one to do that.
>> >> >> >
>> >> >> > because this the only to start to do it right
>> >> >> > I had a very long discussion woth LinusW and Grant the Gpio need to stop to
>> >> >> > use gpio_to_irq & co for irq.
>> >> >> How can you get the value of the gpio that is also an interrupt source then ?
>> >> >> Can you give a short example?
>> >> >
>> >> > you just have to check the irq source
>> >> >
>> >> > failing or raising
>> >> >
>> >> > but on 9261 impossible the gpio does not have such detail
>> >> >
>> >> > but anyway this is the invert you need to get the information from the IRQ no the way
>> >> > arround
>> >>
>> >> Should I modify the touchscreen driver to use irq_to_gpio() in this
>> >> case then ? or is this also not proper ?
>> >>
>> > no as said by arnd irq_to_gpio does not exsit
>> >
>> > that's why I said the irq need to provide you the information as it's a
>> > raising or failing irq
>>
>> Even when this kind of information is available it's not enough to
>> know for sure the state of the gpio. Think of short pulses such as the
>> glitches you have when you press a button.
>
> this why you have debounce in the gpio IP to solve this

Unfortunately, the debounce filter in the IP not appropriate to handle
the glitches such as what you have in the case of a button.
It may be good enough to filter EMI induced pulses but not mechanical
ones which are way too long.

>
> Best Regards,
> J.
>>
>> >
>> > Best Regards,
>> > J.

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:30 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> > On 15:41 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >> > On 14:44 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> >> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >> >> > On 14:04 Wed 15 Jan     , Jean-Jacques Hiblot wrote:
> >> >> >> 2014/1/15 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>:
> >> >> >> > On 12:05 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
> >> >> >> >> Hi Boris,
> >> >> >> >>
> >> >> >> >> 2014/1/13 boris brezillon <b.brezillon@overkiz.com>:
> >> >> >> >> > On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
> >> >> >> >> >>
> >> >> >> >> >> Hello Nicolas, Jean-Christophe,
> >> >> >> >> >>
> >> >> >> >> >> As I was trying to enable the touchscreen on the at91sam9261ek with
> >> >> >> >> >> device-tree support, I ran into an issue. The touchscreen driver needs
> >> >> >> >> >> to know the state of the pendown gpio and also needs it as an
> >> >> >> >> >> interrupt source.
> >> >> >> >> >>
> >> >> >> >> >> The problem is that when a gpio is used as an interrupt, it's
> >> >> >> >> >> requested by the pinctrl driver during the xlate stage, marking it
> >> >> >> >> >> unavaliable for the other driver.
> >> >> >> >> >> It looks like the at91 pinctrl driver is the only one to use
> >> >> >> >> >> gpio_request() in the xlate stage. Maybe we should remove this:
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> > You should only request it as a GPIO and then use gpio_to_irq to get the
> >> >> >> >> > related IRQ.
> >> >> >> >> > Because what is done here, is to solve the case where only the irq
> >> >> >> >> > is request, and in this specific case we need to request the pin as a
> >> >> >> >> > GPIO.
> >> >> >> >> >
> >> >> >> >>
> >> >> >> >> That's what I did first, and was about to submit the patch for the
> >> >> >> >> touchscreen driver.
> >> >> >> >> However it doesn't feel right. Being able to get the state of a gpio
> >> >> >> >> that is also an interrupt seems very useful to me, not only for a
> >> >> >> >> touchscreen controller.
> >> >> >> >>
> >> >> >> >> I understand why it's being done here. It's a matter of being sure
> >> >> >> >> that the GPIO is an input and that it'll not be configured otherwise
> >> >> >> >> latter.
> >> >> >> >> But:
> >> >> >> >> 1) I'm wondering why the atmel pinctrl is the only one to do that.
> >> >> >> >
> >> >> >> > because this the only to start to do it right
> >> >> >> > I had a very long discussion woth LinusW and Grant the Gpio need to stop to
> >> >> >> > use gpio_to_irq & co for irq.
> >> >> >> How can you get the value of the gpio that is also an interrupt source then ?
> >> >> >> Can you give a short example?
> >> >> >
> >> >> > you just have to check the irq source
> >> >> >
> >> >> > failing or raising
> >> >> >
> >> >> > but on 9261 impossible the gpio does not have such detail
> >> >> >
> >> >> > but anyway this is the invert you need to get the information from the IRQ no the way
> >> >> > arround
> >> >>
> >> >> Should I modify the touchscreen driver to use irq_to_gpio() in this
> >> >> case then ? or is this also not proper ?
> >> >>
> >> > no as said by arnd irq_to_gpio does not exsit
> >> >
> >> > that's why I said the irq need to provide you the information as it's a
> >> > raising or failing irq
> >>
> >> Even when this kind of information is available it's not enough to
> >> know for sure the state of the gpio. Think of short pulses such as the
> >> glitches you have when you press a button.
> >
> > this why you have debounce in the gpio IP to solve this
> 
> Unfortunately, the debounce filter in the IP not appropriate to handle
> the glitches such as what you have in the case of a button.
> It may be good enough to filter EMI induced pulses but not mechanical
> ones which are way too long.

so use deglitch one this will do the tric

Best Regards,
J.

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Nicolas Ferre]

On 13/01/2014 11:35, boris brezillon :
> On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>> Hello Nicolas, Jean-Christophe,
>>
>> As I was trying to enable the touchscreen on the at91sam9261ek with
>> device-tree support, I ran into an issue. The touchscreen driver needs
>> to know the state of the pendown gpio and also needs it as an
>> interrupt source.
>>
>> The problem is that when a gpio is used as an interrupt, it's
>> requested by the pinctrl driver during the xlate stage, marking it
>> unavaliable for the other driver.
>> It looks like the at91 pinctrl driver is the only one to use
>> gpio_request() in the xlate stage. Maybe we should remove this:
> 
> You should only request it as a GPIO and then use gpio_to_irq to get the
> related IRQ.
> Because what is done here, is to solve the case where only the irq
> is request, and in this specific case we need to request the pin as a
> GPIO.

Yes, this is what we do.

It seems simple and obvious to me, but some may say that "you shall not
do that, it is horrible!". Well... I always tend to choose a solution
that works. It is one of my weaknesses, I admit ;-)

Linus W. any advice on this, before we hit again one of those infinite
threads that leads no progress at all?


>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>> index a7549c4..cf91a35 100644
>> --- a/drivers/pinctrl/pinctrl-at91.c
>> +++ b/drivers/pinctrl/pinctrl-at91.c
>> @@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
>> irq_domain *d,
>>          *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;
>>   }
>>
>> Jean-Jacques
> 
> 
> 


-- 
Nicolas Ferre

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Linus Walleij]

On Wed, Jan 15, 2014 at 6:28 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
> On 13/01/2014 11:35, boris brezillon :

>> You should only request it as a GPIO and then use gpio_to_irq to get the
>> related IRQ.
>> Because what is done here, is to solve the case where only the irq
>> is request, and in this specific case we need to request the pin as a
>> GPIO.
>
> Yes, this is what we do.
>
> It seems simple and obvious to me, but some may say that "you shall not
> do that, it is horrible!". Well... I always tend to choose a solution
> that works. It is one of my weaknesses, I admit ;-)
>
> Linus W. any advice on this, before we hit again one of those infinite
> threads that leads no progress at all?

So we recently established that it is actually OK for any IRQ
consumer to request an IRQ from any irq_chip no matter if
that is a combined GPIO+IRQ driver. This was concluded after
a long discussion involving several parties.

gpio_to_irq() is just a convenience function and should not be
relied upon to have been called before the IRQ is used.

The basic premise is that gpio_chip and irq_chip are
orthogonal, and offering their services independent of each
other.

Especially in the device tree use case it is very hard to
dictate that a certain semantic need to be followed to use
a certain interrupt-controller to have dependencies to a
certain gpio-controller. So they need to be orthogonal.

First step is: always prepare the hardware and make it
ready for action in respective callbacks from the gpio_chip API
and irq_chip API. Do not rely on gpio_to_irq() having been
called first anymore.

This leads to ambiguities that we need to solve: if there is
competition inside the subsystem which side is using
the resource (a certain GPIO line and register for example)
it needs to deny certain operations and keep track of usage
inside of the gpiolib subsystem.

We have added a new API to help with this situation:

 gpio_lock_as_irq(struct gpio_chip *chip, unsigned int offset)
 gpio_unlock_as_irq(struct gpio_chip *chip,
                                      unsigned int offset)

These should be called from the irq_chip startup() and
.shutdown() callbacks to flag that the line is now in use by
an IRQ. For example the GPIOlib core will deny the line to
be set as output after this.

If we need more infrastructure to help with this, I'm game.

Clear as mud? ;-)

Yours,
Linus Walleij

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

2014/1/22 Linus Walleij <linus.walleij@linaro.org>:
> On Wed, Jan 15, 2014 at 6:28 PM, Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>> On 13/01/2014 11:35, boris brezillon :
>
>>> You should only request it as a GPIO and then use gpio_to_irq to get the
>>> related IRQ.
>>> Because what is done here, is to solve the case where only the irq
>>> is request, and in this specific case we need to request the pin as a
>>> GPIO.
>>
>> Yes, this is what we do.
>>
>> It seems simple and obvious to me, but some may say that "you shall not
>> do that, it is horrible!". Well... I always tend to choose a solution
>> that works. It is one of my weaknesses, I admit ;-)
>>
>> Linus W. any advice on this, before we hit again one of those infinite
>> threads that leads no progress at all?
>
> So we recently established that it is actually OK for any IRQ
> consumer to request an IRQ from any irq_chip no matter if
> that is a combined GPIO+IRQ driver. This was concluded after
> a long discussion involving several parties.
>
> gpio_to_irq() is just a convenience function and should not be
> relied upon to have been called before the IRQ is used.
>
> The basic premise is that gpio_chip and irq_chip are
> orthogonal, and offering their services independent of each
> other.
>
> Especially in the device tree use case it is very hard to
> dictate that a certain semantic need to be followed to use
> a certain interrupt-controller to have dependencies to a
> certain gpio-controller. So they need to be orthogonal.
>
> First step is: always prepare the hardware and make it
> ready for action in respective callbacks from the gpio_chip API
> and irq_chip API. Do not rely on gpio_to_irq() having been
> called first anymore.
>
> This leads to ambiguities that we need to solve: if there is
> competition inside the subsystem which side is using
> the resource (a certain GPIO line and register for example)
> it needs to deny certain operations and keep track of usage
> inside of the gpiolib subsystem.
>
> We have added a new API to help with this situation:
>
>  gpio_lock_as_irq(struct gpio_chip *chip, unsigned int offset)
>  gpio_unlock_as_irq(struct gpio_chip *chip,
>                                       unsigned int offset)
>
> These should be called from the irq_chip startup() and
> .shutdown() callbacks to flag that the line is now in use by
> an IRQ. For example the GPIOlib core will deny the line to
> be set as output after this.
>
> If we need more infrastructure to help with this, I'm game.
>
> Clear as mud? ;-)
it's actually crystal clear.

>
> Yours,
> Linus Walleij

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:35 Mon 13 Jan     , boris brezillon wrote:
> On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
> >Hello Nicolas, Jean-Christophe,
> >
> >As I was trying to enable the touchscreen on the at91sam9261ek with
> >device-tree support, I ran into an issue. The touchscreen driver needs
> >to know the state of the pendown gpio and also needs it as an
> >interrupt source.
> >
> >The problem is that when a gpio is used as an interrupt, it's
> >requested by the pinctrl driver during the xlate stage, marking it
> >unavaliable for the other driver.
> >It looks like the at91 pinctrl driver is the only one to use
> >gpio_request() in the xlate stage. Maybe we should remove this:
> 
> You should only request it as a GPIO and then use gpio_to_irq to get the
> related IRQ.

This here a HUGE issue in the hole kernel

You should NEVER known ti's a gpio in the driver at all gpio_to_irq should never
exist you need to only see the irq

> Because what is done here, is to solve the case where only the irq
> is request, and in this specific case we need to request the pin as a
> GPIO.
this need to be handled at irq level not drivers

Best Regards,
J.

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[boris brezillon]

On 15/01/2014 19:06, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:35 Mon 13 Jan     , boris brezillon wrote:
>> On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>>> Hello Nicolas, Jean-Christophe,
>>>
>>> As I was trying to enable the touchscreen on the at91sam9261ek with
>>> device-tree support, I ran into an issue. The touchscreen driver needs
>>> to know the state of the pendown gpio and also needs it as an
>>> interrupt source.
>>>
>>> The problem is that when a gpio is used as an interrupt, it's
>>> requested by the pinctrl driver during the xlate stage, marking it
>>> unavaliable for the other driver.
>>> It looks like the at91 pinctrl driver is the only one to use
>>> gpio_request() in the xlate stage. Maybe we should remove this:
>> You should only request it as a GPIO and then use gpio_to_irq to get the
>> related IRQ.
> This here a HUGE issue in the hole kernel
>
> You should NEVER known ti's a gpio in the driver at all gpio_to_irq should never
> exist you need to only see the irq
>
>> Because what is done here, is to solve the case where only the irq
>> is request, and in this specific case we need to request the pin as a
>> GPIO.
> this need to be handled at irq level not drivers

Then propose something (or at least give us a deadline for this new 
interrupt
model to come out), because the ADS7843E touchscreen is not working anymore
(at least on at91 platforms).

What this driver needs is a level IRQ (not an edge IRQ). The code in 
ads7846_hard_irq<http://lxr.free-electrons.com/ident?i=ads7846_hard_irq>
interrupt handler is here to transform an edge IRQ into a level IRQ.

Is there a way to provide a generic framework to transform edge IRQs 
into level IRQs
(because some GPIO controllers do not support level IRQs, and this is 
the case for the
at91sam9261 one) ?


Of cource the gpio_to_irq approach is not perfect, but at least it as 
the benefit to quickly
fix the bug, and we will still be able to improve this later, when we 
have enough tools
(or mechanisms) to do it.

Best Regards,

Boris
> Best Regards,
> J.

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Nicolas Ferre]

On 16/01/2014 09:54, boris brezillon :
> On 15/01/2014 19:06, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> On 11:35 Mon 13 Jan     , boris brezillon wrote:
>>> On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>>>> Hello Nicolas, Jean-Christophe,
>>>>
>>>> As I was trying to enable the touchscreen on the at91sam9261ek with
>>>> device-tree support, I ran into an issue. The touchscreen driver needs
>>>> to know the state of the pendown gpio and also needs it as an
>>>> interrupt source.
>>>>
>>>> The problem is that when a gpio is used as an interrupt, it's
>>>> requested by the pinctrl driver during the xlate stage, marking it
>>>> unavaliable for the other driver.
>>>> It looks like the at91 pinctrl driver is the only one to use
>>>> gpio_request() in the xlate stage. Maybe we should remove this:
>>> You should only request it as a GPIO and then use gpio_to_irq to get the
>>> related IRQ.
>> This here a HUGE issue in the hole kernel
>>
>> You should NEVER known ti's a gpio in the driver at all gpio_to_irq should never
>> exist you need to only see the irq
>>
>>> Because what is done here, is to solve the case where only the irq
>>> is request, and in this specific case we need to request the pin as a
>>> GPIO.
>> this need to be handled at irq level not drivers
> 
> Then propose something (or at least give us a deadline for this new 
> interrupt
> model to come out), because the ADS7843E touchscreen is not working anymore
> (at least on at91 platforms).
> 
> What this driver needs is a level IRQ (not an edge IRQ). The code in 
> ads7846_hard_irq<http://lxr.free-electrons.com/ident?i=ads7846_hard_irq>
> interrupt handler is here to transform an edge IRQ into a level IRQ.
> 
> Is there a way to provide a generic framework to transform edge IRQs 
> into level IRQs
> (because some GPIO controllers do not support level IRQs, and this is 
> the case for the
> at91sam9261 one) ?
> 
> 
> Of cource the gpio_to_irq approach is not perfect, but at least it as 
> the benefit to quickly
> fix the bug, and we will still be able to improve this later, when we 
> have enough tools
> (or mechanisms) to do it.

Moreover, I do not see the rationale behind the concept of "interrupt
with an electrical value". An interrupt signals an event and this event
can be a transition or a state but an electrical value is the
responsibility of a GPIO line, not the other way around.

I see a code that could give the value of an electrical line related to
an interrupt as a twisted interpretation of the notion of "interrupt".
It surprises me that it could be thought as an enhancement that an IRQ
coming from a GPIO could give a value!

Isn't it why other people are also using this simple distinction to use
their GPIO/IRQ mechanism? Maybe this is why we are the only ones to
completely forbid this possibility.

So, let's fix the bug, submit the modification to mainline, make
platform work and see if somebody can convince me that retrieving an
electrical line status from a GPIO is a "bad" thing...

Bye,
-- 
Nicolas Ferre

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

2014/1/16 Nicolas Ferre <nicolas.ferre@atmel.com>:
> On 16/01/2014 09:54, boris brezillon :
>> On 15/01/2014 19:06, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>> On 11:35 Mon 13 Jan     , boris brezillon wrote:
>>>> On 13/01/2014 11:29, Jean-Jacques Hiblot wrote:
>>>>> Hello Nicolas, Jean-Christophe,
>>>>>
>>>>> As I was trying to enable the touchscreen on the at91sam9261ek with
>>>>> device-tree support, I ran into an issue. The touchscreen driver needs
>>>>> to know the state of the pendown gpio and also needs it as an
>>>>> interrupt source.
>>>>>
>>>>> The problem is that when a gpio is used as an interrupt, it's
>>>>> requested by the pinctrl driver during the xlate stage, marking it
>>>>> unavaliable for the other driver.
>>>>> It looks like the at91 pinctrl driver is the only one to use
>>>>> gpio_request() in the xlate stage. Maybe we should remove this:
>>>> You should only request it as a GPIO and then use gpio_to_irq to get the
>>>> related IRQ.
>>> This here a HUGE issue in the hole kernel
>>>
>>> You should NEVER known ti's a gpio in the driver at all gpio_to_irq should never
>>> exist you need to only see the irq
>>>
>>>> Because what is done here, is to solve the case where only the irq
>>>> is request, and in this specific case we need to request the pin as a
>>>> GPIO.
>>> this need to be handled at irq level not drivers
>>
>> Then propose something (or at least give us a deadline for this new
>> interrupt
>> model to come out), because the ADS7843E touchscreen is not working anymore
>> (at least on at91 platforms).
>>
>> What this driver needs is a level IRQ (not an edge IRQ). The code in
>> ads7846_hard_irq<http://lxr.free-electrons.com/ident?i=ads7846_hard_irq>
>> interrupt handler is here to transform an edge IRQ into a level IRQ.
>>
>> Is there a way to provide a generic framework to transform edge IRQs
>> into level IRQs
>> (because some GPIO controllers do not support level IRQs, and this is
>> the case for the
>> at91sam9261 one) ?
>>
>>
>> Of cource the gpio_to_irq approach is not perfect, but at least it as
>> the benefit to quickly
>> fix the bug, and we will still be able to improve this later, when we
>> have enough tools
>> (or mechanisms) to do it.
>
> Moreover, I do not see the rationale behind the concept of "interrupt
> with an electrical value". An interrupt signals an event and this event
> can be a transition or a state but an electrical value is the
> responsibility of a GPIO line, not the other way around.
>
> I see a code that could give the value of an electrical line related to
> an interrupt as a twisted interpretation of the notion of "interrupt".
> It surprises me that it could be thought as an enhancement that an IRQ
> coming from a GPIO could give a value!
>
> Isn't it why other people are also using this simple distinction to use
> their GPIO/IRQ mechanism? Maybe this is why we are the only ones to
> completely forbid this possibility.
>
> So, let's fix the bug, submit the modification to mainline, make
> platform work and see if somebody can convince me that retrieving an
> electrical line status from a GPIO is a "bad" thing...
>

I share your view on this.

Linus,
 the root of the issue is the fact that a GPIO can't be requested
twice. But IMO it's safe for a single device to request it more than
once and use it for different purposes (irq and electrical signal
value). Maybe we can rework the GPIO request to include this ownership
information. I can post a draft implementation for this if you think
it's worthwhile.

Jean-Jacques

> Bye,
> --
> Nicolas Ferre

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Linus Walleij]

On Thu, Jan 16, 2014 at 1:02 PM, Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:

> Linus,
>  the root of the issue is the fact that a GPIO can't be requested
> twice. But IMO it's safe for a single device to request it more than
> once and use it for different purposes (irq and electrical signal
> value). Maybe we can rework the GPIO request to include this ownership
> information. I can post a draft implementation for this if you think
> it's worthwhile.

You are right and actually Jean-Christophe's patch is a good
start.

You also need to use the gpio_lock_as_irq() API as shown in
other patches to the subsystem e.g. this.
commit eb7cce1ea96b6399672abce787598f6e7a4352c3

Also check my other mail in this thread.

Yours,
Linus Walleij

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Jacques Hiblot]

2014/1/22 Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Jan 16, 2014 at 1:02 PM, Jean-Jacques Hiblot
> <jjhiblot@traphandler.com> wrote:
>
>> Linus,
>>  the root of the issue is the fact that a GPIO can't be requested
>> twice. But IMO it's safe for a single device to request it more than
>> once and use it for different purposes (irq and electrical signal
>> value). Maybe we can rework the GPIO request to include this ownership
>> information. I can post a draft implementation for this if you think
>> it's worthwhile.
>
> You are right and actually Jean-Christophe's patch is a good
> start.

Hi Linus,

I've been thinking hard on the gpio ownership and I'd like your
opinion on the following assumptions:

- a gpio used as an interrupt is always a gpio configured as an input.
(corollary : a gpio used for interrupt cannot not be an ouput)
- a gpio used as an input or interrupt only, could be safely accessed
by multiple users.
- a gpio used as an output should be used as such by only one user.
Still other users should be allowed to read its value.

If those assumptions are true, I believe they can lead to a new
exclusion scheme:
- ouput is mutually exclusive with interrupt but not with input
- only 1 ouput user at a time.

It would do away with our sharing issue and more (chained interrupt
handlers on gpio interrupt, read/interrupt access through sysfs to a
gpio requested by a driver)

And I believe that most of the infrastructure is in place to implement this :
- direction flags in gpio_request_one
- gpio_lock_as_irq/gpio_unlock_as_irq


Jean-Jacques

>
> You also need to use the gpio_lock_as_irq() API as shown in
> other patches to the subsystem e.g. this.
> commit eb7cce1ea96b6399672abce787598f6e7a4352c3
>
> Also check my other mail in this thread.
>
> Yours,
> Linus Walleij

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Linus Walleij]

On Thu, Jan 23, 2014 at 2:16 PM, Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:

> I've been thinking hard on the gpio ownership and I'd like your
> opinion on the following assumptions:
>
> - a gpio used as an interrupt is always a gpio configured as an input.
> (corollary : a gpio used for interrupt cannot not be an ouput)

That is the assumption made by the core. Until the day someone
comes up with a weird usecase...

> - a gpio used as an input or interrupt only, could be safely accessed
> by multiple users.

Multiple consumers is the term we would use. But yes.

> - a gpio used as an output should be used as such by only one user.
> Still other users should be allowed to read its value.

Sounds reasonable.

> If those assumptions are true, I believe they can lead to a new
> exclusion scheme:
> - ouput is mutually exclusive with interrupt but not with input
> - only 1 ouput user at a time.

Logical conclusion from the above yes.

> It would do away with our sharing issue and more (chained interrupt
> handlers on gpio interrupt, read/interrupt access through sysfs to a
> gpio requested by a driver)
>
> And I believe that most of the infrastructure is in place to implement this :
> - direction flags in gpio_request_one
> - gpio_lock_as_irq/gpio_unlock_as_irq

So I think what I need to see is a proposed patch, whether it will
hit a particular driver or the gpiolib core, but either would probably
be interesting.

Pls keep linux-gpio and Alexandre on CC for this discussion.

Yours,
Linus Walleij

device-tree: at91: irq and gpios: problem while requesting a gpio used as an interrupt source.

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:29 Mon 13 Jan     , Jean-Jacques Hiblot wrote:
> Hello Nicolas, Jean-Christophe,
> 
> As I was trying to enable the touchscreen on the at91sam9261ek with
> device-tree support, I ran into an issue. The touchscreen driver needs
> to know the state of the pendown gpio and also needs it as an
> interrupt source.
> 
> The problem is that when a gpio is used as an interrupt, it's
> requested by the pinctrl driver during the xlate stage, marking it
> unavaliable for the other driver.
> It looks like the at91 pinctrl driver is the only one to use
> gpio_request() in the xlate stage. Maybe we should remove this:
> 
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a7549c4..cf91a35 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -1463,14 +1463,6 @@ static int at91_gpio_irq_domain_xlate(struct
> irq_domain *d,
>         *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;
> -
Nack

the gpio is request automaticaly as the driver NEVER need to known
that is a GPIO

Best Regards,
J.
>         return 0;
>  }
> 
> Jean-Jacques
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel