| Message ID | d9f8b9413c10fcf067658979d16a4f5c7abe69e7.camel@hadess.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] USB: core: Add wireless_status sysfs attribute | expand |
On Tue, Jan 17, 2023 at 04:17:23PM +0100, Bastien Nocera wrote: > Hey, > > TLDR: new sysfs attribute that makes it possible to leave receivers for > wireless headsets plugged in. At the USB level, or at the base driver > level? > > Longer version: > I started working on implementing support for some wireless headsets > that use USB receivers to communicate to the headset itself. > > The USB receivers have multiple interfaces, and independent drivers for > each, as is wont to do for USB devices. There's usually a HID interface > to do the custom stuff (LEDs, battery status, connection status, etc.) > and a standard audio class interface. > > Those drivers don't know anything about each other, and getting them to > talk to each other would be rather complicated. Additionally the audio > interface is still somewhat functional when the headset is > disconnected. > > In the end, I came up with this new sysfs attribute that would make it > possible for user-space (PulseAudio or Pipewire) to know whether the > receiver is plugged in or not. > > That allows user-space to not show the battery information for the > device (rather than 0 percent), not offer the headset as an output, and > potentially automatically switch to it when the headset is powered on. > > The question is whether this should be a USB sysfs attribute, or one at > the base driver level. Example implementation of the USB sysfs > attribute itself below. Do you know of any non-USB devices using the receiver/emitter approach? > I have a patch for a USB API as well, but I'm having some problems > creating deferred work on a soft irq. > > Cheers > > ---- > Add a wireless_status sysfs attribute to USB devices to keep track of > whether a USB device that uses a receiver/emitter combo has its > emitter connected or disconnected. > > By default, the USB device will declare not to use a receiver/emitter. How do you plan to tell which devices do use a receiver/emitter? Is this something the drivers already knowo about? Is it conceivable that a single device might have more than one receiver? If so, should the attribute belong to an interface rather than to the USB device? Alan Stern
On Tue, Jan 17, 2023 at 04:17:23PM +0100, Bastien Nocera wrote: > Hey, > > TLDR: new sysfs attribute that makes it possible to leave receivers for > wireless headsets plugged in. At the USB level, or at the base driver > level? > > Longer version: > I started working on implementing support for some wireless headsets > that use USB receivers to communicate to the headset itself. Would this also include wireless keyboard/mice recievers? Why is "wireless" somehow a special attribute that userspace needs to know about? > The USB receivers have multiple interfaces, and independent drivers for > each, as is wont to do for USB devices. There's usually a HID interface > to do the custom stuff (LEDs, battery status, connection status, etc.) > and a standard audio class interface. This probably should be an interface attribute (as Alan points out), as it's not a device attribute (think about updating the firmware for one of these, that's on an interface for the reciever you plugged in, not on the other end of the wireless connection...) > Those drivers don't know anything about each other, and getting them to > talk to each other would be rather complicated. Additionally the audio > interface is still somewhat functional when the headset is > disconnected. Those drivers shouldn't know about each other, that's up to userspace to group and control if needed. No kernel interactions should be needed. > In the end, I came up with this new sysfs attribute that would make it > possible for user-space (PulseAudio or Pipewire) to know whether the > receiver is plugged in or not. Again, should be an interface attribute, if at all. > That allows user-space to not show the battery information for the > device (rather than 0 percent), not offer the headset as an output, and > potentially automatically switch to it when the headset is powered on. Same for a keyboard/mouse, right? thanks, greg k-h
On Tue, 2023-01-17 at 11:01 -0500, Alan Stern wrote: > On Tue, Jan 17, 2023 at 04:17:23PM +0100, Bastien Nocera wrote: > > Hey, > > > > TLDR: new sysfs attribute that makes it possible to leave receivers > > for > > wireless headsets plugged in. At the USB level, or at the base > > driver > > level? > > > > Longer version: > > I started working on implementing support for some wireless > > headsets > > that use USB receivers to communicate to the headset itself. > > > > The USB receivers have multiple interfaces, and independent drivers > > for > > each, as is wont to do for USB devices. There's usually a HID > > interface > > to do the custom stuff (LEDs, battery status, connection status, > > etc.) > > and a standard audio class interface. > > > > Those drivers don't know anything about each other, and getting > > them to > > talk to each other would be rather complicated. Additionally the > > audio > > interface is still somewhat functional when the headset is > > disconnected. > > > > In the end, I came up with this new sysfs attribute that would make > > it > > possible for user-space (PulseAudio or Pipewire) to know whether > > the > > receiver is plugged in or not. > > > > That allows user-space to not show the battery information for the > > device (rather than 0 percent), not offer the headset as an output, > > and > > potentially automatically switch to it when the headset is powered > > on. > > > > The question is whether this should be a USB sysfs attribute, or > > one at > > the base driver level. Example implementation of the USB sysfs > > attribute itself below. > > Do you know of any non-USB devices using the receiver/emitter > approach? I don't know of any. > > > I have a patch for a USB API as well, but I'm having some problems > > creating deferred work on a soft irq. > > > > Cheers > > > > ---- > > Add a wireless_status sysfs attribute to USB devices to keep track > > of > > whether a USB device that uses a receiver/emitter combo has its > > emitter connected or disconnected. > > > > By default, the USB device will declare not to use a > > receiver/emitter. > > How do you plan to tell which devices do use a receiver/emitter? Is > this something the drivers already knowo about? This is something that will need to be done on a device-by-device basis. For example, I have some patches to the hid-logitech-hidpp driver to set the wireless status when the headset is turned on. This would allow leaving the receiver plugged in, and user-space would know when the headset was actually available. > > Is it conceivable that a single device might have more than one > receiver? If so, should the attribute belong to an interface rather > than to the USB device? The USB device is usually (always?) the receiver, so this kind of setup wouldn't make much sense to me. We could have it at the interface level, certainly, as Greg mentioned. That's probably the path I will take.
On Tue, 2023-01-17 at 17:14 +0100, Greg Kroah-Hartman wrote: > On Tue, Jan 17, 2023 at 04:17:23PM +0100, Bastien Nocera wrote: > > Hey, > > > > TLDR: new sysfs attribute that makes it possible to leave receivers > > for > > wireless headsets plugged in. At the USB level, or at the base > > driver > > level? > > > > Longer version: > > I started working on implementing support for some wireless > > headsets > > that use USB receivers to communicate to the headset itself. > > Would this also include wireless keyboard/mice recievers? This could also be used by certain keyboard or mice receivers, if the keyboard or mouse is kept constantly connected and there's a way to find out whether it's connected or not. > Why is "wireless" somehow a special attribute that userspace needs to > know about? "wireless" isn't the attribute user-space would be interested in, "wireless status" would be. If the headset's wireless status is "disconnected", then the headset is unavailable. Then user-space can route the audio accordingly. > > The USB receivers have multiple interfaces, and independent drivers > > for > > each, as is wont to do for USB devices. There's usually a HID > > interface > > to do the custom stuff (LEDs, battery status, connection status, > > etc.) > > and a standard audio class interface. > > This probably should be an interface attribute (as Alan points out), > as > it's not a device attribute (think about updating the firmware for > one > of these, that's on an interface for the reciever you plugged in, not > on > the other end of the wireless connection...) OK, fair enough. > > Those drivers don't know anything about each other, and getting > > them to > > talk to each other would be rather complicated. Additionally the > > audio > > interface is still somewhat functional when the headset is > > disconnected. > > Those drivers shouldn't know about each other, that's up to userspace > to > group and control if needed. No kernel interactions should be > needed. > > > In the end, I came up with this new sysfs attribute that would make > > it > > possible for user-space (PulseAudio or Pipewire) to know whether > > the > > receiver is plugged in or not. > > Again, should be an interface attribute, if at all. > > > That allows user-space to not show the battery information for the > > device (rather than 0 percent), not offer the headset as an output, > > and > > potentially automatically switch to it when the headset is powered > > on. > > Same for a keyboard/mouse, right? Yes, although I haven't found devices where this would be useful. I'll reimplement this as an interface attribute. Thanks very much to you and Alan for the quick replies. Cheers
diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb index 568103d3376e..23ba756d40f7 100644 --- a/Documentation/ABI/testing/sysfs-bus-usb +++ b/Documentation/ABI/testing/sysfs-bus-usb @@ -166,6 +166,17 @@ Description: The file will be present for all speeds of USB devices, and will always read "no" for USB 1.1 and USB 2.0 devices. +What: /sys/bus/usb/devices/.../wireless_status +Date: December 2022 +Contact: Bastien Nocera <hadess@hadess.net> +Description: + Some USB devices use a small USB receiver coupled with a larger + wireless device, usually communicating using proprietary + wireless protocols. This attribute will read either "connected" + or "disconnected" depending on whether the emitter is turned on, + in range and connected. If the device does not use a receiver/ + emitter combo, then this attribute will not exist. + What: /sys/bus/usb/devices/.../<hub_interface>/port<X> Date: August 2012 Contact: Lan Tianyu <tianyu.lan@intel.com> diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c index 631574718d8a..6e963fc9ed73 100644 --- a/drivers/usb/core/sysfs.c +++ b/drivers/usb/core/sysfs.c @@ -849,12 +849,62 @@ static const struct attribute_group dev_string_attr_grp = { .is_visible = dev_string_attrs_are_visible, }; +static ssize_t wireless_status_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct usb_device *udev; + + udev = to_usb_device(dev); + if (udev->wireless_status == USB_WIRELESS_STATUS_DISCONNECTED) + return sysfs_emit(buf, "%s\n", "disconnected"); + return sysfs_emit(buf, "%s\n", "connected"); +} +static DEVICE_ATTR_RO(wireless_status); + +static struct attribute *dev_wireless_status_attrs[] = { + &dev_attr_wireless_status.attr, + NULL +}; + +static umode_t dev_wireless_status_attr_is_visible(struct kobject *kobj, + struct attribute *a, int n) +{ + struct device *dev = kobj_to_dev(kobj); + struct usb_device *udev = to_usb_device(dev); + + if (a != &dev_attr_wireless_status.attr || + udev->wireless_status != USB_WIRELESS_STATUS_NA) + return a->mode; + return 0; +} + +static const struct attribute_group dev_wireless_status_attr_grp = { + .attrs = dev_wireless_status_attrs, + .is_visible = dev_wireless_status_attr_is_visible, +}; + const struct attribute_group *usb_device_groups[] = { &dev_attr_grp, &dev_string_attr_grp, + &dev_wireless_status_attr_grp, NULL }; +int usb_update_wireless_status_attr(struct usb_device *udev) +{ + struct device *dev = &udev->dev; + int ret; + + ret = sysfs_update_group(&dev->kobj, &dev_wireless_status_attr_grp); + if (ret < 0) + return ret; + + sysfs_notify(&dev->kobj, NULL, "wireless_status"); + kobject_uevent(&dev->kobj, KOBJ_CHANGE); + + return 0; +} + /* Binary descriptors */ static ssize_t diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h index 0eac7d4285d1..33d42d1b7d99 100644 --- a/drivers/usb/core/usb.h +++ b/drivers/usb/core/usb.h @@ -13,6 +13,7 @@ struct usb_dev_state; extern int usb_create_sysfs_dev_files(struct usb_device *dev); extern void usb_remove_sysfs_dev_files(struct usb_device *dev); +extern int usb_update_wireless_status_attr(struct usb_device *dev); extern void usb_create_sysfs_intf_files(struct usb_interface *intf); extern void usb_remove_sysfs_intf_files(struct usb_interface *intf); extern int usb_create_ep_devs(struct device *parent, diff --git a/include/linux/usb.h b/include/linux/usb.h index d2d2f41052c0..0c527cbd7165 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -545,6 +545,12 @@ struct usb3_lpm_parameters { int timeout; }; +enum usb_wireless_status { + USB_WIRELESS_STATUS_NA = 0, + USB_WIRELESS_STATUS_DISCONNECTED, + USB_WIRELESS_STATUS_CONNECTED, +}; + /** * struct usb_device - kernel's representation of a USB device * @devnum: device number; address on a USB bus @@ -620,6 +626,8 @@ struct usb3_lpm_parameters { * parent->hub_delay + wHubDelay + tTPTransmissionDelay (40ns) * Will be used as wValue for SetIsochDelay requests. * @use_generic_driver: ask driver core to reprobe using the generic driver. + * @wireless_status: if the USB device uses a receiver/emitter combo, whether + * the emitter is connected. * * Notes: * Usbcore drivers should not set usbdev->state directly. Instead use @@ -708,6 +716,7 @@ struct usb_device { u16 hub_delay; unsigned use_generic_driver:1; + enum usb_wireless_status wireless_status; }; #define to_usb_device(d) container_of(d, struct usb_device, dev)
Hey, TLDR: new sysfs attribute that makes it possible to leave receivers for wireless headsets plugged in. At the USB level, or at the base driver level? Longer version: I started working on implementing support for some wireless headsets that use USB receivers to communicate to the headset itself. The USB receivers have multiple interfaces, and independent drivers for each, as is wont to do for USB devices. There's usually a HID interface to do the custom stuff (LEDs, battery status, connection status, etc.) and a standard audio class interface. Those drivers don't know anything about each other, and getting them to talk to each other would be rather complicated. Additionally the audio interface is still somewhat functional when the headset is disconnected. In the end, I came up with this new sysfs attribute that would make it possible for user-space (PulseAudio or Pipewire) to know whether the receiver is plugged in or not. That allows user-space to not show the battery information for the device (rather than 0 percent), not offer the headset as an output, and potentially automatically switch to it when the headset is powered on. The question is whether this should be a USB sysfs attribute, or one at the base driver level. Example implementation of the USB sysfs attribute itself below. I have a patch for a USB API as well, but I'm having some problems creating deferred work on a soft irq. Cheers ---- Add a wireless_status sysfs attribute to USB devices to keep track of whether a USB device that uses a receiver/emitter combo has its emitter connected or disconnected. By default, the USB device will declare not to use a receiver/emitter. Signed-off-by: Bastien Nocera <hadess@hadess.net> --- Documentation/ABI/testing/sysfs-bus-usb | 11 ++++++ drivers/usb/core/sysfs.c | 50 +++++++++++++++++++++++++ drivers/usb/core/usb.h | 1 + include/linux/usb.h | 9 +++++ 4 files changed, 71 insertions(+)