On Thu, Feb 23, 2023 at 02:24:50PM +0100, Bastien Nocera wrote: > 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. That's going to be very vague, and is starting to get very interface-specific as an attibute here. Why can't it just be an input device attribute? Why is "wireless" suddenly a special case for USB devices (we thought we got rid of the old wireless usb code...) > By default, the USB device will declare not to use a receiver/emitter. I do not understand this statement, what do you mean by this? > Signed-off-by: Bastien Nocera <hadess@xxxxxxxxxx> > --- > Documentation/ABI/testing/sysfs-bus-usb | 12 ++++++ > drivers/usb/core/sysfs.c | 50 +++++++++++++++++++++++++ > drivers/usb/core/usb.h | 1 + > include/linux/usb.h | 10 +++++ > 4 files changed, 73 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb > index 545c2dd97ed0..0bd22ece05cd 100644 > --- a/Documentation/ABI/testing/sysfs-bus-usb > +++ b/Documentation/ABI/testing/sysfs-bus-usb > @@ -166,6 +166,18 @@ 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/<INTERFACE>/wireless_status > +Date: January 2023 January was last month :( > +Contact: Bastien Nocera <hadess@xxxxxxxxxx> > +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, on the interface which is used to detect > + this state. If the device does not use a receiver/emitter combo, > + then this attribute will not exist. So would you declare a wireless network device such a thing? See, it gets tricky, I do not think this should be a generic USB attribute at all. > + > What: /sys/bus/usb/devices/.../<hub_interface>/port<X> > Date: August 2012 > Contact: Lan Tianyu <tianyu.lan@xxxxxxxxx> > diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c > index 8217032dfb85..da3c0f0dd633 100644 > --- a/drivers/usb/core/sysfs.c > +++ b/drivers/usb/core/sysfs.c > @@ -1232,9 +1232,59 @@ static const struct attribute_group intf_assoc_attr_grp = { > .is_visible = intf_assoc_attrs_are_visible, > }; > > +static ssize_t wireless_status_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct usb_interface *intf; > + > + intf = to_usb_interface(dev); > + if (intf->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 *intf_wireless_status_attrs[] = { > + &dev_attr_wireless_status.attr, > + NULL > +}; > + > +static umode_t intf_wireless_status_attr_is_visible(struct kobject *kobj, > + struct attribute *a, int n) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct usb_interface *intf = to_usb_interface(dev); > + > + if (a != &dev_attr_wireless_status.attr || > + intf->wireless_status != USB_WIRELESS_STATUS_NA) > + return a->mode; > + return 0; > +} > + > +static const struct attribute_group intf_wireless_status_attr_grp = { > + .attrs = intf_wireless_status_attrs, > + .is_visible = intf_wireless_status_attr_is_visible, > +}; > + > +int usb_update_wireless_status_attr(struct usb_interface *intf) > +{ > + struct device *dev = &intf->dev; > + int ret; > + > + ret = sysfs_update_group(&dev->kobj, &intf_wireless_status_attr_grp); > + if (ret < 0) > + return ret; > + > + sysfs_notify(&dev->kobj, NULL, "wireless_status"); > + kobject_uevent(&dev->kobj, KOBJ_CHANGE); That could be very noisy, why does that deserve a KOBJ_CHANGE event? thanks, greg k-h