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 <redacted>
Signed-off-by: Daniel Ribeiro <redacted>

---
 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.

To compile this driver as a module...

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 [off-list ref]");
+MODULE_LICENSE("GPL");

Do we need MODULE_ALIAS() here?

Thanks!

-- 
Dmitry