[PATCH 2/2] Input: omap-keypad: dynamically handle register offsets
Felipe Balbi
balbi at ti.com
Mon Mar 19 04:33:39 EDT 2012
Hi,
On Mon, Mar 19, 2012 at 01:27:50PM +0530, Poddar, Sourav wrote:
>
>
> On Sat, Mar 17, 2012 at 11:17 AM, Dmitry Torokhov <dmitry.torokhov at gmail.com>
> wrote:
>
> Hi Sourav,
>
> On Fri, Mar 16, 2012 at 07:56:29PM +0530, Sourav Poddar wrote:
> > From: G, Manjunath Kondaiah <manjugk at ti.com>
> >
> > Keypad controller register offsets are different for omap4
> > and omap5. Handle these offsets through static mapping and
> > assign these mappings during run time.
> >
> > Signed-off-by: Felipe Balbi <balbi at ti.com>
> > Signed-off-by: G, Manjunath Kondaiah <manjugk at ti.com>
> > Signed-off-by: Sourav Poddar <sourav.poddar at ti.com>
> > ---
> > drivers/input/keyboard/Kconfig | 6 +-
> > drivers/input/keyboard/omap4-keypad.c | 122
> ++++++++++++++++++++++++++------
> > 2 files changed, 102 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/
> Kconfig
> > index 1379daa..09d7d0b 100644
> > --- a/drivers/input/keyboard/Kconfig
> > +++ b/drivers/input/keyboard/Kconfig
> > @@ -510,10 +510,10 @@ config KEYBOARD_OMAP
> > To compile this driver as a module, choose M here: the
> > module will be called omap-keypad.
> >
> > -config KEYBOARD_OMAP4
> > - tristate "TI OMAP4 keypad support"
> > +config KEYBOARD_OMAP4+
> > + tristate "TI OMAP4+ keypad support"
> > help
> > - Say Y here if you want to use the OMAP4 keypad.
> > + Say Y here if you want to use the OMAP4+ keypad.
> >
> > To compile this driver as a module, choose M here: the
> > module will be called omap4-keypad.
> > diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/
> keyboard/omap4-keypad.c
> > index e809ac0..6872495 100644
> > --- a/drivers/input/keyboard/omap4-keypad.c
> > +++ b/drivers/input/keyboard/omap4-keypad.c
> > @@ -68,6 +68,11 @@
> >
> > #define OMAP4_MASK_IRQSTATUSDISABLE 0xFFFF
> >
> > +enum {
> > + KBD_REVISION_OMAP4 = 0,
> > + KBD_REVISION_OMAP5,
> > +};
> > +
> > struct omap4_keypad {
> > struct input_dev *input;
> >
> > @@ -76,11 +81,79 @@ struct omap4_keypad {
> >
> > unsigned int rows;
> > unsigned int cols;
> > + unsigned int revision;
> > unsigned int row_shift;
> > unsigned char key_state[8];
> > unsigned short keymap[];
> > };
> >
> > +static int kbd_read_irqstatus(struct omap4_keypad *keypad_data, u32
> offset)
> > +{
> > + if (keypad_data->revision == KBD_REVISION_OMAP4)
> > + return __raw_readl(keypad_data->base + offset);
> > + else if (keypad_data->revision == KBD_REVISION_OMAP5)
> > + return __raw_readl(keypad_data->base + offset + 0x0c);
> > +
>
> This is called from ISR and doing comparisons seems quite wasteful. Why
> don't you stuff register numbers for IRQ manipulations into struct
> omap4_keypad at probe time and your function would just do:
>
> return __raw_readl(keypad_data->base + keypad_data->irqstatus_reg);
>
>
> Yes, can do that way . Will make necessary changes and post it again.
>
> And do the same for irqstatus. FOr the rest of the register store the
>
> I think you mean "irqenable" here.
>
> common offset (seems to be 0x10).
>
> > + return -ENODEV;
> > +}
> > +
> > +static int kbd_write_irqstatus(struct omap4_keypad *keypad_data,
> > + u32 offset, u32 value)
> > +{
> > + if (keypad_data->revision == KBD_REVISION_OMAP4)
> > + return __raw_writel(value, keypad_data->base + offset);
> > + else if (keypad_data->revision == KBD_REVISION_OMAP5)
> > + return __raw_writel(value, keypad_data->base + offset +
> 0x0c);
> > +
> > + return -ENODEV;
> > +}
> > +
> > +static int kbd_write_irqenable(struct omap4_keypad *keypad_data,
> > + u32 offset, u32 value)
> > +{
> > + if (keypad_data->revision == KBD_REVISION_OMAP4)
> > + return __raw_writel(value, keypad_data->base + offset);
> > + else if (keypad_data->revision == KBD_REVISION_OMAP5)
> > + return __raw_writel(value, keypad_data->base + offset +
> 0x0c);
> > +
> > + return -ENODEV;
> > +}
> > +
> > +static int kbd_readl(struct omap4_keypad *keypad_data, u32 offset)
> > +{
> > + if (keypad_data->revision == KBD_REVISION_OMAP4)
> > + return __raw_readl(keypad_data->base + offset);
> > + else if (keypad_data->revision == KBD_REVISION_OMAP5)
> > + return __raw_readl(keypad_data->base + offset + 0x10);
> > +
> > + return -ENODEV;
> > +}
> > +
> > +static void kbd_writel(struct omap4_keypad *keypad_data, u32 offset, u32
> value)
> > +{
> > + if (keypad_data->revision == KBD_REVISION_OMAP4)
> > + __raw_writel(value, keypad_data->base + offset);
> > + else if (keypad_data->revision == KBD_REVISION_OMAP5)
> > + __raw_writel(value, keypad_data->base + offset + 0x10);
> > +}
> > +
> > +static int kbd_read_revision(struct omap4_keypad *keypad_data, u32
> offset)
> > +{
> > + int reg;
> > + reg = __raw_readl(keypad_data->base + offset);
> > + reg &= 0x03 << 30;
> > + reg >>= 30;
> > +
> > + switch (reg) {
> > + case 1:
> > + return KBD_REVISION_OMAP5;
> > + case 0:
> > + return KBD_REVISION_OMAP4;
> > + default:
> > + return -ENODEV;
> > + }
> > +}
> > +
> > /* Interrupt handler */
> > static irqreturn_t omap4_keypad_interrupt(int irq, void *dev_id)
> > {
> > @@ -91,12 +164,11 @@ static irqreturn_t omap4_keypad_interrupt(int irq,
> void *dev_id)
> > u32 *new_state = (u32 *) key_state;
> >
> > /* Disable interrupts */
> > - __raw_writel(OMAP4_VAL_IRQDISABLE,
> > - keypad_data->base + OMAP4_KBD_IRQENABLE);
> > + kbd_write_irqenable(keypad_data, OMAP4_KBD_IRQENABLE,
> > + OMAP4_VAL_IRQDISABLE);
> >
> > - *new_state = __raw_readl(keypad_data->base +
> OMAP4_KBD_FULLCODE31_0);
> > - *(new_state + 1) = __raw_readl(keypad_data->base
> > - + OMAP4_KBD_FULLCODE63_32);
> > + *new_state = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
> > + *(new_state + 1) = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
> >
> > for (row = 0; row < keypad_data->rows; row++) {
> > changed = key_state[row] ^ keypad_data->key_state[row];
> > @@ -121,12 +193,13 @@ static irqreturn_t omap4_keypad_interrupt(int irq,
> void *dev_id)
> > sizeof(keypad_data->key_state));
> >
> > /* clear pending interrupts */
> > - __raw_writel(__raw_readl(keypad_data->base + OMAP4_KBD_IRQSTATUS),
> > - keypad_data->base + OMAP4_KBD_IRQSTATUS);
> > + kbd_write_irqstatus(keypad_data, OMAP4_KBD_IRQSTATUS,
> > + kbd_read_irqstatus(keypad_data,
> OMAP4_KBD_IRQSTATUS));
> >
> > /* enable interrupts */
> > - __raw_writel(OMAP4_DEF_IRQENABLE_EVENTEN |
> OMAP4_DEF_IRQENABLE_LONGKEY,
> > - keypad_data->base + OMAP4_KBD_IRQENABLE);
> > + kbd_write_irqenable(keypad_data, OMAP4_KBD_IRQENABLE,
> > + OMAP4_DEF_IRQENABLE_EVENTEN |
> > + OMAP4_DEF_IRQENABLE_LONGKEY);
> >
> > return IRQ_HANDLED;
> > }
> > @@ -139,16 +212,19 @@ static int omap4_keypad_open(struct input_dev
> *input)
> >
> > disable_irq(keypad_data->irq);
> >
> > - __raw_writel(OMAP4_VAL_FUNCTIONALCFG,
> > - keypad_data->base + OMAP4_KBD_CTRL);
> > - __raw_writel(OMAP4_VAL_DEBOUNCINGTIME,
> > - keypad_data->base + OMAP4_KBD_DEBOUNCINGTIME);
> > - __raw_writel(OMAP4_VAL_IRQDISABLE,
> > - keypad_data->base + OMAP4_KBD_IRQSTATUS);
> > - __raw_writel(OMAP4_DEF_IRQENABLE_EVENTEN |
> OMAP4_DEF_IRQENABLE_LONGKEY,
> > - keypad_data->base + OMAP4_KBD_IRQENABLE);
> > - __raw_writel(OMAP4_DEF_WUP_EVENT_ENA | OMAP4_DEF_WUP_LONG_KEY_ENA,
> > - keypad_data->base + OMAP4_KBD_WAKEUPENABLE);
> > + keypad_data->revision = kbd_read_revision(keypad_data,
> > + OMAP4_KBD_REVISION);
>
> You want to bail out of kbd_read_revision() returns error here.
>
>
> Yes, will include a error check.
Please send text/plain emails. You're currently sending multipart:
Content-Type: multipart/alternative;
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120319/8fe3b937/attachment-0001.sig>
More information about the linux-arm-kernel
mailing list