usb/137189: [usb][patch] create and use sysctl nodes for HID
report descriptors
Eygene Ryabinkin
rea-fbsd at codelabs.ru
Thu Jul 30 20:40:04 UTC 2009
The following reply was made to PR usb/137189; it has been noted by GNATS.
From: Eygene Ryabinkin <rea-fbsd at codelabs.ru>
To: bug-followup at FreeBSD.org, freebsd-usb at FreeBSD.org
Cc: hselasky at c2i.net
Subject: Re: usb/137189: [usb][patch] create and use sysctl nodes for HID
report descriptors
Date: Fri, 31 Jul 2009 00:38:12 +0400
--tThc/1wpZn/ma/RB
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hans Petter, good day.
Tue, Jul 28, 2009 at 07:40:27PM +0400, Eygene Ryabinkin wrote:
> HPS wrote:
> > > Why do you dislike the sysctl approach? It is simple and reliable.
> >
> > It's duplicating access to data. There is not that much wrong about
> > it, except it will not work if the device is of another kind. I.E. you
> > would have to patch the HID sysctl node into every driver accessing
> > HID descriptors?
>
> Just now -- yes, I'll need it. But probably I can move this
> functionality into the USB bus level -- it will automatically create
> this sysctl node for all HID children and will dispose it on the detach.
> usb_probe_and_attach() is a candidate for such functionality. Will it
> be bad?
OK, attached is the reworked version of the sysctl patch: it now creates
the needed nodes automatically (though they still can be created by
explicit call from the driver, as in uhid(4)). As a bonus, kernel got the
ability to install per-USB class post-attach and pre-detach handlers, so
we can do some class-specific things for every driver. What do you think
of it?
Thanks!
--
Eygene
_ ___ _.--. #
\`.|\..----...-'` `-._.-'_.-'` # Remember that it is hard
/ ' ` , __.--' # to read the on-line manual
)/' _/ \ `-_, / # while single-stepping the kernel.
`-'" `"\_ ,_.-;_.-\_ ', fsc/as #
_.-'_./ {_.' ; / # -- FreeBSD Developers handbook
{_.-``-' {_/ #
--tThc/1wpZn/ma/RB
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0001-USB-implement-new-sysctl-node-hiddesc-for-HID-device.patch"
Content-Transfer-Encoding: quoted-printable
=46rom 4f8fd6e07cc388e96ab567ef29c6d592231ffb8a Mon Sep 17 00:00:00 2001
=46rom: Eygene Ryabinkin <rea-fbsd at codelabs.ru>
Date: Mon, 27 Jul 2009 08:36:07 +0400
Subject: [PATCH 1/3] USB: implement new sysctl node, "hiddesc" for HID devi=
ces
=2E..and also implement per-class attach/detach hooks -- now USB classes
can register theis handlers that will be called just after attachment
of device from their class and just before detachment of such device.
This functionality provides the foundation to implement the needed
sysctl in a generic manner where all HID-type devices will automatically
gain this sysctl.
The sysctl node will give out HID descriptor for the given device. It
should be useful both for debugging and for the usbhidctl(1) utility --
it will be able to get (and dump) report descriptors from the devices
that are "children" of the uhid(4): ukbd(4), ums(4) and others.
We can't export this data via the ioctl, because ums(4) and ukbd(4)
devices are typically used (by moused, Xorg, etc), so we just can't
open them to pass the file descriptor to the ioctl call.
Any driver can override the automated sysctl creation (that will read
the report descriptor from the device-supplied data): this is handy for
uhid(4) -- it sometimes uses hand-crafted report descriptors and we want
to export the descriptor that driver really uses.
Signed-off-by: Eygene Ryabinkin <rea-fbsd at codelabs.ru>
---
sys/dev/usb/input/uhid.c | 5 +
sys/dev/usb/usb_device.c | 68 ++++++++++++---
sys/dev/usb/usb_hid.c | 214 ++++++++++++++++++++++++++++++++++++++++++=
++++
sys/dev/usb/usb_hooks.h | 48 ++++++++++
sys/dev/usb/usbhid.h | 1 +
5 files changed, 325 insertions(+), 11 deletions(-)
create mode 100644 sys/dev/usb/usb_hooks.h
diff --git a/sys/dev/usb/input/uhid.c b/sys/dev/usb/input/uhid.c
index 411aeb6..2432491 100644
--- a/sys/dev/usb/input/uhid.c
+++ b/sys/dev/usb/input/uhid.c
@@ -756,6 +756,11 @@ uhid_attach(device_t dev)
if (error) {
goto detach;
}
+
+ /* Sysctl stuff */
+ hid_install_hiddesc_sysctl_handler(dev,
+ sc->sc_repdesc_ptr, sc->sc_repdesc_size);
+
return (0); /* success */
=20
detach:
diff --git a/sys/dev/usb/usb_device.c b/sys/dev/usb/usb_device.c
index 6617297..df2d531 100644
--- a/sys/dev/usb/usb_device.c
+++ b/sys/dev/usb/usb_device.c
@@ -50,6 +50,7 @@
#include <dev/usb/usbdi.h>
#include <dev/usb/usbdi_util.h>
#include <dev/usb/usb_ioctl.h>
+#include <dev/usb/usb_hooks.h>
#include "usbdevs.h"
=20
#define USB_DEBUG_VAR usb_debug
@@ -81,8 +82,8 @@ static void usb_init_endpoint(struct usb_device *, uint8_=
t,
struct usb_endpoint_descriptor *, struct usb_endpoint *);
static void usb_unconfigure(struct usb_device *, uint8_t);
static void usb_detach_device(struct usb_device *, uint8_t, uint8_t);
-static void usb_detach_device_sub(struct usb_device *, device_t *,
- uint8_t);
+static void usb_detach_device_sub(struct usb_device *,
+ struct usb_interface *, uint8_t);
static uint8_t usb_probe_and_attach_sub(struct usb_device *,
struct usb_attach_arg *);
static void usb_init_attach_arg(struct usb_device *,
@@ -122,6 +123,15 @@ usb_statestr(enum usb_dev_state state)
return ((state < USB_STATE_MAX) ? statestr[state] : "UNKNOWN");
}
=20
+/* List of the class-specific attach/detach hooks. */
+static struct usb_perclass_hooks {
+ usb_postattach_hook_t attach;
+ usb_predetach_hook_t detach;
+} usb_perclass_hooks[UICLASS_VENDOR];
+
+/* Useful macro */
+#define COUNTOF(x) (sizeof(x)/sizeof(*(x)))
+
/*------------------------------------------------------------------------*
* usbd_get_ep_by_addr
*
@@ -988,7 +998,7 @@ usb_reset_iface_endpoints(struct usb_device *udev, uint=
8_t iface_index)
* Flag values, see "USB_UNCFG_FLAG_XXX".
*------------------------------------------------------------------------=
*/
static void
-usb_detach_device_sub(struct usb_device *udev, device_t *ppdev,
+usb_detach_device_sub(struct usb_device *udev, struct usb_interface *iface,
uint8_t flag)
{
device_t dev;
@@ -996,17 +1006,17 @@ usb_detach_device_sub(struct usb_device *udev, devic=
e_t *ppdev,
=20
if (!(flag & USB_UNCFG_FLAG_FREE_SUBDEV)) {
=20
- *ppdev =3D NULL;
+ iface->subdev =3D NULL;
=20
- } else if (*ppdev) {
+ } else if (iface->subdev) {
=20
/*
- * NOTE: It is important to clear "*ppdev" before deleting
- * the child due to some device methods being called late
- * during the delete process !
+ * NOTE: It is important to clear "iface->subdev" before
+ * deleting the child due to some device methods being
+ * called late during the delete process!
*/
- dev =3D *ppdev;
- *ppdev =3D NULL;
+ dev =3D iface->subdev;
+ iface->subdev =3D NULL;
=20
device_printf(dev, "at %s, port %d, addr %d "
"(disconnected)\n",
@@ -1014,12 +1024,21 @@ usb_detach_device_sub(struct usb_device *udev, devi=
ce_t *ppdev,
udev->port_no, udev->address);
=20
if (device_is_attached(dev)) {
+ uByte class;
+
if (udev->flags.peer_suspended) {
err =3D DEVICE_RESUME(dev);
if (err) {
device_printf(dev, "Resume failed!\n");
}
}
+ /* Run class-specific pre-detach hooks. */
+ class =3D iface->idesc->bInterfaceClass;
+ if(class < COUNTOF(usb_perclass_hooks) &&
+ usb_perclass_hooks[class].detach) {
+ usb_perclass_hooks[class].detach(dev);
+ }
+
if (device_detach(dev)) {
goto error;
}
@@ -1081,7 +1100,7 @@ usb_detach_device(struct usb_device *udev, uint8_t if=
ace_index,
/* looks like the end of the USB interfaces */
break;
}
- usb_detach_device_sub(udev, &iface->subdev, flag);
+ usb_detach_device_sub(udev, iface, flag);
}
}
=20
@@ -1147,6 +1166,17 @@ usb_probe_and_attach_sub(struct usb_device *udev,
iface->subdev =3D uaa->temp_dev;
=20
if (device_probe_and_attach(iface->subdev) =3D=3D 0) {
+ uByte class;
+ /*
+ * Run per-class post-attach hooks. They require
+ * ivars to be set, so we should call them here.
+ */
+ class =3D iface->idesc->bInterfaceClass;
+ if(class < COUNTOF(usb_perclass_hooks) &&
+ usb_perclass_hooks[class].attach) {
+ usb_perclass_hooks[class].attach(iface->subdev);
+ }
+
/*
* The USB attach arguments are only available during probe
* and attach !
@@ -2447,3 +2477,19 @@ usbd_device_attached(struct usb_device *udev)
{
return (udev->state > USB_STATE_DETACHED);
}
+
+/*
+ * Sets per-class hook handlers in our hook registry.
+ * Arguments:
+ * - class: device class;
+ * - attach: post-attach hook, NULL disables the hook;
+ * - detach: pre-detach hook, NULL disable the hook.
+ */
+void usb_set_perclass_hooks(uByte class,
+ usb_postattach_hook_t attach, usb_predetach_hook_t detach)
+{
+ if (class >=3D COUNTOF(usb_perclass_hooks))
+ return;
+ usb_perclass_hooks[class].attach =3D attach;
+ usb_perclass_hooks[class].detach =3D detach;
+}
diff --git a/sys/dev/usb/usb_hid.c b/sys/dev/usb/usb_hid.c
index 220ffa2..c86325e 100644
--- a/sys/dev/usb/usb_hid.c
+++ b/sys/dev/usb/usb_hid.c
@@ -64,6 +64,7 @@ __FBSDID("$FreeBSD$");
#include <dev/usb/usbdi.h>
#include <dev/usb/usbdi_util.h>
#include <dev/usb/usbhid.h>
+#include <dev/usb/usb_hooks.h>
=20
#define USB_DEBUG_VAR usb_debug
=20
@@ -76,6 +77,34 @@ __FBSDID("$FreeBSD$");
static void hid_clear_local(struct hid_item *);
static uint8_t hid_get_byte(struct hid_data *s, const uint16_t wSize);
=20
+/*
+ * Infrastructure for automatical registration and unregistration
+ * of the HID generic sysctl nodes. hid_autosysctl holds the list
+ * of all HID devices for which sysctl handler was created, explicitely
+ * (by the driver) or implicitely (by the HID subsystem automation).
+ *
+ * Data items that were automatically initialized and should be freed on
+ * device detach will have non-NULL value; NULL value tells that the
+ * request for sysctl creation was explicit and driver manages memory by
+ * himself.
+ *
+ * Just now we have only one automatical sysctl node -- 'hiddesc'
+ * and all that we want to free is the report descriptor data, so
+ * 'struct hid_autosysctl' is very simple. If more sysctls are to
+ * be handled -- probably one should add the flag that will tell
+ * to which sysctl the data belongs to.
+ */
+static struct mtx hid_autosysctl_mtx;
+static LIST_HEAD(, hid_autosysctl) hid_autosysctl =3D
+ LIST_HEAD_INITIALIZER(hid_autosysctl);
+
+struct hid_autosysctl {
+ device_t owner; /* Who owns the sysctl */
+ void *data; /* Data that should be freed */
+ struct sysctl_oid *oid; /* OID of the created sysctl node */
+ LIST_ENTRY(hid_autosysctl) links;
+};
+
#define MAXUSAGE 64
#define MAXPUSH 4
struct hid_data {
@@ -742,3 +771,188 @@ usbd_req_get_hid_desc(struct usb_device *udev, struct=
mtx *mtx,
}
return (USB_ERR_NORMAL_COMPLETION);
}
+
+/*------------------------------------------------------------------------*
+ * hid_sysctl_handler_hiddesc
+ *
+ * Sysctl(3) handler for the device's HID descriptor. Mean to be used
+ * by all HID children drivers (if they want such sysctl).
+ *------------------------------------------------------------------------=
*/
+static int
+hid_sysctl_handler_hiddesc(SYSCTL_HANDLER_ARGS)
+{
+ int err;
+
+ KASSERT(arg1 !=3D NULL, ("HID descriptor ptr is NULL "
+ "inside 'hiddesc' sysctl handler"));
+ err =3D SYSCTL_OUT(req, arg1, arg2);
+ return (err);
+}
+
+/*------------------------------------------------------------------------*
+ * hid_install_sysctl_handler_hiddesc
+ *
+ * Installs sysctl handler for the device's "hiddesc" leaf and registers
+ * the fact of the installation.
+ *
+ * We're registering the installation of all handlers to allow this
+ * function to be called multiple times, e.g.:
+ * - "explicitely", by device driver itself;
+ * - "implicitely", by the generic registration mechanism.
+ *
+ * But only one sysctl handler will be installed -- the one initiated
+ * by the first invocation of this function.
+ *
+ * Arguments:
+ * - dev: the device for which the handler is installed;
+ * - desc: pointer to the memory chunk with the report descriptor
+ * data. If it is NULL, then the caller wants us to fetch
+ * its descriptor and do deallocation on the device detach
+ * (via the HID generic pre-detach hook).
+ * - size: size of the report descriptor; ignored when desc =3D=3D NULL.
+ *------------------------------------------------------------------------=
*/
+void
+hid_install_hiddesc_sysctl_handler(device_t dev, void *desc, size_t size)
+{
+ unsigned char grabbed_desc =3D 0;
+ struct hid_autosysctl *entry, *cur;
+
+ /*
+ * Get descriptor data (if needed) and grab memory while we're
+ * not locked.
+ */
+ if (desc =3D=3D NULL) {
+ int err;
+ uint16_t len;
+ struct usb_attach_arg *uaa =3D device_get_ivars(dev);
+
+ err =3D usbd_req_get_hid_desc(uaa->device, NULL,
+ &desc, &len, M_USBDEV, uaa->info.bIfaceIndex);
+ if (err)
+ return; /* Sorry, folks. */
+
+ size =3D len;
+ grabbed_desc =3D 1;
+ }
+ entry =3D malloc(sizeof(*entry), M_USBDEV, M_WAITOK | M_ZERO);
+ entry->owner =3D dev;
+ entry->data =3D (grabbed_desc ? desc : NULL);
+
+ /* Start manipulations with the autosysctl list */
+ mtx_lock(&hid_autosysctl_mtx);
+
+ /* Look if we already have sysctl for this device. */
+ LIST_FOREACH(cur, &hid_autosysctl, links) {
+ if (cur->owner =3D=3D dev) {
+ mtx_unlock(&hid_autosysctl_mtx);
+ if (grabbed_desc)
+ free(desc, M_USBDEV);
+ free(entry, M_USBDEV);
+ return;
+ }
+ }
+
+ LIST_INSERT_HEAD(&hid_autosysctl, entry, links);
+
+ /* End manipulations with the autosysctl list */
+ mtx_unlock(&hid_autosysctl_mtx);
+
+ entry->oid =3D SYSCTL_ADD_PROC(device_get_sysctl_ctx(dev),
+ SYSCTL_CHILDREN(device_get_sysctl_tree(dev)),
+ OID_AUTO, "hiddesc", CTLTYPE_STRING|CTLFLAG_RD,
+ desc, (int)size, hid_sysctl_handler_hiddesc,
+ "O", "HID report descriptor for device");
+}
+
+/*------------------------------------------------------------------------*
+ * hid_remove_sysctl_handler_hiddesc
+ *
+ * Frees the data that possibly had been automatically allocated for the
+ * 'hiddesc' sysctl handler. The removal of the sysctl node is done here
+ * too: we should first delete the node and free the associated data only
+ * afterwards, because otherwise we can have sysctl handler to be called
+ * with the already freed pointer.
+ *
+ * Arguments:
+ * - dev: the device for which the handler was installed previously.
+ *------------------------------------------------------------------------=
*/
+static void
+hid_remove_hiddesc_sysctl_handler(device_t dev)
+{
+ struct hid_autosysctl *cur, *np_temp;
+
+ mtx_lock(&hid_autosysctl_mtx);
+ LIST_FOREACH_SAFE(cur, &hid_autosysctl, links, np_temp) {
+ if (cur->owner =3D=3D dev) {
+ LIST_REMOVE(cur, links);
+ /* Drop the lock before sysctl manipulations */
+ mtx_unlock(&hid_autosysctl_mtx);
+
+ /*
+ * We must both remove sysctl from context
+ * and free it. Otherwise device removal
+ * procedures will try to remove it automatically
+ * and that's bad idea to delete removed sysctl.
+ */
+ sysctl_ctx_entry_del(device_get_sysctl_ctx(dev),
+ cur->oid);
+ sysctl_remove_oid(cur->oid, 1, 0);
+
+ if (cur->data)
+ free(cur->data, M_USBDEV);
+ free(cur, M_USBDEV);
+ return;
+ }
+ }
+ mtx_unlock(&hid_autosysctl_mtx);
+}
+
+/*------------------------------------------------------------------------*
+ * hid_hiddev_postattach_hook
+ *
+ * Called by the device attachment code inside usb_device.c after
+ * successfull attachment of the HID device.
+ *------------------------------------------------------------------------=
*/
+static void
+hid_hiddev_postattach_hook(device_t dev)
+{
+ hid_install_hiddesc_sysctl_handler(dev, NULL, 0);
+}
+
+/*------------------------------------------------------------------------*
+ * hid_hiddev_predetach_hook
+ *
+ * Called by the device detachment code inside usb_device.c before the
+ * detachment of the HID device.
+ *------------------------------------------------------------------------=
*/
+static void
+hid_hiddev_predetach_hook(device_t dev)
+{
+ hid_remove_hiddesc_sysctl_handler(dev);
+}
+
+/*------------------------------------------------------------------------*
+ * Methods to register and unregister our sysctl mutex and class-specific
+ * attach/detach hooks.
+ *------------------------------------------------------------------------=
*/
+static void
+uhid_mtx_init(void *arg)
+{
+ /* initialize mutex */
+ mtx_init(&hid_autosysctl_mtx, "HID autosysctl", NULL, MTX_DEF);
+ /* register hooks */
+ usb_set_perclass_hooks(UICLASS_HID, hid_hiddev_postattach_hook,
+ hid_hiddev_predetach_hook);
+}
+
+static void
+uhid_mtx_uninit(void *arg)
+{
+ /* destroy mutex */
+ mtx_destroy(&hid_autosysctl_mtx);
+ /* unregister hooks */
+ usb_set_perclass_hooks(UICLASS_HID, NULL, NULL);
+}
+
+SYSINIT(uhid_mtx_init, SI_SUB_LOCK, SI_ORDER_FIRST, uhid_mtx_init, NULL);
+SYSUNINIT(uhid_mtx_uninit, SI_SUB_LOCK, SI_ORDER_ANY, uhid_mtx_uninit, NUL=
L);
diff --git a/sys/dev/usb/usb_hooks.h b/sys/dev/usb/usb_hooks.h
new file mode 100644
index 0000000..a356594
--- /dev/null
+++ b/sys/dev/usb/usb_hooks.h
@@ -0,0 +1,48 @@
+/* $FreeBSD$ */
+/*-
+ * Copyright (c) 2009 Eygene Ryabinkin. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURP=
OSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENT=
IAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STR=
ICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY W=
AY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#ifndef _USB_HOOKS_H_
+#define _USB_HOOKS_H_
+
+#include <sys/types.h>
+
+/*
+ * There is the possibility to have per-class post-attach and pre-detach
+ * hooks for USB devices. These hooks are class-specific, but are generic
+ * to the class itself.
+ *
+ * Hooks can be set (or cleared) using usb_set_perclass_hooks. The usual
+ * place where hooks are to be registered is the SYSINIT entry for the
+ * class-specific interface.
+ */
+
+typedef void (*usb_postattach_hook_t)(device_t);
+typedef void (*usb_predetach_hook_t)(device_t);
+
+void usb_set_perclass_hooks(uByte class,
+ usb_postattach_hook_t attach, usb_predetach_hook_t);
+
+#endif /* _USB_HOOKS_H_ */
diff --git a/sys/dev/usb/usbhid.h b/sys/dev/usb/usbhid.h
index 4c5f313..e3f6105 100644
--- a/sys/dev/usb/usbhid.h
+++ b/sys/dev/usb/usbhid.h
@@ -235,5 +235,6 @@ struct usb_hid_descriptor *hid_get_descriptor_from_usb(
usb_error_t usbd_req_get_hid_desc(struct usb_device *udev, struct mtx *mtx,
void **descp, uint16_t *sizep, struct malloc_type *mem,
uint8_t iface_index);
+void hid_install_hiddesc_sysctl_handler(device_t dev, void *desc, size_t s=
ize);
#endif /* _KERNEL */
#endif /* _USB_HID_H_ */
--=20
1.6.3.3
--tThc/1wpZn/ma/RB--
More information about the freebsd-usb
mailing list