Re: [PATCH 1/2] OMAP: Add keypad driver #3
- From: Komal Shah <komal_shah802003@xxxxxxxxx>
- Date: Thu, 27 Jul 2006 02:26:29 -0700 (PDT)
--- Russell King <rmk+lkml@xxxxxxxxxxxxxxxx> wrote:
+
+ /* 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]));
+ } else
+ /* disable keyboard interrupt and schedule for handling */
+ omap_writew(1, OMAP_MPUIO_BASE + OMAP_MPUIO_KBD_MASKIT);
+
+ if (!cpu_is_omap24xx()) {
This seems obfuscated. It would be trivial to combine these two if()
clauses.
Oops. I will update this in the next patch.
And a general note about the omap24xx vs !omap24xx differences in
this
file - would it make more sense for code readability to have two
completely separate drivers?
Yeah, I had same thought when I did the integration of omap24xx H4 gpio
based keypad driver from TI OMAP tree to omap-git. But, if Tony, Juha
and Richard agrees, then I can roll-out new omap2-gpio-keypad driver
patch along with changes into existing omap-keypad.c(will become
omap1-keypad.c then). It will be also easy for me to maintain omap2
keypad driver, as I don't have access to OMAP1 based boards.
+
+ if (machine_is_omap_osk() || machine_is_omap_h2()
+ || machine_is_omap_h3())
+ udelay(9);
+ else
+ udelay(4);
Wouldn't it be better to pass this via the platform device driver?
It
seems likely that other delays may be required with differing
hardware.
Yes, we can. I will make that change.
+
+ if (machine_is_omap_h2() || machine_is_omap_h3() ||
+ machine_is_omap_perseus2()) {
+ omap_writew(0xff, OMAP_MPUIO_BASE + OMAP_MPUIO_GPIO_DEBOUNCING);
+ }
Maybe this should be a flag or something? Why does h2, h3 and
perseus2
require this and not others?
Yes we can put the flag there through platform data, but OMAP1
(h2/h3/perseus) owners should comment on that. Kevin/Tony?
Thanx for the detailed review.
---Komal Shah
http://komalshah.blogspot.com/
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- References:
- Re: [PATCH 1/2] OMAP: Add keypad driver #3
- From: Russell King
- Re: [PATCH 1/2] OMAP: Add keypad driver #3
- Prev by Date: Re: [BUG?] possible recursive locking detected
- Next by Date: Re: [BUG?] possible recursive locking detected
- Previous by thread: Re: [PATCH 1/2] OMAP: Add keypad driver #3
- Next by thread: We got them all, check it
- Index(es):
Relevant Pages
|
|