* [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled
@ 2010-09-15 20:12 Anatolij Gustschin
2010-09-16 2:38 ` Grant Likely
2010-10-14 14:55 ` Anatolij Gustschin
0 siblings, 2 replies; 6+ messages in thread
From: Anatolij Gustschin @ 2010-09-15 20:12 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anatolij Gustschin
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 <agust@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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled
2010-09-15 20:12 [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled Anatolij Gustschin
@ 2010-09-16 2:38 ` Grant Likely
2010-09-25 20:22 ` Anatolij Gustschin
2010-10-14 14:55 ` Anatolij Gustschin
1 sibling, 1 reply; 6+ messages in thread
From: Grant Likely @ 2010-09-16 2:38 UTC (permalink / raw)
To: Anatolij Gustschin; +Cc: linuxppc-dev
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 <agust@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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled
2010-09-16 2:38 ` Grant Likely
@ 2010-09-25 20:22 ` Anatolij Gustschin
2010-10-27 13:40 ` Grant Likely
0 siblings, 1 reply; 6+ messages in thread
From: Anatolij Gustschin @ 2010-09-25 20:22 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
On Wed, 15 Sep 2010 20:38:23 -0600
Grant Likely <grant.likely@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled
2010-09-25 20:22 ` Anatolij Gustschin
@ 2010-10-27 13:40 ` Grant Likely
0 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2010-10-27 13:40 UTC (permalink / raw)
To: Anatolij Gustschin; +Cc: linuxppc-dev
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.likely@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled
2010-09-15 20:12 [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled Anatolij Gustschin
2010-09-16 2:38 ` Grant Likely
@ 2010-10-14 14:55 ` Anatolij Gustschin
2010-10-16 3:52 ` Grant Likely
1 sibling, 1 reply; 6+ messages in thread
From: Anatolij Gustschin @ 2010-10-14 14:55 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
Hi Grant,
On Wed, 15 Sep 2010 22:12:57 +0200
Anatolij Gustschin <agust@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 <agust@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled
2010-10-14 14:55 ` Anatolij Gustschin
@ 2010-10-16 3:52 ` Grant Likely
0 siblings, 0 replies; 6+ messages in thread
From: Grant Likely @ 2010-10-16 3:52 UTC (permalink / raw)
To: Anatolij Gustschin; +Cc: linuxppc-dev
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 <agust@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 <agust@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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-27 13:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-15 20:12 [PATCH] powerpc/5121: pdm360ng: fix touch irq if 8xxx gpio driver is enabled Anatolij Gustschin
2010-09-16 2:38 ` Grant Likely
2010-09-25 20:22 ` Anatolij Gustschin
2010-10-27 13:40 ` Grant Likely
2010-10-14 14:55 ` Anatolij Gustschin
2010-10-16 3:52 ` Grant Likely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).