Re: [PATCHv3 1/3] TCA6416 keypad : Implement keypad driver for keys interfaced to TCA6416

From: Abraham Arce <abraham.arce.moreno@gmail.com>
To: Sriramakrishnan <srk@ti.com>
Cc: linux-omap@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCHv3 1/3] TCA6416 keypad : Implement keypad driver for keys interfaced to TCA6416
Date: Sun, 4 Apr 2010 13:29:08 -0500	[thread overview]
Message-ID: <s2wcb8016981004041129y8db79t8bbd941411a179ba@mail.gmail.com> (raw)
In-Reply-To: <1269357035-19754-2-git-send-email-srk@ti.com>

Sriramakrishnan,

Find minor changes proposals...

> +
> +         Say Y here if your device has keys connected to
> +         TCA6416 IO expander. Your board-specific setup logic
> +         must also provide pin-mask details(of which TCA6416 pins
> +         are used for keypad).

space in " details ( "

<snip>

> +       if (chip->use_polling)
> +               schedule_delayed_work(&chip->dwork, msecs_to_jiffies(100));
> +       else
> +               enable_irq(chip->irqnum);
> +
> +}
> +
> +

remove one blank line

> +
> +       /* initialize cached registers from their original values.
> +        * we can't share this chip with another i2c master.
> +        */

should we start our comment section as below?

/*
 * initialize...

<snip>

> +
> +       ret = tca6416_read_reg(chip, TCA6416_INPUT, &chip->reg_input);
> +       if (ret)
> +               goto fail1;
> +
> +       i2c_set_clientdata(client, chip);
> +
> +

remove one blank line

<snip>

> +
> +       if (!chip->use_polling) {
> +               if (pdata->irq_is_gpio)
> +                       chip->irqnum = gpio_to_irq(client->irq);
> +               else
> +                       chip->irqnum = client->irq;
> +
> +               ret = request_irq(chip->irqnum, tca6416_keys_isr,
> +                               IRQF_SHARED | IRQF_TRIGGER_FALLING ,
> +                               "tca6416-keypad", chip);
> +               if (ret) {
> +                       dev_dbg(&client->dev,
> +                               "Unable to claim irq %d; error %d\n",
> +                               chip->irqnum, ret);
> +                       goto fail3;
> +               }
> +               disable_irq(chip->irqnum);

why not using threaded irq?

Best Regards
Abraham
--
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