| 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: <CAN+gG=HqecBrhdnZtfgxhByhtXf+uGELPHGsbcD9OAj6Gx7odg@mail.gmail.com>
Date: Thu, 29 Nov 2012 16:31:43 +0100
From: Benjamin Tissoires <benjamin.tissoires@...il.com>
To: Henrik Rydberg <rydberg@...omail.se>
Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>,
Jiri Kosina <jkosina@...e.cz>,
Stephane Chatty <chatty@...c.fr>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/11] HID: hid-input: export hidinput_allocation function
On Tue, Nov 27, 2012 at 9:21 PM, Henrik Rydberg <rydberg@...omail.se> wrote:
> On Fri, Nov 23, 2012 at 04:31:26PM +0100, Benjamin Tissoires wrote:
>> During the probe, third party drivers can now safely create a new
>> input devices depending on the parsing of the reports descriptor.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...il.com>
>> ---
>> drivers/hid/hid-input.c | 14 +++++++++++---
>> include/linux/hid.h | 1 +
>> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> I can think of two mechanisms that might be useful in finding a
> way to achieve this cleanly: a) Let a driver return a value telling
> whether to change input device, and b) Let a second driver have a go
> at the same device report. Some return value or state could determine
> logic in the hid core saying "we are not done with this device, try
> another driver". Or something. Just not this way, please.
Hi Henrik,
ok for the implementation of this patch series, it has to be reworked.
As for your proposals:
a) We can not rely on input_mapping because there is a temporal issue:
we already want to have the new input when we are in input_mapping.
So, this implies to create a new callback.
b) This would implies just too much work in hid-core for taking into
account a special case of one type of devices.
Here, we have in the same usb interface 2 different type of reports
coming from different sensors. It's far too different from the usual
configuration we have with legacy devices: when we have several hid
drivers handling the same usb device, it was when hardware makers
split the different sensors in different interfaces. This situation is
correctly handled in the usb subsystem and the hid layer has only to
deal with one driver at a time for a specific interface.
So, in the next series, I propose a new callback ("new_report" -- the
name is awful, but I can not manage to find another one ATM) which
will be called before we call input_mapping for a whole report.
The driver will then have the possibility to:
- continue normally (default behavior)
- ask for a new input device
- skip the entire report
Anyway, Henrik , could you also have a look at patches 7 to 11, they
have nothing to do with pen support, and I'm sure that you want to say
something on them too.
Cheers,
Benjamin
>
>>
>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>> index eea02b0..b0572d0 100644
>> --- a/drivers/hid/hid-input.c
>> +++ b/drivers/hid/hid-input.c
>> @@ -1163,7 +1163,7 @@ static void report_features(struct hid_device *hid)
>> }
>> }
>>
>> -static struct hid_input *hidinput_allocate(struct hid_device *hid)
>> +struct hid_input *hidinput_allocate(struct hid_device *hid)
>> {
>> struct hid_input *hidinput = kzalloc(sizeof(*hidinput), GFP_KERNEL);
>> struct input_dev *input_dev = input_allocate_device();
>> @@ -1190,10 +1190,16 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid)
>> input_dev->id.version = hid->version;
>> input_dev->dev.parent = hid->dev.parent;
>> hidinput->input = input_dev;
>> - list_add_tail(&hidinput->list, &hid->inputs);
>> + list_add(&hidinput->list, &hid->inputs);
>>
>> return hidinput;
>> }
>> +EXPORT_SYMBOL_GPL(hidinput_allocate);
>> +
>> +static struct hid_input *hid_get_latest_hidinput(struct hid_device *hid)
>> +{
>> + return list_first_entry(&hid->inputs, struct hid_input, list);
>> +}
>>
>> /*
>> * Register the input device; print a message.
>> @@ -1243,9 +1249,11 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
>> }
>>
>> for (i = 0; i < report->maxfield; i++)
>> - for (j = 0; j < report->field[i]->maxusage; j++)
>> + for (j = 0; j < report->field[i]->maxusage; j++) {
>> + hidinput = hid_get_latest_hidinput(hid);
>> hidinput_configure_usage(hidinput, report->field[i],
>> report->field[i]->usage + j);
>> + }
>>
>> if (hid->quirks & HID_QUIRK_MULTI_INPUT) {
>> /* This will leave hidinput NULL, so that it
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index d2c42dd..42b02d6 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -701,6 +701,7 @@ extern void hidinput_hid_event(struct hid_device *, struct hid_field *, struct h
>> extern void hidinput_report_event(struct hid_device *hid, struct hid_report *report);
>> extern int hidinput_connect(struct hid_device *hid, unsigned int force);
>> extern void hidinput_disconnect(struct hid_device *);
>> +extern struct hid_input *hidinput_allocate(struct hid_device *hid);
>>
>> int hid_set_field(struct hid_field *, unsigned, __s32);
>> int hid_input_report(struct hid_device *, int type, u8 *, int, int);
>> --
>> 1.8.0
>>
>
> Thanks,
> Henrik
--
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