Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-24 Thread Sricharan R 
Hi,
On Monday 23 September 2013 10:37 PM, Tony Lindgren wrote:
 * Javier Martinez Canillas javier.marti...@collabora.co.uk [130923 10:09]:
 On 09/23/2013 06:45 PM, Tony Lindgren wrote:
 Hmm does this still work for legacy platform data based
 drivers that are doing gpio_request() first?

 Yes it still work when booting using board files. I tested on my OMAP3 board 
 and
 it worked in both DT and legacy booting mode.
 OK great.
  
 And what's the path for clearing things for PM when free_irq()
 gets called? It seems that this would leave the GPIO bank
 enabled causing a PM regression?

 Indeed, I did set bank-mod_usage |= 1  offset so the bank is enabled if 
 the
 device goes to suspended and then resumed but I completely forget about the
 clearing path when the IRQ is freed.

 Which makes me think that we should probably maintain two usage variables, 
 one
 for GPIO and another one for IRQ and check both of them on the 
 suspend/resume pm
 functions.
 Yes that it seems that they should be treated separately.
 To understand, why cant the flag be cleared in gpio_irq_shutdown ?

Regards,
 Sricharan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-24 Thread Javier Martinez Canillas 
On 09/24/2013 09:39 AM, Sricharan R wrote:
 Hi,
 On Monday 23 September 2013 10:37 PM, Tony Lindgren wrote:
 * Javier Martinez Canillas javier.marti...@collabora.co.uk [130923 10:09]:
 On 09/23/2013 06:45 PM, Tony Lindgren wrote:
 Hmm does this still work for legacy platform data based
 drivers that are doing gpio_request() first?

 Yes it still work when booting using board files. I tested on my OMAP3 
 board and
 it worked in both DT and legacy booting mode.
 OK great.
  
 And what's the path for clearing things for PM when free_irq()
 gets called? It seems that this would leave the GPIO bank
 enabled causing a PM regression?

 Indeed, I did set bank-mod_usage |= 1  offset so the bank is enabled if 
 the
 device goes to suspended and then resumed but I completely forget about the
 clearing path when the IRQ is freed.

 Which makes me think that we should probably maintain two usage variables, 
 one
 for GPIO and another one for IRQ and check both of them on the 
 suspend/resume pm
 functions.
 Yes that it seems that they should be treated separately.
  To understand, why cant the flag be cleared in gpio_irq_shutdown ?

Hi Sricharan,

Without this patch today drivers do this:

gpio_request(gpio, foo IRQ); // bank-mod_usage |= 1  offset
gpio_direction_input(gpio);

and then request a IRQ with:

irq = gpio_to_irq(gpio);
request_irq(irq, ...);

later on its cleanup path:

free_irq(irq, dev);
gpio_free(gpio) // bank-mod_usage = ~(1  offset);

So if you clear the flag on gpio_irq_shutdown then bank module won't be enabled
after a suspend making drivers using the GPIO after freeing the IRQ to fail.

So the idea is to have something like this:

a) Drivers that request both the GPIO and IRQ

gpio_request(gpio, foo IRQ); // bank-mod_usage |= 1  offset
gpio_direction_input(gpio);

irq = gpio_to_irq(gpio);
request_irq(irq, ...); // bank-irq_usage |= 1  offset

free_irq(irq, dev); // bank-irq_usage = ~(1  offset);
gpio_free(gpio) // bank-mod_usage = ~(1  offset);

b) Drivers that just request the IRQ:

irq = gpio_to_irq(gpio);
request_irq(irq, ...); // bank-irq_usage |= 1  offset
free_irq(irq, dev); // bank-irq_usage = ~(1  offset);

So irq_usage or mod_usage is set means that the bank has to be enabled and if
both are not set means that the bank module can be disabled.

 
 Regards,
  Sricharan
 

Best regards,
Javier
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-24 Thread Sricharan R 
On Tuesday 24 September 2013 01:24 PM, Javier Martinez Canillas wrote:
 On 09/24/2013 09:39 AM, Sricharan R wrote:
 Hi,
 On Monday 23 September 2013 10:37 PM, Tony Lindgren wrote:
 * Javier Martinez Canillas javier.marti...@collabora.co.uk [130923 10:09]:
 On 09/23/2013 06:45 PM, Tony Lindgren wrote:
 Hmm does this still work for legacy platform data based
 drivers that are doing gpio_request() first?

 Yes it still work when booting using board files. I tested on my OMAP3 
 board and
 it worked in both DT and legacy booting mode.
 OK great.
  
 And what's the path for clearing things for PM when free_irq()
 gets called? It seems that this would leave the GPIO bank
 enabled causing a PM regression?

 Indeed, I did set bank-mod_usage |= 1  offset so the bank is enabled if 
 the
 device goes to suspended and then resumed but I completely forget about the
 clearing path when the IRQ is freed.

 Which makes me think that we should probably maintain two usage variables, 
 one
 for GPIO and another one for IRQ and check both of them on the 
 suspend/resume pm
 functions.
 Yes that it seems that they should be treated separately.
  To understand, why cant the flag be cleared in gpio_irq_shutdown ?
 Hi Sricharan,

 Without this patch today drivers do this:

 gpio_request(gpio, foo IRQ); // bank-mod_usage |= 1  offset
 gpio_direction_input(gpio);

 and then request a IRQ with:

 irq = gpio_to_irq(gpio);
 request_irq(irq, ...);

 later on its cleanup path:

 free_irq(irq, dev);
 gpio_free(gpio) // bank-mod_usage = ~(1  offset);

 So if you clear the flag on gpio_irq_shutdown then bank module won't be 
 enabled
 after a suspend making drivers using the GPIO after freeing the IRQ to fail.

 So the idea is to have something like this:

 a) Drivers that request both the GPIO and IRQ

 gpio_request(gpio, foo IRQ); // bank-mod_usage |= 1  offset
 gpio_direction_input(gpio);

 irq = gpio_to_irq(gpio);
 request_irq(irq, ...); // bank-irq_usage |= 1  offset

 free_irq(irq, dev); // bank-irq_usage = ~(1  offset);
 gpio_free(gpio) // bank-mod_usage = ~(1  offset);

 b) Drivers that just request the IRQ:

 irq = gpio_to_irq(gpio);
 request_irq(irq, ...); // bank-irq_usage |= 1  offset
 free_irq(irq, dev); // bank-irq_usage = ~(1  offset);

 So irq_usage or mod_usage is set means that the bank has to be enabled and if
 both are not set means that the bank module can be disabled.

 Ok get it. Thanks.

Regards,
 Sricharan
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-24 Thread Tony Lindgren 
* Javier Martinez Canillas javier.marti...@collabora.co.uk [130923 22:49]:
 On 09/23/2013 10:15 PM, Linus Walleij wrote:
  javier.marti...@collabora.co.uk wrote:
  - When a second caller calls omap_gpio_request() it should
be OK as well, but only if the flags corresponds to the
previously enabled input mode. Else it should be
disallowed.
  
  - The same should happen for _set_gpio_direction() if a pin
previously set up for IRQ gets a request to be used as
output.
  
  If this cannot be tracked in the driver, it is certainly a candidate
  for something that gpiolib should be doing. And then I'm open to
  solutions to how we can do that.
  
 
 Ok, this can be tracked in the driver, will add it when posting v2 soon.
 
  If this needs to be applied pronto to fix the regression I'm
  happy with that too, if we add a big boilerplate stating the above
  problem and that it needs to be *fixed* at some point.

In the mainline kernel, the regression is happening at least for
smsc911x using boards, so that's omap3-igep.dtsi and omap3-tobi.dts
currently. Also MMC card detection for omap4 is failing. Then
of course there are tons of boards where we don't have the .dts
files in the mainline kernel yet.

So maybe let's do a minimal fix for the -rc cycle first?

Then the extra checks can be queued for the next merge window
if it gets too intrusive.

  But in either case I want this to be tested on OMAP1 before
  I apply it, as in a Tested-by tag.
 
 
 Agreed. Even though this is a fix for a long standing issue I prefer it to be
 tested extensively than applying the patch in a rush just to learn that causes
 regressions and have to be reverted as it happens last time.
 
 So as you said let's wait until we have a few Tested-by tags by people using
 different OMAP platforms specially OMAP1.

Yup.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-23 Thread Stephen Warren 
On 09/22/2013 08:40 AM, Javier Martinez Canillas wrote:
 To use a GPIO pin as an interrupt line, two previous configurations
 have to be made:
 
 a) Map the GPIO pin as an interrupt line into the Linux irq space
 b) Enable the GPIO bank and configure the GPIO direction as input
 
 Most GPIO/IRQ chip drivers just create a mapping for every single
 GPIO pin with irq_create_mapping() on .probe so users usually can
 assume a) and only have to do b) by using the following sequence:
 
 gpio_request(gpio, foo IRQ);
 gpio_direction_input(gpio);
 
 and then request a IRQ with:
 
 irq = gpio_to_irq(gpio);
 request_irq(irq, ...);
 
 Some drivers know that their IRQ line is being driven by a GPIO
 and use a similar sequence as the described above but others are
 not aware or don't care wether their IRQ is a real line from an
 interrupt controller or a GPIO pin acting as an IRQ.
 ...

I think that explanation is a bit like retro-actively implying that
drivers /should/ be aware of whether their IRQ is a GPIO or not, and
should be acting differently. However, they should not.

I would much rather see a simpler patch description along the lines of:

The OMAP GPIO controller HW requires that a pin be configured in GPIO
mode in order to operate as an interrupt input. Since drivers should not
be aware of whether an interrupt pin is also a GPIO or not, the HW
should be fully configured/enabled as an IRQ if a driver solely uses IRQ
APIs such as request_irq, and never calls any GPIO-related APIs. As
such, add the missing HW setup to the OMAP GPIO controller's irq_chip
driver.

The code change looks like it does what I would expect though.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-23 Thread Tony Lindgren 
* Javier Martinez Canillas javier.marti...@collabora.co.uk [130922 07:49]:
 --- a/drivers/gpio/gpio-omap.c
 +++ b/drivers/gpio/gpio-omap.c
 @@ -420,6 +420,29 @@ static int _set_gpio_triggering(struct gpio_bank *bank, 
 int gpio,
   return 0;
  }
  
 +static void _set_gpio_mode(struct gpio_bank *bank, unsigned offset)
 +{
 + if (bank-regs-pinctrl) {
 + void __iomem *reg = bank-base + bank-regs-pinctrl;
 +
 + /* Claim the pin for MPU */
 + __raw_writel(__raw_readl(reg) | (1  offset), reg);
 + }
 +
 + if (bank-regs-ctrl  !bank-mod_usage) {
 + void __iomem *reg = bank-base + bank-regs-ctrl;
 + u32 ctrl;
 +
 + ctrl = __raw_readl(reg);
 + /* Module is enabled, clocks are not gated */
 + ctrl = ~GPIO_MOD_CTRL_BIT;
 + __raw_writel(ctrl, reg);
 + bank-context.ctrl = ctrl;
 + }
 +
 + bank-mod_usage |= 1  offset;
 +}
 +
  static int gpio_irq_type(struct irq_data *d, unsigned type)
  {
   struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
 @@ -427,8 +450,8 @@ static int gpio_irq_type(struct irq_data *d, unsigned 
 type)
   int retval;
   unsigned long flags;
  
 - if (WARN_ON(!bank-mod_usage))
 - return -EINVAL;
 + if (!bank-mod_usage)
 + pm_runtime_get_sync(bank-dev);
  
  #ifdef CONFIG_ARCH_OMAP1
   if (d-irq  IH_MPUIO_BASE)
 @@ -438,6 +461,11 @@ static int gpio_irq_type(struct irq_data *d, unsigned 
 type)
   if (!gpio)
   gpio = irq_to_gpio(bank, d-hwirq);
  
 + spin_lock_irqsave(bank-lock, flags);
 + _set_gpio_mode(bank, GPIO_INDEX(bank, gpio));
 + _set_gpio_direction(bank, GPIO_INDEX(bank, gpio), 1);
 + spin_unlock_irqrestore(bank-lock, flags);
 +
   if (type  ~IRQ_TYPE_SENSE_MASK)
   return -EINVAL;
  

Hmm does this still work for legacy platform data based
drivers that are doing gpio_request() first?

And what's the path for clearing things for PM when free_irq()
gets called? It seems that this would leave the GPIO bank
enabled causing a PM regression?

Other than the two concerns above it seems that this might
be the way to go to fix the regression for the -rc cycle.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-23 Thread Javier Martinez Canillas 
On 09/23/2013 06:14 PM, Stephen Warren wrote:
 On 09/22/2013 08:40 AM, Javier Martinez Canillas wrote:
 To use a GPIO pin as an interrupt line, two previous configurations
 have to be made:
 
 a) Map the GPIO pin as an interrupt line into the Linux irq space
 b) Enable the GPIO bank and configure the GPIO direction as input
 
 Most GPIO/IRQ chip drivers just create a mapping for every single
 GPIO pin with irq_create_mapping() on .probe so users usually can
 assume a) and only have to do b) by using the following sequence:
 
 gpio_request(gpio, foo IRQ);
 gpio_direction_input(gpio);
 
 and then request a IRQ with:
 
 irq = gpio_to_irq(gpio);
 request_irq(irq, ...);
 
 Some drivers know that their IRQ line is being driven by a GPIO
 and use a similar sequence as the described above but others are
 not aware or don't care wether their IRQ is a real line from an
 interrupt controller or a GPIO pin acting as an IRQ.
 ...
 
 I think that explanation is a bit like retro-actively implying that
 drivers /should/ be aware of whether their IRQ is a GPIO or not, and
 should be acting differently. However, they should not.


I know the patch description is rather verbose but since we have been discussing
this a lot and people have different opinions I wanted to explain some context
and the motivation for the patch.

 I would much rather see a simpler patch description along the lines of:
 
 The OMAP GPIO controller HW requires that a pin be configured in GPIO
 mode in order to operate as an interrupt input. Since drivers should not
 be aware of whether an interrupt pin is also a GPIO or not, the HW
 should be fully configured/enabled as an IRQ if a driver solely uses IRQ
 APIs such as request_irq, and never calls any GPIO-related APIs. As
 such, add the missing HW setup to the OMAP GPIO controller's irq_chip
 driver.
 

Thanks for the suggestion, I'll use something like that when I do a proper post
as a PATCH and not RFC.

 The code change looks like it does what I would expect though.
 

Great, let's see what is the feedback from Santosh and Kevin about the
implementation since they are the maintainers of this driver.

I really hope we can find a solution to this long standing issue.

Thanks a lot and best regards,
Javier
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-23 Thread Javier Martinez Canillas 
On 09/23/2013 06:45 PM, Tony Lindgren wrote:
 * Javier Martinez Canillas javier.marti...@collabora.co.uk [130922 07:49]:
 --- a/drivers/gpio/gpio-omap.c
 +++ b/drivers/gpio/gpio-omap.c
 @@ -420,6 +420,29 @@ static int _set_gpio_triggering(struct gpio_bank *bank, 
 int gpio,
  return 0;
  }
  
 +static void _set_gpio_mode(struct gpio_bank *bank, unsigned offset)
 +{
 +if (bank-regs-pinctrl) {
 +void __iomem *reg = bank-base + bank-regs-pinctrl;
 +
 +/* Claim the pin for MPU */
 +__raw_writel(__raw_readl(reg) | (1  offset), reg);
 +}
 +
 +if (bank-regs-ctrl  !bank-mod_usage) {
 +void __iomem *reg = bank-base + bank-regs-ctrl;
 +u32 ctrl;
 +
 +ctrl = __raw_readl(reg);
 +/* Module is enabled, clocks are not gated */
 +ctrl = ~GPIO_MOD_CTRL_BIT;
 +__raw_writel(ctrl, reg);
 +bank-context.ctrl = ctrl;
 +}
 +
 +bank-mod_usage |= 1  offset;
 +}
 +
  static int gpio_irq_type(struct irq_data *d, unsigned type)
  {
  struct gpio_bank *bank = irq_data_get_irq_chip_data(d);
 @@ -427,8 +450,8 @@ static int gpio_irq_type(struct irq_data *d, unsigned 
 type)
  int retval;
  unsigned long flags;
  
 -if (WARN_ON(!bank-mod_usage))
 -return -EINVAL;
 +if (!bank-mod_usage)
 +pm_runtime_get_sync(bank-dev);
  
  #ifdef CONFIG_ARCH_OMAP1
  if (d-irq  IH_MPUIO_BASE)
 @@ -438,6 +461,11 @@ static int gpio_irq_type(struct irq_data *d, unsigned 
 type)
  if (!gpio)
  gpio = irq_to_gpio(bank, d-hwirq);
  
 +spin_lock_irqsave(bank-lock, flags);
 +_set_gpio_mode(bank, GPIO_INDEX(bank, gpio));
 +_set_gpio_direction(bank, GPIO_INDEX(bank, gpio), 1);
 +spin_unlock_irqrestore(bank-lock, flags);
 +
  if (type  ~IRQ_TYPE_SENSE_MASK)
  return -EINVAL;


Hi Tony, thanks a lot for your feedback.

 
 Hmm does this still work for legacy platform data based
 drivers that are doing gpio_request() first?
 

Yes it still work when booting using board files. I tested on my OMAP3 board and
it worked in both DT and legacy booting mode.

 And what's the path for clearing things for PM when free_irq()
 gets called? It seems that this would leave the GPIO bank
 enabled causing a PM regression?
 

Indeed, I did set bank-mod_usage |= 1  offset so the bank is enabled if the
device goes to suspended and then resumed but I completely forget about the
clearing path when the IRQ is freed.

Which makes me think that we should probably maintain two usage variables, one
for GPIO and another one for IRQ and check both of them on the suspend/resume pm
functions.

 Other than the two concerns above it seems that this might
 be the way to go to fix the regression for the -rc cycle.
 
 Regards,
 
 Tony
 

Great, I'll do these changes when posting as a proper PATCH.

Thanks a lot and best regards,
Javier
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-23 Thread Tony Lindgren 
* Javier Martinez Canillas javier.marti...@collabora.co.uk [130923 10:09]:
 On 09/23/2013 06:45 PM, Tony Lindgren wrote:
  
  Hmm does this still work for legacy platform data based
  drivers that are doing gpio_request() first?
  
 
 Yes it still work when booting using board files. I tested on my OMAP3 board 
 and
 it worked in both DT and legacy booting mode.

OK great.
 
  And what's the path for clearing things for PM when free_irq()
  gets called? It seems that this would leave the GPIO bank
  enabled causing a PM regression?
  
 
 Indeed, I did set bank-mod_usage |= 1  offset so the bank is enabled if the
 device goes to suspended and then resumed but I completely forget about the
 clearing path when the IRQ is freed.
 
 Which makes me think that we should probably maintain two usage variables, one
 for GPIO and another one for IRQ and check both of them on the suspend/resume 
 pm
 functions.

Yes that it seems that they should be treated separately.

Regards,

Tony
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-23 Thread Santosh Shilimkar 
Javier,

On Monday 23 September 2013 01:07 PM, Tony Lindgren wrote:
 * Javier Martinez Canillas javier.marti...@collabora.co.uk [130923 10:09]:
 On 09/23/2013 06:45 PM, Tony Lindgren wrote:

 Hmm does this still work for legacy platform data based
 drivers that are doing gpio_request() first?


 Yes it still work when booting using board files. I tested on my OMAP3 board 
 and
 it worked in both DT and legacy booting mode.
 
 OK great.
  
 And what's the path for clearing things for PM when free_irq()
 gets called? It seems that this would leave the GPIO bank
 enabled causing a PM regression?


 Indeed, I did set bank-mod_usage |= 1  offset so the bank is enabled if 
 the
 device goes to suspended and then resumed but I completely forget about the
 clearing path when the IRQ is freed.

 Which makes me think that we should probably maintain two usage variables, 
 one
 for GPIO and another one for IRQ and check both of them on the 
 suspend/resume pm
 functions.
 
 Yes that it seems that they should be treated separately.
 
As discussed on IRC, the patch as such is fine after the mentioned fixup,
I would like to hear back if Linus W/Grant is fine with the approach. Not sure
if I missed the discussion, but the proposed patch is deviation from
traditional method of doing gpio_request() first up to perform other
gpio operations.

Regards,
Santosh


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-23 Thread Linus Walleij 
On Sun, Sep 22, 2013 at 4:40 PM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:

 To use a GPIO pin as an interrupt line, two previous configurations
 have to be made:

 a) Map the GPIO pin as an interrupt line into the Linux irq space
 b) Enable the GPIO bank and configure the GPIO direction as input

 Most GPIO/IRQ chip drivers just create a mapping for every single
 GPIO pin with irq_create_mapping() on .probe so users usually can
 assume a) and only have to do b) by using the following sequence:

 gpio_request(gpio, foo IRQ);
 gpio_direction_input(gpio);

 and then request a IRQ with:

 irq = gpio_to_irq(gpio);
 request_irq(irq, ...);

I guess I have to live with this approach, but I'd like to - if possible -
address my pet issue.

- It is OK that the HW get set up as GPIO input by the IRQ
  request function alone. (Through gpio_irq_type I guess).

- When a second caller calls omap_gpio_request() it should
  be OK as well, but only if the flags corresponds to the
  previously enabled input mode. Else it should be
  disallowed.

- The same should happen for _set_gpio_direction() if a pin
  previously set up for IRQ gets a request to be used as
  output.

If this cannot be tracked in the driver, it is certainly a candidate
for something that gpiolib should be doing. And then I'm open to
solutions to how we can do that.

If this needs to be applied pronto to fix the regression I'm
happy with that too, if we add a big boilerplate stating the above
problem and that it needs to be *fixed* at some point.

But in either case I want this to be tested on OMAP1 before
I apply it, as in a Tested-by tag.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] gpio/omap: auto-setup a GPIO when used as an IRQ

2013-09-23 Thread Javier Martinez Canillas 
On 09/23/2013 10:15 PM, Linus Walleij wrote:
 On Sun, Sep 22, 2013 at 4:40 PM, Javier Martinez Canillas
 javier.marti...@collabora.co.uk wrote:
 
 To use a GPIO pin as an interrupt line, two previous configurations
 have to be made:

 a) Map the GPIO pin as an interrupt line into the Linux irq space
 b) Enable the GPIO bank and configure the GPIO direction as input

 Most GPIO/IRQ chip drivers just create a mapping for every single
 GPIO pin with irq_create_mapping() on .probe so users usually can
 assume a) and only have to do b) by using the following sequence:

 gpio_request(gpio, foo IRQ);
 gpio_direction_input(gpio);

 and then request a IRQ with:

 irq = gpio_to_irq(gpio);
 request_irq(irq, ...);
 
 I guess I have to live with this approach, but I'd like to - if possible -
 address my pet issue.
 
 - It is OK that the HW get set up as GPIO input by the IRQ
   request function alone. (Through gpio_irq_type I guess).
 

Yes, this is how is made on this patch indeed.

 - When a second caller calls omap_gpio_request() it should
   be OK as well, but only if the flags corresponds to the
   previously enabled input mode. Else it should be
   disallowed.
 
 - The same should happen for _set_gpio_direction() if a pin
   previously set up for IRQ gets a request to be used as
   output.
 
 If this cannot be tracked in the driver, it is certainly a candidate
 for something that gpiolib should be doing. And then I'm open to
 solutions to how we can do that.
 

Ok, this can be tracked in the driver, will add it when posting v2 soon.

 If this needs to be applied pronto to fix the regression I'm
 happy with that too, if we add a big boilerplate stating the above
 problem and that it needs to be *fixed* at some point.
 
 But in either case I want this to be tested on OMAP1 before
 I apply it, as in a Tested-by tag.


Agreed. Even though this is a fix for a long standing issue I prefer it to be
tested extensively than applying the patch in a rush just to learn that causes
regressions and have to be reverted as it happens last time.

So as you said let's wait until we have a few Tested-by tags by people using
different OMAP platforms specially OMAP1.

 Yours,
 Linus Walleij
 

Thanks a lot and best regards,
Javier

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html