Em Qua, 2009-04-01 às 15:52 +0300, Ilya Petrov escreveu:
> Ok, second try.
> 
+ *  Inpit driver for PCAP events:
Typo

+ *  Copyright (c) 2008 Ily Petrov <[email protected]>
Two typos

+#include <linux/platform_device.h>
It would be great if this one was actually used! :)

+#include <asm/io.h>
Is this really needed?

+        if (err) {
+          printk(KERN_ERR "pcap_powerkey: Failed to register device\n");
+          goto fail_irq;
+        }
style, checkpatch.pl may help you spot these.

+        ezx_pcap_unregister_event(PCAP_IRQ_ONOFF);
+        ezx_pcap_unregister_event(PCAP_IRQ_MB2 | PCAP_IRQ_A1);
a single unregister_event with the 3 bits ORed is cleaner.


The biggest problem is the lack of a platform_device/platform_driver
pair. We really need it because we want to conditionally register the
platform_device only on gen2 phones, so the driver doesnt get active
case it is running on a gen1 phone.

Take a look at drivers/rtc/rtc-pcap.c, its a small and simple driver,
and it is a platform_driver. See arch/arm/mach-pxa/ezx.c for the
corresponding platform_device.

Submit two patches, one that adds the driver, and another that adds the
platform_device to ezx.c (to avoid conflicts on git merges, and for a
good looking topgit dependency tree).

Also, we need a Signed-off-by: line from you to submit this driver
upstream.

Thanks for your contribution!

-- 
Daniel Ribeiro


Reply via email to