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
