Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2 4/6] USB: core: Add wireless_status sysfs attribute
From: Bastien Nocera @ 2023-03-01 12:23 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230301122310.3579-1-hadess@hadess.net>

Add a wireless_status sysfs attribute to USB devices to keep track of
whether a USB device that's comprised of a receiver dongle and an emitter
device over a, most of the time proprietary, wireless link has its emitter
connected or disconnected.

This will be used by user-space OS components to determine whether the
battery-powered part of the device is wirelessly connected or not,
allowing, for example:
- upower to hide the battery for devices where the device is turned off
  but the receiver plugged in, rather than showing 0%, or other values
  that could be confusing to users
- Pipewire to hide a headset from the list of possible inputs or outputs
  or route audio appropriately if the headset is suddenly turned off, or
  turned on
- libinput to determine whether a keyboard or mouse is present when its
  receiver is plugged in.

This is done at the USB interface level as:
- the interface on which the wireless status is detected is sometimes
  not the same as where it could be consumed (eg. the audio interface
  on a headset dongle will still appear even if the headset is turned
  off), and we cannot have synchronisation of status across subsystems.
- this behaviour is not specific to HID devices, even if the protocols
  used to determine whether or not the remote device is connected can
  be HID.

This is not an attribute that is meant to replace protocol specific
APIs, such as the ones available for WWAN, WLAN/Wi-Fi, or Bluetooth
or any other sort of networking, but solely for wireless devices with
an ad-hoc “lose it and your device is e-waste” receiver dongle.

The USB interface will only be exporting the wireless_status sysfs
attribute if it gets set through the API exported in the next commit.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
Updated commit message and documentation in v2 so that the commit
doesn't need to reference older discussions.

 Documentation/ABI/testing/sysfs-bus-usb | 16 ++++++++
 drivers/usb/core/sysfs.c                | 50 +++++++++++++++++++++++++
 drivers/usb/core/usb.h                  |  1 +
 include/linux/usb.h                     |  9 +++++
 4 files changed, 76 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
index 545c2dd97ed0..45f1feb04dde 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -166,6 +166,22 @@ 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:		March 2023
+Contact:	Bastien Nocera <hadess@hadess.net>
+Description:
+		Some USB devices use a USB receiver dongle to communicate wirelessly
+		with their device using proprietary protocols. This attribute allows
+		user-space to know whether the device is connected to its receiver
+		dongle, and, for example, consider the device to be absent when choosing
+		whether to show the device's battery, show a headset in a list of
+		outputs, or show an on-screen keyboard if the only wireless keyboard
+		is turned off.
+		This attribute is not to be used to replace protocol specific statuses
+		available in WWAN, WLAN/Wi-Fi, Bluetooth, etc.
+		If the device does not use a receiver dongle with a wireless device,
+		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 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);
+
+	return 0;
+}
+
 const struct attribute_group *usb_interface_groups[] = {
 	&intf_attr_grp,
 	&intf_assoc_attr_grp,
+	&intf_wireless_status_attr_grp,
 	NULL
 };
 
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 0eac7d4285d1..3f14e15f07f6 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -15,6 +15,7 @@ extern int usb_create_sysfs_dev_files(struct usb_device *dev);
 extern void usb_remove_sysfs_dev_files(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_update_wireless_status_attr(struct usb_interface *intf);
 extern int usb_create_ep_devs(struct device *parent,
 				struct usb_host_endpoint *endpoint,
 				struct usb_device *udev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 86d1c8e79566..46fc85aba0df 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -170,6 +170,12 @@ usb_find_last_int_out_endpoint(struct usb_host_interface *alt,
 	return usb_find_common_endpoints_reverse(alt, NULL, NULL, NULL, int_out);
 }
 
+enum usb_wireless_status {
+	USB_WIRELESS_STATUS_NA = 0,
+	USB_WIRELESS_STATUS_DISCONNECTED,
+	USB_WIRELESS_STATUS_CONNECTED,
+};
+
 /**
  * struct usb_interface - what usb device drivers talk to
  * @altsetting: array of interface structures, one for each alternate
@@ -203,6 +209,8 @@ usb_find_last_int_out_endpoint(struct usb_host_interface *alt,
  * @reset_ws: Used for scheduling resets from atomic context.
  * @resetting_device: USB core reset the device, so use alt setting 0 as
  *	current; needs bandwidth alloc after reset.
+ * @wireless_status: if the USB device uses a receiver/emitter combo, whether
+ *	the emitter is connected.
  *
  * USB device drivers attach to interfaces on a physical device.  Each
  * interface encapsulates a single high level function, such as feeding
@@ -253,6 +261,7 @@ struct usb_interface {
 	unsigned needs_binding:1;	/* needs delayed unbind/rebind */
 	unsigned resetting_device:1;	/* true: bandwidth alloc after reset */
 	unsigned authorized:1;		/* used for interface authorization */
+	enum usb_wireless_status wireless_status;
 
 	struct device dev;		/* interface specific device info */
 	struct device *usb_dev;
-- 
2.39.2


^ permalink raw reply related

* [PATCH v2 5/6] USB: core: Add API to change the wireless_status
From: Bastien Nocera @ 2023-03-01 12:23 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230301122310.3579-1-hadess@hadess.net>

This adds the API that allows device specific drivers to tell user-space
about whether the wireless device is connected to its receiver dongle.

See "USB: core: Add wireless_status sysfs attribute" for a detailed
explanation of what this attribute should be used for.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
Fixed locking/use-after-free in v2, thanks to Alan Stern

 drivers/usb/core/message.c | 40 ++++++++++++++++++++++++++++++++++++++
 include/linux/usb.h        |  5 +++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 127fac1af676..3867d9a85145 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1908,6 +1908,45 @@ static void __usb_queue_reset_device(struct work_struct *ws)
 	usb_put_intf(iface);	/* Undo _get_ in usb_queue_reset_device() */
 }
 
+/*
+ * Internal function to set the wireless_status sysfs attribute
+ * See usb_set_wireless_status() for more details
+ */
+static void __usb_wireless_status_intf(struct work_struct *ws)
+{
+	struct usb_interface *iface =
+		container_of(ws, struct usb_interface, wireless_status_work);
+
+	device_lock(iface->dev.parent);
+	if (iface->sysfs_files_created)
+		usb_update_wireless_status_attr(iface);
+	usb_put_intf(iface);	/* Undo _get_ in usb_set_wireless_status() */
+	device_unlock(iface->dev.parent);
+}
+
+/**
+ * usb_set_wireless_status - sets the wireless_status struct member
+ * @dev: the device to modify
+ * @status: the new wireless status
+ *
+ * Set the wireless_status struct member to the new value, and emit
+ * sysfs changes as necessary.
+ *
+ * Returns: 0 on success, -EALREADY if already set.
+ */
+int usb_set_wireless_status(struct usb_interface *iface,
+		enum usb_wireless_status status)
+{
+	if (iface->wireless_status == status)
+		return -EALREADY;
+
+	usb_get_intf(iface);
+	iface->wireless_status = status;
+	schedule_work(&iface->wireless_status_work);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_set_wireless_status);
 
 /*
  * usb_set_configuration - Makes a particular device setting be current
@@ -2100,6 +2139,7 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
 		intf->dev.type = &usb_if_device_type;
 		intf->dev.groups = usb_interface_groups;
 		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
+		INIT_WORK(&intf->wireless_status_work, __usb_wireless_status_intf);
 		intf->minor = -1;
 		device_initialize(&intf->dev);
 		pm_runtime_no_callbacks(&intf->dev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 46fc85aba0df..a48eeec62a66 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -262,6 +262,7 @@ struct usb_interface {
 	unsigned resetting_device:1;	/* true: bandwidth alloc after reset */
 	unsigned authorized:1;		/* used for interface authorization */
 	enum usb_wireless_status wireless_status;
+	struct work_struct wireless_status_work;
 
 	struct device dev;		/* interface specific device info */
 	struct device *usb_dev;
@@ -896,6 +897,10 @@ static inline int usb_interface_claimed(struct usb_interface *iface)
 
 extern void usb_driver_release_interface(struct usb_driver *driver,
 			struct usb_interface *iface);
+
+int usb_set_wireless_status(struct usb_interface *iface,
+			enum usb_wireless_status status);
+
 const struct usb_device_id *usb_match_id(struct usb_interface *interface,
 					 const struct usb_device_id *id);
 extern int usb_match_one_id(struct usb_interface *interface,
-- 
2.39.2


^ permalink raw reply related

* [PATCH v2 6/6] HID: logitech-hidpp: Set wireless_status for G935 receiver
From: Bastien Nocera @ 2023-03-01 12:23 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230301122310.3579-1-hadess@hadess.net>

Set the USB interface "wireless_status" for the G935 receiver when
receiving battery notifications.

This will allow sound daemons such as Pipewire or PulseAudio to know
whether or not the headset is turned on and connected.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
 drivers/hid/hid-logitech-hidpp.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 4708819a6d79..c7d81b4241ad 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -74,6 +74,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS	BIT(27)
 #define HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS	BIT(28)
 #define HIDPP_QUIRK_HI_RES_SCROLL_1P0		BIT(29)
+#define HIDPP_QUIRK_WIRELESS_STATUS		BIT(30)
 
 /* These are just aliases for now */
 #define HIDPP_QUIRK_KBD_SCROLL_WHEEL HIDPP_QUIRK_HIDPP_WHEELS
@@ -472,6 +473,26 @@ static void hidpp_prefix_name(char **name, int name_length)
 	*name = new_name;
 }
 
+/*
+ * Updates the USB wireless_status based on whether the headset
+ * is turned on and reachable.
+ */
+static void hidpp_update_usb_wireless_status(struct hidpp_device *hidpp)
+{
+	struct hid_device *hdev = hidpp->hid_dev;
+	struct usb_interface *intf;
+
+	if (!(hidpp->quirks & HIDPP_QUIRK_WIRELESS_STATUS))
+		return;
+	if (!hid_is_usb(hdev))
+		return;
+
+	intf = to_usb_interface(hdev->dev.parent);
+	usb_set_wireless_status(intf, hidpp->battery.online ?
+				USB_WIRELESS_STATUS_CONNECTED :
+				USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
 /**
  * hidpp_scroll_counter_handle_scroll() - Send high- and low-resolution scroll
  *                                        events given a high-resolution wheel
@@ -1872,6 +1893,7 @@ static int hidpp20_query_adc_measurement_info_1f20(struct hidpp_device *hidpp)
 								 &hidpp->battery.voltage);
 	hidpp->battery.capacity = hidpp20_map_adc_measurement_1f20_capacity(hidpp->hid_dev,
 									    hidpp->battery.voltage);
+	hidpp_update_usb_wireless_status(hidpp);
 
 	return 0;
 }
@@ -1896,6 +1918,7 @@ static int hidpp20_adc_measurement_event_1f20(struct hidpp_device *hidpp,
 		hidpp->battery.capacity = hidpp20_map_adc_measurement_1f20_capacity(hidpp->hid_dev, voltage);
 		if (hidpp->battery.ps)
 			power_supply_changed(hidpp->battery.ps);
+		hidpp_update_usb_wireless_status(hidpp);
 	}
 	return 0;
 }
@@ -4557,7 +4580,8 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
 
 	{ /* G935 Gaming Headset */
-	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87) },
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87),
+		.driver_data = HIDPP_QUIRK_WIRELESS_STATUS },
 
 	{ /* MX5000 keyboard over Bluetooth */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb305),
-- 
2.39.2


^ permalink raw reply related

* Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support
From: Andy Shevchenko @ 2023-03-01 14:59 UTC (permalink / raw)
  To: Daniel Kaehn, Hans de Goede
  Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
	bartosz.golaszewski, dmitry.torokhov, devicetree, linux-input,
	ethan.twardy
In-Reply-To: <CAP+ZCCe=f3AtxvC1Z6zPErMEG9BcnCOjApc26n_9yjq2+U72pw@mail.gmail.com>

+Cc: Hans (as some DT/ACPI interesting cases are being discussed).

On Tue, Feb 28, 2023 at 01:05:54PM -0600, Daniel Kaehn wrote:
> On Mon, Feb 27, 2023 at 5:07 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Feb 27, 2023 at 08:07:58AM -0600, Danny Kaehn wrote:
> > > Bind I2C and GPIO interfaces to subnodes with names
> > > "i2c" and "gpio" if they exist, respectively. This
> > > allows the GPIO and I2C controllers to be described
> > > in firmware as usual. Additionally, support configuring the
> > > I2C bus speed from the clock-frequency device property.
> >
> > A bit shorten indentation...
> >
> > Nevertheless what I realized now is that this change, despite being OF
> > independent by used APIs, still OF-only.
> 
> I assumed this would practically be the case -- not because of the casing
> reason you gave (wasn't aware of that, thanks for the FYI), but because it
> doesn't seem that there's any way to describe USB devices connected to
> a USB port in ACPI, at least as far as I can tell (ACPI is still largely a black
> box to me).

That's not true :-)

Microsoft created a schema that is not part of the specification, but let's
say a standard de facto. Linux supports that and I even played with it [1]
to get connection of the arbitrary device to USB-2-GPIO/I²C/SPI adapter.

> But it seems reasonable that we should try to use the interface
> in a way so that it could be described using ACPI at some point (assuming
> that it isn't currently possible).
> 
> > Would it be possible to allow indexed access to child nodes as well, so if
> > there are no names, we may still be able to use firmware nodes from the correct
> > children?
> 
> Sure, you mean to fallback to using child nodes by index rather than by name
> in the case that that device_get_named_child_node() fails?  Would we need to
> somehow verify that those nodes are the nodes we expect them to be? (a.e.
> node 0 is actually the i2c-controller, node 1 is actually the
> gpio-controller).

Something like that, but I don't know if we can actually validate this without
having a preliminary agreement (hard-coded values) that index 0 is always let's
say I²C controller and so on.

> I don't see a reason why not, though I am curious if there is
> precedence for this
> strategy, a.e. in other drivers that use named child nodes. In my initial search
> through the kernel, I don't think I found anything like this -- does that mean
> those drivers also inherently won't work with ACPI?

If they are relying on names, yes, they won't work. It might be that some of them
have specific ACPI approach where different description is in use.

> The only driver I can find which uses device_get_named_child_node and has
> an acpi_device_id is drivers/platform/x86/intel/chtwc_int33fe.c

Yes, and you may notice the capitalization of the name. Also note, that relying
on the name in ACPI like now is quite fragile due to no common standard between
OEMs on how to name the same hardware nodes, they are free in that.

> > P.S. The problem with ACPI is that "name" of the child node will be in capital
> > letters as it's in accordance with the specification.
> 
> Knowing that this is the limitation, some other potential resolutions
> to potentially
> consider might be:
> 
> - Uppercase the names of the child nodes for the DT binding -- it appears
>   that the child node that chtwc_int33fe.c (the driver mentioned earlier)
>   accesses does indeed have an upper-cased name -- though that driver doesn't
>   have an of_device_id (makes sense, x86...). It seems named child nodes are
>   always lowercase in DT bindings -- not sure if that's a rule, or just how
>   it currently happens to be.

Not an option AFAIK, the DT and ACPI specifications are requiring conflicting
naming schema.

Possible way is to lowering case for ACPI names, but I dunno. We need more
opinions on this.

> - Do a case invariant compare on the names (and/or check for both lowercase
>   and uppercase)

For ACPI it will be always capital. For DT I have no clue if they allow capital
letters there, but I assume they don't.

> - Remove the use of child nodes, and combine the i2c and gpio nodes into the
>     cp2112's fwnode. I didn't do this initially because I wanted to avoid
>     namespace collisions between GPIO hogs and i2c child devices, and thought
>     that logically made sense to keep them separate, but that was before
>     knowing this limitation of ACPI.

Seems to me not an option at all, we need to define hardware as is. If it
combines two devices under a hood, should be two devices under a hood in
DT/ACPI.

[1]: https://stackoverflow.com/a/60855157/2511795

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support
From: Andy Shevchenko @ 2023-03-01 15:04 UTC (permalink / raw)
  To: Daniel Kaehn, Hans de Goede
  Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
	bartosz.golaszewski, dmitry.torokhov, devicetree, linux-input,
	ethan.twardy
In-Reply-To: <Y/9oO1AE6GK6CQmp@smile.fi.intel.com>

On Wed, Mar 01, 2023 at 04:59:07PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 28, 2023 at 01:05:54PM -0600, Daniel Kaehn wrote:
> > On Mon, Feb 27, 2023 at 5:07 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Mon, Feb 27, 2023 at 08:07:58AM -0600, Danny Kaehn wrote:
> > > > Bind I2C and GPIO interfaces to subnodes with names
> > > > "i2c" and "gpio" if they exist, respectively. This
> > > > allows the GPIO and I2C controllers to be described
> > > > in firmware as usual. Additionally, support configuring the
> > > > I2C bus speed from the clock-frequency device property.
> > >
> > > A bit shorten indentation...
> > >
> > > Nevertheless what I realized now is that this change, despite being OF
> > > independent by used APIs, still OF-only.
> > 
> > I assumed this would practically be the case -- not because of the casing
> > reason you gave (wasn't aware of that, thanks for the FYI), but because it
> > doesn't seem that there's any way to describe USB devices connected to
> > a USB port in ACPI, at least as far as I can tell (ACPI is still largely a black
> > box to me).
> 
> That's not true :-)
> 
> Microsoft created a schema that is not part of the specification, but let's
> say a standard de facto. Linux supports that and I even played with it [1]
> to get connection of the arbitrary device to USB-2-GPIO/I²C/SPI adapter.
> 
> > But it seems reasonable that we should try to use the interface
> > in a way so that it could be described using ACPI at some point (assuming
> > that it isn't currently possible).
> > 
> > > Would it be possible to allow indexed access to child nodes as well, so if
> > > there are no names, we may still be able to use firmware nodes from the correct
> > > children?
> > 
> > Sure, you mean to fallback to using child nodes by index rather than by name
> > in the case that that device_get_named_child_node() fails?  Would we need to
> > somehow verify that those nodes are the nodes we expect them to be? (a.e.
> > node 0 is actually the i2c-controller, node 1 is actually the
> > gpio-controller).
> 
> Something like that, but I don't know if we can actually validate this without
> having a preliminary agreement (hard-coded values) that index 0 is always let's
> say I²C controller and so on.
> 
> > I don't see a reason why not, though I am curious if there is
> > precedence for this
> > strategy, a.e. in other drivers that use named child nodes. In my initial search
> > through the kernel, I don't think I found anything like this -- does that mean
> > those drivers also inherently won't work with ACPI?
> 
> If they are relying on names, yes, they won't work. It might be that some of them
> have specific ACPI approach where different description is in use.
> 
> > The only driver I can find which uses device_get_named_child_node and has
> > an acpi_device_id is drivers/platform/x86/intel/chtwc_int33fe.c
> 
> Yes, and you may notice the capitalization of the name. Also note, that relying
> on the name in ACPI like now is quite fragile due to no common standard between
> OEMs on how to name the same hardware nodes, they are free in that.

Forgot to mention, that you need to take into consideration the drivers that are:
1) can be compiled with CONFIG_OF=n (i.e. using agnostic APIs);
2) having OF ID table;
3) being platform independent (no Kconfig dependency to something that's known
   to be an ARCH/CPU/etc one).

All of those can be instantiated in ACPI via special PRP0001 ACPI ID [2].

> > > P.S. The problem with ACPI is that "name" of the child node will be in capital
> > > letters as it's in accordance with the specification.
> > 
> > Knowing that this is the limitation, some other potential resolutions
> > to potentially
> > consider might be:
> > 
> > - Uppercase the names of the child nodes for the DT binding -- it appears
> >   that the child node that chtwc_int33fe.c (the driver mentioned earlier)
> >   accesses does indeed have an upper-cased name -- though that driver doesn't
> >   have an of_device_id (makes sense, x86...). It seems named child nodes are
> >   always lowercase in DT bindings -- not sure if that's a rule, or just how
> >   it currently happens to be.
> 
> Not an option AFAIK, the DT and ACPI specifications are requiring conflicting
> naming schema.
> 
> Possible way is to lowering case for ACPI names, but I dunno. We need more
> opinions on this.
> 
> > - Do a case invariant compare on the names (and/or check for both lowercase
> >   and uppercase)
> 
> For ACPI it will be always capital. For DT I have no clue if they allow capital
> letters there, but I assume they don't.
> 
> > - Remove the use of child nodes, and combine the i2c and gpio nodes into the
> >     cp2112's fwnode. I didn't do this initially because I wanted to avoid
> >     namespace collisions between GPIO hogs and i2c child devices, and thought
> >     that logically made sense to keep them separate, but that was before
> >     knowing this limitation of ACPI.
> 
> Seems to me not an option at all, we need to define hardware as is. If it
> combines two devices under a hood, should be two devices under a hood in
> DT/ACPI.
> 
> [1]: https://stackoverflow.com/a/60855157/2511795

[2]: https://docs.kernel.org/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply

* Re: [PATCH v2 5/6] USB: core: Add API to change the wireless_status
From: Alan Stern @ 2023-03-01 15:33 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230301122310.3579-5-hadess@hadess.net>

On Wed, Mar 01, 2023 at 01:23:09PM +0100, Bastien Nocera wrote:
> This adds the API that allows device specific drivers to tell user-space
> about whether the wireless device is connected to its receiver dongle.
> 
> See "USB: core: Add wireless_status sysfs attribute" for a detailed
> explanation of what this attribute should be used for.
> 
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> ---
> Fixed locking/use-after-free in v2, thanks to Alan Stern
> 
>  drivers/usb/core/message.c | 40 ++++++++++++++++++++++++++++++++++++++
>  include/linux/usb.h        |  5 +++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 127fac1af676..3867d9a85145 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1908,6 +1908,45 @@ static void __usb_queue_reset_device(struct work_struct *ws)
>  	usb_put_intf(iface);	/* Undo _get_ in usb_queue_reset_device() */
>  }
>  
> +/*
> + * Internal function to set the wireless_status sysfs attribute
> + * See usb_set_wireless_status() for more details
> + */
> +static void __usb_wireless_status_intf(struct work_struct *ws)
> +{
> +	struct usb_interface *iface =
> +		container_of(ws, struct usb_interface, wireless_status_work);
> +
> +	device_lock(iface->dev.parent);
> +	if (iface->sysfs_files_created)
> +		usb_update_wireless_status_attr(iface);
> +	usb_put_intf(iface);	/* Undo _get_ in usb_set_wireless_status() */
> +	device_unlock(iface->dev.parent);

Whoops!  Calling usb_put_intf() means the iface pointer is no longer 
valid.  The device_unlock() call should come before it, not after.

Alan

PS: You might also want to edit the sysfs documentation in the preceding 
patch, to make sure the text doesn't extend beyond the 80-column limit. 
I know people don't pay too much attention to that restriction in code 
any more, but in documentation it helps to keep the lines fairly short.  
People have trouble reading text when the lines get too long.

^ permalink raw reply

* Re: [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"
From: Werner Sembach @ 2023-03-01 16:57 UTC (permalink / raw)
  To: dmitry.torokhov, swboyd, gregkh, hdegoede, mkorpershoek,
	chenhuacai, wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <20230227185907.569154-1-wse@tuxedocomputers.com>


Am 27.02.23 um 19:59 schrieb Werner Sembach:
> This is a continuation of
> https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/
>
> That fix did fix the keyboard not responding at all sometimes after resume,
> but at the price of it being laggy for some time after boot. Additionally
> setting atkbd.reset removes that lag.
>
> This patch comes in 2 parts: The first one adds a quirk to atkbd to set
> atkbd.reset and the second one then applies that and the i8042 quirks to
> the affected devices.
>
>
Somehow, for my testing last week these patches seemed work, but now i still see 
occasional laggy keyboard after boot. So sadly the atkbd_reset quirk didn't fix 
the issue after all.

^ permalink raw reply

* Re: [PATCH v2 0/2] Fix "Input: i8042 - add TUXEDO devices to i8042 quirk tables for partial fix"
From: Hans de Goede @ 2023-03-01 17:52 UTC (permalink / raw)
  To: Werner Sembach, dmitry.torokhov, swboyd, gregkh, mkorpershoek,
	chenhuacai, wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <192c2eab-3b57-8a09-dfd7-5720b2b419b8@tuxedocomputers.com>

Hi,

On 3/1/23 17:57, Werner Sembach wrote:
> 
> Am 27.02.23 um 19:59 schrieb Werner Sembach:
>> This is a continuation of
>> https://lore.kernel.org/linux-input/20220708161005.1251929-3-wse@tuxedocomputers.com/
>>
>> That fix did fix the keyboard not responding at all sometimes after resume,
>> but at the price of it being laggy for some time after boot. Additionally
>> setting atkbd.reset removes that lag.
>>
>> This patch comes in 2 parts: The first one adds a quirk to atkbd to set
>> atkbd.reset and the second one then applies that and the i8042 quirks to
>> the affected devices.
>>
>>
> Somehow, for my testing last week these patches seemed work, but now i still see occasional laggy keyboard after boot. So sadly the atkbd_reset quirk didn't fix the issue after all.

Ok, well I guess that also resolves the discussion about trying to avoid
the duplicate DMI table entries.

FWIW I did not realize that the other quirk was in another module and
I don't have a good answer how to solve this in a way that avoids
adding the DMI matches twice.

Regards,

Hans


^ permalink raw reply

* [PATCH v5 00/18] Self-encapsulate the thermal zone device structure
From: Daniel Lezcano @ 2023-03-01 20:14 UTC (permalink / raw)
  To: rafael, daniel.lezcano
  Cc: linux-pm, linux-kernel, Zhang Rui, Len Brown, Damien Le Moal,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Jean Delvare, Guenter Roeck, Jonathan Cameron,
	Lars-Peter Clausen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
	Dmitry Torokhov, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Ido Schimmel, Petr Machata, Gregory Greenman,
	Kalle Valo, Sebastian Reichel, Liam Girdwood, Mark Brown,
	Miquel Raynal, Amit Kucheria, Florian Fainelli,
	Broadcom internal kernel review list, Ray Jui, Scott Branden,
	Markus Mayer, Support Opensource, Andy Gross, Bjorn Andersson,
	Konrad Dybcio, Thara Gopinath, Niklas Söderlund,
	Heiko Stuebner, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski,
	Alim Akhtar, Orson Zhai, Baolin Wang, Chunyan Zhang,
	Vasily Khoruzhick, Yangtao Li, Thierry Reding, Jonathan Hunter,
	Talel Shenhar, Eduardo Valentin, Keerthy, Kunihiko Hayashi,
	Masami Hiramatsu, Matthias Brugger, AngeloGioacchino Del Regno,
	Stefan Wahren, Zheng Yongjun, Yang Li, Srinivas Pandruvada,
	Daniel Golle, Balsam CHIHI, Mikko Perttunen, linux-acpi,
	linux-ide, linux-arm-kernel, linux-hwmon, linux-iio, linux-sunxi,
	linux-input, netdev, linux-wireless, linux-rpi-kernel,
	linux-arm-msm, linux-renesas-soc, linux-rockchip,
	linux-samsung-soc, linux-tegra, linux-omap, linux-mediatek

The exported thermal headers expose the thermal core structure while those
should be private to the framework. The initial idea was the thermal sensor
drivers use the thermal zone device structure pointer to pass it around from
the ops to the thermal framework API like a handler.

Unfortunately, different drivers are using and abusing the internals of this
structure to hook the associated struct device, read the internals values, take
the lock, etc ...

In order to fix this situation, let's encapsulate the structure leaking the
more in the different drivers: the thermal_zone_device structure.

This series revisit the existing drivers using the thermal zone private
structure internals to change the access to something else. For instance, the
get_temp() ops is using the tz->dev to write a debug trace. Despite the trace
is not helpful, we can check the return value for the get_temp() ops in the
call site and show the message in this place.

With this set of changes, the thermal_zone_device is almost self-encapsulated.
As usual, the acpi driver needs a more complex changes, so that will come in a
separate series along with the structure moved the private core headers.

Changelog:
	- V5:
	   - Dropped patch 19 : "thermal/tegra: Do not enable ... is already enabled"
	   - Changed the init sequence of the hw channels on tegra3 to close
	     the race window
	   - Collected more tags
	- V4:
	   - Collected more tags
	   - Fixed a typo therma_zone_device_priv() for db8500
	   - Remove traces patch [20/20] to be submitted separetely
	- V3:
	   - Split the first patch into three to reduce the number of
	     recipients per change
	   - Collected more tags
	   - Added missing changes for ->devdata in some drivers
	   - Added a 'type' accessor
	   - Replaced the 'type' to 'id' changes by the 'type' accessor
	   - Used the 'type' accessor in the drivers
	- V2:
	   - Collected tags
	   - Added missing changes for ->devdata for the tsens driver
	   - Renamed thermal_zone_device_get_data() to thermal_zone_priv()
	   - Added stubs when CONFIG_THERMAL is not set
	   - Dropped hwmon change where we remove the tz->lock usage

Thank you all for your comments

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Lars-Peter Clausen <lars@metafoo.de>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Samuel Holland <samuel@sholland.org>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Ido Schimmel <idosch@nvidia.com>
Cc: Petr Machata <petrm@nvidia.com>
Cc: Gregory Greenman <gregory.greenman@intel.com>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: Sebastian Reichel <sre@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Amit Kucheria <amitk@kernel.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: Markus Mayer <mmayer@broadcom.com>
Cc: Support Opensource <support.opensource@diasemi.com>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Thara Gopinath <thara.gopinath@gmail.com>
Cc: "Niklas Söderlund" <niklas.soderlund@ragnatech.se>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Vasily Khoruzhick <anarsoul@gmail.com>
Cc: Yangtao Li <tiny.windzz@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Talel Shenhar <talel@amazon.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Zheng Yongjun <zhengyongjun3@huawei.com>
Cc: Yang Li <yang.lee@linux.alibaba.com>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Daniel Golle <daniel@makrotopia.org>
Cc: Balsam CHIHI <bchihi@baylibre.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
Cc: linux-acpi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-ide@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-hwmon@vger.kernel.org
Cc: linux-iio@vger.kernel.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-input@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-rpi-kernel@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
Cc: linux-rockchip@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-tegra@vger.kernel.org
Cc: linux-omap@vger.kernel.org
Cc: linux-mediatek@lists.infradead.org

Daniel Lezcano (18):
  thermal/core: Add a thermal zone 'devdata' accessor
  thermal/core: Use the thermal zone 'devdata' accessor in thermal
    located drivers
  thermal/core: Use the thermal zone 'devdata' accessor in hwmon located
    drivers
  thermal/core: Use the thermal zone 'devdata' accessor in remaining
    drivers
  thermal/core: Show a debug message when get_temp() fails
  thermal: Remove debug or error messages in get_temp() ops
  thermal/hwmon: Do not set no_hwmon before calling
    thermal_add_hwmon_sysfs()
  thermal/hwmon: Use the right device for devm_thermal_add_hwmon_sysfs()
  thermal: Don't use 'device' internal thermal zone structure field
  thermal/core: Add thermal_zone_device structure 'type' accessor
  thermal/drivers/spear: Don't use tz->device but pdev->dev
  thermal: Add a thermal zone id accessor
  thermal: Use thermal_zone_device_type() accessor
  thermal/drivers/da9062: Don't access the thermal zone device fields
  thermal/hwmon: Use the thermal_core.h header
  thermal/drivers/tegra: Remove unneeded lock when setting a trip point
  thermal/drivers/acerhdf: Make interval setting only at module load
    time
  thermal/drivers/acerhdf: Remove pointless governor test

 drivers/acpi/thermal.c                        | 18 +++----
 drivers/ata/ahci_imx.c                        |  2 +-
 drivers/hwmon/hwmon.c                         |  4 +-
 drivers/hwmon/pmbus/pmbus_core.c              |  2 +-
 drivers/hwmon/scmi-hwmon.c                    |  4 +-
 drivers/hwmon/scpi-hwmon.c                    |  2 +-
 drivers/iio/adc/sun4i-gpadc-iio.c             |  2 +-
 drivers/input/touchscreen/sun4i-ts.c          |  2 +-
 .../ethernet/chelsio/cxgb4/cxgb4_thermal.c    |  2 +-
 .../ethernet/mellanox/mlxsw/core_thermal.c    | 16 +++----
 drivers/net/wireless/intel/iwlwifi/mvm/tt.c   |  4 +-
 drivers/platform/x86/acerhdf.c                | 19 ++------
 drivers/power/supply/power_supply_core.c      |  2 +-
 drivers/regulator/max8973-regulator.c         |  2 +-
 drivers/thermal/amlogic_thermal.c             |  4 +-
 drivers/thermal/armada_thermal.c              | 14 ++----
 drivers/thermal/broadcom/bcm2711_thermal.c    |  3 +-
 drivers/thermal/broadcom/bcm2835_thermal.c    |  3 +-
 drivers/thermal/broadcom/brcmstb_thermal.c    |  8 ++--
 drivers/thermal/broadcom/ns-thermal.c         |  2 +-
 drivers/thermal/broadcom/sr-thermal.c         |  2 +-
 drivers/thermal/da9062-thermal.c              | 13 +++--
 drivers/thermal/db8500_thermal.c              |  2 +-
 drivers/thermal/dove_thermal.c                |  7 +--
 drivers/thermal/hisi_thermal.c                |  5 +-
 drivers/thermal/imx8mm_thermal.c              |  4 +-
 drivers/thermal/imx_sc_thermal.c              |  9 ++--
 drivers/thermal/imx_thermal.c                 | 47 +++++--------------
 .../intel/int340x_thermal/int3400_thermal.c   |  2 +-
 .../int340x_thermal/int340x_thermal_zone.c    |  4 +-
 .../processor_thermal_device_pci.c            |  4 +-
 drivers/thermal/intel/intel_pch_thermal.c     |  2 +-
 .../thermal/intel/intel_quark_dts_thermal.c   |  6 +--
 drivers/thermal/intel/intel_soc_dts_iosf.c    | 13 ++---
 drivers/thermal/intel/x86_pkg_temp_thermal.c  |  4 +-
 drivers/thermal/k3_bandgap.c                  |  4 +-
 drivers/thermal/k3_j72xx_bandgap.c            |  2 +-
 drivers/thermal/kirkwood_thermal.c            |  7 +--
 drivers/thermal/max77620_thermal.c            |  6 +--
 drivers/thermal/mediatek/auxadc_thermal.c     |  4 +-
 drivers/thermal/mediatek/lvts_thermal.c       | 10 ++--
 drivers/thermal/qcom/qcom-spmi-adc-tm5.c      |  6 +--
 drivers/thermal/qcom/qcom-spmi-temp-alarm.c   |  6 +--
 drivers/thermal/qcom/tsens.c                  |  6 +--
 drivers/thermal/qoriq_thermal.c               |  4 +-
 drivers/thermal/rcar_gen3_thermal.c           |  5 +-
 drivers/thermal/rcar_thermal.c                |  8 +---
 drivers/thermal/rockchip_thermal.c            |  8 +---
 drivers/thermal/rzg2l_thermal.c               |  3 +-
 drivers/thermal/samsung/exynos_tmu.c          |  4 +-
 drivers/thermal/spear_thermal.c               | 10 ++--
 drivers/thermal/sprd_thermal.c                |  2 +-
 drivers/thermal/st/st_thermal.c               |  5 +-
 drivers/thermal/st/stm_thermal.c              |  4 +-
 drivers/thermal/sun8i_thermal.c               |  4 +-
 drivers/thermal/tegra/soctherm.c              |  6 +--
 drivers/thermal/tegra/tegra-bpmp-thermal.c    |  6 ++-
 drivers/thermal/tegra/tegra30-tsensor.c       | 31 ++++++------
 drivers/thermal/thermal-generic-adc.c         |  7 ++-
 drivers/thermal/thermal_core.c                | 18 +++++++
 drivers/thermal/thermal_helpers.c             |  3 ++
 drivers/thermal/thermal_hwmon.c               |  9 ++--
 drivers/thermal/thermal_hwmon.h               |  4 +-
 drivers/thermal/thermal_mmio.c                |  2 +-
 .../ti-soc-thermal/ti-thermal-common.c        | 10 ++--
 drivers/thermal/uniphier_thermal.c            |  2 +-
 include/linux/thermal.h                       | 19 ++++++++
 67 files changed, 218 insertions(+), 246 deletions(-)

-- 
2.34.1


^ permalink raw reply

* [PATCH v5 04/18] thermal/core: Use the thermal zone 'devdata' accessor in remaining drivers
From: Daniel Lezcano @ 2023-03-01 20:14 UTC (permalink / raw)
  To: rafael, daniel.lezcano
  Cc: linux-pm, linux-kernel, Mark Brown, Ido Schimmel,
	Gregory Greenman, Sebastian Reichel, Damien Le Moal,
	Rafael J . Wysocki, Zhang Rui, Len Brown, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	Jonathan Cameron, Lars-Peter Clausen, Chen-Yu Tsai,
	Jernej Skrabec, Samuel Holland, Dmitry Torokhov, Raju Rangoju,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Petr Machata, Kalle Valo, Sebastian Reichel, Liam Girdwood,
	open list:ACPI THERMAL DRIVER,
	open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	open list:IIO SUBSYSTEM AND DRIVERS,
	open list:ARM/Allwinner sunXi SoC support,
	open list:INPUT (KEYBOARD, MOUSE, JOYSTICK , TOUCHSCREEN)...,
	open list:CXGB4 ETHERNET DRIVER (CXGB4),
	open list:INTEL WIRELESS WIFI LINK (iwlwifi)
In-Reply-To: <20230301201446.3713334-1-daniel.lezcano@linaro.org>

The thermal zone device structure is exposed to the different drivers
and obviously they access the internals while that should be
restricted to the core thermal code.

In order to self-encapsulate the thermal core code, we need to prevent
the drivers accessing directly the thermal zone structure and provide
accessor functions to deal with.

Use the devdata accessor introduced in the previous patch.

No functional changes intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Ido Schimmel <idosch@nvidia.com> #mlxsw
Acked-by: Gregory Greenman <gregory.greenman@intel.com> #iwlwifi
Acked-by: Sebastian Reichel <sebastian.reichel@collabora.com> #power_supply
Acked-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> #ahci
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/thermal.c                           | 16 ++++++++--------
 drivers/ata/ahci_imx.c                           |  2 +-
 drivers/iio/adc/sun4i-gpadc-iio.c                |  2 +-
 drivers/input/touchscreen/sun4i-ts.c             |  2 +-
 .../net/ethernet/chelsio/cxgb4/cxgb4_thermal.c   |  2 +-
 .../net/ethernet/mellanox/mlxsw/core_thermal.c   | 14 +++++++-------
 drivers/net/wireless/intel/iwlwifi/mvm/tt.c      |  4 ++--
 drivers/power/supply/power_supply_core.c         |  2 +-
 drivers/regulator/max8973-regulator.c            |  2 +-
 9 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 0b4b844f9d4c..392b73b3e269 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -498,7 +498,7 @@ static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
 
 static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
 {
-	struct acpi_thermal *tz = thermal->devdata;
+	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
 	int result;
 
 	if (!tz)
@@ -516,7 +516,7 @@ static int thermal_get_temp(struct thermal_zone_device *thermal, int *temp)
 static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 				 int trip, enum thermal_trip_type *type)
 {
-	struct acpi_thermal *tz = thermal->devdata;
+	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
 	int i;
 
 	if (!tz || trip < 0)
@@ -560,7 +560,7 @@ static int thermal_get_trip_type(struct thermal_zone_device *thermal,
 static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 				 int trip, int *temp)
 {
-	struct acpi_thermal *tz = thermal->devdata;
+	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
 	int i;
 
 	if (!tz || trip < 0)
@@ -613,7 +613,7 @@ static int thermal_get_trip_temp(struct thermal_zone_device *thermal,
 static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
 				int *temperature)
 {
-	struct acpi_thermal *tz = thermal->devdata;
+	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
 
 	if (tz->trips.critical.flags.valid) {
 		*temperature = deci_kelvin_to_millicelsius_with_offset(
@@ -628,7 +628,7 @@ static int thermal_get_crit_temp(struct thermal_zone_device *thermal,
 static int thermal_get_trend(struct thermal_zone_device *thermal,
 			     int trip, enum thermal_trend *trend)
 {
-	struct acpi_thermal *tz = thermal->devdata;
+	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
 	enum thermal_trip_type type;
 	int i;
 
@@ -670,7 +670,7 @@ static int thermal_get_trend(struct thermal_zone_device *thermal,
 
 static void acpi_thermal_zone_device_hot(struct thermal_zone_device *thermal)
 {
-	struct acpi_thermal *tz = thermal->devdata;
+	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
 
 	acpi_bus_generate_netlink_event(tz->device->pnp.device_class,
 					dev_name(&tz->device->dev),
@@ -679,7 +679,7 @@ static void acpi_thermal_zone_device_hot(struct thermal_zone_device *thermal)
 
 static void acpi_thermal_zone_device_critical(struct thermal_zone_device *thermal)
 {
-	struct acpi_thermal *tz = thermal->devdata;
+	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
 
 	acpi_bus_generate_netlink_event(tz->device->pnp.device_class,
 					dev_name(&tz->device->dev),
@@ -693,7 +693,7 @@ static int acpi_thermal_cooling_device_cb(struct thermal_zone_device *thermal,
 					  bool bind)
 {
 	struct acpi_device *device = cdev->devdata;
-	struct acpi_thermal *tz = thermal->devdata;
+	struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
 	struct acpi_device *dev;
 	acpi_handle handle;
 	int i;
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c
index a950767f7948..e45e91f5e703 100644
--- a/drivers/ata/ahci_imx.c
+++ b/drivers/ata/ahci_imx.c
@@ -418,7 +418,7 @@ static int __sata_ahci_read_temperature(void *dev, int *temp)
 
 static int sata_ahci_read_temperature(struct thermal_zone_device *tz, int *temp)
 {
-	return __sata_ahci_read_temperature(tz->devdata, temp);
+	return __sata_ahci_read_temperature(thermal_zone_device_priv(tz), temp);
 }
 
 static ssize_t sata_ahci_show_temp(struct device *dev,
diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
index a6ade70dedf8..a5322550c422 100644
--- a/drivers/iio/adc/sun4i-gpadc-iio.c
+++ b/drivers/iio/adc/sun4i-gpadc-iio.c
@@ -414,7 +414,7 @@ static int sun4i_gpadc_runtime_resume(struct device *dev)
 
 static int sun4i_gpadc_get_temp(struct thermal_zone_device *tz, int *temp)
 {
-	struct sun4i_gpadc_iio *info = tz->devdata;
+	struct sun4i_gpadc_iio *info = thermal_zone_device_priv(tz);
 	int val, scale, offset;
 
 	if (sun4i_gpadc_temp_read(info->indio_dev, &val))
diff --git a/drivers/input/touchscreen/sun4i-ts.c b/drivers/input/touchscreen/sun4i-ts.c
index 73eb8f80be6e..1117fba30020 100644
--- a/drivers/input/touchscreen/sun4i-ts.c
+++ b/drivers/input/touchscreen/sun4i-ts.c
@@ -194,7 +194,7 @@ static int sun4i_get_temp(const struct sun4i_ts_data *ts, int *temp)
 
 static int sun4i_get_tz_temp(struct thermal_zone_device *tz, int *temp)
 {
-	return sun4i_get_temp(tz->devdata, temp);
+	return sun4i_get_temp(thermal_zone_device_priv(tz), temp);
 }
 
 static const struct thermal_zone_device_ops sun4i_ts_tz_ops = {
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
index 95e1b415ba13..dea9d2907666 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
@@ -12,7 +12,7 @@
 static int cxgb4_thermal_get_temp(struct thermal_zone_device *tzdev,
 				  int *temp)
 {
-	struct adapter *adap = tzdev->devdata;
+	struct adapter *adap = thermal_zone_device_priv(tzdev);
 	u32 param, val;
 	int ret;
 
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index c5240d38c9db..722e4a40afef 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -201,7 +201,7 @@ mlxsw_thermal_module_trips_update(struct device *dev, struct mlxsw_core *core,
 static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
 			      struct thermal_cooling_device *cdev)
 {
-	struct mlxsw_thermal *thermal = tzdev->devdata;
+	struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
 	struct device *dev = thermal->bus_info->dev;
 	int i, err;
 
@@ -227,7 +227,7 @@ static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
 static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
 				struct thermal_cooling_device *cdev)
 {
-	struct mlxsw_thermal *thermal = tzdev->devdata;
+	struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
 	struct device *dev = thermal->bus_info->dev;
 	int i;
 	int err;
@@ -249,7 +249,7 @@ static int mlxsw_thermal_unbind(struct thermal_zone_device *tzdev,
 static int mlxsw_thermal_get_temp(struct thermal_zone_device *tzdev,
 				  int *p_temp)
 {
-	struct mlxsw_thermal *thermal = tzdev->devdata;
+	struct mlxsw_thermal *thermal = thermal_zone_device_priv(tzdev);
 	struct device *dev = thermal->bus_info->dev;
 	char mtmp_pl[MLXSW_REG_MTMP_LEN];
 	int temp;
@@ -281,7 +281,7 @@ static struct thermal_zone_device_ops mlxsw_thermal_ops = {
 static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
 				     struct thermal_cooling_device *cdev)
 {
-	struct mlxsw_thermal_module *tz = tzdev->devdata;
+	struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
 	struct mlxsw_thermal *thermal = tz->parent;
 	int i, j, err;
 
@@ -310,7 +310,7 @@ static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
 static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
 				       struct thermal_cooling_device *cdev)
 {
-	struct mlxsw_thermal_module *tz = tzdev->devdata;
+	struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
 	struct mlxsw_thermal *thermal = tz->parent;
 	int i;
 	int err;
@@ -356,7 +356,7 @@ mlxsw_thermal_module_temp_and_thresholds_get(struct mlxsw_core *core,
 static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
 					 int *p_temp)
 {
-	struct mlxsw_thermal_module *tz = tzdev->devdata;
+	struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
 	struct mlxsw_thermal *thermal = tz->parent;
 	int temp, crit_temp, emerg_temp;
 	struct device *dev;
@@ -391,7 +391,7 @@ static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
 static int mlxsw_thermal_gearbox_temp_get(struct thermal_zone_device *tzdev,
 					  int *p_temp)
 {
-	struct mlxsw_thermal_module *tz = tzdev->devdata;
+	struct mlxsw_thermal_module *tz = thermal_zone_device_priv(tzdev);
 	struct mlxsw_thermal *thermal = tz->parent;
 	char mtmp_pl[MLXSW_REG_MTMP_LEN];
 	u16 index;
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
index 232c200af38f..354d95222b1b 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/tt.c
@@ -615,7 +615,7 @@ int iwl_mvm_send_temp_report_ths_cmd(struct iwl_mvm *mvm)
 static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device,
 				  int *temperature)
 {
-	struct iwl_mvm *mvm = (struct iwl_mvm *)device->devdata;
+	struct iwl_mvm *mvm = thermal_zone_device_priv(device);
 	int ret;
 	int temp;
 
@@ -641,7 +641,7 @@ static int iwl_mvm_tzone_get_temp(struct thermal_zone_device *device,
 static int iwl_mvm_tzone_set_trip_temp(struct thermal_zone_device *device,
 				       int trip, int temp)
 {
-	struct iwl_mvm *mvm = (struct iwl_mvm *)device->devdata;
+	struct iwl_mvm *mvm = thermal_zone_device_priv(device);
 	struct iwl_mvm_thermal_device *tzone;
 	int ret;
 
diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
index 7c790c41e2fe..83fd19079d8b 100644
--- a/drivers/power/supply/power_supply_core.c
+++ b/drivers/power/supply/power_supply_core.c
@@ -1142,7 +1142,7 @@ static int power_supply_read_temp(struct thermal_zone_device *tzd,
 	int ret;
 
 	WARN_ON(tzd == NULL);
-	psy = tzd->devdata;
+	psy = thermal_zone_device_priv(tzd);
 	ret = power_supply_get_property(psy, POWER_SUPPLY_PROP_TEMP, &val);
 	if (ret)
 		return ret;
diff --git a/drivers/regulator/max8973-regulator.c b/drivers/regulator/max8973-regulator.c
index 7e00a45db26a..303426135276 100644
--- a/drivers/regulator/max8973-regulator.c
+++ b/drivers/regulator/max8973-regulator.c
@@ -436,7 +436,7 @@ static int max8973_init_dcdc(struct max8973_chip *max,
 
 static int max8973_thermal_read_temp(struct thermal_zone_device *tz, int *temp)
 {
-	struct max8973_chip *mchip = tz->devdata;
+	struct max8973_chip *mchip = thermal_zone_device_priv(tz);
 	unsigned int val;
 	int ret;
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v3 1/6] HID: logitech-hidpp: Simplify array length check
From: Bastien Nocera @ 2023-03-02 10:55 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado

Use the compiler to force a 100-length array, rather than check the
length after the fact.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
New in v2, following a review comment in the 1f20 enablement patch.
No changes in v3.

 drivers/hid/hid-logitech-hidpp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index ff1fcebf2ec7..f55b2233dbea 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1356,7 +1356,7 @@ static int hidpp20_map_battery_capacity(struct hid_device *hid_dev, int voltage)
 	 * there are a few devices that use different battery technology.
 	 */
 
-	static const int voltages[] = {
+	static const int voltages[100] = {
 		4186, 4156, 4143, 4133, 4122, 4113, 4103, 4094, 4086, 4075,
 		4067, 4059, 4051, 4043, 4035, 4027, 4019, 4011, 4003, 3997,
 		3989, 3983, 3976, 3969, 3961, 3955, 3949, 3942, 3935, 3929,
@@ -1371,8 +1371,6 @@ static int hidpp20_map_battery_capacity(struct hid_device *hid_dev, int voltage)
 
 	int i;
 
-	BUILD_BUG_ON(ARRAY_SIZE(voltages) != 100);
-
 	if (unlikely(voltage < 3500 || voltage >= 5000))
 		hid_warn_once(hid_dev,
 			      "%s: possibly using the wrong voltage curve\n",
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 2/6] HID: logitech-hidpp: Add support for ADC measurement feature
From: Bastien Nocera @ 2023-03-02 10:55 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230302105555.51417-1-hadess@hadess.net>

This is used in a number of Logitech headsets to report the voltage
of the battery.

Tested on a Logitech G935.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216483
---
Fixed array length checking in v2.
No changes in v3

 drivers/hid/hid-logitech-hidpp.c | 172 ++++++++++++++++++++++++++++++-
 1 file changed, 170 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index f55b2233dbea..5a95f01a6129 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -94,6 +94,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL	BIT(7)
 #define HIDPP_CAPABILITY_HIDPP20_HI_RES_SCROLL	BIT(8)
 #define HIDPP_CAPABILITY_HIDPP10_FAST_SCROLL	BIT(9)
+#define HIDPP_CAPABILITY_ADC_MEASUREMENT	BIT(10)
 
 #define lg_map_key_clear(c)  hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
 
@@ -145,6 +146,7 @@ struct hidpp_battery {
 	u8 feature_index;
 	u8 solar_feature_index;
 	u8 voltage_feature_index;
+	u8 adc_measurement_feature_index;
 	struct power_supply_desc desc;
 	struct power_supply *ps;
 	char name[64];
@@ -1742,6 +1744,162 @@ static int hidpp_set_wireless_feature_index(struct hidpp_device *hidpp)
 	return ret;
 }
 
+/* -------------------------------------------------------------------------- */
+/* 0x1f20: ADC measurement                                                    */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_ADC_MEASUREMENT 0x1f20
+
+#define CMD_ADC_MEASUREMENT_GET_ADC_MEASUREMENT 0x00
+
+#define EVENT_ADC_MEASUREMENT_STATUS_BROADCAST 0x00
+
+static int hidpp20_map_adc_measurement_1f20_capacity(struct hid_device *hid_dev, int voltage)
+{
+	/* NB: This voltage curve doesn't necessarily map perfectly to all
+	 * devices that implement the ADC_MEASUREMENT feature. This is because
+	 * there are a few devices that use different battery technology.
+	 *
+	 * Adapted from:
+	 * https://github.com/Sapd/HeadsetControl/blob/acd972be0468e039b93aae81221f20a54d2d60f7/src/devices/logitech_g633_g933_935.c#L44-L52
+	 */
+	static const int voltages[100] = {
+		4030, 4024, 4018, 4011, 4003, 3994, 3985, 3975, 3963, 3951,
+		3937, 3922, 3907, 3893, 3880, 3868, 3857, 3846, 3837, 3828,
+		3820, 3812, 3805, 3798, 3791, 3785, 3779, 3773, 3768, 3762,
+		3757, 3752, 3747, 3742, 3738, 3733, 3729, 3724, 3720, 3716,
+		3712, 3708, 3704, 3700, 3696, 3692, 3688, 3685, 3681, 3677,
+		3674, 3670, 3667, 3663, 3660, 3657, 3653, 3650, 3646, 3643,
+		3640, 3637, 3633, 3630, 3627, 3624, 3620, 3617, 3614, 3611,
+		3608, 3604, 3601, 3598, 3595, 3592, 3589, 3585, 3582, 3579,
+		3576, 3573, 3569, 3566, 3563, 3560, 3556, 3553, 3550, 3546,
+		3543, 3539, 3536, 3532, 3529, 3525, 3499, 3466, 3433, 3399,
+	};
+
+	int i;
+
+	if (voltage == 0)
+		return 0;
+
+	if (unlikely(voltage < 3400 || voltage >= 5000))
+		hid_warn_once(hid_dev,
+			      "%s: possibly using the wrong voltage curve\n",
+			      __func__);
+
+	for (i = 0; i < ARRAY_SIZE(voltages); i++) {
+		if (voltage >= voltages[i])
+			return ARRAY_SIZE(voltages) - i;
+	}
+
+	return 0;
+}
+
+static int hidpp20_map_adc_measurement_1f20(u8 data[3], int *voltage)
+{
+	int status;
+	u8 flags;
+
+	flags = data[2];
+
+	switch (flags) {
+	case 0x01:
+		status = POWER_SUPPLY_STATUS_DISCHARGING;
+		break;
+	case 0x03:
+		status = POWER_SUPPLY_STATUS_CHARGING;
+		break;
+	case 0x07:
+		status = POWER_SUPPLY_STATUS_FULL;
+		break;
+	case 0x0F:
+	default:
+		status = POWER_SUPPLY_STATUS_UNKNOWN;
+		break;
+	}
+
+	*voltage = get_unaligned_be16(data);
+
+	dbg_hid("Parsed 1f20 data as flag 0x%02x voltage %dmV\n",
+		flags, *voltage);
+
+	return status;
+}
+
+/* Return value is whether the device is online */
+static bool hidpp20_get_adc_measurement_1f20(struct hidpp_device *hidpp,
+						 u8 feature_index,
+						 int *status, int *voltage)
+{
+	struct hidpp_report response;
+	int ret;
+	u8 *params = (u8 *)response.fap.params;
+
+	*status = POWER_SUPPLY_STATUS_UNKNOWN;
+	*voltage = 0;
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+					  CMD_ADC_MEASUREMENT_GET_ADC_MEASUREMENT,
+					  NULL, 0, &response);
+
+	if (ret > 0) {
+		hid_dbg(hidpp->hid_dev, "%s: received protocol error 0x%02x\n",
+			__func__, ret);
+		return false;
+	}
+
+	*status = hidpp20_map_adc_measurement_1f20(params, voltage);
+	return true;
+}
+
+static int hidpp20_query_adc_measurement_info_1f20(struct hidpp_device *hidpp)
+{
+	u8 feature_type;
+
+	if (hidpp->battery.adc_measurement_feature_index == 0xff) {
+		int ret;
+
+		ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_ADC_MEASUREMENT,
+					     &hidpp->battery.adc_measurement_feature_index,
+					     &feature_type);
+		if (ret)
+			return ret;
+
+		hidpp->capabilities |= HIDPP_CAPABILITY_ADC_MEASUREMENT;
+	}
+
+	hidpp->battery.online = hidpp20_get_adc_measurement_1f20(hidpp,
+								 hidpp->battery.adc_measurement_feature_index,
+								 &hidpp->battery.status,
+								 &hidpp->battery.voltage);
+	hidpp->battery.capacity = hidpp20_map_adc_measurement_1f20_capacity(hidpp->hid_dev,
+									    hidpp->battery.voltage);
+
+	return 0;
+}
+
+static int hidpp20_adc_measurement_event_1f20(struct hidpp_device *hidpp,
+					    u8 *data, int size)
+{
+	struct hidpp_report *report = (struct hidpp_report *)data;
+	int status, voltage;
+
+	if (report->fap.feature_index != hidpp->battery.adc_measurement_feature_index ||
+		report->fap.funcindex_clientid != EVENT_ADC_MEASUREMENT_STATUS_BROADCAST)
+		return 0;
+
+	status = hidpp20_map_adc_measurement_1f20(report->fap.params, &voltage);
+
+	hidpp->battery.online = status != POWER_SUPPLY_STATUS_UNKNOWN;
+
+	if (voltage != hidpp->battery.voltage || status != hidpp->battery.status) {
+		hidpp->battery.status = status;
+		hidpp->battery.voltage = voltage;
+		hidpp->battery.capacity = hidpp20_map_adc_measurement_1f20_capacity(hidpp->hid_dev, voltage);
+		if (hidpp->battery.ps)
+			power_supply_changed(hidpp->battery.ps);
+	}
+	return 0;
+}
+
 /* -------------------------------------------------------------------------- */
 /* 0x2120: Hi-resolution scrolling                                            */
 /* -------------------------------------------------------------------------- */
@@ -3660,6 +3818,9 @@ static int hidpp_raw_hidpp_event(struct hidpp_device *hidpp, u8 *data,
 		ret = hidpp20_battery_voltage_event(hidpp, data, size);
 		if (ret != 0)
 			return ret;
+		ret = hidpp20_adc_measurement_event_1f20(hidpp, data, size);
+		if (ret != 0)
+			return ret;
 	}
 
 	if (hidpp->capabilities & HIDPP_CAPABILITY_HIDPP10_BATTERY) {
@@ -3783,6 +3944,7 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 	hidpp->battery.feature_index = 0xff;
 	hidpp->battery.solar_feature_index = 0xff;
 	hidpp->battery.voltage_feature_index = 0xff;
+	hidpp->battery.adc_measurement_feature_index = 0xff;
 
 	if (hidpp->protocol_major >= 2) {
 		if (hidpp->quirks & HIDPP_QUIRK_CLASS_K750)
@@ -3796,6 +3958,8 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 				ret = hidpp20_query_battery_info_1004(hidpp);
 			if (ret)
 				ret = hidpp20_query_battery_voltage_info(hidpp);
+			if (ret)
+				ret = hidpp20_query_adc_measurement_info_1f20(hidpp);
 		}
 
 		if (ret)
@@ -3825,7 +3989,8 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 
 	if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_MILEAGE ||
 	    hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_PERCENTAGE ||
-	    hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE)
+	    hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE ||
+	    hidpp->capabilities & HIDPP_CAPABILITY_ADC_MEASUREMENT)
 		battery_props[num_battery_props++] =
 				POWER_SUPPLY_PROP_CAPACITY;
 
@@ -3833,7 +3998,8 @@ static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 		battery_props[num_battery_props++] =
 				POWER_SUPPLY_PROP_CAPACITY_LEVEL;
 
-	if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE)
+	if (hidpp->capabilities & HIDPP_CAPABILITY_BATTERY_VOLTAGE ||
+	    hidpp->capabilities & HIDPP_CAPABILITY_ADC_MEASUREMENT)
 		battery_props[num_battery_props++] =
 			POWER_SUPPLY_PROP_VOLTAGE_NOW;
 
@@ -4006,6 +4172,8 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 			hidpp20_query_battery_voltage_info(hidpp);
 		else if (hidpp->capabilities & HIDPP_CAPABILITY_UNIFIED_BATTERY)
 			hidpp20_query_battery_info_1004(hidpp);
+		else if (hidpp->capabilities & HIDPP_CAPABILITY_ADC_MEASUREMENT)
+			hidpp20_query_adc_measurement_info_1f20(hidpp);
 		else
 			hidpp20_query_battery_info_1000(hidpp);
 	}
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 3/6] HID: logitech-hidpp: Add Logitech G935 headset
From: Bastien Nocera @ 2023-03-02 10:55 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230302105555.51417-1-hadess@hadess.net>

Add the Logitech G935 headset that uses the HID++ protocol to the
list of supported devices.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
No changes since v1

 drivers/hid/hid-logitech-hidpp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5a95f01a6129..4708819a6d79 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4556,6 +4556,9 @@ static const struct hid_device_id hidpp_devices[] = {
 	{ /* Logitech G Pro Gaming Mouse over USB */
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
 
+	{ /* G935 Gaming Headset */
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87) },
+
 	{ /* MX5000 keyboard over Bluetooth */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb305),
 	  .driver_data = HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS },
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 4/6] USB: core: Add wireless_status sysfs attribute
From: Bastien Nocera @ 2023-03-02 10:55 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230302105555.51417-1-hadess@hadess.net>

Add a wireless_status sysfs attribute to USB devices to keep track of
whether a USB device that's comprised of a receiver dongle and an emitter
device over a, most of the time proprietary, wireless link has its emitter
connected or disconnected.

This will be used by user-space OS components to determine whether the
battery-powered part of the device is wirelessly connected or not,
allowing, for example:
- upower to hide the battery for devices where the device is turned off
  but the receiver plugged in, rather than showing 0%, or other values
  that could be confusing to users
- Pipewire to hide a headset from the list of possible inputs or outputs
  or route audio appropriately if the headset is suddenly turned off, or
  turned on
- libinput to determine whether a keyboard or mouse is present when its
  receiver is plugged in.

This is done at the USB interface level as:
- the interface on which the wireless status is detected is sometimes
  not the same as where it could be consumed (eg. the audio interface
  on a headset dongle will still appear even if the headset is turned
  off), and we cannot have synchronisation of status across subsystems.
- this behaviour is not specific to HID devices, even if the protocols
  used to determine whether or not the remote device is connected can
  be HID.

This is not an attribute that is meant to replace protocol specific
APIs, such as the ones available for WWAN, WLAN/Wi-Fi, or Bluetooth
or any other sort of networking, but solely for wireless devices with
an ad-hoc “lose it and your device is e-waste” receiver dongle.

The USB interface will only be exporting the wireless_status sysfs
attribute if it gets set through the API exported in the next commit.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
Updated commit message and documentation in v2 so that the commit
doesn't need to reference older discussions.

Trimmed the width of the sysfs docs in v3.

 Documentation/ABI/testing/sysfs-bus-usb | 17 +++++++++
 drivers/usb/core/sysfs.c                | 50 +++++++++++++++++++++++++
 drivers/usb/core/usb.h                  |  1 +
 include/linux/usb.h                     |  9 +++++
 4 files changed, 77 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-usb b/Documentation/ABI/testing/sysfs-bus-usb
index 545c2dd97ed0..cb172db41b34 100644
--- a/Documentation/ABI/testing/sysfs-bus-usb
+++ b/Documentation/ABI/testing/sysfs-bus-usb
@@ -166,6 +166,23 @@ 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:		February 2023
+Contact:	Bastien Nocera <hadess@hadess.net>
+Description:
+		Some USB devices use a USB receiver dongle to communicate
+		wirelessly with their device using proprietary protocols. This
+		attribute allows user-space to know whether the device is
+		connected to its receiver dongle, and, for example, consider
+		the device to be absent when choosing whether to show the
+		device's battery, show a headset in a list of outputs, or show
+		an on-screen keyboard if the only wireless keyboard is
+		turned off.
+		This attribute is not to be used to replace protocol specific
+		statuses available in WWAN, WLAN/Wi-Fi, Bluetooth, etc.
+		If the device does not use a receiver dongle with a wireless
+		device, 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 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);
+
+	return 0;
+}
+
 const struct attribute_group *usb_interface_groups[] = {
 	&intf_attr_grp,
 	&intf_assoc_attr_grp,
+	&intf_wireless_status_attr_grp,
 	NULL
 };
 
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 0eac7d4285d1..3f14e15f07f6 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -15,6 +15,7 @@ extern int usb_create_sysfs_dev_files(struct usb_device *dev);
 extern void usb_remove_sysfs_dev_files(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_update_wireless_status_attr(struct usb_interface *intf);
 extern int usb_create_ep_devs(struct device *parent,
 				struct usb_host_endpoint *endpoint,
 				struct usb_device *udev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 86d1c8e79566..46fc85aba0df 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -170,6 +170,12 @@ usb_find_last_int_out_endpoint(struct usb_host_interface *alt,
 	return usb_find_common_endpoints_reverse(alt, NULL, NULL, NULL, int_out);
 }
 
+enum usb_wireless_status {
+	USB_WIRELESS_STATUS_NA = 0,
+	USB_WIRELESS_STATUS_DISCONNECTED,
+	USB_WIRELESS_STATUS_CONNECTED,
+};
+
 /**
  * struct usb_interface - what usb device drivers talk to
  * @altsetting: array of interface structures, one for each alternate
@@ -203,6 +209,8 @@ usb_find_last_int_out_endpoint(struct usb_host_interface *alt,
  * @reset_ws: Used for scheduling resets from atomic context.
  * @resetting_device: USB core reset the device, so use alt setting 0 as
  *	current; needs bandwidth alloc after reset.
+ * @wireless_status: if the USB device uses a receiver/emitter combo, whether
+ *	the emitter is connected.
  *
  * USB device drivers attach to interfaces on a physical device.  Each
  * interface encapsulates a single high level function, such as feeding
@@ -253,6 +261,7 @@ struct usb_interface {
 	unsigned needs_binding:1;	/* needs delayed unbind/rebind */
 	unsigned resetting_device:1;	/* true: bandwidth alloc after reset */
 	unsigned authorized:1;		/* used for interface authorization */
+	enum usb_wireless_status wireless_status;
 
 	struct device dev;		/* interface specific device info */
 	struct device *usb_dev;
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 5/6] USB: core: Add API to change the wireless_status
From: Bastien Nocera @ 2023-03-02 10:55 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230302105555.51417-1-hadess@hadess.net>

This adds the API that allows device specific drivers to tell user-space
about whether the wireless device is connected to its receiver dongle.

See "USB: core: Add wireless_status sysfs attribute" for a detailed
explanation of what this attribute should be used for.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
Fixed locking/use-after-free in v2, thanks to Alan Stern

Fixed ordering of locking/put in v3, thanks to Alan Stern again

 drivers/usb/core/message.c | 40 ++++++++++++++++++++++++++++++++++++++
 include/linux/usb.h        |  5 +++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 127fac1af676..7930dca84616 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1908,6 +1908,45 @@ static void __usb_queue_reset_device(struct work_struct *ws)
 	usb_put_intf(iface);	/* Undo _get_ in usb_queue_reset_device() */
 }
 
+/*
+ * Internal function to set the wireless_status sysfs attribute
+ * See usb_set_wireless_status() for more details
+ */
+static void __usb_wireless_status_intf(struct work_struct *ws)
+{
+	struct usb_interface *iface =
+		container_of(ws, struct usb_interface, wireless_status_work);
+
+	device_lock(iface->dev.parent);
+	if (iface->sysfs_files_created)
+		usb_update_wireless_status_attr(iface);
+	device_unlock(iface->dev.parent);
+	usb_put_intf(iface);	/* Undo _get_ in usb_set_wireless_status() */
+}
+
+/**
+ * usb_set_wireless_status - sets the wireless_status struct member
+ * @dev: the device to modify
+ * @status: the new wireless status
+ *
+ * Set the wireless_status struct member to the new value, and emit
+ * sysfs changes as necessary.
+ *
+ * Returns: 0 on success, -EALREADY if already set.
+ */
+int usb_set_wireless_status(struct usb_interface *iface,
+		enum usb_wireless_status status)
+{
+	if (iface->wireless_status == status)
+		return -EALREADY;
+
+	usb_get_intf(iface);
+	iface->wireless_status = status;
+	schedule_work(&iface->wireless_status_work);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_set_wireless_status);
 
 /*
  * usb_set_configuration - Makes a particular device setting be current
@@ -2100,6 +2139,7 @@ int usb_set_configuration(struct usb_device *dev, int configuration)
 		intf->dev.type = &usb_if_device_type;
 		intf->dev.groups = usb_interface_groups;
 		INIT_WORK(&intf->reset_ws, __usb_queue_reset_device);
+		INIT_WORK(&intf->wireless_status_work, __usb_wireless_status_intf);
 		intf->minor = -1;
 		device_initialize(&intf->dev);
 		pm_runtime_no_callbacks(&intf->dev);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 46fc85aba0df..a48eeec62a66 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -262,6 +262,7 @@ struct usb_interface {
 	unsigned resetting_device:1;	/* true: bandwidth alloc after reset */
 	unsigned authorized:1;		/* used for interface authorization */
 	enum usb_wireless_status wireless_status;
+	struct work_struct wireless_status_work;
 
 	struct device dev;		/* interface specific device info */
 	struct device *usb_dev;
@@ -896,6 +897,10 @@ static inline int usb_interface_claimed(struct usb_interface *iface)
 
 extern void usb_driver_release_interface(struct usb_driver *driver,
 			struct usb_interface *iface);
+
+int usb_set_wireless_status(struct usb_interface *iface,
+			enum usb_wireless_status status);
+
 const struct usb_device_id *usb_match_id(struct usb_interface *interface,
 					 const struct usb_device_id *id);
 extern int usb_match_one_id(struct usb_interface *interface,
-- 
2.39.2


^ permalink raw reply related

* [PATCH v3 6/6] HID: logitech-hidpp: Set wireless_status for G935 receiver
From: Bastien Nocera @ 2023-03-02 10:55 UTC (permalink / raw)
  To: linux-usb, linux-input
  Cc: Greg Kroah-Hartman, Alan Stern, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230302105555.51417-1-hadess@hadess.net>

Set the USB interface "wireless_status" for the G935 receiver when
receiving battery notifications.

This will allow sound daemons such as Pipewire or PulseAudio to know
whether or not the headset is turned on and connected.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
No changes since v1

 drivers/hid/hid-logitech-hidpp.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 4708819a6d79..c7d81b4241ad 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -74,6 +74,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_HIDPP_EXTRA_MOUSE_BTNS	BIT(27)
 #define HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS	BIT(28)
 #define HIDPP_QUIRK_HI_RES_SCROLL_1P0		BIT(29)
+#define HIDPP_QUIRK_WIRELESS_STATUS		BIT(30)
 
 /* These are just aliases for now */
 #define HIDPP_QUIRK_KBD_SCROLL_WHEEL HIDPP_QUIRK_HIDPP_WHEELS
@@ -472,6 +473,26 @@ static void hidpp_prefix_name(char **name, int name_length)
 	*name = new_name;
 }
 
+/*
+ * Updates the USB wireless_status based on whether the headset
+ * is turned on and reachable.
+ */
+static void hidpp_update_usb_wireless_status(struct hidpp_device *hidpp)
+{
+	struct hid_device *hdev = hidpp->hid_dev;
+	struct usb_interface *intf;
+
+	if (!(hidpp->quirks & HIDPP_QUIRK_WIRELESS_STATUS))
+		return;
+	if (!hid_is_usb(hdev))
+		return;
+
+	intf = to_usb_interface(hdev->dev.parent);
+	usb_set_wireless_status(intf, hidpp->battery.online ?
+				USB_WIRELESS_STATUS_CONNECTED :
+				USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
 /**
  * hidpp_scroll_counter_handle_scroll() - Send high- and low-resolution scroll
  *                                        events given a high-resolution wheel
@@ -1872,6 +1893,7 @@ static int hidpp20_query_adc_measurement_info_1f20(struct hidpp_device *hidpp)
 								 &hidpp->battery.voltage);
 	hidpp->battery.capacity = hidpp20_map_adc_measurement_1f20_capacity(hidpp->hid_dev,
 									    hidpp->battery.voltage);
+	hidpp_update_usb_wireless_status(hidpp);
 
 	return 0;
 }
@@ -1896,6 +1918,7 @@ static int hidpp20_adc_measurement_event_1f20(struct hidpp_device *hidpp,
 		hidpp->battery.capacity = hidpp20_map_adc_measurement_1f20_capacity(hidpp->hid_dev, voltage);
 		if (hidpp->battery.ps)
 			power_supply_changed(hidpp->battery.ps);
+		hidpp_update_usb_wireless_status(hidpp);
 	}
 	return 0;
 }
@@ -4557,7 +4580,8 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
 
 	{ /* G935 Gaming Headset */
-	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87) },
+	  HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87),
+		.driver_data = HIDPP_QUIRK_WIRELESS_STATUS },
 
 	{ /* MX5000 keyboard over Bluetooth */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb305),
-- 
2.39.2


^ permalink raw reply related

* [PATCH 1/2] HID: logitech-hidpp: Don't use the USB serial for USB devices
From: Bastien Nocera @ 2023-03-02 13:01 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires,
	Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado

For devices that support the 0x0003 feature (Device Information) version 4,
set the serial based on the output of that feature, rather than relying
on the usbhid code setting the USB serial.

This should allow the serial when connected through USB to (nearly)
match the one when connected through a unifying receiver.

For example, on the serials on a G903 wired/wireless mouse:
- Unifying: 4067-e8-ce-cd-45
- USB before patch: 017C385C3837
- USB after patch: c086-e8-ce-cd-45

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
 drivers/hid/hid-logitech-hidpp.c | 51 ++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index c7d81b4241ad..99ce2c49facc 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -969,6 +969,55 @@ static int hidpp_root_get_protocol_version(struct hidpp_device *hidpp)
 	return 0;
 }
 
+/* -------------------------------------------------------------------------- */
+/* 0x0003: Device Information                                                 */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_DEVICE_INFORMATION			0x0003
+
+#define CMD_GET_DEVICE_INFO				0x00
+
+static int hidpp_get_serial(struct hidpp_device *hidpp, u32 *serial)
+{
+	struct hidpp_report response;
+	u8 feature_type;
+	u8 feature_index;
+	int ret;
+
+	ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_DEVICE_INFORMATION,
+				     &feature_index,
+				     &feature_type);
+	if (ret)
+		return ret;
+
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+					  CMD_GET_DEVICE_INFO,
+					  NULL, 0, &response);
+	if (ret)
+		return ret;
+
+	/* See hidpp_unifying_get_serial() */
+	*serial = *((u32 *)&response.rap.params[1]);
+	return 0;
+}
+
+static int hidpp_serial_init(struct hidpp_device *hidpp)
+{
+	struct hid_device *hdev = hidpp->hid_dev;
+	u32 serial;
+	int ret;
+
+	ret = hidpp_get_serial(hidpp, &serial);
+	if (ret)
+		return ret;
+
+	snprintf(hdev->uniq, sizeof(hdev->uniq), "%04x-%4phD",
+		 hdev->product, &serial);
+	dbg_hid("HID++ DeviceInformation: Got serial: %s\n", hdev->uniq);
+
+	return 0;
+}
+
 /* -------------------------------------------------------------------------- */
 /* 0x0005: GetDeviceNameType                                                  */
 /* -------------------------------------------------------------------------- */
@@ -4398,6 +4447,8 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
 		hidpp_unifying_init(hidpp);
+	else if (hid_is_usb(hidpp->hid_dev))
+		hidpp_serial_init(hidpp);
 
 	connected = hidpp_root_get_protocol_version(hidpp) == 0;
 	atomic_set(&hidpp->connected, connected);
-- 
2.39.2


^ permalink raw reply related

* [PATCH 2/2] HID: logitech-hidpp: Reconcile USB and Unifying serials
From: Bastien Nocera @ 2023-03-02 13:01 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires,
	Peter F . Patel-Schneider, Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230302130117.3975-1-hadess@hadess.net>

Now that USB HID++ devices can gather a serial number that matches the
one that would be gathered when connected through a Unifying receiver,
remove the last difference by dropping the product ID as devices
usually have different product IDs when connected through USB or
Unifying.

For example, on the serials on a G903 wired/wireless mouse:
- Unifying before patch: 4067-e8-ce-cd-45
- USB before patch: c086-e8-ce-cd-45
- Unifying and USB after patch: e8-ce-cd-45

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
 drivers/hid/hid-logitech-hidpp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 99ce2c49facc..0cda1d07641f 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -875,8 +875,7 @@ static int hidpp_unifying_init(struct hidpp_device *hidpp)
 	if (ret)
 		return ret;
 
-	snprintf(hdev->uniq, sizeof(hdev->uniq), "%04x-%4phD",
-		 hdev->product, &serial);
+	snprintf(hdev->uniq, sizeof(hdev->uniq), "%4phD", &serial);
 	dbg_hid("HID++ Unifying: Got serial: %s\n", hdev->uniq);
 
 	name = hidpp_unifying_get_name(hidpp);
@@ -1011,8 +1010,7 @@ static int hidpp_serial_init(struct hidpp_device *hidpp)
 	if (ret)
 		return ret;
 
-	snprintf(hdev->uniq, sizeof(hdev->uniq), "%04x-%4phD",
-		 hdev->product, &serial);
+	snprintf(hdev->uniq, sizeof(hdev->uniq), "%4phD", &serial);
 	dbg_hid("HID++ DeviceInformation: Got serial: %s\n", hdev->uniq);
 
 	return 0;
-- 
2.39.2


^ permalink raw reply related

* Re: [PATCH v2 5/6] USB: core: Add API to change the wireless_status
From: Bastien Nocera @ 2023-03-02 13:23 UTC (permalink / raw)
  To: Alan Stern
  Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <Y/9wMsqKMKJUugl4@rowland.harvard.edu>

On Wed, 2023-03-01 at 10:33 -0500, Alan Stern wrote:
> On Wed, Mar 01, 2023 at 01:23:09PM +0100, Bastien Nocera wrote:
> > This adds the API that allows device specific drivers to tell user-
> > space
> > about whether the wireless device is connected to its receiver
> > dongle.
> > 
> > See "USB: core: Add wireless_status sysfs attribute" for a detailed
> > explanation of what this attribute should be used for.
> > 
> > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > ---
> > Fixed locking/use-after-free in v2, thanks to Alan Stern
> > 
> >  drivers/usb/core/message.c | 40
> > ++++++++++++++++++++++++++++++++++++++
> >  include/linux/usb.h        |  5 +++++
> >  2 files changed, 45 insertions(+)
> > 
> > diff --git a/drivers/usb/core/message.c
> > b/drivers/usb/core/message.c
> > index 127fac1af676..3867d9a85145 100644
> > --- a/drivers/usb/core/message.c
> > +++ b/drivers/usb/core/message.c
> > @@ -1908,6 +1908,45 @@ static void __usb_queue_reset_device(struct
> > work_struct *ws)
> >         usb_put_intf(iface);    /* Undo _get_ in
> > usb_queue_reset_device() */
> >  }
> >  
> > +/*
> > + * Internal function to set the wireless_status sysfs attribute
> > + * See usb_set_wireless_status() for more details
> > + */
> > +static void __usb_wireless_status_intf(struct work_struct *ws)
> > +{
> > +       struct usb_interface *iface =
> > +               container_of(ws, struct usb_interface,
> > wireless_status_work);
> > +
> > +       device_lock(iface->dev.parent);
> > +       if (iface->sysfs_files_created)
> > +               usb_update_wireless_status_attr(iface);
> > +       usb_put_intf(iface);    /* Undo _get_ in
> > usb_set_wireless_status() */
> > +       device_unlock(iface->dev.parent);
> 
> Whoops!  Calling usb_put_intf() means the iface pointer is no longer 
> valid.  The device_unlock() call should come before it, not after.

Fixed this in v3.

> PS: You might also want to edit the sysfs documentation in the
> preceding 
> patch, to make sure the text doesn't extend beyond the 80-column
> limit. 
> I know people don't pay too much attention to that restriction in
> code 
> any more, but in documentation it helps to keep the lines fairly
> short.  
> People have trouble reading text when the lines get too long.

That's an easy fix, so done in v3 as well.

^ permalink raw reply

* Re: [PATCH v3 4/6] USB: core: Add wireless_status sysfs attribute
From: Alan Stern @ 2023-03-02 15:27 UTC (permalink / raw)
  To: Bastien Nocera
  Cc: linux-usb, linux-input, Greg Kroah-Hartman, Benjamin Tissoires,
	Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <20230302105555.51417-4-hadess@hadess.net>

On Thu, Mar 02, 2023 at 11:55:53AM +0100, Bastien Nocera wrote:
> Add a wireless_status sysfs attribute to USB devices to keep track of
> whether a USB device that's comprised of a receiver dongle and an emitter
> device over a, most of the time proprietary, wireless link has its emitter
> connected or disconnected.
> 
> This will be used by user-space OS components to determine whether the
> battery-powered part of the device is wirelessly connected or not,
> allowing, for example:
> - upower to hide the battery for devices where the device is turned off
>   but the receiver plugged in, rather than showing 0%, or other values
>   that could be confusing to users
> - Pipewire to hide a headset from the list of possible inputs or outputs
>   or route audio appropriately if the headset is suddenly turned off, or
>   turned on
> - libinput to determine whether a keyboard or mouse is present when its
>   receiver is plugged in.
> 
> This is done at the USB interface level as:
> - the interface on which the wireless status is detected is sometimes
>   not the same as where it could be consumed (eg. the audio interface
>   on a headset dongle will still appear even if the headset is turned
>   off), and we cannot have synchronisation of status across subsystems.
> - this behaviour is not specific to HID devices, even if the protocols
>   used to determine whether or not the remote device is connected can
>   be HID.
> 
> This is not an attribute that is meant to replace protocol specific
> APIs, such as the ones available for WWAN, WLAN/Wi-Fi, or Bluetooth
> or any other sort of networking, but solely for wireless devices with
> an ad-hoc “lose it and your device is e-waste” receiver dongle.
> 
> The USB interface will only be exporting the wireless_status sysfs
> attribute if it gets set through the API exported in the next commit.
> 
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> ---

For parts 4 and 5:

Acked-by: Alan Stern <stern@rowland.harvard.edu>


^ permalink raw reply

* Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support
From: Benjamin Tissoires @ 2023-03-02 17:06 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Daniel Kaehn, Hans de Goede, robh+dt, krzysztof.kozlowski+dt,
	jikos, bartosz.golaszewski, dmitry.torokhov, devicetree,
	linux-input, ethan.twardy
In-Reply-To: <Y/9oO1AE6GK6CQmp@smile.fi.intel.com>

On Mar 01 2023, Andy Shevchenko wrote:
> +Cc: Hans (as some DT/ACPI interesting cases are being discussed).
> 
> On Tue, Feb 28, 2023 at 01:05:54PM -0600, Daniel Kaehn wrote:
> > On Mon, Feb 27, 2023 at 5:07 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Mon, Feb 27, 2023 at 08:07:58AM -0600, Danny Kaehn wrote:
> > > > Bind I2C and GPIO interfaces to subnodes with names
> > > > "i2c" and "gpio" if they exist, respectively. This
> > > > allows the GPIO and I2C controllers to be described
> > > > in firmware as usual. Additionally, support configuring the
> > > > I2C bus speed from the clock-frequency device property.
> > >
> > > A bit shorten indentation...
> > >
> > > Nevertheless what I realized now is that this change, despite being OF
> > > independent by used APIs, still OF-only.
> > 
> > I assumed this would practically be the case -- not because of the casing
> > reason you gave (wasn't aware of that, thanks for the FYI), but because it
> > doesn't seem that there's any way to describe USB devices connected to
> > a USB port in ACPI, at least as far as I can tell (ACPI is still largely a black
> > box to me).
> 
> That's not true :-)
> 
> Microsoft created a schema that is not part of the specification, but let's
> say a standard de facto. Linux supports that and I even played with it [1]
> to get connection of the arbitrary device to USB-2-GPIO/I²C/SPI adapter.
> 
> > But it seems reasonable that we should try to use the interface
> > in a way so that it could be described using ACPI at some point (assuming
> > that it isn't currently possible).
> > 
> > > Would it be possible to allow indexed access to child nodes as well, so if
> > > there are no names, we may still be able to use firmware nodes from the correct
> > > children?
> > 
> > Sure, you mean to fallback to using child nodes by index rather than by name
> > in the case that that device_get_named_child_node() fails?  Would we need to
> > somehow verify that those nodes are the nodes we expect them to be? (a.e.
> > node 0 is actually the i2c-controller, node 1 is actually the
> > gpio-controller).
> 
> Something like that, but I don't know if we can actually validate this without
> having a preliminary agreement (hard-coded values) that index 0 is always let's
> say I²C controller and so on.
> 
> > I don't see a reason why not, though I am curious if there is
> > precedence for this
> > strategy, a.e. in other drivers that use named child nodes. In my initial search
> > through the kernel, I don't think I found anything like this -- does that mean
> > those drivers also inherently won't work with ACPI?
> 
> If they are relying on names, yes, they won't work. It might be that some of them
> have specific ACPI approach where different description is in use.
> 
> > The only driver I can find which uses device_get_named_child_node and has
> > an acpi_device_id is drivers/platform/x86/intel/chtwc_int33fe.c
> 
> Yes, and you may notice the capitalization of the name. Also note, that relying
> on the name in ACPI like now is quite fragile due to no common standard between
> OEMs on how to name the same hardware nodes, they are free in that.
> 
> > > P.S. The problem with ACPI is that "name" of the child node will be in capital
> > > letters as it's in accordance with the specification.
> > 
> > Knowing that this is the limitation, some other potential resolutions
> > to potentially
> > consider might be:
> > 
> > - Uppercase the names of the child nodes for the DT binding -- it appears
> >   that the child node that chtwc_int33fe.c (the driver mentioned earlier)
> >   accesses does indeed have an upper-cased name -- though that driver doesn't
> >   have an of_device_id (makes sense, x86...). It seems named child nodes are
> >   always lowercase in DT bindings -- not sure if that's a rule, or just how
> >   it currently happens to be.
> 
> Not an option AFAIK, the DT and ACPI specifications are requiring conflicting
> naming schema.
> 
> Possible way is to lowering case for ACPI names, but I dunno. We need more
> opinions on this.
> 
> > - Do a case invariant compare on the names (and/or check for both lowercase
> >   and uppercase)
> 
> For ACPI it will be always capital. For DT I have no clue if they allow capital
> letters there, but I assume they don't.
> 
> > - Remove the use of child nodes, and combine the i2c and gpio nodes into the
> >     cp2112's fwnode. I didn't do this initially because I wanted to avoid
> >     namespace collisions between GPIO hogs and i2c child devices, and thought
> >     that logically made sense to keep them separate, but that was before
> >     knowing this limitation of ACPI.
> 
> Seems to me not an option at all, we need to define hardware as is. If it
> combines two devices under a hood, should be two devices under a hood in
> DT/ACPI.
> 
> [1]: https://stackoverflow.com/a/60855157/2511795

Thanks Andy for your help here, and thanks for that link.

I am trying to test Danny's patch as I want to use it for my HID CI,
being an owner of a CP2112 device myself.

The current setup is using out of the tree patches [2] which are
implementing a platform i2c-hid support and some manual addition of a
I2C-HID device on top of it. This works fine but gets busted every now
and then when the tree sees a change that conflicts with these patches.

So with Danny's series, I thought I could have an SSDT override to
declare that very same device instead of patching my kernel before
testing it.

Of course, it gets tricky because I need to run that under qemu.

I am currently stuck at the "sharing the firmware_node from usb with
HID" step and I'd like to know if you could help me.

On my laptop, if I plug the CP2112 (without using a USB hub), I can get:

$> ls -l /sys/bus/hid/devices/0003:10C4:EA90.*            
  lrwxrwxrwx. 1 root root 0 Mar  2 17:02 /sys/bus/hid/devices/0003:10C4:EA90.0079 -> ../../../devices/pci0000:00/0000:00:14.0/usb2/2-9/2-9:1.0/0003:10C4:EA90.0079
$> ls -l /sys/bus/usb/devices/2-9*/firmware_node
  lrwxrwxrwx. 1 root root 0 Mar  2 17:03 /sys/bus/usb/devices/2-9:1.0/firmware_node -> ../../../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:15/device:16/device:25
  lrwxrwxrwx. 1 root root 0 Mar  2 17:02 /sys/bus/usb/devices/2-9/firmware_node -> ../../../../LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:15/device:16/device:25

So AFAIU the USB device is properly assigned a firmware node. My dsdt
also shows the "Device (RHUB)" and I guess everything is fine.

However, playing with qemu is not so easy.

I am running qemu with the following arguments (well, almost because I
have a wrapper script on top of it and I also run the compiled kernel
from the current tree):

#> qemu-system-x86_64 -bios /usr/share/edk2/ovmf/OVMF_CODE.fd \
                      -netdev user,id=hostnet0 \
                      -device virtio-net-pci,netdev=hostnet0 \
                      -m 4G \
                      -enable-kvm \
                      -cpu host \
                      -device qemu-xhci -usb \
                      -device 'usb-host,vendorid=0x10c4,productid=0xea90' \
                      -cdrom ~/Downloads/Fedora-Workstation-Live-x86_64-37-1.7.iso

And this is what I get:

#> ls -l /sys/bus/hid/devices/0003:10C4:EA90.*
  lrwxrwxrwx 1 root root 0 Mar  2 16:10 /sys/bus/hid/devices/0003:10C4:EA90.0001 -> ../../../devices/pci0000:00/0000:00:06.0/usb2/2-1/2-1:1.0/0003:10C4:EA90.0001

#> ls -l /sys/bus/usb/devices/2-1*/firmware_node
  ls: cannot access '/sys/bus/usb/devices/2-1*/firmware_node': No such file or directory

Looking at the DSDT, I do not see any reference to the USB hub, so I
wonder if the firmware_node needs to be populated first in the DSDT.

Also note that if I plug the CP2112 over a docking station, I lose the
firmware_node sysfs entries on the host too.

Do you think it would be achievable to emulate that over qemu and use a
mainline kernel without patches?

Cheers,
Benjamin

[2] https://gitlab.freedesktop.org/bentiss/gitlab-kernel-ci/-/tree/master/VM


^ permalink raw reply

* Re: [PATCH v5 00/18] Self-encapsulate the thermal zone device structure
From: Daniel Lezcano @ 2023-03-03  9:24 UTC (permalink / raw)
  To: rafael
  Cc: linux-pm, linux-kernel, Zhang Rui, Len Brown, Damien Le Moal,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Jean Delvare, Guenter Roeck, Jonathan Cameron,
	Lars-Peter Clausen, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
	Dmitry Torokhov, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Ido Schimmel, Petr Machata, Gregory Greenman,
	Kalle Valo, Sebastian Reichel, Liam Girdwood, Mark Brown,
	Miquel Raynal, Amit Kucheria, Florian Fainelli,
	Broadcom internal kernel review list, Ray Jui, Scott Branden,
	Markus Mayer, Support Opensource, Andy Gross, Bjorn Andersson,
	Konrad Dybcio, Thara Gopinath, Niklas Söderlund,
	Heiko Stuebner, Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski,
	Alim Akhtar, Orson Zhai, Baolin Wang, Chunyan Zhang,
	Vasily Khoruzhick, Yangtao Li, Thierry Reding, Jonathan Hunter,
	Talel Shenhar, Eduardo Valentin, Keerthy, Kunihiko Hayashi,
	Masami Hiramatsu, Matthias Brugger, AngeloGioacchino Del Regno,
	Stefan Wahren, Zheng Yongjun, Yang Li, Srinivas Pandruvada,
	Daniel Golle, Balsam CHIHI, Mikko Perttunen, linux-acpi,
	linux-ide, linux-arm-kernel, linux-hwmon, linux-iio, linux-sunxi,
	linux-input, netdev, linux-wireless, linux-rpi-kernel,
	linux-arm-msm, linux-renesas-soc, linux-rockchip,
	linux-samsung-soc, linux-tegra, linux-omap, linux-mediatek
In-Reply-To: <20230301201446.3713334-1-daniel.lezcano@linaro.org>


Hi Rafael,

Do we have enough ack to apply this series, is it for you ?

Thanks

   -- Daniel


On 01/03/2023 21:14, Daniel Lezcano wrote:
> The exported thermal headers expose the thermal core structure while those
> should be private to the framework. The initial idea was the thermal sensor
> drivers use the thermal zone device structure pointer to pass it around from
> the ops to the thermal framework API like a handler.
> 
> Unfortunately, different drivers are using and abusing the internals of this
> structure to hook the associated struct device, read the internals values, take
> the lock, etc ...
> 
> In order to fix this situation, let's encapsulate the structure leaking the
> more in the different drivers: the thermal_zone_device structure.
> 
> This series revisit the existing drivers using the thermal zone private
> structure internals to change the access to something else. For instance, the
> get_temp() ops is using the tz->dev to write a debug trace. Despite the trace
> is not helpful, we can check the return value for the get_temp() ops in the
> call site and show the message in this place.
> 
> With this set of changes, the thermal_zone_device is almost self-encapsulated.
> As usual, the acpi driver needs a more complex changes, so that will come in a
> separate series along with the structure moved the private core headers.
> 
> Changelog:
> 	- V5:
> 	   - Dropped patch 19 : "thermal/tegra: Do not enable ... is already enabled"
> 	   - Changed the init sequence of the hw channels on tegra3 to close
> 	     the race window
> 	   - Collected more tags
> 	- V4:
> 	   - Collected more tags
> 	   - Fixed a typo therma_zone_device_priv() for db8500
> 	   - Remove traces patch [20/20] to be submitted separetely
> 	- V3:
> 	   - Split the first patch into three to reduce the number of
> 	     recipients per change
> 	   - Collected more tags
> 	   - Added missing changes for ->devdata in some drivers
> 	   - Added a 'type' accessor
> 	   - Replaced the 'type' to 'id' changes by the 'type' accessor
> 	   - Used the 'type' accessor in the drivers
> 	- V2:
> 	   - Collected tags
> 	   - Added missing changes for ->devdata for the tsens driver
> 	   - Renamed thermal_zone_device_get_data() to thermal_zone_priv()
> 	   - Added stubs when CONFIG_THERMAL is not set
> 	   - Dropped hwmon change where we remove the tz->lock usage
> 
> Thank you all for your comments
> 
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Jonathan Cameron <jic23@kernel.org>
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Cc: Samuel Holland <samuel@sholland.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Ido Schimmel <idosch@nvidia.com>
> Cc: Petr Machata <petrm@nvidia.com>
> Cc: Gregory Greenman <gregory.greenman@intel.com>
> Cc: Kalle Valo <kvalo@kernel.org>
> Cc: Sebastian Reichel <sre@kernel.org>
> Cc: Liam Girdwood <lgirdwood@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Amit Kucheria <amitk@kernel.org>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> Cc: Ray Jui <rjui@broadcom.com>
> Cc: Scott Branden <sbranden@broadcom.com>
> Cc: Markus Mayer <mmayer@broadcom.com>
> Cc: Support Opensource <support.opensource@diasemi.com>
> Cc: Andy Gross <agross@kernel.org>
> Cc: Bjorn Andersson <andersson@kernel.org>
> Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
> Cc: Thara Gopinath <thara.gopinath@gmail.com>
> Cc: "Niklas Söderlund" <niklas.soderlund@ragnatech.se>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Cc: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: Orson Zhai <orsonzhai@gmail.com>
> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
> Cc: Chunyan Zhang <zhang.lyra@gmail.com>
> Cc: Vasily Khoruzhick <anarsoul@gmail.com>
> Cc: Yangtao Li <tiny.windzz@gmail.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Jonathan Hunter <jonathanh@nvidia.com>
> Cc: Talel Shenhar <talel@amazon.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Cc: Zheng Yongjun <zhengyongjun3@huawei.com>
> Cc: Yang Li <yang.lee@linux.alibaba.com>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Daniel Golle <daniel@makrotopia.org>
> Cc: Balsam CHIHI <bchihi@baylibre.com>
> Cc: Mikko Perttunen <mperttunen@nvidia.com>
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-ide@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-hwmon@vger.kernel.org
> Cc: linux-iio@vger.kernel.org
> Cc: linux-sunxi@lists.linux.dev
> Cc: linux-input@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: linux-wireless@vger.kernel.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-rpi-kernel@lists.infradead.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-rockchip@lists.infradead.org
> Cc: linux-samsung-soc@vger.kernel.org
> Cc: linux-tegra@vger.kernel.org
> Cc: linux-omap@vger.kernel.org
> Cc: linux-mediatek@lists.infradead.org
> 
> Daniel Lezcano (18):
>    thermal/core: Add a thermal zone 'devdata' accessor
>    thermal/core: Use the thermal zone 'devdata' accessor in thermal
>      located drivers
>    thermal/core: Use the thermal zone 'devdata' accessor in hwmon located
>      drivers
>    thermal/core: Use the thermal zone 'devdata' accessor in remaining
>      drivers
>    thermal/core: Show a debug message when get_temp() fails
>    thermal: Remove debug or error messages in get_temp() ops
>    thermal/hwmon: Do not set no_hwmon before calling
>      thermal_add_hwmon_sysfs()
>    thermal/hwmon: Use the right device for devm_thermal_add_hwmon_sysfs()
>    thermal: Don't use 'device' internal thermal zone structure field
>    thermal/core: Add thermal_zone_device structure 'type' accessor
>    thermal/drivers/spear: Don't use tz->device but pdev->dev
>    thermal: Add a thermal zone id accessor
>    thermal: Use thermal_zone_device_type() accessor
>    thermal/drivers/da9062: Don't access the thermal zone device fields
>    thermal/hwmon: Use the thermal_core.h header
>    thermal/drivers/tegra: Remove unneeded lock when setting a trip point
>    thermal/drivers/acerhdf: Make interval setting only at module load
>      time
>    thermal/drivers/acerhdf: Remove pointless governor test
> 
>   drivers/acpi/thermal.c                        | 18 +++----
>   drivers/ata/ahci_imx.c                        |  2 +-
>   drivers/hwmon/hwmon.c                         |  4 +-
>   drivers/hwmon/pmbus/pmbus_core.c              |  2 +-
>   drivers/hwmon/scmi-hwmon.c                    |  4 +-
>   drivers/hwmon/scpi-hwmon.c                    |  2 +-
>   drivers/iio/adc/sun4i-gpadc-iio.c             |  2 +-
>   drivers/input/touchscreen/sun4i-ts.c          |  2 +-
>   .../ethernet/chelsio/cxgb4/cxgb4_thermal.c    |  2 +-
>   .../ethernet/mellanox/mlxsw/core_thermal.c    | 16 +++----
>   drivers/net/wireless/intel/iwlwifi/mvm/tt.c   |  4 +-
>   drivers/platform/x86/acerhdf.c                | 19 ++------
>   drivers/power/supply/power_supply_core.c      |  2 +-
>   drivers/regulator/max8973-regulator.c         |  2 +-
>   drivers/thermal/amlogic_thermal.c             |  4 +-
>   drivers/thermal/armada_thermal.c              | 14 ++----
>   drivers/thermal/broadcom/bcm2711_thermal.c    |  3 +-
>   drivers/thermal/broadcom/bcm2835_thermal.c    |  3 +-
>   drivers/thermal/broadcom/brcmstb_thermal.c    |  8 ++--
>   drivers/thermal/broadcom/ns-thermal.c         |  2 +-
>   drivers/thermal/broadcom/sr-thermal.c         |  2 +-
>   drivers/thermal/da9062-thermal.c              | 13 +++--
>   drivers/thermal/db8500_thermal.c              |  2 +-
>   drivers/thermal/dove_thermal.c                |  7 +--
>   drivers/thermal/hisi_thermal.c                |  5 +-
>   drivers/thermal/imx8mm_thermal.c              |  4 +-
>   drivers/thermal/imx_sc_thermal.c              |  9 ++--
>   drivers/thermal/imx_thermal.c                 | 47 +++++--------------
>   .../intel/int340x_thermal/int3400_thermal.c   |  2 +-
>   .../int340x_thermal/int340x_thermal_zone.c    |  4 +-
>   .../processor_thermal_device_pci.c            |  4 +-
>   drivers/thermal/intel/intel_pch_thermal.c     |  2 +-
>   .../thermal/intel/intel_quark_dts_thermal.c   |  6 +--
>   drivers/thermal/intel/intel_soc_dts_iosf.c    | 13 ++---
>   drivers/thermal/intel/x86_pkg_temp_thermal.c  |  4 +-
>   drivers/thermal/k3_bandgap.c                  |  4 +-
>   drivers/thermal/k3_j72xx_bandgap.c            |  2 +-
>   drivers/thermal/kirkwood_thermal.c            |  7 +--
>   drivers/thermal/max77620_thermal.c            |  6 +--
>   drivers/thermal/mediatek/auxadc_thermal.c     |  4 +-
>   drivers/thermal/mediatek/lvts_thermal.c       | 10 ++--
>   drivers/thermal/qcom/qcom-spmi-adc-tm5.c      |  6 +--
>   drivers/thermal/qcom/qcom-spmi-temp-alarm.c   |  6 +--
>   drivers/thermal/qcom/tsens.c                  |  6 +--
>   drivers/thermal/qoriq_thermal.c               |  4 +-
>   drivers/thermal/rcar_gen3_thermal.c           |  5 +-
>   drivers/thermal/rcar_thermal.c                |  8 +---
>   drivers/thermal/rockchip_thermal.c            |  8 +---
>   drivers/thermal/rzg2l_thermal.c               |  3 +-
>   drivers/thermal/samsung/exynos_tmu.c          |  4 +-
>   drivers/thermal/spear_thermal.c               | 10 ++--
>   drivers/thermal/sprd_thermal.c                |  2 +-
>   drivers/thermal/st/st_thermal.c               |  5 +-
>   drivers/thermal/st/stm_thermal.c              |  4 +-
>   drivers/thermal/sun8i_thermal.c               |  4 +-
>   drivers/thermal/tegra/soctherm.c              |  6 +--
>   drivers/thermal/tegra/tegra-bpmp-thermal.c    |  6 ++-
>   drivers/thermal/tegra/tegra30-tsensor.c       | 31 ++++++------
>   drivers/thermal/thermal-generic-adc.c         |  7 ++-
>   drivers/thermal/thermal_core.c                | 18 +++++++
>   drivers/thermal/thermal_helpers.c             |  3 ++
>   drivers/thermal/thermal_hwmon.c               |  9 ++--
>   drivers/thermal/thermal_hwmon.h               |  4 +-
>   drivers/thermal/thermal_mmio.c                |  2 +-
>   .../ti-soc-thermal/ti-thermal-common.c        | 10 ++--
>   drivers/thermal/uniphier_thermal.c            |  2 +-
>   include/linux/thermal.h                       | 19 ++++++++
>   67 files changed, 218 insertions(+), 246 deletions(-)
> 

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


^ permalink raw reply

* Re: [PATCH 1/1] HID: logitech-hidpp: Add support for Logitech MX Master 3S mouse
From: Jiri Kosina @ 2023-03-03 14:05 UTC (permalink / raw)
  To: Rafał Szalecki
  Cc: linux-input, linux-kernel, Benjamin Tissoires, Filipe Laíns,
	Bastien Nocera
In-Reply-To: <20230301012356.940756-1-perexist7@gmail.com>

On Wed, 1 Mar 2023, Rafał Szalecki wrote:

> Add signature for the Logitech MX Master 3S mouse over Bluetooth.
> 
> Signed-off-by: Rafał Szalecki <perexist7@gmail.com>
> ---
> Hello,
> 
> I'm sending the patch to add the support for Logitech MX Master 3S mouse. The main reason for adding the support for this mouse was to enable high precision scrolling as it is now supported by Wayland composers. Tested with KDE 5.27. High precision scrolling was configured with Solaar and successfully tested with Brave browser.
> 
>  drivers/hid/hid-logitech-hidpp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 25dcda76d6c7..5fc88a063297 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4399,6 +4399,8 @@ static const struct hid_device_id hidpp_devices[] = {
>  	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb02a) },
>  	{ /* MX Master 3 mouse over Bluetooth */
>  	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb023) },
> +	{ /* MX Master 3S mouse over Bluetooth */
> +	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb034) },
>  	{}
>  };

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH] HID: intel-ish-hid: ipc: Fix potential use-after-free in work function
From: Jiri Kosina @ 2023-03-03 14:06 UTC (permalink / raw)
  To: Reka Norman
  Cc: Srinivas Pandruvada, Benjamin Tissoires, Li Qiong, linux-input,
	linux-kernel
In-Reply-To: <20230227024938.2265017-1-rekanorman@chromium.org>

On Mon, 27 Feb 2023, Reka Norman wrote:

> When a reset notify IPC message is received, the ISR schedules a work
> function and passes the ISHTP device to it via a global pointer
> ishtp_dev. If ish_probe() fails, the devm-managed device resources
> including ishtp_dev are freed, but the work is not cancelled, causing a
> use-after-free when the work function tries to access ishtp_dev. Use
> devm_work_autocancel() instead, so that the work is automatically
> cancelled if probe fails.
> 
> Signed-off-by: Reka Norman <rekanorman@chromium.org>

Applied, thank you.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* [PATCH] Input: i8042 - Add quirk for Fujitsu Lifebook A574/H
From: Jonathan Denose @ 2023-03-03 15:26 UTC (permalink / raw)
  To: LKML
  Cc: Jonathan Denose, Dmitry Torokhov, Hans de Goede, Huacai Chen,
	Mattijs Korpershoek, Takashi Iwai, Werner Sembach, Wolfram Sang,
	linux-input

Fujitsu Lifebook A574/H requires the nomux option to properly
probe the touchpad, especially when waking from sleep.

Signed-off-by: Jonathan Denose <jdenose@google.com>
---

 drivers/input/serio/i8042-acpipnpio.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index efc61736099b..fe7ffe30997c 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -610,6 +610,14 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
 		},
 		.driver_data = (void *)(SERIO_QUIRK_NOMUX)
 	},
+	{
+		/* Fujitsu Lifebook A574/H */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "FUJITSU"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "FMVA0501PZ"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOMUX)
+	},
 	{
 		/* Gigabyte M912 */
 		.matches = {
-- 
2.40.0.rc0.216.gc4246ad0f0-goog


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox