When the multi input quirk is set, there is a new input device
created for every feature report. Since the idea is to present
features per hid device, not per input device, revert back to
the original report loop and change the feature_mapping() callback
to not take the input device as argument.
Signed-off-by: Henrik Rydberg <[email protected]>
---
Hi Jiri, Benjamin,
It seems the feature report callback was a bit intrusive, after all;
for some devices such as ntrig, with multi input, there are additional
input devices created. The following patch seems to fix the problem,
but it has not been tested on any device using the hid-multitouch
driver.
Thanks,
Henrik
drivers/hid/hid-input.c | 30 +++++++++++++++++++++---------
drivers/hid/hid-multitouch.c | 2 +-
include/linux/hid.h | 2 +-
3 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 7f552bf..ebcc02a 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -290,14 +290,6 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
goto ignore;
}
- if (field->report_type == HID_FEATURE_REPORT) {
- if (device->driver->feature_mapping) {
- device->driver->feature_mapping(device, hidinput, field,
- usage);
- }
- goto ignore;
- }
-
if (device->driver->input_mapping) {
int ret = device->driver->input_mapping(device, hidinput, field,
usage, &bit, &max);
@@ -835,6 +827,24 @@ static void hidinput_close(struct input_dev *dev)
hid_hw_close(hid);
}
+static void report_features(struct hid_device *hid)
+{
+ struct hid_driver *drv = hid->driver;
+ struct hid_report_enum *rep_enum;
+ struct hid_report *rep;
+ int i, j;
+
+ if (!drv->feature_mapping)
+ return;
+
+ rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
+ list_for_each_entry(rep, &rep_enum->report_list, list)
+ for (i = 0; i < rep->maxfield; i++)
+ for (j = 0; j < rep->field[i]->maxusage; j++)
+ drv->feature_mapping(hid, rep->field[i],
+ rep->field[i]->usage + j);
+}
+
/*
* Register the input device; print a message.
* Configure the input layer interface
@@ -863,7 +873,9 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
return -1;
}
- for (k = HID_INPUT_REPORT; k <= HID_FEATURE_REPORT; k++) {
+ report_features(hid);
+
+ for (k = HID_INPUT_REPORT; k <= HID_OUTPUT_REPORT; k++) {
if (k == HID_OUTPUT_REPORT &&
hid->quirks & HID_QUIRK_SKIP_OUTPUT_REPORTS)
continue;
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 07d3183..2bbc954 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -122,7 +122,7 @@ struct mt_class mt_classes[] = {
{ }
};
-static void mt_feature_mapping(struct hid_device *hdev, struct hid_input *hi,
+static void mt_feature_mapping(struct hid_device *hdev,
struct hid_field *field, struct hid_usage *usage)
{
if (usage->hid == HID_DG_INPUTMODE) {
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d91c25e..fc5faf6 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -638,7 +638,7 @@ struct hid_driver {
struct hid_input *hidinput, struct hid_field *field,
struct hid_usage *usage, unsigned long **bit, int *max);
void (*feature_mapping)(struct hid_device *hdev,
- struct hid_input *hidinput, struct hid_field *field,
+ struct hid_field *field,
struct hid_usage *usage);
#ifdef CONFIG_PM
int (*suspend)(struct hid_device *hdev, pm_message_t message);
--
1.7.4.1
Hi Henrik,
Thanks for spotting this. BTW, I'm not happy with your solution.
You are sending the feature report before creating the struct
hid_input. To be consistent with the rest, we have to keep the same
signature. Today, the code does not make any use of it. But I use it
in my devel branch to auto-detect the maximum contact count of the
device. This was the safest place to call input_mt_init_slots.
How about adding hidinput as an argument to report_features
and calling it after the " for (k = HID_INPUT_REPORT; k <=
HID_OUTPUT_REPORT; k++) {" loop with
list_for_each_entry_safe(hidinput, next, &hid->inputs, list)
report_features(hid, hidinput);
I did not even try to compile it right now (I don't have any
multitouch device right now) but I'll be able to make further tests
tomorrow.
Cheers,
Benjamin
On Thu, Feb 24, 2011 at 19:30, Henrik Rydberg <[email protected]> wrote:
> When the multi input quirk is set, there is a new input device
> created for every feature report. Since the idea is to present
> features per hid device, not per input device, revert back to
> the original report loop and change the feature_mapping() callback
> to not take the input device as argument.
>
> Signed-off-by: Henrik Rydberg <[email protected]>
> ---
> Hi Jiri, Benjamin,
>
> It seems the feature report callback was a bit intrusive, after all;
> for some devices such as ntrig, with multi input, there are additional
> input devices created. The following patch seems to fix the problem,
> but it has not been tested on any device using the hid-multitouch
> driver.
>
> Thanks,
> Henrik
>
> ?drivers/hid/hid-input.c ? ? ?| ? 30 +++++++++++++++++++++---------
> ?drivers/hid/hid-multitouch.c | ? ?2 +-
> ?include/linux/hid.h ? ? ? ? ?| ? ?2 +-
> ?3 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 7f552bf..ebcc02a 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -290,14 +290,6 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> ? ? ? ? ? ? ? ?goto ignore;
> ? ? ? ?}
>
> - ? ? ? if (field->report_type == HID_FEATURE_REPORT) {
> - ? ? ? ? ? ? ? if (device->driver->feature_mapping) {
> - ? ? ? ? ? ? ? ? ? ? ? device->driver->feature_mapping(device, hidinput, field,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? usage);
> - ? ? ? ? ? ? ? }
> - ? ? ? ? ? ? ? goto ignore;
> - ? ? ? }
> -
> ? ? ? ?if (device->driver->input_mapping) {
> ? ? ? ? ? ? ? ?int ret = device->driver->input_mapping(device, hidinput, field,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?usage, &bit, &max);
> @@ -835,6 +827,24 @@ static void hidinput_close(struct input_dev *dev)
> ? ? ? ?hid_hw_close(hid);
> ?}
>
> +static void report_features(struct hid_device *hid)
> +{
> + ? ? ? struct hid_driver *drv = hid->driver;
> + ? ? ? struct hid_report_enum *rep_enum;
> + ? ? ? struct hid_report *rep;
> + ? ? ? int i, j;
> +
> + ? ? ? if (!drv->feature_mapping)
> + ? ? ? ? ? ? ? return;
> +
> + ? ? ? rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
> + ? ? ? list_for_each_entry(rep, &rep_enum->report_list, list)
> + ? ? ? ? ? ? ? for (i = 0; i < rep->maxfield; i++)
> + ? ? ? ? ? ? ? ? ? ? ? for (j = 0; j < rep->field[i]->maxusage; j++)
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? drv->feature_mapping(hid, rep->field[i],
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?rep->field[i]->usage + j);
> +}
> +
> ?/*
> ?* Register the input device; print a message.
> ?* Configure the input layer interface
> @@ -863,7 +873,9 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
> ? ? ? ? ? ? ? ? ? ? ? ?return -1;
> ? ? ? ?}
>
> - ? ? ? for (k = HID_INPUT_REPORT; k <= HID_FEATURE_REPORT; k++) {
> + ? ? ? report_features(hid);
> +
> + ? ? ? for (k = HID_INPUT_REPORT; k <= HID_OUTPUT_REPORT; k++) {
> ? ? ? ? ? ? ? ?if (k == HID_OUTPUT_REPORT &&
> ? ? ? ? ? ? ? ? ? ? ? ?hid->quirks & HID_QUIRK_SKIP_OUTPUT_REPORTS)
> ? ? ? ? ? ? ? ? ? ? ? ?continue;
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 07d3183..2bbc954 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -122,7 +122,7 @@ struct mt_class mt_classes[] = {
> ? ? ? ?{ }
> ?};
>
> -static void mt_feature_mapping(struct hid_device *hdev, struct hid_input *hi,
> +static void mt_feature_mapping(struct hid_device *hdev,
> ? ? ? ? ? ? ? ?struct hid_field *field, struct hid_usage *usage)
> ?{
> ? ? ? ?if (usage->hid == HID_DG_INPUTMODE) {
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index d91c25e..fc5faf6 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -638,7 +638,7 @@ struct hid_driver {
> ? ? ? ? ? ? ? ? ? ? ? ?struct hid_input *hidinput, struct hid_field *field,
> ? ? ? ? ? ? ? ? ? ? ? ?struct hid_usage *usage, unsigned long **bit, int *max);
> ? ? ? ?void (*feature_mapping)(struct hid_device *hdev,
> - ? ? ? ? ? ? ? ? ? ? ? struct hid_input *hidinput, struct hid_field *field,
> + ? ? ? ? ? ? ? ? ? ? ? struct hid_field *field,
> ? ? ? ? ? ? ? ? ? ? ? ?struct hid_usage *usage);
> ?#ifdef CONFIG_PM
> ? ? ? ?int (*suspend)(struct hid_device *hdev, pm_message_t message);
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
On Thu, Feb 24, 2011 at 08:50:55PM +0100, Benjamin Tissoires wrote:
> Hi Henrik,
>
> Thanks for spotting this. BTW, I'm not happy with your solution.
>
> You are sending the feature report before creating the struct
> hid_input. To be consistent with the rest, we have to keep the same
> signature. Today, the code does not make any use of it. But I use it
> in my devel branch to auto-detect the maximum contact count of the
> device. This was the safest place to call input_mt_init_slots.
Well, the whole point is "which input device". It would only be
well-defined when the hid device has a single input device. The
feature callback could be called last, however, if that helps.
> How about adding hidinput as an argument to report_features
>
> and calling it after the " for (k = HID_INPUT_REPORT; k <=
> HID_OUTPUT_REPORT; k++) {" loop with
>
> list_for_each_entry_safe(hidinput, next, &hid->inputs, list)
> report_features(hid, hidinput);
>
> I did not even try to compile it right now (I don't have any
> multitouch device right now) but I'll be able to make further tests
> tomorrow.
Thanks,
Henrik
Hi Henrik,
after some quick tests, I can deal with our two options (changing
feature_mapping signature or not, and so calling feature_mapping
before or after input_mapping).
So, my questions are:
- do we really need to change feature_mapping signature?
- is feature_mapping tied to an input or to a device?
Cheers,
Benjamin
On Thu, Feb 24, 2011 at 21:43, Henrik Rydberg <[email protected]> wrote:
> On Thu, Feb 24, 2011 at 08:50:55PM +0100, Benjamin Tissoires wrote:
>> Hi Henrik,
>>
>> Thanks for spotting this. BTW, I'm not happy with your solution.
>>
>> You are sending the feature report before creating the struct
>> hid_input. To be consistent with the rest, we have to keep the same
>> signature. Today, the code does not make any use of it. But I use it
>> in my devel branch to auto-detect the maximum contact count of the
>> device. This was the safest place to call input_mt_init_slots.
>
> Well, the whole point is "which input device". It would only be
> well-defined when the hid device has a single input device. The
> feature callback could be called last, however, if that helps.
>
>> How about adding hidinput as an argument to report_features
>>
>> and calling it after the " for (k = HID_INPUT_REPORT; k <=
>> HID_OUTPUT_REPORT; k++) {" loop with
>>
>> list_for_each_entry_safe(hidinput, next, &hid->inputs, list)
>> ? ? report_features(hid, hidinput);
>>
>> I did not even try to compile it right now (I don't have any
>> multitouch device right now) but I'll be able to make further tests
>> tomorrow.
>
> Thanks,
> Henrik
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
Hi Benjamin,
> after some quick tests, I can deal with our two options (changing
> feature_mapping signature or not, and so calling feature_mapping
> before or after input_mapping).
Good, good.
> So, my questions are:
> - do we really need to change feature_mapping signature?
> - is feature_mapping tied to an input or to a device?
The input, output and feature reports are all found on the same level
in the HID protocol, so it makes sense to associate all reports with
the device itself, without any assumed association between different
reports. From a practical point of view, we may assign different input
nodes (input devices) to different input reports, so it is clear that
the mapping between hid device and input device is not 1-to-1.
For output devices, the only supported case is EV_LED, which passes
events to the input device. It is probably assumed that
HID_QUIRK_MULTI_INPUT is false for those devices. Jiri?
For feature reports, the lack of 1-1 correspondence suggests the input
device is ill-defined, and should therefore be left out of the
argument list of the callback. However, I will leave that to Jiri or
anyone more experienced with the HID layer.
Thanks,
Henrik
On Thu, 24 Feb 2011, Henrik Rydberg wrote:
> When the multi input quirk is set, there is a new input device
> created for every feature report. Since the idea is to present
> features per hid device, not per input device, revert back to
> the original report loop and change the feature_mapping() callback
> to not take the input device as argument.
>
> Signed-off-by: Henrik Rydberg <[email protected]>
Hi Henrik,
I agree with this implementation.
Benjamin, did you manage to do the tests with hid-multitouch driver so
that I could put your Tested-by: to the patch and apply it?
Thanks.
> ---
> Hi Jiri, Benjamin,
>
> It seems the feature report callback was a bit intrusive, after all;
> for some devices such as ntrig, with multi input, there are additional
> input devices created. The following patch seems to fix the problem,
> but it has not been tested on any device using the hid-multitouch
> driver.
>
> Thanks,
> Henrik
>
> drivers/hid/hid-input.c | 30 +++++++++++++++++++++---------
> drivers/hid/hid-multitouch.c | 2 +-
> include/linux/hid.h | 2 +-
> 3 files changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 7f552bf..ebcc02a 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -290,14 +290,6 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> goto ignore;
> }
>
> - if (field->report_type == HID_FEATURE_REPORT) {
> - if (device->driver->feature_mapping) {
> - device->driver->feature_mapping(device, hidinput, field,
> - usage);
> - }
> - goto ignore;
> - }
> -
> if (device->driver->input_mapping) {
> int ret = device->driver->input_mapping(device, hidinput, field,
> usage, &bit, &max);
> @@ -835,6 +827,24 @@ static void hidinput_close(struct input_dev *dev)
> hid_hw_close(hid);
> }
>
> +static void report_features(struct hid_device *hid)
> +{
> + struct hid_driver *drv = hid->driver;
> + struct hid_report_enum *rep_enum;
> + struct hid_report *rep;
> + int i, j;
> +
> + if (!drv->feature_mapping)
> + return;
> +
> + rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
> + list_for_each_entry(rep, &rep_enum->report_list, list)
> + for (i = 0; i < rep->maxfield; i++)
> + for (j = 0; j < rep->field[i]->maxusage; j++)
> + drv->feature_mapping(hid, rep->field[i],
> + rep->field[i]->usage + j);
> +}
> +
> /*
> * Register the input device; print a message.
> * Configure the input layer interface
> @@ -863,7 +873,9 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
> return -1;
> }
>
> - for (k = HID_INPUT_REPORT; k <= HID_FEATURE_REPORT; k++) {
> + report_features(hid);
> +
> + for (k = HID_INPUT_REPORT; k <= HID_OUTPUT_REPORT; k++) {
> if (k == HID_OUTPUT_REPORT &&
> hid->quirks & HID_QUIRK_SKIP_OUTPUT_REPORTS)
> continue;
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 07d3183..2bbc954 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -122,7 +122,7 @@ struct mt_class mt_classes[] = {
> { }
> };
>
> -static void mt_feature_mapping(struct hid_device *hdev, struct hid_input *hi,
> +static void mt_feature_mapping(struct hid_device *hdev,
> struct hid_field *field, struct hid_usage *usage)
> {
> if (usage->hid == HID_DG_INPUTMODE) {
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index d91c25e..fc5faf6 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -638,7 +638,7 @@ struct hid_driver {
> struct hid_input *hidinput, struct hid_field *field,
> struct hid_usage *usage, unsigned long **bit, int *max);
> void (*feature_mapping)(struct hid_device *hdev,
> - struct hid_input *hidinput, struct hid_field *field,
> + struct hid_field *field,
> struct hid_usage *usage);
> #ifdef CONFIG_PM
> int (*suspend)(struct hid_device *hdev, pm_message_t message);
> --
> 1.7.4.1
>
--
Jiri Kosina
SUSE Labs, Novell Inc.
Hi Jiri,
Tested-by: Benjamin Tissoires <[email protected]>
Acked-by: Benjamin Tissoires <[email protected]>
Cheers,
Benjamin
On Tue, Mar 1, 2011 at 17:03, Jiri Kosina <[email protected]> wrote:
> On Thu, 24 Feb 2011, Henrik Rydberg wrote:
>
>> When the multi input quirk is set, there is a new input device
>> created for every feature report. Since the idea is to present
>> features per hid device, not per input device, revert back to
>> the original report loop and change the feature_mapping() callback
>> to not take the input device as argument.
>>
>> Signed-off-by: Henrik Rydberg <[email protected]>
>
> Hi Henrik,
>
> I agree with this implementation.
>
> Benjamin, did you manage to do the tests with hid-multitouch driver so
> that I could put your Tested-by: to the patch and apply it?
>
> Thanks.
>
>> ---
>> Hi Jiri, Benjamin,
>>
>> It seems the feature report callback was a bit intrusive, after all;
>> for some devices such as ntrig, with multi input, there are additional
>> input devices created. The following patch seems to fix the problem,
>> but it has not been tested on any device using the hid-multitouch
>> driver.
>>
>> Thanks,
>> Henrik
>>
>> ?drivers/hid/hid-input.c ? ? ?| ? 30 +++++++++++++++++++++---------
>> ?drivers/hid/hid-multitouch.c | ? ?2 +-
>> ?include/linux/hid.h ? ? ? ? ?| ? ?2 +-
>> ?3 files changed, 23 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>> index 7f552bf..ebcc02a 100644
>> --- a/drivers/hid/hid-input.c
>> +++ b/drivers/hid/hid-input.c
>> @@ -290,14 +290,6 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
>> ? ? ? ? ? ? ? goto ignore;
>> ? ? ? }
>>
>> - ? ? if (field->report_type == HID_FEATURE_REPORT) {
>> - ? ? ? ? ? ? if (device->driver->feature_mapping) {
>> - ? ? ? ? ? ? ? ? ? ? device->driver->feature_mapping(device, hidinput, field,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? usage);
>> - ? ? ? ? ? ? }
>> - ? ? ? ? ? ? goto ignore;
>> - ? ? }
>> -
>> ? ? ? if (device->driver->input_mapping) {
>> ? ? ? ? ? ? ? int ret = device->driver->input_mapping(device, hidinput, field,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? usage, &bit, &max);
>> @@ -835,6 +827,24 @@ static void hidinput_close(struct input_dev *dev)
>> ? ? ? hid_hw_close(hid);
>> ?}
>>
>> +static void report_features(struct hid_device *hid)
>> +{
>> + ? ? struct hid_driver *drv = hid->driver;
>> + ? ? struct hid_report_enum *rep_enum;
>> + ? ? struct hid_report *rep;
>> + ? ? int i, j;
>> +
>> + ? ? if (!drv->feature_mapping)
>> + ? ? ? ? ? ? return;
>> +
>> + ? ? rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
>> + ? ? list_for_each_entry(rep, &rep_enum->report_list, list)
>> + ? ? ? ? ? ? for (i = 0; i < rep->maxfield; i++)
>> + ? ? ? ? ? ? ? ? ? ? for (j = 0; j < rep->field[i]->maxusage; j++)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? drv->feature_mapping(hid, rep->field[i],
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?rep->field[i]->usage + j);
>> +}
>> +
>> ?/*
>> ? * Register the input device; print a message.
>> ? * Configure the input layer interface
>> @@ -863,7 +873,9 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
>> ? ? ? ? ? ? ? ? ? ? ? return -1;
>> ? ? ? }
>>
>> - ? ? for (k = HID_INPUT_REPORT; k <= HID_FEATURE_REPORT; k++) {
>> + ? ? report_features(hid);
>> +
>> + ? ? for (k = HID_INPUT_REPORT; k <= HID_OUTPUT_REPORT; k++) {
>> ? ? ? ? ? ? ? if (k == HID_OUTPUT_REPORT &&
>> ? ? ? ? ? ? ? ? ? ? ? hid->quirks & HID_QUIRK_SKIP_OUTPUT_REPORTS)
>> ? ? ? ? ? ? ? ? ? ? ? continue;
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 07d3183..2bbc954 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -122,7 +122,7 @@ struct mt_class mt_classes[] = {
>> ? ? ? { }
>> ?};
>>
>> -static void mt_feature_mapping(struct hid_device *hdev, struct hid_input *hi,
>> +static void mt_feature_mapping(struct hid_device *hdev,
>> ? ? ? ? ? ? ? struct hid_field *field, struct hid_usage *usage)
>> ?{
>> ? ? ? if (usage->hid == HID_DG_INPUTMODE) {
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index d91c25e..fc5faf6 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -638,7 +638,7 @@ struct hid_driver {
>> ? ? ? ? ? ? ? ? ? ? ? struct hid_input *hidinput, struct hid_field *field,
>> ? ? ? ? ? ? ? ? ? ? ? struct hid_usage *usage, unsigned long **bit, int *max);
>> ? ? ? void (*feature_mapping)(struct hid_device *hdev,
>> - ? ? ? ? ? ? ? ? ? ? struct hid_input *hidinput, struct hid_field *field,
>> + ? ? ? ? ? ? ? ? ? ? struct hid_field *field,
>> ? ? ? ? ? ? ? ? ? ? ? struct hid_usage *usage);
>> ?#ifdef CONFIG_PM
>> ? ? ? int (*suspend)(struct hid_device *hdev, pm_message_t message);
>> --
>> 1.7.4.1
>>
>
> --
> Jiri Kosina
> SUSE Labs, Novell Inc.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
On Tue, 1 Mar 2011, Benjamin Tissoires wrote:
> Tested-by: Benjamin Tissoires <[email protected]>
> Acked-by: Benjamin Tissoires <[email protected]>
Applied, thanks.
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Fri, Feb 25, 2011 at 06:19:11PM +0100, Henrik Rydberg wrote:
> Hi Benjamin,
>
> > after some quick tests, I can deal with our two options (changing
> > feature_mapping signature or not, and so calling feature_mapping
> > before or after input_mapping).
>
> Good, good.
>
> > So, my questions are:
> > - do we really need to change feature_mapping signature?
> > - is feature_mapping tied to an input or to a device?
>
> The input, output and feature reports are all found on the same level
> in the HID protocol, so it makes sense to associate all reports with
> the device itself, without any assumed association between different
> reports. From a practical point of view, we may assign different input
> nodes (input devices) to different input reports, so it is clear that
> the mapping between hid device and input device is not 1-to-1.
>
> For output devices, the only supported case is EV_LED, which passes
> events to the input device. It is probably assumed that
> HID_QUIRK_MULTI_INPUT is false for those devices. Jiri?
>
I am probably late to the party fut the above is not true. Here is an
example of an USB keyboard (wih LEDs) that is split into two:
I: Bus=0003 Vendor=046d Product=c30e Version=0110
N: Name="Logitech HID compliant keyboard"
P: Phys=usb-0000:00:1d.0-1.2/input0
S:
Sysfs=/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/input/input3
U: Uniq=
H: Handlers=sysrq kbd event3
B: PROP=0
B: EV=120013
B: KEY=1000000000007 ff800000000007ff febeffdff3cfffff fffffffffffffffe
B: MSC=10
B: LED=7
I: Bus=0003 Vendor=046d Product=c30e Version=0110
N: Name="Logitech HID compliant keyboard"
P: Phys=usb-0000:00:1d.0-1.2/input1
S:
Sysfs=/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.1/input/input4
U: Uniq=
H: Handlers=kbd event4
B: PROP=0
B: EV=13
B: KEY=fff ffffffffffffffff 2000000 387ad800d001 1e000000000000 0
B: MSC=10
This was done, most likely, because Logitech decided to reuse usage codes
for different keys.
Thanks.
--
Dmitry
Hi Dmitry,
> > For output devices, the only supported case is EV_LED, which passes
> > events to the input device. It is probably assumed that
> > HID_QUIRK_MULTI_INPUT is false for those devices. Jiri?
> >
>
> I am probably late to the party fut the above is not true. Here is an
> example of an USB keyboard (wih LEDs) that is split into two:
>
> I: Bus=0003 Vendor=046d Product=c30e Version=0110
> N: Name="Logitech HID compliant keyboard"
> P: Phys=usb-0000:00:1d.0-1.2/input0
> S:
> Sysfs=/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/input/input3
> U: Uniq=
> H: Handlers=sysrq kbd event3
> B: PROP=0
> B: EV=120013
> B: KEY=1000000000007 ff800000000007ff febeffdff3cfffff fffffffffffffffe
> B: MSC=10
> B: LED=7
>
> I: Bus=0003 Vendor=046d Product=c30e Version=0110
> N: Name="Logitech HID compliant keyboard"
> P: Phys=usb-0000:00:1d.0-1.2/input1
> S:
> Sysfs=/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.1/input/input4
> U: Uniq=
> H: Handlers=kbd event4
> B: PROP=0
> B: EV=13
> B: KEY=fff ffffffffffffffff 2000000 387ad800d001 1e000000000000 0
> B: MSC=10
>
> This was done, most likely, because Logitech decided to reuse usage codes
> for different keys.
This looks like different interfaces though, which should be fine. It
is only in the odd case of mixed input and output reports on the same
interface that the MULTI_INPUT quirk would ever have any strange
effect.
Cheers,
Henrik
On Wed, Mar 02, 2011 at 04:02:27PM +0100, Henrik Rydberg wrote:
> Hi Dmitry,
>
> > > For output devices, the only supported case is EV_LED, which passes
> > > events to the input device. It is probably assumed that
> > > HID_QUIRK_MULTI_INPUT is false for those devices. Jiri?
> > >
> >
> > I am probably late to the party fut the above is not true. Here is an
> > example of an USB keyboard (wih LEDs) that is split into two:
> >
> > I: Bus=0003 Vendor=046d Product=c30e Version=0110
> > N: Name="Logitech HID compliant keyboard"
> > P: Phys=usb-0000:00:1d.0-1.2/input0
> > S:
> > Sysfs=/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.0/input/input3
> > U: Uniq=
> > H: Handlers=sysrq kbd event3
> > B: PROP=0
> > B: EV=120013
> > B: KEY=1000000000007 ff800000000007ff febeffdff3cfffff fffffffffffffffe
> > B: MSC=10
> > B: LED=7
> >
> > I: Bus=0003 Vendor=046d Product=c30e Version=0110
> > N: Name="Logitech HID compliant keyboard"
> > P: Phys=usb-0000:00:1d.0-1.2/input1
> > S:
> > Sysfs=/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.2/2-1.2:1.1/input/input4
> > U: Uniq=
> > H: Handlers=kbd event4
> > B: PROP=0
> > B: EV=13
> > B: KEY=fff ffffffffffffffff 2000000 387ad800d001 1e000000000000 0
> > B: MSC=10
> >
> > This was done, most likely, because Logitech decided to reuse usage codes
> > for different keys.
>
> This looks like different interfaces though, which should be fine. It
> is only in the odd case of mixed input and output reports on the same
> interface that the MULTI_INPUT quirk would ever have any strange
> effect.
>
Gah, when I looked at it before posting I could swore they were on the
same interface. OK, just ignore me...
Thanks.
--
Dmitry
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
A bit on the late side, I can confirm this reverts the annoying proliferation of
input nodes mapped for ntrig hardware.
The new firmwares seem to really enjoy exposing bugs, more on that later.
Rafi
On 03/01/11 11:26, Jiri Kosina wrote:
> On Tue, 1 Mar 2011, Benjamin Tissoires wrote:
>
>> Tested-by: Benjamin Tissoires <[email protected]>
>> Acked-by: Benjamin Tissoires <[email protected]>
>
> Applied, thanks.
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQIcBAEBAgAGBQJNdaYCAAoJEPILXytRLnK214MP/RC61mIowTq/2o4oWp8JQvy3
l7zZumg3v/tInK7OS1XciWzMQl+zH09FfzutgAEULO7hGhN1RFiP8qiBHtuMrMVg
80FWXIl24SykGe/MwgQPi9esipTx6ee8W7hmIpFOfyVIZIl/F4afdPe3a9fFhuH+
0H5yn7+tUw5n7wglD57xkeDW4a3RD3ehf++ehZhmmwWoPKhr9119XN9UDAmHRBX4
1DfdtOTtsVA161DJkiMipEO5KoeD6dH1VMiCBhxkn+WLpVPHP6zhV/3KUhIDEFDd
oWMvldltbpe8UHQmh2KwiwrtJib0bLIrrmzS5EZkVm8bT2EFlM6ZsYt5tkp6NDwl
/bD1gBrMd14KGwVjb7XflfprlUHbbgpvjDUF7L5B3I955LWsowFoFqjquoV6+Phj
KOJRbUABvDa6esRpg79aAjD59Uh3adnmFydFFcHBHNc3Kb6ROJ4LVGlR1Zxt+QoT
rEXEYuxPZwxJ0zZ08yO2Si+FaoPVOrZC7S2XNhv64MMpISGBWEIvRWowDPjpgutY
v2aVdGiWskPsjqxjVXyR9MTqOgdXrp2JoZy2U2Jq7AV3vzwQfoJ+BJjgEAwkVmO3
XpKuxJ2s5vYPTlxPga3PqUPT2r7PzfjakTcXM4UQ3f7iQy4RZ4qr4q6+4aI3saVy
kvTrsCQV6SmmbbZuaFLh
=h0h3
-----END PGP SIGNATURE-----
On Tue, Mar 01, 2011 at 05:26:41PM +0100, Jiri Kosina wrote:
> On Tue, 1 Mar 2011, Benjamin Tissoires wrote:
>
> > Tested-by: Benjamin Tissoires <[email protected]>
> > Acked-by: Benjamin Tissoires <[email protected]>
>
> Applied, thanks.
Hi Jiri,
I was wondering if this patch will appear in the final 2.6.38? It
seems the regression it fixes is pretty severe for ntrig devices, and
once out there, the callback argument change will create problems for
add-on modules such as dkms modules.
Thanks,
Henrik
On Thu, 10 Mar 2011, Henrik Rydberg wrote:
> > > Tested-by: Benjamin Tissoires <[email protected]>
> > > Acked-by: Benjamin Tissoires <[email protected]>
> >
> > Applied, thanks.
>
> Hi Jiri,
>
> I was wondering if this patch will appear in the final 2.6.38? It
> seems the regression it fixes is pretty severe for ntrig devices, and
> once out there, the callback argument change will create problems for
> add-on modules such as dkms modules.
Hi Henrik,
I am not sure Linus will be taking it for .38 still, as he is now offline
and will be releasing final .38 after he appears, so it's unlikely that
he'll take pull requests unless it's something super-severe.
I'll put it on my list of commits to be pushed out to 38-stable right
away. Does that make sense?
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
> > I was wondering if this patch will appear in the final 2.6.38? It
> > seems the regression it fixes is pretty severe for ntrig devices, and
> > once out there, the callback argument change will create problems for
> > add-on modules such as dkms modules.
>
> Hi Henrik,
>
> I am not sure Linus will be taking it for .38 still, as he is now offline
> and will be releasing final .38 after he appears, so it's unlikely that
> he'll take pull requests unless it's something super-severe.
>
> I'll put it on my list of commits to be pushed out to 38-stable right
> away. Does that make sense?
Yes, that will be great. Thanks!
Henrik