[PATCH] PCAP misc input driver (for 2.6.32)

[PATCH] PCAP misc input driver (for 2.6.32)

[Daniel Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 5919 bytes --]

This is a driver for misc input events for the PCAP2 PMIC, it handles
the power button, headphone insertion/removal and the headphone button.

Signed-off-by: Ilya Petrov <ilya.muromec@gmail.com>
Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>

---
 drivers/input/keyboard/Kconfig     |    7 ++
 drivers/input/keyboard/Makefile    |    1 +
 drivers/input/keyboard/pcap_keys.c |  152 ++++++++++++++++++++++++++++++++++++
 3 files changed, 160 insertions(+), 0 deletions(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 9d8f796..ea512b0 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -353,4 +353,11 @@ config KEYBOARD_EP93XX
 	  To compile this driver as a module, choose M here: the
 	  module will be called ep93xx_keypad.
 
+config KEYBOARD_PCAP
+        tristate "Motorola EZX PCAP events"
+        depends on EZX_PCAP
+        help
+          Say Y here if you want to use power key and jack events
+          on Motorola EZX 2nd generation phones
+
 endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 156b647..06b77dd 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -30,3 +30,4 @@ obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
 obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
 obj-$(CONFIG_KEYBOARD_SH_KEYSC)		+= sh_keysc.o
 obj-$(CONFIG_KEYBOARD_EP93XX)		+= ep93xx_keypad.o
+obj-$(CONFIG_KEYBOARD_PCAP)		+= pcap_keys.o
diff --git a/drivers/input/keyboard/pcap_keys.c b/drivers/input/keyboard/pcap_keys.c
new file mode 100644
index 0000000..8a9b533
--- /dev/null
+++ b/drivers/input/keyboard/pcap_keys.c
@@ -0,0 +1,152 @@
+/*
+ *  Input driver for PCAP events:
+ *   * Power key
+ *   * Jack plug/unplug
+ *   * Headphone button
+ *
+ *  Copyright (c) 2008,2009 Ilya Petrov <ilya.muromec@gmail.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/input.h>
+#include <linux/mfd/ezx-pcap.h>
+
+struct pcap_keys {
+	struct pcap_chip *pcap;
+	struct input_dev *input;
+};
+
+/* PCAP2 interrupts us on keypress */
+static irqreturn_t pcap_keys_handler(int irq, void *_pcap_keys)
+{
+	struct pcap_keys *pcap_keys = _pcap_keys;
+	int pirq = irq_to_pcap(pcap_keys->pcap, irq);
+	u32 pstat;
+
+	ezx_pcap_read(pcap_keys->pcap, PCAP_REG_PSTAT, &pstat);
+	pstat &= 1 << pirq;
+
+	switch (pirq) {
+	case PCAP_IRQ_ONOFF:
+		input_report_key(pcap_keys->input, KEY_POWER, !pstat);
+		break;
+	case PCAP_IRQ_HS:
+		input_report_switch(pcap_keys->input,
+				SW_HEADPHONE_INSERT, !pstat);
+		break;
+	case PCAP_IRQ_MIC:
+		input_report_key(pcap_keys->input, KEY_HP, !pstat);
+		break;
+	}
+
+	input_sync(pcap_keys->input);
+
+	return IRQ_HANDLED;
+}
+
+static int __init pcap_keys_probe(struct platform_device *pdev)
+{
+	int err = -ENOMEM;
+	struct pcap_keys *pcap_keys;
+
+	pcap_keys = kmalloc(sizeof(struct pcap_keys), GFP_KERNEL);
+	if (!pcap_keys)
+		return err;
+
+	pcap_keys->pcap = platform_get_drvdata(pdev);
+
+	pcap_keys->input = input_allocate_device();
+	if (!pcap_keys->input)
+		goto fail;
+
+	platform_set_drvdata(pdev, pcap_keys);
+	pcap_keys->input->name = pdev->name;
+	pcap_keys->input->phys = "pcap-keys/input0";
+	pcap_keys->input->dev.parent = &pdev->dev;
+
+	pcap_keys->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_SW);
+	set_bit(KEY_POWER, pcap_keys->input->keybit);
+	set_bit(SW_HEADPHONE_INSERT, pcap_keys->input->swbit);
+	set_bit(KEY_HP, pcap_keys->input->keybit);
+
+	err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF),
+			pcap_keys_handler, 0, "Power key", pcap_keys);
+	if (err)
+		goto fail_dev;
+
+	err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS),
+			pcap_keys_handler, 0, "Headphone jack", pcap_keys);
+	if (err)
+		goto fail_pwrkey;
+
+	err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC),
+			pcap_keys_handler, 0, "MIC jack/button", pcap_keys);
+	if (err)
+		goto fail_jack;
+
+	err = input_register_device(pcap_keys->input);
+	if (err)
+		goto fail_mic;
+
+	return 0;
+
+fail_mic:
+	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC), pcap_keys);
+fail_jack:
+	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS), pcap_keys);
+fail_pwrkey:
+	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF), pcap_keys);
+fail_dev:
+	input_free_device(pcap_keys->input);
+fail:
+	kfree(pcap_keys);
+	return err;
+}
+
+static int pcap_keys_remove(struct platform_device *pdev)
+{
+	struct pcap_keys *pcap_keys = platform_get_drvdata(pdev);
+
+	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF), pcap_keys);
+	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS), pcap_keys);
+	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC), pcap_keys);
+
+	input_unregister_device(pcap_keys->input);
+	kfree(pcap_keys);
+
+	return 0;
+}
+
+static struct platform_driver pcap_keys_device_driver = {
+	.probe		= pcap_keys_probe,
+	.remove		= pcap_keys_remove,
+	.driver		= {
+		.name	= "pcap-keys",
+		.owner	= THIS_MODULE,
+	}
+};
+
+static int __init pcap_keys_init(void)
+{
+	return platform_driver_register(&pcap_keys_device_driver);
+};
+
+static void __exit pcap_keys_exit(void)
+{
+	platform_driver_unregister(&pcap_keys_device_driver);
+};
+
+module_init(pcap_keys_init);
+module_exit(pcap_keys_exit);
+
+MODULE_DESCRIPTION("Motorola PCAP2 input events driver");
+MODULE_AUTHOR("Ilya Petrov <ilya.muromec@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
tg: (22f99ae..) ezx/pcap_keys (depends on: ezx/local/pcap)

-- 
Daniel Ribeiro

[-- Attachment #2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] PCAP misc input driver (for 2.6.32)

[Trilok Soni]

Hi Daniel,

> +
> +static int __init pcap_keys_probe(struct platform_device *pdev)
> +{
> +       int err = -ENOMEM;
> +       struct pcap_keys *pcap_keys;
> +
> +       pcap_keys = kmalloc(sizeof(struct pcap_keys), GFP_KERNEL);
> +       if (!pcap_keys)
> +               return err;
> +
> +       pcap_keys->pcap = platform_get_drvdata(pdev);
> +
> +       pcap_keys->input = input_allocate_device();
> +       if (!pcap_keys->input)
> +               goto fail;
> +
> +       platform_set_drvdata(pdev, pcap_keys);
> +       pcap_keys->input->name = pdev->name;
> +       pcap_keys->input->phys = "pcap-keys/input0";
> +       pcap_keys->input->dev.parent = &pdev->dev;
> +
> +       pcap_keys->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_SW);
> +       set_bit(KEY_POWER, pcap_keys->input->keybit);
> +       set_bit(SW_HEADPHONE_INSERT, pcap_keys->input->swbit);
> +       set_bit(KEY_HP, pcap_keys->input->keybit);

__set_bit please.

> +
> +       err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF),
> +                       pcap_keys_handler, 0, "Power key", pcap_keys);
> +       if (err)
> +               goto fail_dev;
> +
> +       err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS),
> +                       pcap_keys_handler, 0, "Headphone jack", pcap_keys);
> +       if (err)
> +               goto fail_pwrkey;
> +
> +       err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC),
> +                       pcap_keys_handler, 0, "MIC jack/button", pcap_keys);
> +       if (err)
> +               goto fail_jack;
> +
> +       err = input_register_device(pcap_keys->input);
> +       if (err)
> +               goto fail_mic;

Same comment as given in PCAP touchscreen driver.

> +
> +static int pcap_keys_remove(struct platform_device *pdev)
> +{
> +       struct pcap_keys *pcap_keys = platform_get_drvdata(pdev);
> +
> +       free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF), pcap_keys);
> +       free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS), pcap_keys);
> +       free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC), pcap_keys);
> +
> +       input_unregister_device(pcap_keys->input);
> +       kfree(pcap_keys);
> +
> +       return 0;
> +}
> +
> +static struct platform_driver pcap_keys_device_driver = {
> +       .probe          = pcap_keys_probe,
> +       .remove         = pcap_keys_remove,

__devexit_p ?

-- 
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] PCAP misc input driver (for 2.6.32)

[Daniel Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 1519 bytes --]

Em Dom, 2009-06-28 às 12:49 +0530, Trilok Soni escreveu:
> > +       pcap_keys->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_SW);
> > +       set_bit(KEY_POWER, pcap_keys->input->keybit);
> > +       set_bit(SW_HEADPHONE_INSERT, pcap_keys->input->swbit);
> > +       set_bit(KEY_HP, pcap_keys->input->keybit);
> 
> __set_bit please.

Ok.

> > +
> > +       err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF),
> > +                       pcap_keys_handler, 0, "Power key", pcap_keys);
> > +       if (err)
> > +               goto fail_dev;
> > +
> > +       err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS),
> > +                       pcap_keys_handler, 0, "Headphone jack", pcap_keys);
> > +       if (err)
> > +               goto fail_pwrkey;
> > +
> > +       err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC),
> > +                       pcap_keys_handler, 0, "MIC jack/button", pcap_keys);
> > +       if (err)
> > +               goto fail_jack;
> > +
> > +       err = input_register_device(pcap_keys->input);
> > +       if (err)
> > +               goto fail_mic;
> 
> Same comment as given in PCAP touchscreen driver.

Ok.

> > +static struct platform_driver pcap_keys_device_driver = {
> > +       .probe          = pcap_keys_probe,
> > +       .remove         = pcap_keys_remove,
> 
> __devexit_p ?

Ok.

Thanks for the review, I will send new versions addressing your comments
as soon as possible.

-- 
Daniel Ribeiro

[-- Attachment #2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] PCAP misc input driver (for 2.6.32)

[Dmitry Torokhov]

Hi Daniel,

On Sat, Jun 27, 2009 at 02:09:52PM -0300, Daniel Ribeiro wrote:
> This is a driver for misc input events for the PCAP2 PMIC, it handles
> the power button, headphone insertion/removal and the headphone button.
> 
> Signed-off-by: Ilya Petrov <ilya.muromec@gmail.com>
> Signed-off-by: Daniel Ribeiro <drwyrm@gmail.com>
> 
> ---
>  drivers/input/keyboard/Kconfig     |    7 ++
>  drivers/input/keyboard/Makefile    |    1 +
>  drivers/input/keyboard/pcap_keys.c |  152 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 160 insertions(+), 0 deletions(-)
> 

First of all I think the driver should live in misc, not in keyboard,
since it is not a full-fledged keyboard.

> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 9d8f796..ea512b0 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -353,4 +353,11 @@ config KEYBOARD_EP93XX
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ep93xx_keypad.
>  
> +config KEYBOARD_PCAP
> +        tristate "Motorola EZX PCAP events"
> +        depends on EZX_PCAP
> +        help
> +          Say Y here if you want to use power key and jack events
> +          on Motorola EZX 2nd generation phones
> +

To compile this driver as a module...

>  endif
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 156b647..06b77dd 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -30,3 +30,4 @@ obj-$(CONFIG_KEYBOARD_MAPLE)		+= maple_keyb.o
>  obj-$(CONFIG_KEYBOARD_BFIN)		+= bf54x-keys.o
>  obj-$(CONFIG_KEYBOARD_SH_KEYSC)		+= sh_keysc.o
>  obj-$(CONFIG_KEYBOARD_EP93XX)		+= ep93xx_keypad.o
> +obj-$(CONFIG_KEYBOARD_PCAP)		+= pcap_keys.o
> diff --git a/drivers/input/keyboard/pcap_keys.c b/drivers/input/keyboard/pcap_keys.c
> new file mode 100644
> index 0000000..8a9b533
> --- /dev/null
> +++ b/drivers/input/keyboard/pcap_keys.c
> @@ -0,0 +1,152 @@
> +/*
> + *  Input driver for PCAP events:
> + *   * Power key
> + *   * Jack plug/unplug
> + *   * Headphone button
> + *
> + *  Copyright (c) 2008,2009 Ilya Petrov <ilya.muromec@gmail.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/input.h>
> +#include <linux/mfd/ezx-pcap.h>
> +
> +struct pcap_keys {
> +	struct pcap_chip *pcap;
> +	struct input_dev *input;
> +};
> +
> +/* PCAP2 interrupts us on keypress */
> +static irqreturn_t pcap_keys_handler(int irq, void *_pcap_keys)
> +{
> +	struct pcap_keys *pcap_keys = _pcap_keys;
> +	int pirq = irq_to_pcap(pcap_keys->pcap, irq);
> +	u32 pstat;
> +
> +	ezx_pcap_read(pcap_keys->pcap, PCAP_REG_PSTAT, &pstat);
> +	pstat &= 1 << pirq;
> +
> +	switch (pirq) {
> +	case PCAP_IRQ_ONOFF:
> +		input_report_key(pcap_keys->input, KEY_POWER, !pstat);
> +		break;
> +	case PCAP_IRQ_HS:
> +		input_report_switch(pcap_keys->input,
> +				SW_HEADPHONE_INSERT, !pstat);
> +		break;
> +	case PCAP_IRQ_MIC:
> +		input_report_key(pcap_keys->input, KEY_HP, !pstat);

Why not SW_MICROPHONE_INSERT?

> +		break;
> +	}
> +
> +	input_sync(pcap_keys->input);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int __init pcap_keys_probe(struct platform_device *pdev)

__devinit, not __init should be used on driver's probe() methods.

> +{
> +	int err = -ENOMEM;
> +	struct pcap_keys *pcap_keys;
> +
> +	pcap_keys = kmalloc(sizeof(struct pcap_keys), GFP_KERNEL);
> +	if (!pcap_keys)
> +		return err;
> +
> +	pcap_keys->pcap = platform_get_drvdata(pdev);
> +
> +	pcap_keys->input = input_allocate_device();
> +	if (!pcap_keys->input)
> +		goto fail;
> +
> +	platform_set_drvdata(pdev, pcap_keys);
> +	pcap_keys->input->name = pdev->name;
> +	pcap_keys->input->phys = "pcap-keys/input0";
> +	pcap_keys->input->dev.parent = &pdev->dev;

I do like a temp for input_dev, it usually makes code a bit smaller.
Also it would be nice to have but type set (BUS_HOST I think).

> +
> +	pcap_keys->input->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_SW);
> +	set_bit(KEY_POWER, pcap_keys->input->keybit);
> +	set_bit(SW_HEADPHONE_INSERT, pcap_keys->input->swbit);
> +	set_bit(KEY_HP, pcap_keys->input->keybit);
> +

__set_bit() please, like Trolok said.

> +	err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF),
> +			pcap_keys_handler, 0, "Power key", pcap_keys);
> +	if (err)
> +		goto fail_dev;
> +
> +	err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS),
> +			pcap_keys_handler, 0, "Headphone jack", pcap_keys);
> +	if (err)
> +		goto fail_pwrkey;
> +
> +	err = request_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC),
> +			pcap_keys_handler, 0, "MIC jack/button", pcap_keys);
> +	if (err)
> +		goto fail_jack;
> +
> +	err = input_register_device(pcap_keys->input);
> +	if (err)
> +		goto fail_mic;
> +
> +	return 0;
> +
> +fail_mic:
> +	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC), pcap_keys);
> +fail_jack:
> +	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS), pcap_keys);
> +fail_pwrkey:
> +	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF), pcap_keys);
> +fail_dev:
> +	input_free_device(pcap_keys->input);
> +fail:
> +	kfree(pcap_keys);
> +	return err;
> +}
> +
> +static int pcap_keys_remove(struct platform_device *pdev)

__devexit here.

> +{
> +	struct pcap_keys *pcap_keys = platform_get_drvdata(pdev);
> +
> +	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_ONOFF), pcap_keys);
> +	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_HS), pcap_keys);
> +	free_irq(pcap_to_irq(pcap_keys->pcap, PCAP_IRQ_MIC), pcap_keys);
> +
> +	input_unregister_device(pcap_keys->input);
> +	kfree(pcap_keys);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver pcap_keys_device_driver = {
> +	.probe		= pcap_keys_probe,
> +	.remove		= pcap_keys_remove,

__devexit_p()

> +	.driver		= {
> +		.name	= "pcap-keys",
> +		.owner	= THIS_MODULE,
> +	}
> +};
> +
> +static int __init pcap_keys_init(void)
> +{
> +	return platform_driver_register(&pcap_keys_device_driver);
> +};
> +
> +static void __exit pcap_keys_exit(void)
> +{
> +	platform_driver_unregister(&pcap_keys_device_driver);
> +};
> +
> +module_init(pcap_keys_init);
> +module_exit(pcap_keys_exit);
> +
> +MODULE_DESCRIPTION("Motorola PCAP2 input events driver");
> +MODULE_AUTHOR("Ilya Petrov <ilya.muromec@gmail.com>");
> +MODULE_LICENSE("GPL");

Do we need MODULE_ALIAS() here?

Thanks!

-- 
Dmitry

Re: [PATCH] PCAP misc input driver (for 2.6.32)

[Mark Brown]

On Sat, Jun 27, 2009 at 02:09:52PM -0300, Daniel Ribeiro wrote:
> This is a driver for misc input events for the PCAP2 PMIC, it handles
> the power button, headphone insertion/removal and the headphone button.

Depending on the power management capabilities of the CODEC it may be
better to support at laest the headphone insert/removal via the audio
driver as a jack to allow automatic management of the power for the
relevant paths within the CODEC.

Re: [PATCH] PCAP misc input driver (for 2.6.32)

[Ilya Petrov]

2009/6/29 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> Depending on the power management capabilities of the CODEC it may be
> better to support at laest the headphone insert/removal via the audio
> driver as a jack to allow automatic management of the power for the
> relevant paths within the CODEC.

we need to control this manually from userspace - for example, even with
headphone plugged in ringtone should be played via speaker.



-- 
 wbr, Ilya
 ICQ: none, Jabber: ilya.muromec@jabber.ru

Re: [PATCH] PCAP misc input driver (for 2.6.32)

[Mark Brown]

On Mon, Jun 29, 2009 at 07:27:24PM +0300, Ilya Petrov wrote:
> 2009/6/29 Mark Brown <broonie@opensource.wolfsonmicro.com>:

> > Depending on the power management capabilities of the CODEC it may be
> > better to support at laest the headphone insert/removal via the audio
> > driver as a jack to allow automatic management of the power for the
> > relevant paths within the CODEC.

> we need to control this manually from userspace - for example, even with
> headphone plugged in ringtone should be played via speaker.

ALSA jacks are still visible to user space as input devices.  The
difference it will make here is that the kernel can autonomously control
power for paths where that makes sense.

Re: [PATCH] PCAP misc input driver (for 2.6.32)

[Daniel Ribeiro]

[-- Attachment #1: Type: text/plain, Size: 1924 bytes --]

Em Dom, 2009-06-28 às 23:14 -0700, Dmitry Torokhov escreveu:
> >  drivers/input/keyboard/Kconfig     |    7 ++
> >  drivers/input/keyboard/Makefile    |    1 +
> >  drivers/input/keyboard/pcap_keys.c |  152 ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 160 insertions(+), 0 deletions(-)
> > 
> 
> First of all I think the driver should live in misc, not in keyboard,
> since it is not a full-fledged keyboard.

Ok.

> > +          Say Y here if you want to use power key and jack events
> > +          on Motorola EZX 2nd generation phones
> > +
> 
> To compile this driver as a module...

Ok.

> > +	case PCAP_IRQ_MIC:
> > +		input_report_key(pcap_keys->input, KEY_HP, !pstat);
> 
> Why not SW_MICROPHONE_INSERT?

Its actually a button.

The device has a single jack for headphone and microphone. And the
headset that we connect to it has a button that you use to answer calls,
or dial.

> > +static int __init pcap_keys_probe(struct platform_device *pdev)
> 
> __devinit, not __init should be used on driver's probe() methods.

Ok.

> > +	pcap_keys->input->name = pdev->name;
> > +	pcap_keys->input->phys = "pcap-keys/input0";
> > +	pcap_keys->input->dev.parent = &pdev->dev;
> 
> I do like a temp for input_dev, it usually makes code a bit smaller.
> Also it would be nice to have but type set (BUS_HOST I think).

Ok.

> > +	set_bit(KEY_HP, pcap_keys->input->keybit);

> __set_bit() please, like Trolok said.

Ok.

> > +static int pcap_keys_remove(struct platform_device *pdev)
> 
> __devexit here.

Ok.

> > +	.remove		= pcap_keys_remove,
> 
> __devexit_p()

Ok.

> > +MODULE_DESCRIPTION("Motorola PCAP2 input events driver");
> > +MODULE_AUTHOR("Ilya Petrov <ilya.muromec@gmail.com>");
> > +MODULE_LICENSE("GPL");
> 
> Do we need MODULE_ALIAS() here?

Do we? I think we don't, but well... It costs nothing. ;)

-- 
Daniel Ribeiro

[-- Attachment #2: Esta é uma parte de mensagem assinada digitalmente --]
[-- Type: application/pgp-signature, Size: 197 bytes --]