Hi Raphael,
On Mon, Sep 21, 2009 at 12:35:21PM -0300, Raphael Derosso Pereira wrote:
> Hello Dimitry,
>
> 2009/9/15 Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>:
> >
> > This version is much better, thank you very much for making the changes
> > I requested.
>
> You're very welcomed, let's see this version I'm sending now!
>
> >
> >> +
> >> +#define QT2160_CMD_CHIPID (00)
> >> +#define QT2160_CMD_CODEVER (01)
> >> +#define QT2160_CMD_GSTAT (02)
> >> +#define QT2160_CMD_KEYS3 (03)
> >> +#define QT2160_CMD_KEYS4 (04)
> >> +#define QT2160_CMD_SLIDE (05)
> >> +#define QT2160_CMD_GPIOS (06)
> >> +#define QT2160_CMD_SUBVER (07)
> >> +#define QT2160_CMD_CALIBRATE (10)
> >
> > I am a bit concerned about this chunk. The first 8 commands are written
> > as octal while the last (calibrate) as decimal. Is this intentional?
>
> You are right. There's no need to declare them as octal.
>
> >
> > I also made a few more changes. Could you please trythe patch below and
> > if everything is still working I will apply to my tree.
>
> As I had already made other modifications, I added your changes into
> my code, correct a bug you inserted (qt2160_read must return the
> variable value and not 0 at the end), and I'm sending a patch against
> 2.6.31. Hope you don't mind.
>
> --
>
> From: Raphael Derosso Pereira <raphaelpereira@xxxxxxxxx>
>
> Input: AT42QT2160
>
> Inclusion of input->open and input->close plus Dmitry fixups plus other cleanups
> needed after checking code.
>
> Signed-off-by: Raphael Derosso Pereira <raphaelpereira@xxxxxxxxx>
> --
> diff -pruN linux-2.6.31_orig/drivers/input/keyboard/Kconfig
> linux-2.6.31_rp/drivers/input/keyboard/Kconfig
> --- linux-2.6.31_orig/drivers/input/keyboard/Kconfig 2009-09-09
> 19:13:59.000000000 -0300
> +++ linux-2.6.31_rp/drivers/input/keyboard/Kconfig 2009-09-21
> 12:09:49.000000000 -0300
> @@ -361,4 +361,22 @@ config KEYBOARD_XTKBD
> To compile this driver as a module, choose M here: the
> module will be called xtkbd.
>
> +config QT2160
> + tristate "Atmel AT42QT2160 Touch Sensor Chip"
> + depends on EXPERIMENTAL
> + select I2C
> + help
> + If you say yes here you get support for Atmel AT42QT2160 Touch
> + Sensor chip as a keyboard input.
> +
> + This driver can also be built as a module. If so, the module
> + will be called qt2160.
> +
> +config QT2160_DEBUG
> + bool "AT42QT2160 Debug Messages"
> + depends on QT2160
> + help
> + Generates lots of debug from driver to help show what is going
> + on under the hoods.
I don't think we need a Kconfig entry for debugging. A person who wants
to debug such driver can easily turn on debugging manually.
> +
> +static int qt2160_open(struct input_dev *dev)
> +{
> + int error;
> + struct qt2160_data *qt2160;
> + struct i2c_client *client;
> +
> + qt2160 = input_get_drvdata(dev);
> + client = qt2160->client;
> +
> + /* Calibrate device */
> + error = qt2160_write(client, QT2160_CMD_CALIBRATE, 1);
> + if (error) {
> + dev_err(&client->dev,
> + "failed to calibrate device\n");
> + return error;
> + }
> +
> + /* Initialize IRQ structure */
> + schedule_delayed_work(&qt2160->dwork, QT2160_CYCLE_INTERVAL);
> +
> + if (client->irq) {
> + error = request_irq(client->irq, qt2160_irq,
> + (IRQF_TRIGGER_FALLING), "qt2160", qt2160);
> +
> + if (error) {
> + dev_err(&client->dev,
> + "failed to allocate irq %d\n", client->irq);
> + return error;
> + }
> + }
> +
We normally try to allocate all necessary resources in probe() so that
if device is bound to a driver it should work unless it is broken. Since
there is no way to shut off IRQs while IRQ handler is registered it does
not make sense to have open and close if we allocate IRQ in probe().
Please take a look at the latest version that I have in 'for-linus'
branch in my tree on kernel.org and shout if you see something wrong.
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
