[PATCH 0/2] omap-keypad fixes for OMAP2

[PATCH 0/2] omap-keypad fixes for OMAP2

[Peter Ujfalusi]

Fixes for the omap-keypad driver on OMAP2

Peter Ujfalusi (2)
  OMAP2: omap-keypad: interrupt disable fix
  OMAP2: omap-keypad: Enable more than 6 rows

 drivers/input/keyboard/omap-keypad.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

[PATCH 1/2] OMAP2: omap-keypad: interrupt disable fix

[Peter Ujfalusi]

The GPIO interrupts has been disabled several times
after the first key press.
No need to disable - again - the interrupts in the omap_kp_scan_keypad
function on OMAP2.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 drivers/input/keyboard/omap-keypad.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/omap-keypad.c b/drivers/input/keyboard/omap-keypad.c
index d57d2cb..d6cd79b 100644
--- a/drivers/input/keyboard/omap-keypad.c
+++ b/drivers/input/keyboard/omap-keypad.c
@@ -134,10 +134,6 @@ static void omap_kp_scan_keypad(struct omap_kp *omap_kp, unsigned char *state)
 
 	/* read the keypad status */
 	if (cpu_is_omap24xx()) {
-		int i;
-		for (i = 0; i < omap_kp->rows; i++)
-			disable_irq(OMAP_GPIO_IRQ(row_gpios[i]));
-
 		/* read the keypad status */
 		for (col = 0; col < omap_kp->cols; col++) {
 			set_col_gpio_val(omap_kp, ~(1 << col));
-- 
1.5.3.7

[PATCH 2/2] OMAP2: omap-keypad: Enable more than 6 rows

[Peter Ujfalusi]

There is no reason to limit the GPIO rows to 6 for
OMAP2.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@nokia.com>
---
 drivers/input/keyboard/omap-keypad.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/input/keyboard/omap-keypad.c b/drivers/input/keyboard/omap-keypad.c
index d6cd79b..0f95046 100644
--- a/drivers/input/keyboard/omap-keypad.c
+++ b/drivers/input/keyboard/omap-keypad.c
@@ -137,7 +137,7 @@ static void omap_kp_scan_keypad(struct omap_kp *omap_kp, unsigned char *state)
 		/* read the keypad status */
 		for (col = 0; col < omap_kp->cols; col++) {
 			set_col_gpio_val(omap_kp, ~(1 << col));
-			state[col] = ~(get_row_gpio_val(omap_kp)) & 0x3f;
+			state[col] = ~(get_row_gpio_val(omap_kp)) & 0xff;
 		}
 		set_col_gpio_val(omap_kp, 0);
 
-- 
1.5.3.7

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi@nokia.com> [081002 08:29]:
> Fixes for the omap-keypad driver on OMAP2
> 
> Peter Ujfalusi (2)
>   OMAP2: omap-keypad: interrupt disable fix
>   OMAP2: omap-keypad: Enable more than 6 rows
> 
>  drivers/input/keyboard/omap-keypad.c |    6 +-----
>  1 files changed, 1 insertions(+), 5 deletions(-)

Looks good to me, here's my ack:

Acked-by: Tony Lindgren <tony@atomide.com>

Can you please send your series to linux-input@vger.kernel.org
for inclusion into the mainline kernel? Also cc linux-omap list.

You might also want to also include other pending patches sitting in
linux-omap into your series so we get this driver into sync with
mainline too.

Thanks,

Tony

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Peter Ujfalusi]

Hello,

the diff between l-o and mainline are these commits regarding to omap-keypad.c:

commit fdee8764947cde1e6933e7d981ce5b9de00e83e6
Author: Tony Lindgren <tony@atomide.com>
Merge current mainline tree into linux-omap tree

commit ed39c59ff267249e7e789ea3bb64afc1e9afb5bd
Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
omap: fix a load of "warning: symbol 'xxx' was not declared. Should it be static?"

commit e85a9daa48453a4e7ffedf174b4358cdb444fbbb
Author: David Brownell <dbrownell@users.sourceforge.net>
some drivers switch away from OMAP-specific GPIO calls

Should I take the relevant parts from these commits and send it to linux-input?

Also there are other things broken in the omap-keypad driver.
One thing came to my mind is in the omap_kp_enable_store: 
No matter what platform the code is running it tries to
enable/disable IRQ number 33 (IH2_BASE + 1), which is not a nice thing
to do...

Péter


On Thursday 02 October 2008 09:12:39 ext Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi@nokia.com> [081002 08:29]:
> > Fixes for the omap-keypad driver on OMAP2
> > 
> > Peter Ujfalusi (2)
> >   OMAP2: omap-keypad: interrupt disable fix
> >   OMAP2: omap-keypad: Enable more than 6 rows
> > 
> >  drivers/input/keyboard/omap-keypad.c |    6 +-----
> >  1 files changed, 1 insertions(+), 5 deletions(-)
> 
> Looks good to me, here's my ack:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>
> 
> Can you please send your series to linux-input@vger.kernel.org
> for inclusion into the mainline kernel? Also cc linux-omap list.
> 
> You might also want to also include other pending patches sitting in
> linux-omap into your series so we get this driver into sync with
> mainline too.
> 
> Thanks,
> 
> Tony
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Felipe Balbi]

On Thu, Oct 02, 2008 at 04:01:17PM +0300, ext Peter Ujfalusi wrote:
> 
> Hello,
> 
> the diff between l-o and mainline are these commits regarding to omap-keypad.c:
> 
> commit fdee8764947cde1e6933e7d981ce5b9de00e83e6
> Author: Tony Lindgren <tony@atomide.com>
> Merge current mainline tree into linux-omap tree
> 
> commit ed39c59ff267249e7e789ea3bb64afc1e9afb5bd
> Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
> omap: fix a load of "warning: symbol 'xxx' was not declared. Should it be static?"
> 
> commit e85a9daa48453a4e7ffedf174b4358cdb444fbbb
> Author: David Brownell <dbrownell@users.sourceforge.net>
> some drivers switch away from OMAP-specific GPIO calls
> 
> Should I take the relevant parts from these commits and send it to linux-input?
> 
> Also there are other things broken in the omap-keypad driver.
> One thing came to my mind is in the omap_kp_enable_store: 
> No matter what platform the code is running it tries to
> enable/disable IRQ number 33 (IH2_BASE + 1), which is not a nice thing
> to do...

you can pass the irq number via a struct resource and use
platform_get_irq() on driver probe and exit calls to handle the
request_irq() and free_irq();

Then you're sure the irq is correct as long as the struct resource as
well initialized.


-- 
balbi

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Peter Ujfalusi]

On Thursday 02 October 2008 16:06:41 Felipe Balbi wrote:
> On Thu, Oct 02, 2008 at 04:01:17PM +0300, ext Peter Ujfalusi wrote:
> > 
> > Hello,
> > 
> > the diff between l-o and mainline are these commits regarding to omap-keypad.c:
> > 
> > commit fdee8764947cde1e6933e7d981ce5b9de00e83e6
> > Author: Tony Lindgren <tony@atomide.com>
> > Merge current mainline tree into linux-omap tree
> > 
> > commit ed39c59ff267249e7e789ea3bb64afc1e9afb5bd
> > Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
> > omap: fix a load of "warning: symbol 'xxx' was not declared. Should it be static?"
> > 
> > commit e85a9daa48453a4e7ffedf174b4358cdb444fbbb
> > Author: David Brownell <dbrownell@users.sourceforge.net>
> > some drivers switch away from OMAP-specific GPIO calls
> > 
> > Should I take the relevant parts from these commits and send it to linux-input?
> > 
> > Also there are other things broken in the omap-keypad driver.
> > One thing came to my mind is in the omap_kp_enable_store: 
> > No matter what platform the code is running it tries to
> > enable/disable IRQ number 33 (IH2_BASE + 1), which is not a nice thing
> > to do...
> 
> you can pass the irq number via a struct resource and use
> platform_get_irq() on driver probe and exit calls to handle the
> request_irq() and free_irq();
> 
> Then you're sure the irq is correct as long as the struct resource as
> well initialized.

Yes, I know that, I just mentioned that other things are still broken in the omap-keypad driver.
In the OMAP1 board files the resource is filled in, but in the driver you can see this:

static ssize_t omap_kp_enable_store(struct device *dev, struct device_attribute *attr,
				    const char *buf, size_t count)
.
.
	if (state != kp_enable) {
		if (state)
			enable_irq(INT_KEYBOARD);
		else
			disable_irq(INT_KEYBOARD);
		kp_enable = state;
	}
.
.

So, pretty much not a sane thing to do...
It is easy to fix, but that was not my point.
I'll make a patch to fix this one.

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

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Felipe Balbi]

On Thu, Oct 02, 2008 at 04:32:31PM +0300, Peter Ujfalusi wrote:
> Yes, I know that, I just mentioned that other things are still broken in the omap-keypad driver.
> In the OMAP1 board files the resource is filled in, but in the driver you can see this:
> 
> static ssize_t omap_kp_enable_store(struct device *dev, struct device_attribute *attr,
> 				    const char *buf, size_t count)
> .
> .
> 	if (state != kp_enable) {
> 		if (state)
> 			enable_irq(INT_KEYBOARD);
> 		else
> 			disable_irq(INT_KEYBOARD);
> 		kp_enable = state;
> 	}
> .
> .
> 
> So, pretty much not a sane thing to do...
> It is easy to fix, but that was not my point.
> I'll make a patch to fix this one.

that is surely broken :-)

-- 
balbi

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[David Brownell]

On Thursday 02 October 2008, Peter Ujfalusi wrote:
> commit e85a9daa48453a4e7ffedf174b4358cdb444fbbb
> Author: David Brownell <dbrownell@users.sourceforge.net>
> some drivers switch away from OMAP-specific GPIO calls
> 
> Should I take the relevant parts from these commits and send it to linux-input?

For that one, sure.  The mainline patch doesn't seem to
have gotten queued anywhere yet -- Andrew was off for
a while.

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[David Brownell]

On Thursday 02 October 2008, Peter Ujfalusi wrote:
> commit ed39c59ff267249e7e789ea3bb64afc1e9afb5bd
> Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
> omap: fix a load of "warning: symbol 'xxx' was not declared. Should it be static?"
> 
> Should I take the relevant parts from these commits and send it to linux-input?

I think those may be in Russell's GIT waiting for a 2.6.28 merge request.

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Tony Lindgren]

* David Brownell <david-b@pacbell.net> [081002 18:33]:
> On Thursday 02 October 2008, Peter Ujfalusi wrote:
> > commit ed39c59ff267249e7e789ea3bb64afc1e9afb5bd
> > Author: Russell King <rmk@dyn-67.arm.linux.org.uk>
> > omap: fix a load of "warning: symbol 'xxx' was not declared. Should it be static?"
> > 
> > Should I take the relevant parts from these commits and send it to linux-input?
> 
> I think those may be in Russell's GIT waiting for a 2.6.28 merge request.
> 

Yeah that is already queued up.

Tony

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi@nokia.com> [081002 16:03]:
> 
> Hello,
> 
> the diff between l-o and mainline are these commits regarding to omap-keypad.c:
> 
> commit fdee8764947cde1e6933e7d981ce5b9de00e83e6
> Author: Tony Lindgren <tony@atomide.com>
> Merge current mainline tree into linux-omap tree

This should not be needed, but it's always worth checking there was no
automerge weirdness introduced by that patch.

Tony

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [081002 23:22]:
> * Peter Ujfalusi <peter.ujfalusi@nokia.com> [081002 16:03]:
> > 
> > Hello,
> > 
> > the diff between l-o and mainline are these commits regarding to omap-keypad.c:
> > 
> > commit fdee8764947cde1e6933e7d981ce5b9de00e83e6
> > Author: Tony Lindgren <tony@atomide.com>
> > Merge current mainline tree into linux-omap tree
> 
> This should not be needed, but it's always worth checking there was no
> automerge weirdness introduced by that patch.

OK, I'll push these two to l-o tree, hopefully you're still planning
to send them to the mainline kernel?

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

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Peter Ujfalusi]

On Saturday 18 October 2008 01:02:19 ext Tony Lindgren wrote:
> OK, I'll push these two to l-o tree, hopefully you're still planning
> to send them to the mainline kernel?
> 
> > Tony

Sorry, it seams that I was sleeping...
I have sent the patches to linux-input (l-o cc-d).

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

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi@nokia.com> [081022 22:56]:
> 
> On Saturday 18 October 2008 01:02:19 ext Tony Lindgren wrote:
> > OK, I'll push these two to l-o tree, hopefully you're still planning
> > to send them to the mainline kernel?
> > 
> > > Tony
> 
> Sorry, it seams that I was sleeping...
> I have sent the patches to linux-input (l-o cc-d).

Great, thanks!

Tony

[PATCH 0/2] omap-keypad fixes for OMAP2

[Peter Ujfalusi]

Hello,
The following two patches provides fixes for the omap-keypad
driver on OMAP2.

Peter Ujfalusi (2)
  OMAP2: omap-keypad: interrupt disable fix
  OMAP2: omap-keypad: Enable more than 6 rows

 drivers/input/keyboard/omap-keypad.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

Re: [PATCH 0/2] omap-keypad fixes for OMAP2

[Dmitry Torokhov]

Hi Peter,

On Thu, Oct 23, 2008 at 08:51:30AM +0300, Peter Ujfalusi wrote:
> Hello,
> The following two patches provides fixes for the omap-keypad
> driver on OMAP2.
> 
> Peter Ujfalusi (2)
>   OMAP2: omap-keypad: interrupt disable fix
>   OMAP2: omap-keypad: Enable more than 6 rows
> 
>  drivers/input/keyboard/omap-keypad.c |    6 +-----
>  1 files changed, 1 insertions(+), 5 deletions(-)
> 

Looks good, will apply to 'next' branch.

-- 
Dmitry