--- Russell King <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
