* [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 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 3/6] HID: logitech-hidpp: Add Logitech G935 headset
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 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 in v2
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 v2 2/6] HID: logitech-hidpp: Add support for ADC measurement feature
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 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.
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 v2 1/6] HID: logitech-hidpp: Simplify array length check
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
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.
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
* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Hans de Goede @ 2023-03-01 11:41 UTC (permalink / raw)
To: Benjamin Tissoires, Linux regressions mailing list
Cc: Jiri Kosina, Dmitry Torokhov, open list:HID CORE LAYER, LKML,
Gé Koerkamp
In-Reply-To: <CAO-hwJJ2OMFgpmrXK_Z43z0ddujaS1fNjaAJSWwao4qQN+pJ6w@mail.gmail.com>
Hi,
On 2/28/23 14:26, Benjamin Tissoires wrote:
> On Tue, Feb 28, 2023 at 12:32 PM Thorsten Leemhuis
> <regressions@leemhuis.info> wrote:
>>
>> On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
>> wrote:
>>> Hi, this is your Linux kernel regression tracker.
>>>
>>> I noticed a regression report in bugzilla.kernel.org. As many (most?)
>>> kernel developer don't keep an eye on it, I decided to forward it by
>>> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
>>
>> The reporter recently confirmed in the ticket that the issue still
>> happens with 6.2.
>>
>> There wasn't any reply from any of the input developers here or in
>> bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
>> take a minute and at least briefly look into this as answer something
>> like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
>> for an option", or "we have no idea what might cause this, this needs to
>> be bisected"? That would help a lot.
>
> The working dmesg shows a line with:
> hid-generic 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00 Device
> [SIS0817:00 0457:10FB] on i2c-SIS0817:00
> and then
> hid-multitouch 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00
> Device [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>
> But these 2 lines do not appear on the 6.1.4 logs.
>
> So the device is not properly enumerated by ACPI or I2C. Hans might
> have an idea on how to debug/solve that issue.
I actually have a Toshiba satellite click mini l9w-b lying around
myself. I already made a note to try and reproduce this
But I'm very much swamped with too much kernel work, so no promises
when I will get around to this ...
Regards,
Hans
>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>> --
>> Everything you wanna know about Linux kernel regression tracking:
>> https://linux-regtracking.leemhuis.info/about/#tldr
>> If I did something stupid, please tell me, as explained on that page.
>>
>> #regzbot poke
>>>> Gé Koerkamp 2023-01-17 20:21:51 UTC
>>>>
>>>> Created attachment 303619 [details]
>>>> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
>>>>
>>>> Overview:
>>>> The touchscreen does not react on touch events.
>>>> Touchscreen display and touchpad are working.
>>>>
>>>> Step to reproduce:
>>>> Open any UI page
>>>> Try to use touch on relevant UI controls (buttons etc.)
>>>>
>>>> Result:
>>>> No reaction on screen touches
>>>>
>>>> Expected result:
>>>> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
>>>>
>>>> Build information:
>>>> The error happens with kernel version 6.1.4
>>>> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
>>>>
>>>> Additional builds:
>>>> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
>>>>
>>>> Important remark:
>>>> Touchscreen still works fine with kernel 6.1.4 using
>>>> - an HP x2 detachable 10-p0xx or
>>>> - a Lenovo yoga 520-14ikb
>>>>
>>>> So it could be a hardware dependent issue
>>>
>>> See the ticket for more details.
>>>
>>>
>>> [TLDR for the rest of this mail: I'm adding this report to the list of
>>> tracked Linux kernel regressions; the text you find below is based on a
>>> few templates paragraphs you might have encountered already in similar
>>> form.]
>>>
>>> BTW, let me use this mail to also add the report to the list of tracked
>>> regressions to ensure it's doesn't fall through the cracks:
>>>
>>> #regzbot introduced: v5.15..v5.16
>>> https://bugzilla.kernel.org/show_bug.cgi?id=216946
>>> #regzbot title: hid: touchscreen broken with Toshiba satellite click
>>> mini l9w-b
>>> #regzbot ignore-activity
>>>
>>> This isn't a regression? This issue or a fix for it are already
>>> discussed somewhere else? It was fixed already? You want to clarify when
>>> the regression started to happen? Or point out I got the title or
>>> something else totally wrong? Then just reply and tell me -- ideally
>>> while also telling regzbot about it, as explained by the page listed in
>>> the footer of this mail.
>>>
>>> Developers: When fixing the issue, remember to add 'Link:' tags pointing
>>> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
>>> this thread sees some discussion). See page linked in footer for details.
>>>
>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>> --
>>> Everything you wanna know about Linux kernel regression tracking:
>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>> If I did something stupid, please tell me, as explained on that page.
>>
>
^ permalink raw reply
* Re: AUXdisplay for LED arrays, keyboards with per-key LEDs -- was Re: [PATCH v2 2/2] leds: add aw20xx driver
From: Jani Nikula @ 2023-03-01 9:38 UTC (permalink / raw)
To: Pavel Machek, Martin Kurbanov, ojeda
Cc: devicetree, Lee Jones, linux-kernel, dri-devel, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, kernel, linux-leds,
Dmitry Torokhov, linux-input
In-Reply-To: <Y/50tKxpNVZO4Hfb@duo.ucw.cz>
On Tue, 28 Feb 2023, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> > +config LEDS_AW200XX
>> > + tristate "LED support for Awinic AW20036/AW20054/AW20072"
>> > + depends on LEDS_CLASS
>> > + depends on I2C
>> > + help
>> > + This option enables support for the AW20036/AW20054/AW20072 LED driver.
>> > + It is a 3x12/6x9/6x12 matrix LED driver programmed via
>> > + an I2C interface, up to 36/54/72 LEDs or 12/18/24 RGBs,
>> > + 3 pattern controllers for auto breathing or group dimming control.
>>
>> I'm afraid this should be handled as a display, not as an array of
>> individual LEDs.
>
> You probably want to see
>
> AUXILIARY DISPLAY DRIVERS
> M: Miguel Ojeda <ojeda@kernel.org>
> S: Maintained
> F: Documentation/devicetree/bindings/auxdisplay/
> F: drivers/auxdisplay/
> F: include/linux/cfag12864b.h
>
> And this brings another question...
>
> ...sooner or later we'll see LED displays with around 100 pixels in
> almost rectangular grid. Minority of the pixels will have funny
> shapes. How will we handle those? Pretend it is regular display with
> some pixels missing? How do we handle cellphone displays with rounded
> corners and holes for front camera?
>
> And yes, such crazy displays are being manufactured -- it is called
> keyboard with per-key backlight...
>
> https://www.reddit.com/r/MechanicalKeyboards/comments/8dtvgo/keyboard_with_individually_programmable_leds/
But... is that a display or a HID?
Only half-joking, really. This somewhat reminds me of using input system
force feedback stuff for touch screen vibrations.
Cc: Dmitry & linux-input.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
^ permalink raw reply
* Re: [PATCH 1/1] HID: logitech-hidpp: Add support for Logitech MX Master 3S mouse
From: Bastien Nocera @ 2023-03-01 8:21 UTC (permalink / raw)
To: Rafał Szalecki, linux-input
Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Filipe Laíns
In-Reply-To: <20230301012356.940756-1-perexist7@gmail.com>
On Wed, 2023-03-01 at 02:23 +0100, Rafał Szalecki wrote:
> Add signature for the Logitech MX Master 3S mouse over Bluetooth.
>
> Signed-off-by: Rafał Szalecki <perexist7@gmail.com>
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Thanks!
> ---
> 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) },
> {}
> };
>
^ permalink raw reply
* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Hector Martin @ 2023-03-01 3:13 UTC (permalink / raw)
To: Mark Kettenis
Cc: sven, fnkl.kernel, alyssa, dmitry.torokhov, robh+dt,
krzysztof.kozlowski+dt, asahi, rydberg, linux-arm-kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <87wn41qzpm.fsf@bloch.sibelius.xs4all.nl>
On 01/03/2023 05.53, Mark Kettenis wrote:
>> Date: Tue, 28 Feb 2023 11:58:28 +0900
>> From: Hector Martin <marcan@marcan.st>
>>
>> On 24/02/2023 20.08, Sven Peter wrote:
>>> Hi,
>>>
>>>
>>> On Fri, Feb 24, 2023, at 12:04, Sasha Finkelstein wrote:
>>>> On Fri, 24 Feb 2023 at 11:55, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>>>>
>>>>> What is the motivation for including the firmware name in the device
>>>>> tree rather than constructing it in the driver like what is done for
>>>>> the broadcom wireless?
>>>> There is no way to identify the device subtype before the firmware is
>>>> uploaded, and so i need some way of figuring out which firmware to use.
>>>
>>> Some Broadcom bluetooth boards use the compatible of the root node (see
>>> btbcm_get_board_name in drivers/bluetooth/btbcm.c) which would be "apple,jXXX"
>>> for Apple Silicon. I believe the Broadcom WiFi driver has similar logic as well
>>> which marcan had to extend to instead of "brcm,board-type" because different
>>> WiFi boards can me matched to different Apple Silicon boards. I don't think
>>> that's the case for this touchscreen though.
>>
>> The reason why the brcmfmac stuff needs to construct the firmware name
>> itself is that parts of it come from the OTP contents, so there is no
>> way to know from the bootloader what the right firmware is.
>
> The name of the "nvram" file is constructed as well, and that uses the
> compatible of the machine (the root of the device tree). I suppose
> what is special in that case is that several files are tried so a
> single 'firmware-name" property wouldn't cut it.
No, if you look at the way the name is constructed, some of it comes
from OTP. The plain compatible stuff is for non-Apple platforms. Apple
platforms need lookup of nvram/etc per specific fields in the OTP, and
then we try multiple firmware names from most to least specific because
the distinction often isn't relevant (but in some cases it is, and this
even changes from macOS version to macOS version). Our firmware
extractor actually attempts to prune the firmware tree by deduplicating
and promoting the most popular variant up towards the root then pruning
redundant branches, because otherwise we'd end up with hundreds of
copies or links (which is what macOS does, they don't try multiple
firmware filenames).
If the OTP were easily readable from the bootloader I'd just have thrown
this in m1n1 and kept a fixed firmware-name property, but that involves
full PCIe init and power-up of the wlan module and that's way too much
junk to put in there. Hence, dynamically computing firmware names in the
kernel driver.
If it were a simple 1:1 mapping from device tree blob to firmware files,
I would certainly have advocated for "firmware-name" instead of the more
complex thing we do now.
>> That is not the case here, so it makes perfect sense to specify the
>> firmware with `firmware-name` (which is a standard DT property).
>
> It certainly provides the flexibility to cater for all potential
> nonsense names Apple comes up with for future hardware.
We actually make up the firmware names ourselves in the extractor, so
that's not the reason. But if nothing else I'm pretty sure we already
have n:1 mappings (M2 Pro/Max laptops almost certainly share the same
touchpad firmware for at least the same size chassis models, if not all
4 - haven't looked at that yet though), so using a separate property
means we don't have to play symlink/hardlink games.
>
>> As for the layout, both bare names and paths are in common use:
>>
>> qcom/sm8450-qrd.dts: firmware-name = "qcom/sm8450/slpi.mbn";
>> ti/k3-am64-main.dtsi: firmware-name = "am64-main-r5f0_0-fw";
>>
>> ... but the bare names in particular, judging by some Google searches,
>> are *actually* mapped to bare files in /lib/firmware anyway. So the
>> firmware-name property contains the firmware path in the linux-firmware
>> standard hierarchy, in every case.
>
> Well, I think the device tree should not be tied to a particular OS
> and therefore not be tied to things like linux-firmware.
That's fine, but we need *some* source of truth, and just like the Linux
kernel tree is the system of record for device tree bindings today, I
don't see a good reason not to use linux-firmware as the defacto
standard for firmware organization. There's nothing OS-specific about,
effectively, a list of identifiers that particular firmwares should be
listed under. Think of it as a "path key => expected firmware blob"
mapping. How each OS implements that is up to the OS.
This is similar to the whole vendorfw mechanism I constructed for these
platforms. Sure, it's based on Linuxisms, but the whole thing is trivial
enough to reimplement on any OS without much trouble (just a
fixed-format CPIO archive in the ESP at a known path).
>> I already did the same thing for the touchpad on M2s (which requires
>> analogous Z2 firmware passed to it, just in a different format):
>>
>> dts/apple/t8112-j413.dts: firmware-name = "apple/tpmtfw-j413.bin";
>>
>> Why is having a directory a problem for OpenBSD? Regardless of how
>> firmware is handled behind the scenes, it seems logical to organize it
>> by vendor somehow. It seems to me that gratuitously diverging from the
>> standard firmware hierarchy is only going to cause trouble for OpenBSD.
>> Obviously it's fine to store it somewhere other than /lib/firmware or
>> use a completely unrelated mechanism other than files, but why does the
>> *organization* of the firmware have to diverge? There can only be one DT
>> binding, so we need to agree on a way of specifying firmwares that works
>> cross-OS, and I don't see why "apple/foo.bin" couldn't be made to work
>> for everyone in some way or another.
>
> We organize the firmware by driver. And driver names in *BSD differ
> from Linux since there are different constraints. The firmware is
> organized by driver because we have separate firmware packages for
> each driver that get installed as-needed by a tool that matches on the
> driver name.
That's fair, but you can still have another level of hierarchy after the
driver, no? Or just throw away that level when you parse the
`firmware-name` if you prefer.
> Rather than have the device tree dictate the layout of the firmware
> files, I think it would be better to have the OS driver prepend the
> directory to match the convention of the OS in question. This is what
> we typically do in OpenBSD.
The thing is that for better or for worse, some drivers drive devices
with firmware provided by multiple vendors, and then it can still make
sense to split off by vendor. E.g. brcmfmac wifi is already in the
process of diverging into three firmware lineages provided by
two(/three?) vendors, even if you ignore the entire Apple special
snowflake case. I expect pain to come out of that one for everyone
involved... (well, at least for Apple we can always special case
conditionals on "has OTP" which is effectively the "is Apple" flag).
ISTR that radeon/amdgpu also ended up with separate roots, but it's
mixed and with different firmware formats for each and a fallback.
> Now I did indeed forget about the "dockchannel" touchpad firmware that
> I already handle in OpenBSD. That means I could handle the touchbar
> firmware in the same way. But that is mostly because these firmwares
> are non-distributable, so we don't have firmware packages for them.
> Instead we rely on the Asahi installer to make the firmware available
> on the EFI partition and the OpenBSD installer to move the firmware in
> place on the root filesystem.
>
> So this isn't a big issue.
:)
(Do let me know if you have any big issues of course, you know I don't
want to gratuitously make your life hard!)
- Hector
^ permalink raw reply
* [PATCH 1/1] HID: logitech-hidpp: Add support for Logitech MX Master 3S mouse
From: Rafał Szalecki @ 2023-03-01 1:23 UTC (permalink / raw)
To: linux-input
Cc: Rafał Szalecki, linux-kernel, Jiri Kosina,
Benjamin Tissoires, Filipe Laíns, Bastien Nocera
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) },
{}
};
--
2.37.2
^ permalink raw reply related
* Re: [PATCH RFC 1/4] dt-bindings: input: touchscreen: Add Z2 controller bindings.
From: Mark Kettenis @ 2023-02-28 20:53 UTC (permalink / raw)
To: Hector Martin
Cc: sven, fnkl.kernel, alyssa, dmitry.torokhov, robh+dt,
krzysztof.kozlowski+dt, asahi, rydberg, linux-arm-kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <1874e194-5210-460b-3e8f-0f48962f8a47@marcan.st>
> Date: Tue, 28 Feb 2023 11:58:28 +0900
> From: Hector Martin <marcan@marcan.st>
>
> On 24/02/2023 20.08, Sven Peter wrote:
> > Hi,
> >
> >
> > On Fri, Feb 24, 2023, at 12:04, Sasha Finkelstein wrote:
> >> On Fri, 24 Feb 2023 at 11:55, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >>
> >>> What is the motivation for including the firmware name in the device
> >>> tree rather than constructing it in the driver like what is done for
> >>> the broadcom wireless?
> >> There is no way to identify the device subtype before the firmware is
> >> uploaded, and so i need some way of figuring out which firmware to use.
> >
> > Some Broadcom bluetooth boards use the compatible of the root node (see
> > btbcm_get_board_name in drivers/bluetooth/btbcm.c) which would be "apple,jXXX"
> > for Apple Silicon. I believe the Broadcom WiFi driver has similar logic as well
> > which marcan had to extend to instead of "brcm,board-type" because different
> > WiFi boards can me matched to different Apple Silicon boards. I don't think
> > that's the case for this touchscreen though.
>
> The reason why the brcmfmac stuff needs to construct the firmware name
> itself is that parts of it come from the OTP contents, so there is no
> way to know from the bootloader what the right firmware is.
The name of the "nvram" file is constructed as well, and that uses the
compatible of the machine (the root of the device tree). I suppose
what is special in that case is that several files are tried so a
single 'firmware-name" property wouldn't cut it.
> That is not the case here, so it makes perfect sense to specify the
> firmware with `firmware-name` (which is a standard DT property).
It certainly provides the flexibility to cater for all potential
nonsense names Apple comes up with for future hardware.
> As for the layout, both bare names and paths are in common use:
>
> qcom/sm8450-qrd.dts: firmware-name = "qcom/sm8450/slpi.mbn";
> ti/k3-am64-main.dtsi: firmware-name = "am64-main-r5f0_0-fw";
>
> ... but the bare names in particular, judging by some Google searches,
> are *actually* mapped to bare files in /lib/firmware anyway. So the
> firmware-name property contains the firmware path in the linux-firmware
> standard hierarchy, in every case.
Well, I think the device tree should not be tied to a particular OS
and therefore not be tied to things like linux-firmware.
> I already did the same thing for the touchpad on M2s (which requires
> analogous Z2 firmware passed to it, just in a different format):
>
> dts/apple/t8112-j413.dts: firmware-name = "apple/tpmtfw-j413.bin";
>
> Why is having a directory a problem for OpenBSD? Regardless of how
> firmware is handled behind the scenes, it seems logical to organize it
> by vendor somehow. It seems to me that gratuitously diverging from the
> standard firmware hierarchy is only going to cause trouble for OpenBSD.
> Obviously it's fine to store it somewhere other than /lib/firmware or
> use a completely unrelated mechanism other than files, but why does the
> *organization* of the firmware have to diverge? There can only be one DT
> binding, so we need to agree on a way of specifying firmwares that works
> cross-OS, and I don't see why "apple/foo.bin" couldn't be made to work
> for everyone in some way or another.
We organize the firmware by driver. And driver names in *BSD differ
from Linux since there are different constraints. The firmware is
organized by driver because we have separate firmware packages for
each driver that get installed as-needed by a tool that matches on the
driver name.
Rather than have the device tree dictate the layout of the firmware
files, I think it would be better to have the OS driver prepend the
directory to match the convention of the OS in question. This is what
we typically do in OpenBSD.
Now I did indeed forget about the "dockchannel" touchpad firmware that
I already handle in OpenBSD. That means I could handle the touchbar
firmware in the same way. But that is mostly because these firmwares
are non-distributable, so we don't have firmware packages for them.
Instead we rely on the Asahi installer to make the firmware available
on the EFI partition and the OpenBSD installer to move the firmware in
place on the root filesystem.
So this isn't a big issue.
Cheers,
Mark
^ permalink raw reply
* RE: [PATCH] HID: usbhid: enable remote wakeup for mice
From: Limonciello, Mario @ 2023-02-28 19:07 UTC (permalink / raw)
To: Greg KH
Cc: Oliver Neukum, Michael Wu, jikos@kernel.org,
benjamin.tissoires@redhat.com, linux-usb@vger.kernel.org,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Gong, Richard
In-Reply-To: <Y/5QX1gYsGinrPNF@kroah.com>
[AMD Official Use Only - General]
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Tuesday, February 28, 2023 13:05
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Oliver Neukum <oneukum@suse.com>; Michael Wu
> <michael@allwinnertech.com>; jikos@kernel.org;
> benjamin.tissoires@redhat.com; linux-usb@vger.kernel.org; linux-
> input@vger.kernel.org; linux-kernel@vger.kernel.org; Gong, Richard
> <Richard.Gong@amd.com>
> Subject: Re: [PATCH] HID: usbhid: enable remote wakeup for mice
>
> On Tue, Feb 28, 2023 at 06:50:18PM +0000, Limonciello, Mario wrote:
> > I still keep getting inquiries about this where teams that work on the same
> > hardware for Windows and Linux complain about this difference during
> > their testing.
> >
> > I keep educating them to change it in sysfs (or to use a udev rule), but
> > you have to question if you keep getting something asked about policy
> > over and over if it's actually the right policy.
>
> Why not complain to the Windows team to get them to change their policy
> back as they are the ones that changed it over time and are not
> backwards-compatible with older systems?
>
Heh.
I don't think that's actually true though - Modern Standby is relatively new.
Picking new policies tied to that shouldn't break backwards compatibility.
^ permalink raw reply
* Re: [PATCH v8 3/3] HID: cp2112: Fwnode Support
From: Daniel Kaehn @ 2023-02-28 19:05 UTC (permalink / raw)
To: Andy Shevchenko
Cc: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires,
bartosz.golaszewski, dmitry.torokhov, devicetree, linux-input,
ethan.twardy
In-Reply-To: <Y/03to4XFXPwkGH1@smile.fi.intel.com>
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). 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).
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?
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
> 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.
- Do a case invariant compare on the names (and/or check for both lowercase
and uppercase)
- 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.
What are your / others' thoughts?
Thanks,
Danny Kaehn
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH] HID: usbhid: enable remote wakeup for mice
From: Greg KH @ 2023-02-28 19:05 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Oliver Neukum, Michael Wu, jikos@kernel.org,
benjamin.tissoires@redhat.com, linux-usb@vger.kernel.org,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Gong, Richard
In-Reply-To: <MN0PR12MB61017A45BEF80013FD7B77D5E2AC9@MN0PR12MB6101.namprd12.prod.outlook.com>
On Tue, Feb 28, 2023 at 06:50:18PM +0000, Limonciello, Mario wrote:
> I still keep getting inquiries about this where teams that work on the same
> hardware for Windows and Linux complain about this difference during
> their testing.
>
> I keep educating them to change it in sysfs (or to use a udev rule), but
> you have to question if you keep getting something asked about policy
> over and over if it's actually the right policy.
Why not complain to the Windows team to get them to change their policy
back as they are the ones that changed it over time and are not
backwards-compatible with older systems?
:)
thanks,
greg k-h
^ permalink raw reply
* RE: [PATCH] HID: usbhid: enable remote wakeup for mice
From: Limonciello, Mario @ 2023-02-28 18:50 UTC (permalink / raw)
To: Oliver Neukum, Greg KH, Michael Wu
Cc: jikos@kernel.org, benjamin.tissoires@redhat.com,
linux-usb@vger.kernel.org, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Gong, Richard
In-Reply-To: <f2142d88-259f-302d-da61-e0fc39d1f041@suse.com>
[Public]
> -----Original Message-----
> From: Oliver Neukum <oneukum@suse.com>
> Sent: Tuesday, February 28, 2023 03:03
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Oliver Neukum
> <oneukum@suse.com>; Greg KH <gregkh@linuxfoundation.org>; Michael
> Wu <michael@allwinnertech.com>
> Cc: jikos@kernel.org; benjamin.tissoires@redhat.com; linux-
> usb@vger.kernel.org; linux-input@vger.kernel.org; linux-
> kernel@vger.kernel.org; Gong, Richard <Richard.Gong@amd.com>
> Subject: Re: [PATCH] HID: usbhid: enable remote wakeup for mice
>
> On 23.02.23 20:41, Limonciello, Mario wrote:
>
> Hi,
>
> >> As a system wakeup source a mouse that generates events when
> >> it is moved, however, would make the system unsuspendable, whenever
> >> even
> >> a bit of vibration is acting on the system.
> >> And as S4 is used in many setups to prevent an uncontrolled shutdown
> >> at low power, this must work.
> >
> > At least in my version of the series, this is part of the reason that it was
> > only intended to be used with s2idle.
>
> Yes, that is sensible. If these patches are to be taken at all, that will
> be a necessary condition to meet. But it is not sufficient.
Ack.
>
> > The kernel driver is well aware of what power state you're in the suspend
> > callback (pm_suspend_target_state).
> >
> > What about if we agreed to treat this one special by examining that?
> >
> > If the sysfs is set to "enabled"
>
> If user space needs to manipulate sysfs at all, we can have user space
> tell kernel space exactly what to do. Hence I see no point in
> conditional interpretations values in sysfs at that point.
>
> We are discussing the kernel's default here.
Right, I was meaning if the kernel defaulted to enabled or if userspace
changed it to enabled to follow this behavior.
>
> > * During suspend if your target is s2idle -> program it
> > * During suspend if your target is mem -> disable it
> > * During suspend if your target is hibernate -> disable it
>
> To my mind these defaults make sense.
> However, do they make much more sense than what we are doing now?
If you're talking about purely "policy default", I think it makes more sense.
Userspace can still change it, and it better aligns with what Windows does
out of the box.
>
> > With that type of policy on how to handle the suspend call in place
> > perhaps we could set it to enabled by default?
>
> It pains me to say, but I am afraid in that regard the only
> decision that will not cause ugly surprises is to follow Windows.
> Yet, what is wrong about the current defaults?
I still keep getting inquiries about this where teams that work on the same
hardware for Windows and Linux complain about this difference during
their testing.
I keep educating them to change it in sysfs (or to use a udev rule), but
you have to question if you keep getting something asked about policy
over and over if it's actually the right policy.
^ permalink raw reply
* Re: [PATCH] input/misc: hp_sdc_rtc: mark an unused function as __maybe_unused
From: Helge Deller @ 2023-02-28 18:39 UTC (permalink / raw)
To: Geert Uytterhoeven, Randy Dunlap
Cc: linux-kernel, David Howells, linux-m68k, James E.J. Bottomley,
linux-parisc, Al Viro, Dmitry Torokhov, linux-input
In-Reply-To: <CAMuHMdXWLAbXFUHg+TT45AqVq6kHxK=FN=5wEHnpdiR0dn2L_g@mail.gmail.com>
On 2/9/23 14:56, Geert Uytterhoeven wrote:
> On Thu, Feb 9, 2023 at 2:04 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>> When CONFIG_PROC_FS is not set, one procfs-related function is not
>> used, causing a build error or warning.
>> Mark this function as __maybe_unused to quieten the build.
>>
>> ../drivers/input/misc/hp_sdc_rtc.c:268:12: warning: 'hp_sdc_rtc_proc_show' defined but not used [-Wunused-function]
>> 268 | static int hp_sdc_rtc_proc_show(struct seq_file *m, void *v)
>> | ^~~~~~~~~~~~~~~~~~~~
>>
>> Fixes: c18bd9a1ff47 ("hp_sdc_rtc: Don't use create_proc_read_entry()")
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
applied to parisc tree.
Thanks,
Helge
^ permalink raw reply
* Re: (subset) [PATCH v3 0/9] fix reset line polarity for Goodix touchscreen controllers
From: Quentin Schulz @ 2023-02-28 17:36 UTC (permalink / raw)
To: Bjorn Andersson, samuel, agx, megous, heiko, hdegoede, robh+dt,
wens, michael.riesch, lukma, icenowy, kernel, david, shawnguo,
foss+kernel, linux-imx, festevam, pgwipeout, jagan, agross,
hadess, dmitry.torokhov, jernej.skrabec,
angelogioacchino.delregno, mamlinav, frieder.schrempf, angus,
Sascha Hauer, konrad.dybcio, krzysztof.kozlowski+dt
Cc: linux-kernel, devicetree, linux-input, linux-arm-kernel,
linux-sunxi, linux-arm-msm, linux-rockchip
In-Reply-To: <f6a8a8f1-0eec-2716-d4f1-9115c9d156b6@theobroma-systems.com>
Hi all,
On 1/16/23 13:37, Quentin Schulz wrote:
> Hi Bjorn, all,
>
> On 1/10/23 17:17, Bjorn Andersson wrote:
>> On Mon, 5 Dec 2022 14:40:29 +0100, Quentin Schulz wrote:
>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>
>>> The Goodix touchscreen controller has a reset line active low. It
>>> happens to
>>> also be used to configure its i2c address at runtime. If the reset
>>> line is
>>> incorrectly asserted, the address will be wrongly configured. This
>>> cost me a few
>>> hours, trying to figure out why the touchscreen wouldn't work.
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [8/9] arm64: dts: qcom: msm8998-fxtec: fix touchscreen reset GPIO
>> polarity
>> commit: 8a0721dae68fdb4534e220fc9faae7a0ef2f3785
>>
>
> Thank you for the merge, however I think there could be some issue here.
>
> This requires the patches 1, 2 and 3 all modifying the input driver in
> order to not introduce a regression.
>
> I mistakenly removed the RFC tag and seemingly didn't make it clear
> enough that I had some question on how to properly merge this patch
> series, c.f. "Do we also make this patch series only one patchset since
> the DT patches depend
> on the driver patch and vice-versa? In which tree would this go?" in the
> cover letter.
>
> So please, how do we go on with the rest of the patch series? Should I
> submit a v4 which would be only one patch with DT and input changes all
> at once and Bjorn reverts the patch they had just merged?
>
> @Dmitry, since you would have to merge at least patches 1 to 3 in your
> tree (I assume), would you be willing to take the DT patches at the same
> time through your tree too? Are the appropriate device DT maintainers OK
> with this?
>
Ping.
Cheers,
Quentin
^ permalink raw reply
* Re: [PATCH 4/5] USB: core: Add API to change the wireless_status
From: Bastien Nocera @ 2023-02-28 16: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/giLni7cwDGjLpr@rowland.harvard.edu>
On Thu, 2023-02-23 at 21:34 -0500, Alan Stern wrote:
> On Fri, Feb 24, 2023 at 12:04:12AM +0100, Bastien Nocera wrote:
> > On Thu, 2023-02-23 at 12:07 -0500, Alan Stern wrote:
> > > The refcounting in your patch guarantees that when the work
> > > function
> > > runs, the interface structure will still exist. But refcounting
> > > does
> > > not guarantee that the interface will still be registered in
> > > sysfs,
> > > and
> > > this can actually happen if the work is scheduled immediately
> > > before
> > > the
> > > interface is unregistered.
> > >
> > > So my question is: What will happen when sysfs_update_group(),
> > > sysfs_notify(), and kobject_uevent() are called after the
> > > interface
> > > has
> > > been unregistered from sysfs? Maybe they will work okay -- I
> > > simply
> > > don't know, and I wanted to find out whether you had considered
> > > the
> > > issue.
> >
> > A long week-end started for me a couple of hours ago, but I wanted
> > to
> > dump my thoughts before either I forgot, or it took over my whole
> > week-
> > end ;)
> >
> > I had thought about the problem, and didn't think that sysfs files
> > would get removed before the interface got released/unref'ed and
> > usb_remove_sysfs_intf_files() got called.
> >
> > If the device gets removed from the device bus before it's
> > released,
> > then this patch should fix it:
> > --- a/drivers/usb/core/message.c
> > +++ b/drivers/usb/core/message.c
> > @@ -1917,7 +1917,8 @@ static void __usb_wireless_status_intf(struct
> > work_struct *ws)
> > struct usb_interface *iface =
> > container_of(ws, struct usb_interface,
> > wireless_status_work);
> >
> > - usb_update_wireless_status_attr(iface);
> > + if (intf->sysfs_files_created)
> > + usb_update_wireless_status_attr(iface);
> > usb_put_intf(iface); /* Undo _get_ in
> > usb_set_wireless_status() */
> >
> > The callback would be a no-op if the device's sysfs is already
> > unregistered, just unref'ing the reference it held.
> >
> > What do you think? I'll amend that into my patchset on Monday.
>
> That's a good way to do it, but it does race with
> usb_remove_sysfs_intf_files(). To prevent this race, you can protect
> the test and function call with device_lock(iface->dev.parent) (that
> is,
> lock the interface's parent usb_device).
Perfect, I've done that locally.
I'll send an updated patchset once I've been able to test it against
the hardware I have.
Cheers
^ permalink raw reply
* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-02-28 15:11 UTC (permalink / raw)
To: Benjamin Tissoires, Linux regressions mailing list, Hans De Goede
Cc: Jiri Kosina, Dmitry Torokhov, open list:HID CORE LAYER, LKML,
Gé Koerkamp
In-Reply-To: <CAO-hwJJ2OMFgpmrXK_Z43z0ddujaS1fNjaAJSWwao4qQN+pJ6w@mail.gmail.com>
On 28.02.23 14:26, Benjamin Tissoires wrote:
> On Tue, Feb 28, 2023 at 12:32 PM Thorsten Leemhuis
> <regressions@leemhuis.info> wrote:
>>
>> On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
>> wrote:
>>> Hi, this is your Linux kernel regression tracker.
>>>
>>> I noticed a regression report in bugzilla.kernel.org. As many (most?)
>>> kernel developer don't keep an eye on it, I decided to forward it by
>>> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
>>
>> The reporter recently confirmed in the ticket that the issue still
>> happens with 6.2.
>>
>> There wasn't any reply from any of the input developers here or in
>> bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
>> take a minute and at least briefly look into this as answer something
>> like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
>> for an option", or "we have no idea what might cause this, this needs to
>> be bisected"? That would help a lot.
>
> The working dmesg shows a line with:
> hid-generic 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00 Device
> [SIS0817:00 0457:10FB] on i2c-SIS0817:00
> and then
> hid-multitouch 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00
> Device [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>
> But these 2 lines do not appear on the 6.1.4 logs.
>
> So the device is not properly enumerated by ACPI or I2C. Hans might
> have an idea on how to debug/solve that issue.
>
> Also there were no changes between v5.15 and v5.16 in i2c-hid.ko, so
> it's unlikely to be an issue there (unless '5.16' means '5.16.x').
Great, many thx for your help!
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>> --
>> Everything you wanna know about Linux kernel regression tracking:
>> https://linux-regtracking.leemhuis.info/about/#tldr
>> If I did something stupid, please tell me, as explained on that page.
>>
>> #regzbot poke
>>>> Gé Koerkamp 2023-01-17 20:21:51 UTC
>>>>
>>>> Created attachment 303619 [details]
>>>> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
>>>>
>>>> Overview:
>>>> The touchscreen does not react on touch events.
>>>> Touchscreen display and touchpad are working.
>>>>
>>>> Step to reproduce:
>>>> Open any UI page
>>>> Try to use touch on relevant UI controls (buttons etc.)
>>>>
>>>> Result:
>>>> No reaction on screen touches
>>>>
>>>> Expected result:
>>>> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
>>>>
>>>> Build information:
>>>> The error happens with kernel version 6.1.4
>>>> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
>>>>
>>>> Additional builds:
>>>> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
>>>>
>>>> Important remark:
>>>> Touchscreen still works fine with kernel 6.1.4 using
>>>> - an HP x2 detachable 10-p0xx or
>>>> - a Lenovo yoga 520-14ikb
>>>>
>>>> So it could be a hardware dependent issue
>>>
>>> See the ticket for more details.
>>>
>>>
>>> [TLDR for the rest of this mail: I'm adding this report to the list of
>>> tracked Linux kernel regressions; the text you find below is based on a
>>> few templates paragraphs you might have encountered already in similar
>>> form.]
>>>
>>> BTW, let me use this mail to also add the report to the list of tracked
>>> regressions to ensure it's doesn't fall through the cracks:
>>>
>>> #regzbot introduced: v5.15..v5.16
>>> https://bugzilla.kernel.org/show_bug.cgi?id=216946
>>> #regzbot title: hid: touchscreen broken with Toshiba satellite click
>>> mini l9w-b
>>> #regzbot ignore-activity
>>>
>>> This isn't a regression? This issue or a fix for it are already
>>> discussed somewhere else? It was fixed already? You want to clarify when
>>> the regression started to happen? Or point out I got the title or
>>> something else totally wrong? Then just reply and tell me -- ideally
>>> while also telling regzbot about it, as explained by the page listed in
>>> the footer of this mail.
>>>
>>> Developers: When fixing the issue, remember to add 'Link:' tags pointing
>>> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
>>> this thread sees some discussion). See page linked in footer for details.
>>>
>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>> --
>>> Everything you wanna know about Linux kernel regression tracking:
>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>> If I did something stupid, please tell me, as explained on that page.
>>
>
>
>
^ permalink raw reply
* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Benjamin Tissoires @ 2023-02-28 13:26 UTC (permalink / raw)
To: Linux regressions mailing list, Hans De Goede
Cc: Jiri Kosina, Dmitry Torokhov, open list:HID CORE LAYER, LKML,
Gé Koerkamp
In-Reply-To: <abb495f7-f973-4614-846b-d3922dc0fe25@leemhuis.info>
On Tue, Feb 28, 2023 at 12:32 PM Thorsten Leemhuis
<regressions@leemhuis.info> wrote:
>
> On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
> wrote:
> > Hi, this is your Linux kernel regression tracker.
> >
> > I noticed a regression report in bugzilla.kernel.org. As many (most?)
> > kernel developer don't keep an eye on it, I decided to forward it by
> > mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
>
> The reporter recently confirmed in the ticket that the issue still
> happens with 6.2.
>
> There wasn't any reply from any of the input developers here or in
> bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
> take a minute and at least briefly look into this as answer something
> like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
> for an option", or "we have no idea what might cause this, this needs to
> be bisected"? That would help a lot.
The working dmesg shows a line with:
hid-generic 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00 Device
[SIS0817:00 0457:10FB] on i2c-SIS0817:00
and then
hid-multitouch 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00
Device [SIS0817:00 0457:10FB] on i2c-SIS0817:00
But these 2 lines do not appear on the 6.1.4 logs.
So the device is not properly enumerated by ACPI or I2C. Hans might
have an idea on how to debug/solve that issue.
Also there were no changes between v5.15 and v5.16 in i2c-hid.ko, so
it's unlikely to be an issue there (unless '5.16' means '5.16.x').
Cheers,
Benjamin
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
> >> Gé Koerkamp 2023-01-17 20:21:51 UTC
> >>
> >> Created attachment 303619 [details]
> >> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
> >>
> >> Overview:
> >> The touchscreen does not react on touch events.
> >> Touchscreen display and touchpad are working.
> >>
> >> Step to reproduce:
> >> Open any UI page
> >> Try to use touch on relevant UI controls (buttons etc.)
> >>
> >> Result:
> >> No reaction on screen touches
> >>
> >> Expected result:
> >> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
> >>
> >> Build information:
> >> The error happens with kernel version 6.1.4
> >> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
> >>
> >> Additional builds:
> >> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
> >>
> >> Important remark:
> >> Touchscreen still works fine with kernel 6.1.4 using
> >> - an HP x2 detachable 10-p0xx or
> >> - a Lenovo yoga 520-14ikb
> >>
> >> So it could be a hardware dependent issue
> >
> > See the ticket for more details.
> >
> >
> > [TLDR for the rest of this mail: I'm adding this report to the list of
> > tracked Linux kernel regressions; the text you find below is based on a
> > few templates paragraphs you might have encountered already in similar
> > form.]
> >
> > BTW, let me use this mail to also add the report to the list of tracked
> > regressions to ensure it's doesn't fall through the cracks:
> >
> > #regzbot introduced: v5.15..v5.16
> > https://bugzilla.kernel.org/show_bug.cgi?id=216946
> > #regzbot title: hid: touchscreen broken with Toshiba satellite click
> > mini l9w-b
> > #regzbot ignore-activity
> >
> > This isn't a regression? This issue or a fix for it are already
> > discussed somewhere else? It was fixed already? You want to clarify when
> > the regression started to happen? Or point out I got the title or
> > something else totally wrong? Then just reply and tell me -- ideally
> > while also telling regzbot about it, as explained by the page listed in
> > the footer of this mail.
> >
> > Developers: When fixing the issue, remember to add 'Link:' tags pointing
> > to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
> > this thread sees some discussion). See page linked in footer for details.
> >
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> > --
> > Everything you wanna know about Linux kernel regression tracking:
> > https://linux-regtracking.leemhuis.info/about/#tldr
> > If I did something stupid, please tell me, as explained on that page.
>
^ permalink raw reply
* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Gé Koerkamp @ 2023-02-28 12:10 UTC (permalink / raw)
To: Linux regressions mailing list
Cc: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov,
open list:HID CORE LAYER, LKML
In-Reply-To: <abb495f7-f973-4614-846b-d3922dc0fe25@leemhuis.info>
Hi,
additionally, I searched for any reported SIS0817 issues, yet did not find any.
But perhaps it rings a bell.
Cheers - Gé
On 28 Feb 2023, at 12:32, Thorsten Leemhuis <regressions@leemhuis.info> wrote:
On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
wrote:
> Hi, this is your Linux kernel regression tracker.
>
> I noticed a regression report in bugzilla.kernel.org. As many (most?)
> kernel developer don't keep an eye on it, I decided to forward it by
> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
The reporter recently confirmed in the ticket that the issue still
happens with 6.2.
There wasn't any reply from any of the input developers here or in
bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
take a minute and at least briefly look into this as answer something
like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
for an option", or "we have no idea what might cause this, this needs to
be bisected"? That would help a lot.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
>> Gé Koerkamp 2023-01-17 20:21:51 UTC
>>
>> Created attachment 303619 [details]
>> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
>>
>> Overview:
>> The touchscreen does not react on touch events.
>> Touchscreen display and touchpad are working.
>>
>> Step to reproduce:
>> Open any UI page
>> Try to use touch on relevant UI controls (buttons etc.)
>>
>> Result:
>> No reaction on screen touches
>>
>> Expected result:
>> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
>>
>> Build information:
>> The error happens with kernel version 6.1.4
>> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
>>
>> Additional builds:
>> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
>>
>> Important remark:
>> Touchscreen still works fine with kernel 6.1.4 using
>> - an HP x2 detachable 10-p0xx or
>> - a Lenovo yoga 520-14ikb
>>
>> So it could be a hardware dependent issue
>
> See the ticket for more details.
>
>
> [TLDR for the rest of this mail: I'm adding this report to the list of
> tracked Linux kernel regressions; the text you find below is based on a
> few templates paragraphs you might have encountered already in similar
> form.]
>
> BTW, let me use this mail to also add the report to the list of tracked
> regressions to ensure it's doesn't fall through the cracks:
>
> #regzbot introduced: v5.15..v5.16
> https://bugzilla.kernel.org/show_bug.cgi?id=216946
> #regzbot title: hid: touchscreen broken with Toshiba satellite click
> mini l9w-b
> #regzbot ignore-activity
>
> This isn't a regression? This issue or a fix for it are already
> discussed somewhere else? It was fixed already? You want to clarify when
> the regression started to happen? Or point out I got the title or
> something else totally wrong? Then just reply and tell me -- ideally
> while also telling regzbot about it, as explained by the page listed in
> the footer of this mail.
>
> Developers: When fixing the issue, remember to add 'Link:' tags pointing
> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
> this thread sees some discussion). See page linked in footer for details.
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
^ permalink raw reply
* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Thorsten Leemhuis @ 2023-02-28 11:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Dmitry Torokhov
Cc: open list:HID CORE LAYER, LKML, Linux kernel regressions list,
Gé Koerkamp
In-Reply-To: <32a14a8a-9795-4c8c-7e00-da9012f548f8@leemhuis.info>
On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
wrote:
> Hi, this is your Linux kernel regression tracker.
>
> I noticed a regression report in bugzilla.kernel.org. As many (most?)
> kernel developer don't keep an eye on it, I decided to forward it by
> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
The reporter recently confirmed in the ticket that the issue still
happens with 6.2.
There wasn't any reply from any of the input developers here or in
bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
take a minute and at least briefly look into this as answer something
like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
for an option", or "we have no idea what might cause this, this needs to
be bisected"? That would help a lot.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
>> Gé Koerkamp 2023-01-17 20:21:51 UTC
>>
>> Created attachment 303619 [details]
>> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
>>
>> Overview:
>> The touchscreen does not react on touch events.
>> Touchscreen display and touchpad are working.
>>
>> Step to reproduce:
>> Open any UI page
>> Try to use touch on relevant UI controls (buttons etc.)
>>
>> Result:
>> No reaction on screen touches
>>
>> Expected result:
>> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
>>
>> Build information:
>> The error happens with kernel version 6.1.4
>> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
>>
>> Additional builds:
>> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
>>
>> Important remark:
>> Touchscreen still works fine with kernel 6.1.4 using
>> - an HP x2 detachable 10-p0xx or
>> - a Lenovo yoga 520-14ikb
>>
>> So it could be a hardware dependent issue
>
> See the ticket for more details.
>
>
> [TLDR for the rest of this mail: I'm adding this report to the list of
> tracked Linux kernel regressions; the text you find below is based on a
> few templates paragraphs you might have encountered already in similar
> form.]
>
> BTW, let me use this mail to also add the report to the list of tracked
> regressions to ensure it's doesn't fall through the cracks:
>
> #regzbot introduced: v5.15..v5.16
> https://bugzilla.kernel.org/show_bug.cgi?id=216946
> #regzbot title: hid: touchscreen broken with Toshiba satellite click
> mini l9w-b
> #regzbot ignore-activity
>
> This isn't a regression? This issue or a fix for it are already
> discussed somewhere else? It was fixed already? You want to clarify when
> the regression started to happen? Or point out I got the title or
> something else totally wrong? Then just reply and tell me -- ideally
> while also telling regzbot about it, as explained by the page listed in
> the footer of this mail.
>
> Developers: When fixing the issue, remember to add 'Link:' tags pointing
> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
> this thread sees some discussion). See page linked in footer for details.
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
^ permalink raw reply
* [PATCH v4 04/19] thermal/core: Use the thermal zone 'devdata' accessor in remaining drivers
From: Daniel Lezcano @ 2023-02-28 11:22 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: <20230228112238.2312273-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 v4 00/19] Self-encapsulate the thermal zone device structure
From: Daniel Lezcano @ 2023-02-28 11:22 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:
- 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 (19):
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/tegra: Do not enable the thermal zone, it is already enabled
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, 217 insertions(+), 247 deletions(-)
--
2.34.1
^ 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-02-28 11:07 UTC (permalink / raw)
To: Hans de Goede, dmitry.torokhov, swboyd, gregkh, mkorpershoek,
chenhuacai, wsa+renesas, tiwai, linux-input, linux-kernel
In-Reply-To: <a2216cc2-b719-12e1-264c-374fc467db14@redhat.com>
Am 28.02.23 um 09:41 schrieb Hans de Goede:
> Hi Werner,
>
> On 2/27/23 19:59, Werner Sembach wrote:
>> 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.
> Can you please rework this series so that the quirk based setting of
> the "atkbd.reset" equivalent on the kernel commandline becomes another
> SERIO_QUIRK_* flag and avoid the duplication of the DMI ids?
I'm not sure how to cleanly do this, since atkbd is an own module?
Kind Regards,
Werner
>
> Regards,
>
> Hans
>
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox