Hi David,
On Mon, Nov 5, 2012 at 9:58 AM, David Herrmann
<dh.herrmann@xxxxxxxxxxxxxx> wrote:
> Hi Andre
>
> On Thu, Nov 1, 2012 at 10:55 PM, Andre Guedes
> <andre.guedes@xxxxxxxxxxxxx> wrote:
>> This patches adds support for handling LED input events in uhid
>> driver.
>>
>> When uhid receives an input LED event, we set the proper field,
>> create the output report and send it to userspace as UHID_OUTPUT
>> event. Others input events are sent to userspace as UHID_OUTPUT_EV
>> events.
>>
>> Signed-off-by: Andre Guedes <andre.guedes@xxxxxxxxxxxxx>
>> ---
>> drivers/hid/uhid.c | 33 +++++++++++++++++++++++++++++----
>> 1 file changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
>> index 714cd8c..e7b7cdc 100644
>> --- a/drivers/hid/uhid.c
>> +++ b/drivers/hid/uhid.c
>> @@ -122,15 +122,40 @@ static int uhid_hid_input(struct input_dev *input, unsigned int type,
>> struct uhid_device *uhid = hid->driver_data;
>> unsigned long flags;
>> struct uhid_event *ev;
>> + struct hid_field *field;
>> + struct hid_report *report;
>> + int offset;
>>
>> ev = kzalloc(sizeof(*ev), GFP_ATOMIC);
>> if (!ev)
>> return -ENOMEM;
>>
>> - ev->type = UHID_OUTPUT_EV;
>> - ev->u.output_ev.type = type;
>> - ev->u.output_ev.code = code;
>> - ev->u.output_ev.value = value;
>> + switch (type) {
>> + case EV_LED:
>> + offset = hidinput_find_field(hid, type, code, &field);
>> + if (offset == -1) {
>> + hid_warn(input, "event field not found\n");
>> + kfree(ev);
>> + return -1;
>> + }
>> +
>> + hid_set_field(field, offset, value);
>> +
>> + report = field->report;
>> +
>> + ev->type = UHID_OUTPUT;
>> + ev->u.output.rtype = UHID_OUTPUT_REPORT;
>> + hid_output_report(report, ev->u.output.data);
>> + ev->u.output.size = ((report->size - 1) >> 3) + 1 +
>> + (report->id > 0);
>> + break;
>> +
>> + default:
>> + ev->type = UHID_OUTPUT_EV;
>> + ev->u.output_ev.type = type;
>> + ev->u.output_ev.code = code;
>> + ev->u.output_ev.value = value;
>
> I think kernel-coding-style uses "break" in default rules, too.
Ok, I'll check this.
>> + }
>>
>> spin_lock_irqsave(&uhid->qlock, flags);
>> uhid_queue(uhid, ev);
>
> It looks like this was copied almost verbatim from usbhid/hid-core.c
> usb_hidinput_input_event() and __usbhid_submit_report() so I wonder
> why we do not provide a generic helper in hid-input.c.
>
> The code itself looks good, so how about pushing this into a helper in
> hid-input.c and using hid_output_raw_report(). We use this helper if
> hidinput_input_event is NULL and hid_output_raw_report() is non-NULL.
> Maybe Jiri has some comments on that, too. Adding him to CC.
I can work on that and send a new version. Lets see for Jiri have comments so.
Regards,
Andre
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
