On Mon, 5 Nov 2012, David Herrmann wrote:
> > 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.
>
> > + }
> >
> > 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.
Fully agreed, thanks.
Andre, I will happily merge the patch if you resend it with the suggested
changes applied.
Thanks,
--
Jiri Kosina
SUSE Labs
--
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
