| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <AANLkTi=CL=7Aax1dfBHUzszR8KdrJaE0nCHg2tNFAqqJ@mail.gmail.com>
Date: Mon, 10 Jan 2011 20:13:41 +0100
From: Benjamin Tissoires <benjamin.tissoires@...c.fr>
To: Henrik Rydberg <rydberg@...omail.se>
Cc: Stephane Chatty <chatty@...c.fr>,
Dmitry Torokhov <dmitry.torokhov@...il.com>,
Jiri Kosina <jkosina@...e.cz>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [RFC v3 4/5] hid-multitouch: added support for Cypress TrueTouch panels
On Fri, Jan 7, 2011 at 9:00 PM, Henrik Rydberg <rydberg@...omail.se> wrote:
> On Fri, Jan 07, 2011 at 07:42:41PM +0100, Benjamin Tissoires wrote:
>> Added support for Cypress TrueTouch panels, which detect up to 10 fingers
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...c.fr>
>> Signed-off-by: Stéphane Chatty <chatty@...c.fr>
>> ---
>
> Hi, just minor things.
>
>> drivers/hid/Kconfig | 1 +
>> drivers/hid/hid-core.c | 1 +
>> drivers/hid/hid-ids.h | 1 +
>> drivers/hid/hid-multitouch.c | 19 +++++++++++++++++++
>> 4 files changed, 22 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 511554d..de31d75 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -293,6 +293,7 @@ config HID_MULTITOUCH
>>
>> Say Y here if you have one of the following devices:
>> - PixCir touchscreen
>> + - Cypress TrueTouch
>>
>> config HID_NTRIG
>> tristate "N-Trig touch screen"
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 2b4d9b9..e6a86bf 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1298,6 +1298,7 @@ static const struct hid_device_id hid_blacklist[] = {
>> { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_2) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_3) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_MOUSE) },
>> + { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_TRUETOUCH) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_DRAGONRISE, 0x0006) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_DWAV, USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH) },
>> { HID_USB_DEVICE(USB_VENDOR_ID_DWAV, USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH1) },
>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>> index 17b444b..c258c42 100644
>> --- a/drivers/hid/hid-ids.h
>> +++ b/drivers/hid/hid-ids.h
>> @@ -180,6 +180,7 @@
>> #define USB_DEVICE_ID_CYPRESS_BARCODE_1 0xde61
>> #define USB_DEVICE_ID_CYPRESS_BARCODE_2 0xde64
>> #define USB_DEVICE_ID_CYPRESS_BARCODE_3 0xbca1
>> +#define USB_DEVICE_ID_CYPRESS_TRUETOUCH 0xc001
>>
>> #define USB_VENDOR_ID_DEALEXTREAME 0x10c5
>> #define USB_DEVICE_ID_DEALEXTREAME_RADIO_SI4701 0x819a
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 3b05dfe..7af9f71 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -32,6 +32,7 @@ MODULE_LICENSE("GPL");
>> /* quirks to control the device */
>> #define MT_QUIRK_NOT_SEEN_MEANS_UP (1 << 0)
>> #define MT_QUIRK_SLOT_IS_CONTACTID (1 << 1)
>> +#define MT_QUIRK_CYPRESS (1 << 2)
>
> Missing tab.
Ok
>
>>
>> struct mt_slot {
>> __s32 x, y, p, w, h;
>> @@ -62,6 +63,7 @@ struct mt_class {
>> /* classes of device behavior */
>> #define MT_CLS_DEFAULT 0
>> #define MT_CLS_DUAL1 1
>> +#define MT_CLS_CYPRESS 2
>
> Missing tabs... goes for the previous patch as well, coming to think of it.
ok and ok
>
> It does seem slightly complicated, doesn't it. How about dropping
> these, and referring to explicit static structures instead?
I don't want people to place those static structures anywhere in the code.
In v4, I've added a field .name and I retrieve it in the mt_probe function.
>
>>
>> /*
>> * these device-dependent functions determine what slot corresponds
>> @@ -73,6 +75,14 @@ static int slot_is_contactid(struct mt_device *td)
>> return td->curdata.contactid;
>> }
>>
>> +static int cypress_compute_slot(struct mt_device *td)
>> +{
>> + if (td->curdata.contactid != 0 || td->num_received == 0)
>> + return td->curdata.contactid;
>> + else
>> + return -1;
>> +}
>> +
>> static int find_slot_from_contactid(struct mt_device *td)
>> {
>> int i;
>> @@ -95,6 +105,7 @@ static int find_slot_from_contactid(struct mt_device *td)
>> struct mt_class mt_classes[] = {
>> { 0, 0, 0, 10 }, /* MT_CLS_DEFAULT */
>> { MT_QUIRK_SLOT_IS_CONTACTID, 0, 0, 2 }, /* MT_CLS_DUAL1 */
>> + { MT_QUIRK_CYPRESS | MT_QUIRK_NOT_SEEN_MEANS_UP, 0, 0, 10 }, /* MT_CLS_CYPRESS */
>> };
>
> These could be named structs instead of an array, e.g.,
>
> static const struct mt_class mt_cls_dual1 = {
> .quirks = MT_QUIRK_SLOT_IS_CONTACTID,
> .max_contacts = 2,
> };
>
>>
>> static void mt_feature_mapping(struct hid_device *hdev, struct hid_input *hi,
>> @@ -223,6 +234,9 @@ static int mt_compute_slot(struct mt_device *td)
>> if (cls->quirks & MT_QUIRK_SLOT_IS_CONTACTID)
>> return slot_is_contactid(td);
>>
>> + if (cls->quirks & MT_QUIRK_CYPRESS)
>> + return cypress_compute_slot(td);
>> +
>> return find_slot_from_contactid(td);
>> }
>>
>> @@ -422,6 +436,11 @@ static void mt_remove(struct hid_device *hdev)
>>
>> static const struct hid_device_id mt_devices[] = {
>>
>> + /* Cypress panel */
>> + { .driver_data = MT_CLS_CYPRESS,
>> + HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS,
>> + USB_DEVICE_ID_CYPRESS_TRUETOUCH) },
>> +
>> /* PixCir-based panels */
>> { .driver_data = MT_CLS_DUAL1,
>> HID_USB_DEVICE(USB_VENDOR_ID_HANVON,
>
> Could use pointers here instead.
>
> Thanks!
> Henrik
Thanks,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists