| Message ID | 1361979498-24446-2-git-send-email-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Wed, 27 Feb 2013, Benjamin Tissoires wrote: > Some drivers send the idle command directly to underlying device, > creating an unwanted dependency on the underlying transport layer. > This patch adds hid_hw_idle() to the interface, thereby removing > usbhid from the lion share of the drivers. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > drivers/hid/usbhid/hid-core.c | 15 +++++++++++++++ > include/linux/hid.h | 19 +++++++++++++++++++ > 2 files changed, 34 insertions(+) > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c > index 420466b..effcd3d 100644 > --- a/drivers/hid/usbhid/hid-core.c > +++ b/drivers/hid/usbhid/hid-core.c > @@ -1253,6 +1253,20 @@ static void usbhid_request(struct hid_device *hid, struct hid_report *rep, int r > } > } > > +static int usbhid_idle(struct hid_device *hid, int report, int idle, > + int reqtype) Where does the need for passing the report argument come from please?
On Thu, Mar 7, 2013 at 3:26 PM, Jiri Kosina <jkosina@suse.cz> wrote: > On Wed, 27 Feb 2013, Benjamin Tissoires wrote: > >> Some drivers send the idle command directly to underlying device, >> creating an unwanted dependency on the underlying transport layer. >> This patch adds hid_hw_idle() to the interface, thereby removing >> usbhid from the lion share of the drivers. >> >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> >> --- >> drivers/hid/usbhid/hid-core.c | 15 +++++++++++++++ >> include/linux/hid.h | 19 +++++++++++++++++++ >> 2 files changed, 34 insertions(+) >> >> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c >> index 420466b..effcd3d 100644 >> --- a/drivers/hid/usbhid/hid-core.c >> +++ b/drivers/hid/usbhid/hid-core.c >> @@ -1253,6 +1253,20 @@ static void usbhid_request(struct hid_device *hid, struct hid_report *rep, int r >> } >> } >> >> +static int usbhid_idle(struct hid_device *hid, int report, int idle, >> + int reqtype) > > Where does the need for passing the report argument come from please? Well, I haven't checked in the USB spec, but hid_set_idle() in usbhid/hid-core.c does require the reportID, so I add it to the request. I just gave a quick look at the HID/I2C spec, and it also requires the report ID. The set_idle request is report specific. Benjamin > > -- > Jiri Kosina > SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 7 Mar 2013, Benjamin Tissoires wrote: > >> +static int usbhid_idle(struct hid_device *hid, int report, int idle, > >> + int reqtype) > > > > Where does the need for passing the report argument come from please? > > Well, I haven't checked in the USB spec, but hid_set_idle() in > usbhid/hid-core.c does require the reportID, so I add it to the > request. > I just gave a quick look at the HID/I2C spec, and it also requires the > report ID. The set_idle request is report specific. You are right, I have missed the special meaning of lower byte of wValue. Okay, I think it makes sense. I will be taking the patches. Thanks Benjamin.
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index 420466b..effcd3d 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1253,6 +1253,20 @@ static void usbhid_request(struct hid_device *hid, struct hid_report *rep, int r } } +static int usbhid_idle(struct hid_device *hid, int report, int idle, + int reqtype) +{ + struct usb_device *dev = hid_to_usb_dev(hid); + struct usb_interface *intf = to_usb_interface(hid->dev.parent); + struct usb_host_interface *interface = intf->cur_altsetting; + int ifnum = interface->desc.bInterfaceNumber; + + if (reqtype != HID_REQ_SET_IDLE) + return -EINVAL; + + return hid_set_idle(dev, ifnum, report, idle); +} + static struct hid_ll_driver usb_hid_driver = { .parse = usbhid_parse, .start = usbhid_start, @@ -1263,6 +1277,7 @@ static struct hid_ll_driver usb_hid_driver = { .hidinput_input_event = usb_hidinput_input_event, .request = usbhid_request, .wait = usbhid_wait_io, + .idle = usbhid_idle, }; static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id) diff --git a/include/linux/hid.h b/include/linux/hid.h index 7071eb3..863744c 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -664,6 +664,7 @@ struct hid_driver { * shouldn't allocate anything to not leak memory * @request: send report request to device (e.g. feature report) * @wait: wait for buffered io to complete (send/recv reports) + * @idle: send idle request to device */ struct hid_ll_driver { int (*start)(struct hid_device *hdev); @@ -683,6 +684,7 @@ struct hid_ll_driver { struct hid_report *report, int reqtype); int (*wait)(struct hid_device *hdev); + int (*idle)(struct hid_device *hdev, int report, int idle, int reqtype); }; @@ -907,6 +909,23 @@ static inline void hid_hw_request(struct hid_device *hdev, } /** + * hid_hw_idle - send idle request to device + * + * @hdev: hid device + * @report: report to control + * @idle: idle state + * @reqtype: hid request type + */ +static inline int hid_hw_idle(struct hid_device *hdev, int report, int idle, + int reqtype) +{ + if (hdev->ll_driver->idle) + return hdev->ll_driver->idle(hdev, report, idle, reqtype); + + return 0; +} + +/** * hid_hw_wait - wait for buffered io to complete * * @hdev: hid device
Some drivers send the idle command directly to underlying device, creating an unwanted dependency on the underlying transport layer. This patch adds hid_hw_idle() to the interface, thereby removing usbhid from the lion share of the drivers. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/usbhid/hid-core.c | 15 +++++++++++++++ include/linux/hid.h | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+)