Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled

2010-10-27 Thread Grant Likely 
On Sat, Sep 25, 2010 at 10:22:44PM +0200, Anatolij Gustschin wrote:
 On Wed, 15 Sep 2010 20:38:23 -0600
 Grant Likely grant.lik...@secretlab.ca wrote:
 
  On Wed, Sep 15, 2010 at 10:12:57PM +0200, Anatolij Gustschin wrote:
   Enabling the MPC8xxx GPIO driver with MPC512x GPIO extension
   breaks touch screen support on this board since the GPIO
   interrupt will be mapped to 8xxx GPIO irq host resulting in
   a not requestable interrupt in the touch screen driver. Fix
   it by mapping the touch interrupt on 8xxx GPIO irq host.
  
  This looks wrong to me.  The touchscreen code should not go mucking
  about in the GPIO controller registers; that is the job of the gpio
  driver.
 
 But if there is no GPIO driver (as it was the case before adding
 mpc512x support in the 8xxx gpio driver) or if the driver is not
 enabled in the kernel configuration? Then the platform specific
 callback (called from touchscreen driver) returns the pin state
 and acknowlegdes the interrupt.

So, basically the touchscreen device node has an interrupts property
which does not use the gpio controller as the interrupt controller,
but instead points directly and the interrupt controller that the gpio
controller is cascaded from.

Really it sounds like the device tree data is broken.  The preferred
solution is be to fix the device tree to declare the gpio node as
an interrupt controller.

 
   What is the reason that the touch interrupt isn't
  requestable?
 
 The 8xxx gpio driver sets up gpio irq host and installs
 the chained irq handler for GPIO interrupt 78 using
 set_irq_chained_handler() which sets the status field of
 the irq_desc structure to IRQ_NOREQUEST | IRQ_NOPROBE.
 Other drivers can't request this GPIO interrupt any more,
 request_threaded_irq() checks the IRQ_NOREQUEST status
 flag and returns -EINVAL if it is set. The gpio interrupts
 for each gpio pin are now handled by the
 mpc8xxx_gpio_irq_cascade() handler as they should.
 
   It looks like the 8xxx gpio driver is designed to hand
  out a separate virq number for each gpio pin (I've not had time to dig
  into details, so you'll need to educate me on the problem details)
 
 Yes, exactly. This patch adds code to request the
 board's pen_down gpio pin and to use it's virq number in
 the touchscreen driver. The touchscreen driver can
 request this virq interrupt and it is now properly handled
 by the chained handler in the gpio driver.

...but it does so by hard coding the irq number (via the GPIO number)
into the board code; a situation which I've tried very hard to avoid.
This is what I'm not okay with.

Another solution is to modify the 8xxx gpio driver to cascade off the
normal request_irq() path instead of via set_irq_chained_handler(),
but that *might* have unacceptable performance impact for other users.

Unfortunately, I've been slow on this patch, so I cannot get anything
into 2.6.37 (sorry).  However, I've not asked Linus to pull the 8xxx
gpio driver changes either so nothing in mainline will get broken.

g.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled

2010-10-15 Thread Grant Likely 
On Thu, Oct 14, 2010 at 04:55:49PM +0200, Anatolij Gustschin wrote:
 Hi Grant,
 
 On Wed, 15 Sep 2010 22:12:57 +0200
 Anatolij Gustschin ag...@denx.de wrote:
 
  Enabling the MPC8xxx GPIO driver with MPC512x GPIO extension
  breaks touch screen support on this board since the GPIO
  interrupt will be mapped to 8xxx GPIO irq host resulting in
  a not requestable interrupt in the touch screen driver. Fix
  it by mapping the touch interrupt on 8xxx GPIO irq host.
  
  Signed-off-by: Anatolij Gustschin ag...@denx.de
  ---
   arch/powerpc/platforms/512x/pdm360ng.c |   26 ++
   1 files changed, 22 insertions(+), 4 deletions(-)
 
 Can this patch go in for 2.6.37 ?

I hope so, but I haven't looked at it properly yet and I'm trying to
dig myself out of a patch backlog.

g.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled

2010-10-14 Thread Anatolij Gustschin 
Hi Grant,

On Wed, 15 Sep 2010 22:12:57 +0200
Anatolij Gustschin ag...@denx.de wrote:

 Enabling the MPC8xxx GPIO driver with MPC512x GPIO extension
 breaks touch screen support on this board since the GPIO
 interrupt will be mapped to 8xxx GPIO irq host resulting in
 a not requestable interrupt in the touch screen driver. Fix
 it by mapping the touch interrupt on 8xxx GPIO irq host.
 
 Signed-off-by: Anatolij Gustschin ag...@denx.de
 ---
  arch/powerpc/platforms/512x/pdm360ng.c |   26 ++
  1 files changed, 22 insertions(+), 4 deletions(-)

Can this patch go in for 2.6.37 ?

Thanks,
Anatolij

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled

2010-09-25 Thread Anatolij Gustschin 
On Wed, 15 Sep 2010 20:38:23 -0600
Grant Likely grant.lik...@secretlab.ca wrote:

 On Wed, Sep 15, 2010 at 10:12:57PM +0200, Anatolij Gustschin wrote:
  Enabling the MPC8xxx GPIO driver with MPC512x GPIO extension
  breaks touch screen support on this board since the GPIO
  interrupt will be mapped to 8xxx GPIO irq host resulting in
  a not requestable interrupt in the touch screen driver. Fix
  it by mapping the touch interrupt on 8xxx GPIO irq host.
 
 This looks wrong to me.  The touchscreen code should not go mucking
 about in the GPIO controller registers; that is the job of the gpio
 driver.

But if there is no GPIO driver (as it was the case before adding
mpc512x support in the 8xxx gpio driver) or if the driver is not
enabled in the kernel configuration? Then the platform specific
callback (called from touchscreen driver) returns the pin state
and acknowlegdes the interrupt.

  What is the reason that the touch interrupt isn't
 requestable?

The 8xxx gpio driver sets up gpio irq host and installs
the chained irq handler for GPIO interrupt 78 using
set_irq_chained_handler() which sets the status field of
the irq_desc structure to IRQ_NOREQUEST | IRQ_NOPROBE.
Other drivers can't request this GPIO interrupt any more,
request_threaded_irq() checks the IRQ_NOREQUEST status
flag and returns -EINVAL if it is set. The gpio interrupts
for each gpio pin are now handled by the
mpc8xxx_gpio_irq_cascade() handler as they should.

  It looks like the 8xxx gpio driver is designed to hand
 out a separate virq number for each gpio pin (I've not had time to dig
 into details, so you'll need to educate me on the problem details)

Yes, exactly. This patch adds code to request the
board's pen_down gpio pin and to use it's virq number in
the touchscreen driver. The touchscreen driver can
request this virq interrupt and it is now properly handled
by the chained handler in the gpio driver.

Anatolij
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled

2010-09-15 Thread Grant Likely 
On Wed, Sep 15, 2010 at 10:12:57PM +0200, Anatolij Gustschin wrote:
 Enabling the MPC8xxx GPIO driver with MPC512x GPIO extension
 breaks touch screen support on this board since the GPIO
 interrupt will be mapped to 8xxx GPIO irq host resulting in
 a not requestable interrupt in the touch screen driver. Fix
 it by mapping the touch interrupt on 8xxx GPIO irq host.

This looks wrong to me.  The touchscreen code should not go mucking
about in the GPIO controller registers; that is the job of the gpio
driver.  What is the reason that the touch interrupt isn't
requestable?  It looks like the 8xxx gpio driver is designed to hand
out a separate virq number for each gpio pin (I've not had time to dig
into details, so you'll need to educate me on the problem details)

g.

 
 Signed-off-by: Anatolij Gustschin ag...@denx.de
 ---
  arch/powerpc/platforms/512x/pdm360ng.c |   26 ++
  1 files changed, 22 insertions(+), 4 deletions(-)
 
 diff --git a/arch/powerpc/platforms/512x/pdm360ng.c 
 b/arch/powerpc/platforms/512x/pdm360ng.c
 index 0575e85..558eb9e 100644
 --- a/arch/powerpc/platforms/512x/pdm360ng.c
 +++ b/arch/powerpc/platforms/512x/pdm360ng.c
 @@ -27,6 +27,7 @@
  #include linux/spi/ads7846.h
  #include linux/spi/spi.h
  #include linux/notifier.h
 +#include asm/gpio.h
  
  static void *pdm360ng_gpio_base;
  
 @@ -50,7 +51,7 @@ static struct ads7846_platform_data pdm360ng_ads7846_pdata 
 = {
   .irq_flags  = IRQF_TRIGGER_LOW,
  };
  
 -static int __init pdm360ng_penirq_init(void)
 +static int pdm360ng_penirq_init(void)
  {
   struct device_node *np;
  
 @@ -73,6 +74,9 @@ static int __init pdm360ng_penirq_init(void)
   return 0;
  }
  
 +#define GPIO_NR(x)   (ARCH_NR_GPIOS - 32 + (x))
 +#define PENDOWN_GPIO GPIO_NR(25)
 +
  static int pdm360ng_touchscreen_notifier_call(struct notifier_block *nb,
   unsigned long event, void *__dev)
  {
 @@ -80,7 +84,24 @@ static int pdm360ng_touchscreen_notifier_call(struct 
 notifier_block *nb,
  
   if ((event == BUS_NOTIFY_ADD_DEVICE) 
   of_device_is_compatible(dev-of_node, ti,ads7846)) {
 + struct spi_device *spi = to_spi_device(dev);
 + int gpio = PENDOWN_GPIO;
 +
   dev-platform_data = pdm360ng_ads7846_pdata;
 + if (pdm360ng_penirq_init())
 + return NOTIFY_DONE;
 +
 + if (gpio_request_one(gpio, GPIOF_IN, ads7845_pen_down)  0) {
 + pr_err(Failed to request GPIO %d for 
 + ads7845 pen down IRQ\n, gpio);
 + return NOTIFY_DONE;
 + }
 + spi-irq = gpio_to_irq(gpio);
 + if (spi-irq  0) {
 + pr_err(Can't map GPIO IRQ\n);
 + gpio_free(gpio);
 + return NOTIFY_DONE;
 + }
   return NOTIFY_OK;
   }
   return NOTIFY_DONE;
 @@ -92,9 +113,6 @@ static struct notifier_block pdm360ng_touchscreen_nb = {
  
  static void __init pdm360ng_touchscreen_init(void)
  {
 - if (pdm360ng_penirq_init())
 - return;
 -
   bus_register_notifier(spi_bus_type, pdm360ng_touchscreen_nb);
  }
  #else
 -- 
 1.7.0.4
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev