* [PATCH 2/3] HID: steelseries: Add battery request info (byte flags) to device info.
From: Benjamin Wheeler @ 2026-01-20 20:02 UTC (permalink / raw)
To: srimanachanta; +Cc: bentiss, jikos, linux-input, linux-kernel
In-Reply-To: <20260120200202.1225216-1-benjaminwheeler0510@gmail.com>
This way, the device info is decoupled from the functions that call it.
This will allow for simplification of calling these functions.
Signed-off-by: Benjamin Wheeler <benjaminwheeler0510@gmail.com>
---
drivers/hid/hid-steelseries.c | 79 +++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index 63b086fa6df0..dabc4763f072 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -31,12 +31,22 @@
/* Legacy quirk flag for SRW-S1 */
#define STEELSERIES_SRWS1 BIT(0)
+/**
+ * Stores info about how to send a report to this device.
+ */
+struct steelseries_report_data {
+ const u8 data[2];
+ const u8 len; // Only ever 1 or 2, no need for size_t.
+ const enum hid_report_type type;
+};
+
struct steelseries_device_info {
u16 product_id;
const char *name;
u8 interface_binding_mode; /* 0 = first enumerated, 1 = specific interface(s) */
u16 valid_interfaces; /* Bitmask when mode = 1, ignored when mode = 0 */
unsigned long capabilities;
+ const struct steelseries_report_data *const report_data;
};
struct steelseries_device {
@@ -382,12 +392,55 @@ static const __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev,
return rdesc;
}
+// Report data for: arctis 1, 1x, 7p, 7x
+static const struct steelseries_report_data report_data__1_1x_7p_7x = {
+ .data = { 0x06, 0x12 },
+ .len = 2,
+ .type = HID_FEATURE_REPORT
+};
+
+// Report data for: arctis 7, 7 v2 (2019 ed.)
+static const struct steelseries_report_data report_data__7_7v2 = {
+ .data = { 0x06, 0x18 },
+ .len = 2,
+ .type = HID_OUTPUT_REPORT
+};
+
+// Report data for: 7 plus, 7 plus P, 7 plus X, 7 plus Destiny, all novas besides nova 3p, nova 3x
+static const struct steelseries_report_data report_data__7plus_nova = {
+ .data = { 0x00, 0xb0 },
+ .len = 2,
+ .type = HID_OUTPUT_REPORT
+};
+
+// Report data for: arctis 9
+static const struct steelseries_report_data report_data__9 = {
+ .data = { 0x00, 0x20 },
+ .len = 2,
+ .type = HID_FEATURE_REPORT
+};
+
+// Report data for: Pro
+static const struct steelseries_report_data report_data__pro = {
+ .data = { 0x40, 0xAA },
+ .len = 2,
+ .type = HID_OUTPUT_REPORT
+};
+
+// Report data for: Nova 3p, 3x
+static const struct steelseries_report_data report_data__nova_3p_3x = {
+ .data = { 0xb0 },
+ .len = 1,
+ .type = HID_OUTPUT_REPORT
+};
+
static const struct steelseries_device_info arctis_1_info = {
.product_id = USB_DEVICE_ID_STEELSERIES_ARCTIS_1,
.name = "Arctis 1 Wireless",
.interface_binding_mode = 1,
.valid_interfaces = BIT(3),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__1_1x_7p_7x,
};
static const struct steelseries_device_info arctis_1_x_info = {
@@ -396,6 +449,7 @@ static const struct steelseries_device_info arctis_1_x_info = {
.interface_binding_mode = 1,
.valid_interfaces = BIT(3),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__1_1x_7p_7x,
};
static const struct steelseries_device_info arctis_7_info = {
@@ -404,6 +458,7 @@ static const struct steelseries_device_info arctis_7_info = {
.interface_binding_mode = 1,
.valid_interfaces = BIT(5),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__7_7v2,
};
static const struct steelseries_device_info arctis_7_p_info = {
@@ -412,6 +467,7 @@ static const struct steelseries_device_info arctis_7_p_info = {
.interface_binding_mode = 1,
.valid_interfaces = BIT(3),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__1_1x_7p_7x,
};
static const struct steelseries_device_info arctis_7_x_info = {
@@ -420,6 +476,7 @@ static const struct steelseries_device_info arctis_7_x_info = {
.interface_binding_mode = 1,
.valid_interfaces = BIT(3),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__1_1x_7p_7x,
};
static const struct steelseries_device_info arctis_7_gen2_info = {
@@ -428,6 +485,7 @@ static const struct steelseries_device_info arctis_7_gen2_info = {
.interface_binding_mode = 1,
.valid_interfaces = BIT(5),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__7_7v2,
};
static const struct steelseries_device_info arctis_7_plus_info = {
@@ -437,6 +495,7 @@ static const struct steelseries_device_info arctis_7_plus_info = {
.valid_interfaces = BIT(3),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY |
SS_CAP_INACTIVE_TIME | SS_CAP_CHATMIX,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_7_plus_p_info = {
@@ -446,6 +505,7 @@ static const struct steelseries_device_info arctis_7_plus_p_info = {
.valid_interfaces = BIT(3),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY |
SS_CAP_INACTIVE_TIME | SS_CAP_CHATMIX,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_7_plus_x_info = {
@@ -455,6 +515,7 @@ static const struct steelseries_device_info arctis_7_plus_x_info = {
.valid_interfaces = BIT(3),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY |
SS_CAP_INACTIVE_TIME | SS_CAP_CHATMIX,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_7_plus_destiny_info = {
@@ -464,6 +525,7 @@ static const struct steelseries_device_info arctis_7_plus_destiny_info = {
.valid_interfaces = BIT(3),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY |
SS_CAP_INACTIVE_TIME | SS_CAP_CHATMIX,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_9_info = {
@@ -473,6 +535,7 @@ static const struct steelseries_device_info arctis_9_info = {
.valid_interfaces = 0,
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY |
SS_CAP_INACTIVE_TIME | SS_CAP_CHATMIX,
+ .report_data = &report_data__9,
};
static const struct steelseries_device_info arctis_pro_info = {
@@ -481,6 +544,7 @@ static const struct steelseries_device_info arctis_pro_info = {
.interface_binding_mode = 0,
.valid_interfaces = 0,
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__pro,
};
static const struct steelseries_device_info arctis_nova_3_info = {
@@ -490,6 +554,7 @@ static const struct steelseries_device_info arctis_nova_3_info = {
.valid_interfaces = BIT(4),
.capabilities = SS_CAP_SIDETONE | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_3_p_info = {
@@ -499,6 +564,7 @@ static const struct steelseries_device_info arctis_nova_3_p_info = {
.valid_interfaces = BIT(0),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY |
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_VOLUME,
+ .report_data = &report_data__nova_3p_3x,
};
static const struct steelseries_device_info arctis_nova_3_x_info = {
@@ -508,6 +574,7 @@ static const struct steelseries_device_info arctis_nova_3_x_info = {
.valid_interfaces = BIT(0),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY |
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_VOLUME,
+ .report_data = &report_data__nova_3p_3x,
};
static const struct steelseries_device_info arctis_nova_5_info = {
@@ -518,6 +585,7 @@ static const struct steelseries_device_info arctis_nova_5_info = {
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_CHATMIX |
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_5_x_info = {
@@ -528,6 +596,7 @@ static const struct steelseries_device_info arctis_nova_5_x_info = {
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_CHATMIX |
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_7_info = {
@@ -539,6 +608,7 @@ static const struct steelseries_device_info arctis_nova_7_info = {
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER |
SS_CAP_BT_POWER_ON | SS_CAP_BT_CALL_VOL,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_7_x_info = {
@@ -550,6 +620,7 @@ static const struct steelseries_device_info arctis_nova_7_x_info = {
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER |
SS_CAP_BT_POWER_ON | SS_CAP_BT_CALL_VOL,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_7_p_info = {
@@ -561,6 +632,7 @@ static const struct steelseries_device_info arctis_nova_7_p_info = {
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER |
SS_CAP_BT_POWER_ON | SS_CAP_BT_CALL_VOL,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_7_x_rev2_info = {
@@ -572,6 +644,7 @@ static const struct steelseries_device_info arctis_nova_7_x_rev2_info = {
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER |
SS_CAP_BT_POWER_ON | SS_CAP_BT_CALL_VOL,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_7_diablo_info = {
@@ -583,6 +656,7 @@ static const struct steelseries_device_info arctis_nova_7_diablo_info = {
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER |
SS_CAP_BT_POWER_ON | SS_CAP_BT_CALL_VOL,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_7_wow_info = {
@@ -594,6 +668,7 @@ static const struct steelseries_device_info arctis_nova_7_wow_info = {
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER |
SS_CAP_BT_POWER_ON | SS_CAP_BT_CALL_VOL,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_7_gen2_info = {
@@ -605,6 +680,7 @@ static const struct steelseries_device_info arctis_nova_7_gen2_info = {
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER |
SS_CAP_BT_POWER_ON | SS_CAP_BT_CALL_VOL,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_7_x_gen2_info = {
@@ -616,6 +692,7 @@ static const struct steelseries_device_info arctis_nova_7_x_gen2_info = {
SS_CAP_INACTIVE_TIME | SS_CAP_MIC_MUTE_LED |
SS_CAP_MIC_VOLUME | SS_CAP_VOLUME_LIMITER |
SS_CAP_BT_POWER_ON | SS_CAP_BT_CALL_VOL,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_pro_info = {
@@ -624,6 +701,7 @@ static const struct steelseries_device_info arctis_nova_pro_info = {
.interface_binding_mode = 1,
.valid_interfaces = BIT(4),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__7plus_nova,
};
static const struct steelseries_device_info arctis_nova_pro_x_info = {
@@ -632,6 +710,7 @@ static const struct steelseries_device_info arctis_nova_pro_x_info = {
.interface_binding_mode = 1,
.valid_interfaces = BIT(4),
.capabilities = SS_CAP_SIDETONE | SS_CAP_BATTERY | SS_CAP_INACTIVE_TIME,
+ .report_data = &report_data__7plus_nova,
};
#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000
--
2.52.0
^ permalink raw reply related
* [PATCH 3/3] HID: steelseries: Use device data for battery requests
From: Benjamin Wheeler @ 2026-01-20 20:02 UTC (permalink / raw)
To: srimanachanta; +Cc: bentiss, jikos, linux-input, linux-kernel
In-Reply-To: <20260120200202.1225216-1-benjaminwheeler0510@gmail.com>
This eliminates several functions that are no longer needed.
When making a battery request, the required data to do so is simply
pulled from the device struct.
Signed-off-by: Benjamin Wheeler <benjaminwheeler0510@gmail.com>
---
drivers/hid/hid-steelseries.c | 93 +++++------------------------------
1 file changed, 11 insertions(+), 82 deletions(-)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index dabc4763f072..1200213bc8f3 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -809,44 +809,17 @@ static enum power_supply_property steelseries_battery_props[] = {
POWER_SUPPLY_PROP_CAPACITY_LEVEL,
};
-/* Forward declarations for battery request functions */
-static int steelseries_arctis_1_request_battery(struct hid_device *hdev);
-static int steelseries_arctis_7_plus_request_battery(struct hid_device *hdev);
-static int steelseries_arctis_9_request_battery(struct hid_device *hdev);
-static int steelseries_arctis_nova_request_battery(struct hid_device *hdev);
-static int steelseries_arctis_nova_3p_request_battery(struct hid_device *hdev);
-static int
-steelseries_arctis_pro_wireless_request_battery(struct hid_device *hdev);
+static int __steelseries_send_report(struct hid_device *const hdev,
+ const u8 *const data, const size_t len,
+ const enum hid_report_type type);
-static int steelseries_request_battery(struct hid_device *hdev)
+static int
+steelseries_request_battery(const struct steelseries_device *const sd)
{
- u16 product = hdev->product;
-
- /* Route to device-specific battery request handler */
- if (product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1 ||
- product == USB_DEVICE_ID_STEELSERIES_ARCTIS_1_X ||
- product == USB_DEVICE_ID_STEELSERIES_ARCTIS_7_P ||
- product == USB_DEVICE_ID_STEELSERIES_ARCTIS_7_X)
- return steelseries_arctis_1_request_battery(hdev);
-
- if (product == USB_DEVICE_ID_STEELSERIES_ARCTIS_7_PLUS ||
- product == USB_DEVICE_ID_STEELSERIES_ARCTIS_7_PLUS_P ||
- product == USB_DEVICE_ID_STEELSERIES_ARCTIS_7_PLUS_X ||
- product == USB_DEVICE_ID_STEELSERIES_ARCTIS_7_PLUS_DESTINY)
- return steelseries_arctis_7_plus_request_battery(hdev);
-
- if (product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9)
- return steelseries_arctis_9_request_battery(hdev);
-
- if (product == USB_DEVICE_ID_STEELSERIES_ARCTIS_PRO)
- return steelseries_arctis_pro_wireless_request_battery(hdev);
-
- if (product == USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_3_P ||
- product == USB_DEVICE_ID_STEELSERIES_ARCTIS_NOVA_3_X)
- return steelseries_arctis_nova_3p_request_battery(hdev);
-
- /* All other Nova series use the same battery request */
- return steelseries_arctis_nova_request_battery(hdev);
+ const struct steelseries_report_data *const report_data =
+ sd->info->report_data; // Alias
+ return __steelseries_send_report(sd->hdev, report_data->data,
+ report_data->len, report_data->type);
}
static void steelseries_battery_timer_tick(struct work_struct *work)
@@ -854,7 +827,7 @@ static void steelseries_battery_timer_tick(struct work_struct *work)
struct steelseries_device *sd = container_of(
work, struct steelseries_device, battery_work.work);
- steelseries_request_battery(sd->hdev);
+ steelseries_request_battery(sd);
}
static int steelseries_battery_register(struct steelseries_device *sd)
@@ -895,7 +868,7 @@ static int steelseries_battery_register(struct steelseries_device *sd)
power_supply_powers(sd->battery, &sd->hdev->dev);
INIT_DELAYED_WORK(&sd->battery_work, steelseries_battery_timer_tick);
- steelseries_request_battery(sd->hdev);
+ steelseries_request_battery(sd);
/* Arctis 9 may need a retry */
if (sd->hdev->product == USB_DEVICE_ID_STEELSERIES_ARCTIS_9) {
@@ -1685,50 +1658,6 @@ static const struct attribute_group steelseries_attr_group = {
.is_visible = steelseries_attr_is_visible,
};
-static int steelseries_arctis_1_request_battery(struct hid_device *hdev)
-{
- const u8 data[] = { 0x06, 0x12 };
-
- return steelseries_send_feature_report(hdev, data, sizeof(data));
-}
-
-static int steelseries_arctis_7_plus_request_battery(struct hid_device *hdev)
-{
- const u8 data[] = { 0x00, 0xb0 };
-
- return steelseries_send_output_report(hdev, data, sizeof(data));
-}
-
-static int steelseries_arctis_9_request_battery(struct hid_device *hdev)
-{
- const u8 data[] = { 0x00, 0x20 };
-
- return steelseries_send_feature_report(hdev, data, sizeof(data));
-}
-
-static int steelseries_arctis_nova_request_battery(struct hid_device *hdev)
-{
- const u8 data[] = { 0x00, 0xb0 };
-
- return steelseries_send_output_report(hdev, data, sizeof(data));
-}
-
-static int steelseries_arctis_nova_3p_request_battery(struct hid_device *hdev)
-{
- const u8 data[] = { 0xb0 };
-
- return steelseries_send_output_report(hdev, data, sizeof(data));
-}
-
-static int
-steelseries_arctis_pro_wireless_request_battery(struct hid_device *hdev)
-{
- /* Request battery - response will arrive asynchronously via raw_event */
- const u8 data[] = { 0x40, 0xAA };
-
- return steelseries_send_output_report(hdev, data, sizeof(data));
-}
-
static int steelseries_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data, int size)
{
--
2.52.0
^ permalink raw reply related
* [PATCH 1/3] HID: steelseries: Clean up hid send_report functions
From: Benjamin Wheeler @ 2026-01-20 20:02 UTC (permalink / raw)
To: srimanachanta; +Cc: bentiss, jikos, linux-input, linux-kernel
In-Reply-To: <20260120200202.1225216-1-benjaminwheeler0510@gmail.com>
These functions do the same thing, so combine them into one with a
parameter.
Use helper functions to keep callsites the same.
Signed-off-by: Benjamin Wheeler <benjaminwheeler0510@gmail.com>
---
drivers/hid/hid-steelseries.c | 45 ++++++++++++++++-------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index a0046fbc830b..63b086fa6df0 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -829,9 +829,13 @@ static int steelseries_battery_register(struct steelseries_device *sd)
return 0;
}
-/* Helper function to send feature reports */
-static int steelseries_send_feature_report(struct hid_device *hdev,
- const u8 *data, size_t len)
+/**
+ * Send an hid report to the device.
+ * Supported types are output reports and feature reports.
+ */
+static int __steelseries_send_report(struct hid_device *const hdev,
+ const u8 *const data, const size_t len,
+ const enum hid_report_type type)
{
u8 *buf;
int ret;
@@ -840,8 +844,9 @@ static int steelseries_send_feature_report(struct hid_device *hdev,
if (!buf)
return -ENOMEM;
- ret = hid_hw_raw_request(hdev, data[0], buf, len, HID_FEATURE_REPORT,
+ ret = hid_hw_raw_request(hdev, data[0], buf, len, type,
HID_REQ_SET_REPORT);
+
kfree(buf);
if (ret < 0)
@@ -852,28 +857,19 @@ static int steelseries_send_feature_report(struct hid_device *hdev,
return 0;
}
-/* Helper function to send output reports */
-static int steelseries_send_output_report(struct hid_device *hdev,
- const u8 *data, size_t len)
+static inline int steelseries_send_feature_report(struct hid_device *const hdev,
+ const u8 *const data,
+ const size_t len)
{
- u8 *buf;
- int ret;
-
- buf = kmemdup(data, len, GFP_KERNEL);
- if (!buf)
- return -ENOMEM;
-
- /* Use raw_request with OUTPUT_REPORT type for devices without Interrupt OUT */
- ret = hid_hw_raw_request(hdev, data[0], buf, len, HID_OUTPUT_REPORT,
- HID_REQ_SET_REPORT);
- kfree(buf);
-
- if (ret < 0)
- return ret;
- if (ret < len)
- return -EIO;
+ return __steelseries_send_report(hdev, data, len, HID_FEATURE_REPORT);
+}
- return 0;
+static inline int steelseries_send_output_report(struct hid_device *const hdev,
+ const u8 *const data,
+ const size_t len)
+{
+ // NOTE: Output report (HID_OUTPUT_REPORT) is for devices without Interrupt OUT
+ return __steelseries_send_report(hdev, data, len, HID_OUTPUT_REPORT);
}
/* Sidetone level attribute */
@@ -2170,3 +2166,4 @@ MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
MODULE_AUTHOR("Christian Mayer <git@mayer-bgk.de>");
MODULE_AUTHOR("Sriman Achanta <srimanachanta@gmail.com>");
+MODULE_AUTHOR("Benjamin Wheeler <benjaminwheeler0510@gmail.com>");
--
2.52.0
^ permalink raw reply related
* [PATCH 0/3] HID: steelseries: clean up functions, move battery request data to structs
From: Benjamin Wheeler @ 2026-01-20 20:01 UTC (permalink / raw)
To: srimanachanta; +Cc: bentiss, jikos, linux-input, linux-kernel
In-Reply-To: <20260112041941.40531-1-srimanachanta@gmail.com>
Hi Sriman,
As requested, here are my patches for your v2 of the new driver.
They fix an issue where the Arctis 7 incorrectly shows 100% battery due
to submitting an invalid report. They also include some cleanups of a
couple functions.
I've tested these by loading the module in a VM, and seeing that the
Arctis 7 reads the correct battery % through the Ubuntu desktop.
If these don't fit nicely into your v3, feel free to take what you need
out of them, or, once you submit v3, I can re-do my patches and
re-submit.
Thanks!
Benjamin Wheeler (3):
HID: steelseries: Clean up hid send_report functions
HID: steelseries: Add battery request info (byte flags) to device
info.
HID: steelseries: Use device data for battery requests
drivers/hid/hid-steelseries.c | 217 +++++++++++++++++-----------------
1 file changed, 111 insertions(+), 106 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: [PATCH v4 3/6] dt-bindings: input: google,goldfish-events-keypad: Convert to DT schema
From: Dmitry Torokhov @ 2026-01-20 19:56 UTC (permalink / raw)
To: Kuan-Wei Chiu
Cc: airlied, simona, maarten.lankhorst, mripard, tzimmermann, robh,
krzk+dt, conor+dt, sre, gregkh, jirislaby, lgirdwood, broonie,
jserv, eleanor15x, dri-devel, devicetree, linux-kernel,
linux-input, linux-pm, linux-serial, linux-sound
In-Reply-To: <20260113092602.3197681-4-visitorckw@gmail.com>
On Tue, Jan 13, 2026 at 09:25:59AM +0000, Kuan-Wei Chiu wrote:
> Convert the Android Goldfish Events Keypad binding to DT schema format.
> Move the file to the input directory to match the subsystem.
> Update the example node name to 'keypad' to comply with generic node
> naming standards.
>
> Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 2/3] drivers: input: touchscreen: edt-ft5x06: Add FocalTech FT3518
From: Dmitry Torokhov @ 2026-01-20 19:53 UTC (permalink / raw)
To: yedaya.ka
Cc: Kamil Gołda, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, linux-input, devicetree,
linux-kernel, linux-arm-msm, Dmitry Baryshkov
In-Reply-To: <20260118-touchscreen-patches-v3-2-1c6a729c5eb4@gmail.com>
On Sun, Jan 18, 2026 at 10:29:41PM +0200, Yedaya Katsman via B4 Relay wrote:
> From: Yedaya Katsman <yedaya.ka@gmail.com>
>
> The driver also works with FT3518, which supports up to 10 touch points.
> Add compatible data for it.
>
> Co-developed-by: Kamil Gołda <kamil.golda@protonmail.com>
> Signed-off-by: Kamil Gołda <kamil.golda@protonmail.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Yedaya Katsman <yedaya.ka@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: input: touchscreen: edt-ft5x06: Add FocalTech FT3518
From: Dmitry Torokhov @ 2026-01-20 19:53 UTC (permalink / raw)
To: yedaya.ka
Cc: Kamil Gołda, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bjorn Andersson, Konrad Dybcio, linux-input, devicetree,
linux-kernel, linux-arm-msm, Krzysztof Kozlowski
In-Reply-To: <20260118-touchscreen-patches-v3-1-1c6a729c5eb4@gmail.com>
On Sun, Jan 18, 2026 at 10:29:40PM +0200, Yedaya Katsman via B4 Relay wrote:
> From: Yedaya Katsman <yedaya.ka@gmail.com>
>
> Document FocalTech FT3518 support by adding the compatible.
>
> Co-developed-by: Kamil Gołda <kamil.golda@protonmail.com>
> Signed-off-by: Kamil Gołda <kamil.golda@protonmail.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Yedaya Katsman <yedaya.ka@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: Regression: SYNA3602 I2C touchpad broken in Linux 6.17.7 (works in 6.17.6 and previous versions)
From: Andy Shevchenko @ 2026-01-20 19:37 UTC (permalink / raw)
To: Thorsten Leemhuis
Cc: Vijay, regressions, linux-kernel, linux-input, linux-pm,
linux-acpi, Benjamin Tissoires, jikos
In-Reply-To: <2c8d4baa-679f-4af7-a78d-41774410e9a6@leemhuis.info>
On Mon, Dec 08, 2025 at 02:54:20PM +0100, Thorsten Leemhuis wrote:
> On 12/8/25 13:13, Vijay wrote:
> >
> > Yes, the touchpad is not working in 6.18 also, getting the same errors
> > as mentioned previously,
>
> That's really good to know, thx!
>
> Thing is: I fear that nobody will look into this, unless you or somebody
> else affected checks which change broke things. Benjamin mentioned three
> you could try reverting in 6.17.y; alternatively, perform a bisection in
> 6.17.y. For details, see:
> https://docs.kernel.org/admin-guide/verify-bugs-and-bisect-regressions.html
Might be related:
https://bugzilla.kernel.org/show_bug.cgi?id=219799
> > On Thu, 4 Dec 2025 at 20:40, Thorsten Leemhuis
> > <regressions@leemhuis.info <mailto:regressions@leemhuis.info>> wrote:
> >
> > Lo!
> >
> > @AM Vijay: 6.17.y will be EOL in about ten days, so this is unlikely to
> > get fixed there. The big question is:
> >
> > Is 6.18 affected?
> >
> > If it is, we need your help identifying want went wrong; if not, then
> > it's likely not worth looking closer into this
> >
> > Ciao, Thorsten
> >
> > On 11/28/25 09:05, Benjamin Tissoires wrote:
> > > Hi,
> > >
> > > On Fri, Nov 28, 2025 at 7:40 AM Vijay <vijayg0127@gmail.com
> > <mailto:vijayg0127@gmail.com>> wrote:
> > >>
> > >> Hello,
> > >>
> > >> I would like to report a regression in the Linux kernel affecting
> > I2C-HID
> > >> touchpads that run through the Intel ISH + DesignWare I2C controller.
> > >>
> > >> Hardware:
> > >> - Laptop: Infinix Y4 Max
> > >> - CPU: Intel (13th gen core i5)
> > >> - Touchpad: SYNA3602:00 093A:35ED (I2C HID)
> > >> - Bus path: SYNA3602 → i2c_designware → Intel ISH → HID
> > >> - OS: Linux (Arch/CachyOS)
> > >> - Kernel config: Default distro config
> > >>
> > >> Regression summary:
> > >> - Touchpad works perfectly in Linux 6.17.6 and below versions
> > >> - Touchpad stops working in Linux 6.17.7 and all newer versions
> > (6.17.8, 6.17.9, etc.)
> > >> - Desktop environment does not matter (Hyprland/GNOME both fail)
> > >> - The failure happens before userspace loads
> > >> - Touchpad also works fine in Linux 6.12 LTS
> > >>
> > >> This is a kernel-level regression introduced between:
> > >> Good: Linux 6.17.6
> > >> Bad: Linux 6.17.7
> > >>
> > >> **Dmesg logs from broken kernel (6.17.7 and newer):**
> > >>
> > >> i2c-SYNA3602:00: can't add hid device: -110
> > >> hid_sensor_hub: reading report descriptor failed
> > >> intel-hid INTC1078:00: failed to enable HID power button
> > >
> > > Looks like i2c-hid can't even communicate with any I2C device, so this
> > > is slightly worrying.
> > >
> > >>
> > >> And the DesignWare I2C controller logs around the failure:
> > >> i2c_designware 0000:00:15.0: controller timed out
> > >> i2c_designware 0000:00:15.0: lost arbitration
> > >> i2c_designware 0000:00:15.0: transfer aborted (status = -110)
> > >>
> > >> These errors appear only on 6.17.7+ and not on 6.17.6.
> > >>
> > >> On working versions (6.17.6 and 6.12 LTS), the touchpad
> > initializes normally:
> > >>
> > >> input: SYNA3602:00 093A:35ED Touchpad as /devices/.../input/
> > inputX
> > >> hid-multitouch: I2C HID v1.00 device initialized
> > >> i2c_designware 0000:00:15.0: controller operating normally
> > >>
> > >> This narrow regression window should make it possible to identify
> > the offending
> > >> change in either:
> > >> - HID core
> > >> - I2C-HID
> > >> - Intel ISH HID
> > >> - DesignWare I2C controller
> > >> - ACPI timing changes
> > >>
> > >> I can provide:
> > >> - Full dmesg (working and broken)
> > >> - acpidump
> > >
> > > Are you running on a full vanilla kernel?
> > >
> > > The changelog between 6.17.6 and 6.17.7 is rather small, so it should
> > > be easy enough to bisect and get the offending commit.
> > >
> > > I have my suspicions on:
> > > f1971d5ba2ef ("genirq/manage: Add buslock back in to enable_irq()")
> > > b990b4c6ea6b ("genirq/manage: Add buslock back in to
> > __disable_irq_nosync()")
> > > 3c97437239df ("genirq/chip: Add buslock back in to irq_set_handler()")
> > >
> > > Because anything else is unrelated to any component involved in
> > i2c-hid.
> > > (But that's also assuming you are running vanilla kernels without any
> > > extra patches.)
> > >
> > > OTOH, I've booted a 6.17.8 and 6.17.7 shipped by Fedora and I don't
> > > see any issues related to i2c-hid, so those 3 commits might not be the
> > > culprits.
> > >> Please let me know what additional data is needed.
> > >
> > > Can you do a bisect between v6.17.7 and v6.17.6?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH 2/2] input: touchscreen: novatek-nvt-ts: Add support for NT36672A e7t variant
From: Gianluca Boiano @ 2026-01-20 19:36 UTC (permalink / raw)
To: hansg
Cc: dmitry.torokhov, robh, krzk+dt, conor+dt, linux-input, devicetree,
linux-kernel, Gianluca Boiano
In-Reply-To: <20260120193600.1089458-1-morf3089@gmail.com>
Add support for the Novatek NT36672A touchscreen variant found on the
Xiaomi Redmi Note 6 Pro (tulip) which uses a different wake_type value
(0x02 instead of 0x01).
The touchscreen was failing to initialize with error -5 due to the
wake_type parameter mismatch during probe. This adds a new chip data
structure for the e7t variant with the correct wake_type value.
Closes: https://github.com/sdm660-mainline/linux/issues/155
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
---
drivers/input/touchscreen/novatek-nvt-ts.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/input/touchscreen/novatek-nvt-ts.c b/drivers/input/touchscreen/novatek-nvt-ts.c
index 44b58e0dc1ad..b1c379d87de0 100644
--- a/drivers/input/touchscreen/novatek-nvt-ts.c
+++ b/drivers/input/touchscreen/novatek-nvt-ts.c
@@ -323,9 +323,15 @@ static const struct nvt_ts_i2c_chip_data nvt_nt36672a_ts_data = {
.chip_id = 0x08,
};
+static const struct nvt_ts_i2c_chip_data nvt_nt36672a_e7t_ts_data = {
+ .wake_type = 0x02,
+ .chip_id = 0x08,
+};
+
static const struct of_device_id nvt_ts_of_match[] = {
{ .compatible = "novatek,nt11205-ts", .data = &nvt_nt11205_ts_data },
{ .compatible = "novatek,nt36672a-ts", .data = &nvt_nt36672a_ts_data },
+ { .compatible = "novatek,nt36672a-e7t-ts", .data = &nvt_nt36672a_e7t_ts_data },
{ }
};
MODULE_DEVICE_TABLE(of, nvt_ts_of_match);
@@ -333,6 +339,7 @@ MODULE_DEVICE_TABLE(of, nvt_ts_of_match);
static const struct i2c_device_id nvt_ts_i2c_id[] = {
{ "nt11205-ts", (unsigned long) &nvt_nt11205_ts_data },
{ "nt36672a-ts", (unsigned long) &nvt_nt36672a_ts_data },
+ { "nt36672a-e7t-ts", (unsigned long) &nvt_nt36672a_e7t_ts_data },
{ }
};
MODULE_DEVICE_TABLE(i2c, nvt_ts_i2c_id);
--
2.52.0
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: input: novatek,nvt-ts: Add nt36672a-e7t-ts compatible
From: Gianluca Boiano @ 2026-01-20 19:35 UTC (permalink / raw)
To: hansg
Cc: dmitry.torokhov, robh, krzk+dt, conor+dt, linux-input, devicetree,
linux-kernel, Gianluca Boiano
Add compatible string for the Novatek NT36672A e7t touchscreen variant
found on the Xiaomi Redmi Note 6 Pro (tulip).
This variant uses different chip parameters compared to the standard
NT36672A, specifically a different wake_type value.
Signed-off-by: Gianluca Boiano <morf3089@gmail.com>
---
.../devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
index bd6a60486d1f..aaa9976bd65e 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/novatek,nvt-ts.yaml
@@ -17,6 +17,7 @@ properties:
enum:
- novatek,nt11205-ts
- novatek,nt36672a-ts
+ - novatek,nt36672a-e7t-ts
reg:
maxItems: 1
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v2 0/4] HID: steelseries: Add support for Arctis headset lineup
From: Benjamin Wheeler @ 2026-01-20 19:01 UTC (permalink / raw)
To: Sriman Achanta, Jiri Kosina, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20260112041941.40531-1-srimanachanta@gmail.com>
Hello,
I'm Benjamin, and I coincidentally have also been looking into this
driver. Sriman, if you would like, I'd be happy to work together on this
driver with you! In reviewing your patches, I have found and fixed a bug
in my local tree regarding battery requests.
The Steelseries Arctis 7 and Arctis 7 (2019 ed.) are not implemented in
the battery request logic, and simply report 100% (the default value
during battery registration). I discovered this bug while testing your
driver with my Arctis 7 :-)
Would you like me to send a formal patch for this? I also have some
patches I can send that clean up the battery logic in general that make
the code less complex.
I am also happy to look into creating ALSA mixers/switches as well.
Please let me know how I can help! Excellent work so far, and smart
choice in using HeadsetControl, they've done most of the heavy lifting
for us.
Sincerely,
Benjamin Wheeler
On 1/11/26 11:19 PM, Sriman Achanta wrote:
> This patch series adds comprehensive support for the SteelSeries Arctis
> wireless gaming headset lineup to the hid-steelseries driver.
>
> The current driver provides only basic battery monitoring for Arctis 1
> and Arctis 9. This series extends support to 25+ Arctis models with
> full feature control including sidetone, auto-sleep, microphone
> controls, volume limiting, and Bluetooth settings.
>
> The driver restructure uses a capability-based device info system to
> cleanly handle the varying feature sets across different Arctis
> generations while maintaining support for the legacy SRW-S1 racing
> wheel.
>
> Patch 1: Add 27 new device IDs to hid-ids.h
> Patch 2: Add HID quirks for proper device initialization
> Patch 3: Update ABI documentation for new sysfs attributes
> Patch 4: Complete driver implementation with all features
>
> Tested on Arctis Nova 7 (0x2202). All other implementation details are
> based on the reverse engineering done in the HeadsetControl library
> (abe3ac8).
>
> V2:
> - Fix Documentation formatting issues
>
> Sriman Achanta (4):
> HID: hid-ids: Add SteelSeries Arctis headset device IDs
> HID: quirks: Add INPUT_CONFIGURED quirk for SteelSeries Arctis
> headsets
> Documentation: ABI: Document SteelSeries headset sysfs attributes
> HID: steelseries: Add support for Arctis headset lineup
>
> .../ABI/testing/sysfs-driver-hid-srws1 | 21 -
> .../ABI/testing/sysfs-driver-hid-steelseries | 131 ++
> drivers/hid/hid-ids.h | 33 +-
> drivers/hid/hid-quirks.c | 25 +
> drivers/hid/hid-steelseries.c | 2061 ++++++++++++++---
> 5 files changed, 1925 insertions(+), 346 deletions(-)
> delete mode 100644 Documentation/ABI/testing/sysfs-driver-hid-srws1
> create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-steelseries
>
^ permalink raw reply
* Re: [PATCH bpf-next v2 04/13] resolve_btfids: Introduce finalize_btf() step
From: Eduard Zingerman @ 2026-01-20 18:40 UTC (permalink / raw)
To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau
Cc: Mykyta Yatsenko, Tejun Heo, Alan Maguire, Benjamin Tissoires,
Jiri Kosina, Amery Hung, bpf, linux-kernel, linux-input,
sched-ext
In-Reply-To: <74fbaa99-024b-4e42-bda0-99ae792d565b@linux.dev>
On Tue, 2026-01-20 at 10:35 -0800, Ihor Solodrai wrote:
> On 1/20/26 10:19 AM, Eduard Zingerman wrote:
> > On Tue, 2026-01-20 at 10:11 -0800, Ihor Solodrai wrote:
> >
> > [...]
> >
> > > > > @@ -1099,12 +1116,22 @@ int main(int argc, const char **argv)
> > > > > if (obj.efile.idlist_shndx == -1 ||
> > > > > obj.efile.symbols_shndx == -1) {
> > > > > pr_debug("Cannot find .BTF_ids or symbols sections, skip symbols resolution\n");
> > > > > - goto dump_btf;
> > > > > + resolve_btfids = false;
> > > > > }
> > > > >
> > > > > - if (symbols_collect(&obj))
> > > > > + if (resolve_btfids)
> > > > > + if (symbols_collect(&obj))
> > > > > + goto out;
> > > >
> > > > Nit: check obj.efile.idlist_shndx and obj.efile.symbols_shndx inside symbols_collect()?
> > > > To avoid resolve_btfids flag and the `goto dump_btf;` below.
> > >
> > > Hi Eduard, thank you for review.
> > >
> > > The issue is that in case of .BTF_ids section absent we have to skip
> > > some of the steps, specifically:
> > > - symbols_collect()
> > > - sequence between symbols_resolve() and dump_raw_btf_ids()
> >
> > > It's not an exit condition, we still have to do load/dump of the BTF.
> > >
> > > I tried in symbols_collect():
> > >
> > > if (obj.efile.idlist_shndx == -1 || obj.efile.symbols_shndx == -1)
> > > return 0;
> > >
> > > But then, we either have to do the same check in symbols_resolve() and
> > > co, or maybe store a flag in the struct object. So I decided it's
> > > better to have an explicit flag in the main control flow, instead of
> > > hiding it.
> >
> > For symbols_resolve() is any special logic necessary?
> > I think that `id = btf_id__find(root, str);` will just return NULL for
> > every type, thus the whole function would be a noop passing through
> > BTF types once.
> >
> > symbols_patch() will be a noop, as it will attempt traversing empty roots.
> > dump_raw_btf_ids() already returns if there are no .BTF_ids.
>
> Hm... Looks like you're right, those would be noops.
>
> Still, I think it's clearer what steps are skipped with a toplevel
> flag. Otherwise to figure out that those are noops you need to check
> every subroutine (as you just did), and a future change may
> unintentionally break the expectation of noop creating an unnecessary
> debugging session.
I'd argue that resolve_btfids is written in a clear read data /
process data manner, so it should behave correctly even if collected
data is empty. Is there a difference between no .BTF_ids and empty
.BTF_ids?
> And re symbols_resolve(), if we don't like allocating unnecessary
> memory, why are we ok with traversing the BTF with noops? Seems
> a bit inconsistent to me.
He-he, I never heard about consistency :)
Just don't like flags, they make me think more.
^ permalink raw reply
* Re: [PATCH bpf-next v2 04/13] resolve_btfids: Introduce finalize_btf() step
From: Ihor Solodrai @ 2026-01-20 18:35 UTC (permalink / raw)
To: Eduard Zingerman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau
Cc: Mykyta Yatsenko, Tejun Heo, Alan Maguire, Benjamin Tissoires,
Jiri Kosina, Amery Hung, bpf, linux-kernel, linux-input,
sched-ext
In-Reply-To: <b92382bdb9b9b274c0eda1d2bf8aba69c9768ecd.camel@gmail.com>
On 1/20/26 10:19 AM, Eduard Zingerman wrote:
> On Tue, 2026-01-20 at 10:11 -0800, Ihor Solodrai wrote:
>
> [...]
>
>>>> @@ -1099,12 +1116,22 @@ int main(int argc, const char **argv)
>>>> if (obj.efile.idlist_shndx == -1 ||
>>>> obj.efile.symbols_shndx == -1) {
>>>> pr_debug("Cannot find .BTF_ids or symbols sections, skip symbols resolution\n");
>>>> - goto dump_btf;
>>>> + resolve_btfids = false;
>>>> }
>>>>
>>>> - if (symbols_collect(&obj))
>>>> + if (resolve_btfids)
>>>> + if (symbols_collect(&obj))
>>>> + goto out;
>>>
>>> Nit: check obj.efile.idlist_shndx and obj.efile.symbols_shndx inside symbols_collect()?
>>> To avoid resolve_btfids flag and the `goto dump_btf;` below.
>>
>> Hi Eduard, thank you for review.
>>
>> The issue is that in case of .BTF_ids section absent we have to skip
>> some of the steps, specifically:
>> - symbols_collect()
>> - sequence between symbols_resolve() and dump_raw_btf_ids()
>
>> It's not an exit condition, we still have to do load/dump of the BTF.
>>
>> I tried in symbols_collect():
>>
>> if (obj.efile.idlist_shndx == -1 || obj.efile.symbols_shndx == -1)
>> return 0;
>>
>> But then, we either have to do the same check in symbols_resolve() and
>> co, or maybe store a flag in the struct object. So I decided it's
>> better to have an explicit flag in the main control flow, instead of
>> hiding it.
>
> For symbols_resolve() is any special logic necessary?
> I think that `id = btf_id__find(root, str);` will just return NULL for
> every type, thus the whole function would be a noop passing through
> BTF types once.
>
> symbols_patch() will be a noop, as it will attempt traversing empty roots.
> dump_raw_btf_ids() already returns if there are no .BTF_ids.
Hm... Looks like you're right, those would be noops.
Still, I think it's clearer what steps are skipped with a toplevel
flag. Otherwise to figure out that those are noops you need to check
every subroutine (as you just did), and a future change may
unintentionally break the expectation of noop creating an unnecessary
debugging session.
And re symbols_resolve(), if we don't like allocating unnecessary
memory, why are we ok with traversing the BTF with noops? Seems
a bit inconsistent to me.
>
> [...]
^ permalink raw reply
* Re: [PATCH v4 3/3] Input: ili210x - add support for polling mode
From: Dmitry Torokhov @ 2026-01-20 18:31 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
In-Reply-To: <20260117001215.59272-3-marek.vasut+renesas@mailbox.org>
Hi Marek,
On Sat, Jan 17, 2026 at 01:12:04AM +0100, Marek Vasut wrote:
> @@ -860,16 +893,12 @@ static ssize_t ili210x_firmware_update_store(struct device *dev,
> * the touch controller to disable the IRQs during update, so we have
> * to do it this way here.
> */
> - scoped_guard(disable_irq, &client->irq) {
> - dev_dbg(dev, "Firmware update started, firmware=%s\n", fwname);
> -
> - ili210x_hardware_reset(priv->reset_gpio);
> -
> - error = ili210x_do_firmware_update(priv, fwbuf, ac_end, df_end);
> -
> - ili210x_hardware_reset(priv->reset_gpio);
> -
> - dev_dbg(dev, "Firmware update ended, error=%i\n", error);
> + if (client->irq > 0) {
> + scoped_guard(disable_irq, &client->irq) {
> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> + }
You already have a scope here, no need to establish a new one:
guard(disable_irq)(&client->irq);
error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
BTW, not a fan of the "_noirq" suffix... Maybe drop it and add
lockdep_is_held() there?
> + } else {
> + error = ili210x_firmware_update_noirq(dev, fwbuf, ac_end, df_end);
> }
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 2/3] Input: ili210x - convert to dev_err_probe()
From: Dmitry Torokhov @ 2026-01-20 18:25 UTC (permalink / raw)
To: Marek Vasut
Cc: linux-input, Conor Dooley, Frank Li, Job Noorman,
Krzysztof Kozlowski, Rob Herring, devicetree, linux-kernel,
linux-renesas-soc
In-Reply-To: <20260117001215.59272-2-marek.vasut+renesas@mailbox.org>
On Sat, Jan 17, 2026 at 01:12:03AM +0100, Marek Vasut wrote:
> Simplify error return handling, use dev_err_probe() where possible.
> No functional change.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH bpf-next v2 11/13] selftests/bpf: Migrate struct_ops_assoc test to KF_IMPLICIT_ARGS
From: Eduard Zingerman @ 2026-01-20 18:24 UTC (permalink / raw)
To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau
Cc: Mykyta Yatsenko, Tejun Heo, Alan Maguire, Benjamin Tissoires,
Jiri Kosina, Amery Hung, bpf, linux-kernel, linux-input,
sched-ext
In-Reply-To: <4cb38afd-e795-44d5-b71e-e28ef46a713e@linux.dev>
On Tue, 2026-01-20 at 10:20 -0800, Ihor Solodrai wrote:
> On 1/19/26 5:59 PM, Eduard Zingerman wrote:
> > On Fri, 2026-01-16 at 12:16 -0800, Ihor Solodrai wrote:
> >
> > [...]
> >
> > > diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> > > index 2357a0340ffe..225ea30c4e3d 100644
> > > --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> > > +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
> > > @@ -161,7 +161,9 @@ void bpf_kfunc_rcu_task_test(struct task_struct *ptr) __ksym;
> > > struct task_struct *bpf_kfunc_ret_rcu_test(void) __ksym;
> > > int *bpf_kfunc_ret_rcu_test_nostruct(int rdonly_buf_size) __ksym;
> > >
> > > -int bpf_kfunc_multi_st_ops_test_1(struct st_ops_args *args, u32 id) __ksym;
> > > -int bpf_kfunc_multi_st_ops_test_1_impl(struct st_ops_args *args, void *aux__prog) __ksym;
> > > +#ifndef __KERNEL__
> > > +extern int bpf_kfunc_multi_st_ops_test_1(struct st_ops_args *args, u32 id) __weak __ksym;
> > > +extern int bpf_kfunc_multi_st_ops_test_1_assoc(struct st_ops_args *args) __weak __ksym;
> > > +#endif
> >
> > Nit: wbpf_kfunc_multi_st_ops_test_1 change is not necessary, right?
>
> Right, but it felt wrong to only change one of these decls.
>
> This header is weird in that it is included both in the module code
> and in BPF progs, although it is typically not a problem since the
> most kfunc signatures match.
I think it is used this way, so that compiler can warn user about
signature mismatch during development.
> Maybe it should have #ifndef __KERNEL__ followed by kfunc declarations
> that correspond to vmlinux.h format? I haven't tried that, but seems
> logical to me.
^ permalink raw reply
* Re: [PATCH] Input: atmel_mxt_ts - fix NULL pointer dereference in mxt_object_show
From: Dmitry Torokhov @ 2026-01-20 18:22 UTC (permalink / raw)
To: 齐柯宇
Cc: nick, rydberg, jy0922.shim, bleung, ezequiel, linux-input,
linux-kernel
In-Reply-To: <CALEuBan7Knm3G3eXPd8BZ37jVQFtRs7fAssts+picVKHBjGBqQ@mail.gmail.com>
Hi,
On Thu, Jan 15, 2026 at 03:43:37AM +0800, 齐柯宇 wrote:
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c
> b/drivers/input/touchscreen/atmel_mxt_ts.c
> index dd0544cc1bc1..401fcae2264d 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -2859,6 +2859,10 @@ static ssize_t mxt_object_show(struct device *dev,
> int error;
> u8 *obuf;
>
> + /* Check for NULL to prevent race condition during firmware update */
> + if (!data->info || !data->object_table)
> + return -ENODEV;
> +
What stops the pointers from being invalidated right here?
> /* Pre-allocate buffer large enough to hold max sized object. */
> obuf = kmalloc(256, GFP_KERNEL);
> if (!obuf)
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH bpf-next v2 11/13] selftests/bpf: Migrate struct_ops_assoc test to KF_IMPLICIT_ARGS
From: Ihor Solodrai @ 2026-01-20 18:20 UTC (permalink / raw)
To: Eduard Zingerman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau
Cc: Mykyta Yatsenko, Tejun Heo, Alan Maguire, Benjamin Tissoires,
Jiri Kosina, Amery Hung, bpf, linux-kernel, linux-input,
sched-ext
In-Reply-To: <b5fef9672be4395b76619b8bd39697bf28b93350.camel@gmail.com>
On 1/19/26 5:59 PM, Eduard Zingerman wrote:
> On Fri, 2026-01-16 at 12:16 -0800, Ihor Solodrai wrote:
>
> [...]
>
>> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
>> index 2357a0340ffe..225ea30c4e3d 100644
>> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
>> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
>> @@ -161,7 +161,9 @@ void bpf_kfunc_rcu_task_test(struct task_struct *ptr) __ksym;
>> struct task_struct *bpf_kfunc_ret_rcu_test(void) __ksym;
>> int *bpf_kfunc_ret_rcu_test_nostruct(int rdonly_buf_size) __ksym;
>>
>> -int bpf_kfunc_multi_st_ops_test_1(struct st_ops_args *args, u32 id) __ksym;
>> -int bpf_kfunc_multi_st_ops_test_1_impl(struct st_ops_args *args, void *aux__prog) __ksym;
>> +#ifndef __KERNEL__
>> +extern int bpf_kfunc_multi_st_ops_test_1(struct st_ops_args *args, u32 id) __weak __ksym;
>> +extern int bpf_kfunc_multi_st_ops_test_1_assoc(struct st_ops_args *args) __weak __ksym;
>> +#endif
>
> Nit: wbpf_kfunc_multi_st_ops_test_1 change is not necessary, right?
Right, but it felt wrong to only change one of these decls.
This header is weird in that it is included both in the module code
and in BPF progs, although it is typically not a problem since the
most kfunc signatures match.
Maybe it should have #ifndef __KERNEL__ followed by kfunc declarations
that correspond to vmlinux.h format? I haven't tried that, but seems
logical to me.
^ permalink raw reply
* Re: [PATCH bpf-next v2 04/13] resolve_btfids: Introduce finalize_btf() step
From: Eduard Zingerman @ 2026-01-20 18:19 UTC (permalink / raw)
To: Ihor Solodrai, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau
Cc: Mykyta Yatsenko, Tejun Heo, Alan Maguire, Benjamin Tissoires,
Jiri Kosina, Amery Hung, bpf, linux-kernel, linux-input,
sched-ext
In-Reply-To: <fe471a8c-4238-432b-9507-e2039f7fa9d8@linux.dev>
On Tue, 2026-01-20 at 10:11 -0800, Ihor Solodrai wrote:
[...]
> > > @@ -1099,12 +1116,22 @@ int main(int argc, const char **argv)
> > > if (obj.efile.idlist_shndx == -1 ||
> > > obj.efile.symbols_shndx == -1) {
> > > pr_debug("Cannot find .BTF_ids or symbols sections, skip symbols resolution\n");
> > > - goto dump_btf;
> > > + resolve_btfids = false;
> > > }
> > >
> > > - if (symbols_collect(&obj))
> > > + if (resolve_btfids)
> > > + if (symbols_collect(&obj))
> > > + goto out;
> >
> > Nit: check obj.efile.idlist_shndx and obj.efile.symbols_shndx inside symbols_collect()?
> > To avoid resolve_btfids flag and the `goto dump_btf;` below.
>
> Hi Eduard, thank you for review.
>
> The issue is that in case of .BTF_ids section absent we have to skip
> some of the steps, specifically:
> - symbols_collect()
> - sequence between symbols_resolve() and dump_raw_btf_ids()
> It's not an exit condition, we still have to do load/dump of the BTF.
>
> I tried in symbols_collect():
>
> if (obj.efile.idlist_shndx == -1 || obj.efile.symbols_shndx == -1)
> return 0;
>
> But then, we either have to do the same check in symbols_resolve() and
> co, or maybe store a flag in the struct object. So I decided it's
> better to have an explicit flag in the main control flow, instead of
> hiding it.
For symbols_resolve() is any special logic necessary?
I think that `id = btf_id__find(root, str);` will just return NULL for
every type, thus the whole function would be a noop passing through
BTF types once.
symbols_patch() will be a noop, as it will attempt traversing empty roots.
dump_raw_btf_ids() already returns if there are no .BTF_ids.
[...]
^ permalink raw reply
* Re: [PATCH] Input: appletouch - fix use-after-free in work handler during disconnect
From: Dmitry Torokhov @ 2026-01-20 18:18 UTC (permalink / raw)
To: Haowen Tu; +Cc: linux-input, linux-kernel
In-Reply-To: <20260116031113.1003940-1-tuhaowen@uniontech.com>
Hi Haowen,
On Fri, Jan 16, 2026 at 11:11:13AM +0800, Haowen Tu wrote:
> The atp_reinit work item is initialized in atp_probe() and scheduled
> from atp_complete() URB callback when the device becomes idle. During
> device disconnection, the current implementation calls usb_kill_urb()
> in atp_disconnect() but fails to prevent the work from being executed
> after the atp structure has been freed.
>
> Although usb_kill_urb() terminates the URB and its callbacks, there is
> a critical race window: if schedule_work() is called in atp_complete()
> just before usb_kill_urb() takes effect, the work item can still be
> queued. Since atp_disconnect() immediately proceeds to free the atp
> structure without canceling pending work, this leads to a use-after-free
> vulnerability when the work handler executes.
>
> The race condition:
>
> CPU 0 (disconnect path) | CPU 1 (URB completion)
> atp_disconnect() |
> usb_kill_urb(dev->urb) | atp_complete()
> | schedule_work(&dev->work)
> input_unregister_device() |
> usb_free_coherent() |
> usb_free_urb() |
> kfree(dev); // FREE | atp_reinit()
> | dev = container_of(...) // USE
> | atp_geyser_init(dev) // USE
> | dev->urb // USE
> | dev->intf // USE
>
> Fix this by adding disable_work_sync() in atp_disconnect() after
> usb_kill_urb() to ensure the work is properly canceled and cannot
> be rescheduled before the atp structure is freed.
I do not think there is actually a race. The urb is only
submitted/started when the device is open, the work in properly
cancelled in atp_close(), and the input core will ensure atp_close() is
called for opened devices. In fact, that usb_kill_urb() call in
atp_disconnect() is not needed at all.
I see there is a potential race handling dev->open in the driver, I just
posted a patch addressing it.
>
> Signed-off-by: Haowen Tu <tuhaowen@uniontech.com>
> ---
> drivers/input/mouse/appletouch.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
> index e669f86f1882..aa2870a87eee 100644
> --- a/drivers/input/mouse/appletouch.c
> +++ b/drivers/input/mouse/appletouch.c
> @@ -946,6 +946,7 @@ static void atp_disconnect(struct usb_interface *iface)
> usb_set_intfdata(iface, NULL);
> if (dev) {
> usb_kill_urb(dev->urb);
> + disable_work_sync(&dev->work);
> input_unregister_device(dev->input);
> usb_free_coherent(dev->udev, dev->info->datalen,
> dev->data, dev->urb->transfer_dma);
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] Input: appletouch - fix potential race between resume and open
From: Dmitry Torokhov @ 2026-01-20 18:14 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Haowen Tu
Take the input device's mutex in atp_resume() and atp_recover() to make
sure they are not racing with open and close methods, and use
input_device_enabled() helper to see if communication with the device
needs to be restarted after resume.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/mouse/appletouch.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index e669f86f1882..3ce63fb35992 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -200,7 +200,6 @@ struct atp {
u8 *data; /* transferred data */
struct input_dev *input; /* input dev */
const struct atp_info *info; /* touchpad model */
- bool open;
bool valid; /* are the samples valid? */
bool size_detect_done;
bool overflow_warned;
@@ -800,7 +799,6 @@ static int atp_open(struct input_dev *input)
if (usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
- dev->open = true;
return 0;
}
@@ -810,7 +808,6 @@ static void atp_close(struct input_dev *input)
usb_kill_urb(dev->urb);
cancel_work_sync(&dev->work);
- dev->open = false;
}
static int atp_handle_geyser(struct atp *dev)
@@ -963,7 +960,8 @@ static int atp_recover(struct atp *dev)
if (error)
return error;
- if (dev->open && usb_submit_urb(dev->urb, GFP_KERNEL))
+ guard(mutex)(&dev->input->mutex);
+ if (input_device_enabled(dev->input) && usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
return 0;
@@ -981,7 +979,8 @@ static int atp_resume(struct usb_interface *iface)
{
struct atp *dev = usb_get_intfdata(iface);
- if (dev->open && usb_submit_urb(dev->urb, GFP_KERNEL))
+ guard(mutex)(&dev->input->mutex);
+ if (input_device_enabled(dev->input) && usb_submit_urb(dev->urb, GFP_KERNEL))
return -EIO;
return 0;
--
2.52.0.457.g6b5491de43-goog
--
Dmitry
^ permalink raw reply related
* Re: [PATCH bpf-next v2 04/13] resolve_btfids: Introduce finalize_btf() step
From: Ihor Solodrai @ 2026-01-20 18:11 UTC (permalink / raw)
To: Eduard Zingerman, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau
Cc: Mykyta Yatsenko, Tejun Heo, Alan Maguire, Benjamin Tissoires,
Jiri Kosina, Amery Hung, bpf, linux-kernel, linux-input,
sched-ext
In-Reply-To: <c404446ab6d344338592dfa44f5a7e1b95492564.camel@gmail.com>
On 1/19/26 4:13 PM, Eduard Zingerman wrote:
> On Fri, 2026-01-16 at 12:16 -0800, Ihor Solodrai wrote:
>> Since recently [1][2] resolve_btfids executes final adjustments to the
>> kernel/module BTF before it's embedded into the target binary.
>>
>> To keep the implementation simple, a clear and stable "pipeline" of
>> how BTF data flows through resolve_btfids would be helpful. Some BTF
>> modifications may change the ids of the types, so it is important to
>> maintain correct order of operations with respect to .BTF_ids
>> resolution too.
>>
>> This patch refactors the BTF handling to establish the following
>> sequence:
>> - load target ELF sections
>> - load .BTF_ids symbols
>> - this will be a dependency of btf2btf transformations in
>> subsequent patches
>> - load BTF and its base as is
>> - (*) btf2btf transformations will happen here
>> - finalize_btf(), introduced in this patch
>> - does distill base and sort BTF
>> - resolve and patch .BTF_ids
>>
>> This approach helps to avoid fixups in .BTF_ids data in case the ids
>> change at any point of BTF processing, because symbol resolution
>> happens on the finalized, ready to dump, BTF data.
>>
>> This also gives flexibility in BTF transformations, because they will
>> happen on BTF that is not distilled and/or sorted yet, allowing to
>> freely add, remove and modify BTF types.
>>
>> [1] https://lore.kernel.org/bpf/20251219181321.1283664-1-ihor.solodrai@linux.dev/
>> [2] https://lore.kernel.org/bpf/20260109130003.3313716-1-dolinux.peng@gmail.com/
>>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
>> ---
>
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>
>> @@ -1099,12 +1116,22 @@ int main(int argc, const char **argv)
>> if (obj.efile.idlist_shndx == -1 ||
>> obj.efile.symbols_shndx == -1) {
>> pr_debug("Cannot find .BTF_ids or symbols sections, skip symbols resolution\n");
>> - goto dump_btf;
>> + resolve_btfids = false;
>> }
>>
>> - if (symbols_collect(&obj))
>> + if (resolve_btfids)
>> + if (symbols_collect(&obj))
>> + goto out;
>
> Nit: check obj.efile.idlist_shndx and obj.efile.symbols_shndx inside symbols_collect()?
> To avoid resolve_btfids flag and the `goto dump_btf;` below.
Hi Eduard, thank you for review.
The issue is that in case of .BTF_ids section absent we have to skip
some of the steps, specifically:
- symbols_collect()
- sequence between symbols_resolve() and dump_raw_btf_ids()
It's not an exit condition, we still have to do load/dump of the BTF.
I tried in symbols_collect():
if (obj.efile.idlist_shndx == -1 || obj.efile.symbols_shndx == -1)
return 0;
But then, we either have to do the same check in symbols_resolve() and
co, or maybe store a flag in the struct object. So I decided it's
better to have an explicit flag in the main control flow, instead of
hiding it.
lmk if you had something else in mind
>
>> +
>> + if (load_btf(&obj))
>> goto out;
>>
>> + if (finalize_btf(&obj))
>> + goto out;
>> +
>> + if (!resolve_btfids)
>> + goto dump_btf;
>> +
>> if (symbols_resolve(&obj))
>> goto out;
>>
^ permalink raw reply
* Re: [PATCH v11 02/11] HID: asus: initialize additional endpoints only for legacy devices
From: Antheas Kapenekakis @ 2026-01-20 17:41 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Denis Benato, platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede
In-Reply-To: <89a67ace-6fef-72e0-4ef3-be828dd85e3d@linux.intel.com>
On Tue, 20 Jan 2026 at 14:56, Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Sat, 17 Jan 2026, Antheas Kapenekakis wrote:
> > On Sat, 17 Jan 2026 at 18:05, Denis Benato <denis.benato@linux.dev> wrote:
> > > On 1/17/26 17:16, Antheas Kapenekakis wrote:
> > > > On Sat, 17 Jan 2026 at 17:12, Denis Benato <denis.benato@linux.dev> wrote:
> > > >> On 1/17/26 17:10, Antheas Kapenekakis wrote:
> > > >>> On Sat, 17 Jan 2026 at 16:13, Denis Benato <denis.benato@linux.dev> wrote:
> > > >>>> On 1/17/26 16:07, Antheas Kapenekakis wrote:
> > > >>>>> On Sat, 17 Jan 2026 at 14:51, Denis Benato <denis.benato@linux.dev> wrote:
> > > >>>>>> On 1/17/26 00:10, Antheas Kapenekakis wrote:
> > > >>>>>>> On Fri, 16 Jan 2026 at 21:44, Denis Benato <denis.benato@linux.dev> wrote:
> > > >>>>>>>> On 1/16/26 14:31, Antheas Kapenekakis wrote:
> > > >>>>>>>>
> > > >>>>>>>>> Currently, ID1/ID2 initializations are performed for all NKEY devices.
> > > >>>>>>>>> However, ID1 initializations are only required for RGB control and are
> > > >>>>>>>>> only supported for RGB capable devices. ID2 initializations are only
> > > >>>>>>>>> required for initializing the Anime display endpoint which is only
> > > >>>>>>>>> supported on devices with an Anime display. Both of these
> > > >>>>>>>>> initializations are out of scope for this driver (this is a brightness
> > > >>>>>>>>> control and keyboard shortcut driver) and they should not be performed
> > > >>>>>>>>> for devices that do not support them in any case.
> > > >>>>>>>>>
> > > >>>>>>>>> At the same time, there are older NKEY devices that have only been
> > > >>>>>>>>> tested with these initializations in the kernel and it is not possible
> > > >>>>>>>>> to recheck them. There is a possibility that especially with the ID1
> > > >>>>>>>>> initialization, certain laptop models might have their shortcuts stop
> > > >>>>>>>>> working (currently unproven).
> > > >>>>>>>>>
> > > >>>>>>>>> For an abundance of caution, only initialize ID1/ID2 for those older
> > > >>>>>>>>> NKEY devices by introducing a quirk for them and replacing the NKEY
> > > >>>>>>>>> quirk in the block that performs the inits with that.
> > > >>>>>>>>>
> > > >>>>>>>>> In addition, as these initializations might not be supported by the
> > > >>>>>>>>> affected devices, change the function to not bail if they fail.
> > > >>>>>>>>>
> > > >>>>>>>>> Acked-by: Benjamin Tissoires <bentiss@kernel.org>
> > > >>>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > > >>>>>>>>> ---
> > > >>>>>>>>> drivers/hid/hid-asus.c | 16 ++++++----------
> > > >>>>>>>>> 1 file changed, 6 insertions(+), 10 deletions(-)
> > > >>>>>>>>>
> > > >>>>>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > > >>>>>>>>> index 323e6302bac5..dc7af12cf31a 100644
> > > >>>>>>>>> --- a/drivers/hid/hid-asus.c
> > > >>>>>>>>> +++ b/drivers/hid/hid-asus.c
> > > >>>>>>>>> @@ -90,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> > > >>>>>>>>> #define QUIRK_ROG_NKEY_KEYBOARD BIT(11)
> > > >>>>>>>>> #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> > > >>>>>>>>> #define QUIRK_ROG_ALLY_XPAD BIT(13)
> > > >>>>>>>>> +#define QUIRK_ROG_NKEY_LEGACY BIT(14)
> > > >>>>>>>> These past days I have taken a look at new 2025 models and they do make use of ID2,
> > > >>>>>>>> and won't do harm sending ID1 either. I think you can safely remove the if and send regardless.
> > > >>>>>>> Hi Denis,
> > > >>>>>>> it is not the responsibility of this driver. ID2 is used by Anime
> > > >>>>>>> models. It is a concession to make sure that we do not cause a
> > > >>>>>>> regression that will cause warnings for a lot of users.
> > > >>>>>> Who decided it is a concession?
> > > >>>>> I would rather remove the extra calls unless they are shown to be
> > > >>>>> needed, which they might be for these PIDs.
> > > >>>> They are needed on older laptop and to not regress userspace.
> > > >>>>
> > > >>>> You just named _LEGACY an usb pid that is not legacy.
> > > >>>>> The quirk is named legacy because we can't retest these devices. If we
> > > >>>>> can, then we could remove the quirk and the inits if not needed.
> > > >>>> We can't retest every device, and that pid is used in pre-2021 models,
> > > >>>> and these are the unknown, I am criticizing the name of the quirk here,
> > > >>>> not what it does.
> > > >>> If you can test whether your device needs them that would be great.
> > > >> That is pointless.
> > > >>>> I am also questioning if the quirk is even needed since sending
> > > >>>> those commands to (at least) recent hardware that doesn't use
> > > >>>> those endpoints carries no downsides, while removing them
> > > >>>> surely does.
> > > >>> We have not found a device yet that needs them. I do not want to keep
> > > >>> sending unneeded commands. It could cause obscure bugs or interfere
> > > >>> with userspace software such as the one you maintain. So at least for
> > > >>> new hardware that is possible to test we should remove them.
> > > >> There is new hardware that needs them, as I said, including 2025 models.
> > > > I was not aware of that. As far as I know they are not needed. Do you
> > > > have a bug report with a specific laptop model I can look at?
> > > There is current effort to integrate commands that requires those
> > > initializations on 2025 laptop, why would I strip out a command
> > > that I already know is required anyway?
> >
> > Hi,
> > yes ID1 is required for RGB, I have a draft patch for it that would
> > lazily do it if RGB is supported.
> >
> > I recall now a previous discussion about it being required for some
> > laptop shortcuts but we never found a laptop that needs it so I forgot
> >
> > > No, this is not the way to go to knowingly and willingly cause
> > > troubles (both known and unknown) to others just because
> > > you think it's better this way.
> > >
> > > Change the name of _LEGACY to something else, have this accepted
> > > and then if I see it's appropriate to remove the if and send those
> > > regardless I will.
> >
> > Sure, up to you if you want to change the name. What would you like it
> > be? I would like this series to merge
>
> Can you name it e.g. something that should be neutral such as:
>
> QUIRK_ROG_NKEY_ID1_ID2_INIT
>
> I'm not sure if ROG NKEY should still be included into the name based on
> what Denis mentioned about recent models but at least it gets rid of the
> "legacy" connotation. If wider scope is necessary you could use just
> QUIRK_ID1_ID2_INIT.
It's NKEY only so QUIRK_ROG_NKEY_ID1ID2_INIT would be more appropriate
(all recent models are NKEY)
Sounds like a plan, I will resend tomorrow. If anyone wants to leave
any more comments now is the time.
Antheas
> --
> i.
>
^ permalink raw reply
* Re: [PATCH v5 4/4] Input: Add TouchNetix aXiom I2C Touchscreen support
From: Dmitry Torokhov @ 2026-01-20 15:58 UTC (permalink / raw)
To: Marco Felsch
Cc: Luis Chamberlain, Russ Weight, Greg Kroah-Hartman,
Rafael J. Wysocki, Andrew Morton, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kamel Bouhara, Marco Felsch,
Henrik Rydberg, Danilo Krummrich, linux-kernel, devicetree,
linux-input
In-Reply-To: <20260119211750.udvems44y3u7m4x7@pengutronix.de>
Hi Marco,
On Mon, Jan 19, 2026 at 10:17:50PM +0100, Marco Felsch wrote:
> Hi Dmitry,
>
> gentle ping since we never received feedback from one of the input
> maintainers.
Some time ago I did few cleanup/fixes to this driver but did not
actually post them. The patch is below, please pick what still makes
sense and then I'll do the full review.
Thanks!
--
Dmitry
---
drivers/input/touchscreen/touchnetix_axiom.c | 989 +++++++++++++-------------
1 file changed, 495 insertions(+), 494 deletions(-)
diff --git a/drivers/input/touchscreen/touchnetix_axiom.c b/drivers/input/touchscreen/touchnetix_axiom.c
index e8f56a8f7e8a..300e7b76c3b3 100644
--- a/drivers/input/touchscreen/touchnetix_axiom.c
+++ b/drivers/input/touchscreen/touchnetix_axiom.c
@@ -396,24 +396,24 @@ static int axiom_u34_rev1_process_report(struct axiom_data *ts, const u8 *_buf,
static int axiom_u41_rev2_process_report(struct axiom_data *ts, const u8 *buf,
size_t bufsize);
-#define AXIOM_USAGE(num, rev) \
- { \
- .usage_num = num, \
- .rev_num = rev, \
+#define AXIOM_USAGE(num, rev) \
+ { \
+ .usage_num = num, \
+ .rev_num = rev, \
}
-#define AXIOM_RO_USAGE(num, rev) \
- { \
- .usage_num = num, \
- .rev_num = rev, \
- .is_ro = true, \
+#define AXIOM_RO_USAGE(num, rev) \
+ { \
+ .usage_num = num, \
+ .rev_num = rev, \
+ .is_ro = true, \
}
-#define AXIOM_CDU_USAGE(num, rev) \
- { \
- .usage_num = num, \
- .rev_num = rev, \
- .is_cdu = true, \
+#define AXIOM_CDU_USAGE(num, rev) \
+ { \
+ .usage_num = num, \
+ .rev_num = rev, \
+ .is_cdu = true, \
}
#define AXIOM_REPORT_USAGE(num, rev, func) \
@@ -511,20 +511,19 @@ static bool axiom_skip_usage_check(struct axiom_data *ts)
}
}
-static unsigned int
-axiom_usage_baseaddr(struct axiom_data *ts, unsigned char usage_num)
+static unsigned int axiom_usage_baseaddr(struct axiom_data *ts,
+ unsigned char usage_num)
{
return ts->usage_table[usage_num].baseaddr;
}
-static unsigned int
-axiom_usage_size(struct axiom_data *ts, unsigned char usage_num)
+static unsigned int axiom_usage_size(struct axiom_data *ts,
+ unsigned char usage_num)
{
return ts->usage_table[usage_num].size_bytes;
}
-static int
-axiom_usage_rev(struct axiom_data *ts, unsigned char usage_num)
+static int axiom_usage_rev(struct axiom_data *ts, unsigned char usage_num)
{
struct axiom_usage_table_entry *entry = &ts->usage_table[usage_num];
@@ -534,8 +533,8 @@ axiom_usage_rev(struct axiom_data *ts, unsigned char usage_num)
return entry->info->rev_num;
}
-static bool
-axiom_driver_supports_usage(struct axiom_data *ts, unsigned char usage_num)
+static bool axiom_driver_supports_usage(struct axiom_data *ts,
+ unsigned char usage_num)
{
const struct axiom_usage_info *iter = driver_required_usages;
struct device *dev = ts->dev;
@@ -571,14 +570,12 @@ axiom_driver_supports_usage(struct axiom_data *ts, unsigned char usage_num)
return false;
}
-static bool
-axiom_usage_entry_is_report(struct axiom_u31_usage_table_entry *entry)
+static bool axiom_usage_entry_is_report(struct axiom_u31_usage_table_entry *entry)
{
return entry->num_pages == 0;
}
-static unsigned int
-axiom_get_usage_size_bytes(struct axiom_u31_usage_table_entry *entry)
+static unsigned int axiom_get_usage_size_bytes(struct axiom_u31_usage_table_entry *entry)
{
unsigned char max_offset;
@@ -699,9 +696,9 @@ static bool axiom_usage_supported(struct axiom_data *ts, unsigned int baseaddr)
static void axiom_poll(struct input_dev *input);
-static unsigned long
-axiom_wait_for_completion_timeout(struct axiom_data *ts, struct axiom_completion *x,
- long timeout)
+static unsigned long axiom_wait_for_completion_timeout(struct axiom_data *ts,
+ struct axiom_completion *x,
+ long timeout)
{
struct i2c_client *client = to_i2c_client(ts->dev);
unsigned long poll_timeout;
@@ -751,7 +748,7 @@ static void axiom_complete(struct axiom_data *ts, struct axiom_completion *x)
static int axiom_u02_wait_idle(struct axiom_data *ts)
{
unsigned int reg;
- int ret, _ret;
+ int error, ret;
u16 cmd;
if (!axiom_driver_supports_usage(ts, AXIOM_U02))
@@ -764,23 +761,24 @@ static int axiom_u02_wait_idle(struct axiom_data *ts)
* Missing regmap_raw_read_poll_timeout for now. RESP_SUCCESS means that
* the last command successfully completed and the device is idle.
*/
- ret = read_poll_timeout(regmap_raw_read, _ret,
- _ret || cmd == AXIOM_U02_REV1_RESP_SUCCESS,
- 10 * USEC_PER_MSEC, 1 * USEC_PER_SEC, false,
- ts->regmap, reg, &cmd, 2);
- if (ret)
+ error = read_poll_timeout(regmap_raw_read, ret,
+ ret || cmd == AXIOM_U02_REV1_RESP_SUCCESS,
+ 10 * USEC_PER_MSEC, 1 * USEC_PER_SEC, false,
+ ts->regmap, reg, &cmd, 2);
+ if (error) {
dev_err(ts->dev, "Poll u02 timedout with: %#x\n", cmd);
+ return error;
+ }
- return ret;
+ return 0;
}
-static int
-axiom_u02_send_msg(struct axiom_data *ts,
- const struct axiom_u02_rev1_system_manager_msg *msg,
- bool validate_response)
+static int axiom_u02_send_msg(struct axiom_data *ts,
+ const struct axiom_u02_rev1_system_manager_msg *msg,
+ bool validate_response)
{
unsigned int reg;
- int ret;
+ int error;
if (!axiom_driver_supports_usage(ts, AXIOM_U02))
return -EINVAL;
@@ -788,9 +786,9 @@ axiom_u02_send_msg(struct axiom_data *ts,
reg = axiom_usage_baseaddr(ts, AXIOM_U02);
reg += AXIOM_U02_REV1_COMMAND_REG;
- ret = regmap_raw_write(ts->regmap, reg, msg, sizeof(*msg));
- if (ret)
- return ret;
+ error = regmap_raw_write(ts->regmap, reg, msg, sizeof(*msg));
+ if (error)
+ return error;
if (!validate_response)
return 0;
@@ -798,8 +796,7 @@ axiom_u02_send_msg(struct axiom_data *ts,
return axiom_u02_wait_idle(ts);
}
-static int
-axiom_u02_rev1_send_single_cmd(struct axiom_data *ts, u16 cmd)
+static int axiom_u02_rev1_send_single_cmd(struct axiom_data *ts, u16 cmd)
{
struct axiom_u02_rev1_system_manager_msg msg = {
.command = cpu_to_le16(cmd)
@@ -826,7 +823,7 @@ static int axiom_u02_stop(struct axiom_data *ts)
static int axiom_u02_save_config(struct axiom_data *ts)
{
struct axiom_u02_rev1_system_manager_msg msg;
- int ret;
+ int error;
if (!axiom_driver_supports_usage(ts, AXIOM_U02))
return -EINVAL;
@@ -836,45 +833,48 @@ static int axiom_u02_save_config(struct axiom_data *ts)
msg.parameters[1] = cpu_to_le16(AXIOM_U02_REV1_PARAM1_SAVEVLTLCFG2NVM);
msg.parameters[2] = cpu_to_le16(AXIOM_U02_REV1_PARAM2_SAVEVLTLCFG2NVM);
- ret = axiom_u02_send_msg(ts, &msg, false);
- if (ret)
- return ret;
+ error = axiom_u02_send_msg(ts, &msg, false);
+ if (error)
+ return error;
/* Downstream axcfg.py waits for 2sec without checking U01 response */
- ret = axiom_wait_for_completion_timeout(ts, &ts->nvm_write,
- msecs_to_jiffies(2 * MSEC_PER_SEC));
- if (!ret)
+ if (!axiom_wait_for_completion_timeout(ts, &ts->nvm_write,
+ msecs_to_jiffies(2 * MSEC_PER_SEC))) {
dev_err(ts->dev, "Error save volatile config timedout\n");
+ return -ETIMEDOUT;
+ }
- return ret ? 0 : -ETIMEDOUT;
+ return 0;
}
static int axiom_u02_swreset(struct axiom_data *ts)
{
- struct axiom_u02_rev1_system_manager_msg msg = { };
- int ret;
+ struct axiom_u02_rev1_system_manager_msg msg = {
+ .command = cpu_to_le16(AXIOM_U02_REV1_CMD_SOFTRESET),
+ };
+ int error;
if (!axiom_driver_supports_usage(ts, AXIOM_U02))
return -EINVAL;
- msg.command = cpu_to_le16(AXIOM_U02_REV1_CMD_SOFTRESET);
- ret = axiom_u02_send_msg(ts, &msg, false);
- if (ret)
- return ret;
+ error = axiom_u02_send_msg(ts, &msg, false);
+ if (error)
+ return error;
/*
* Downstream axcfg.py waits for 1sec without checking U01 hello. Tests
* showed that waiting for the hello message isn't enough therefore we
- * need both to make it robuster.
+ * need both to make it more robust.
*/
- ret = axiom_wait_for_completion_timeout(ts, &ts->boot_complete,
- msecs_to_jiffies(1 * MSEC_PER_SEC));
- if (!ret)
+ if (!axiom_wait_for_completion_timeout(ts, &ts->boot_complete,
+ msecs_to_jiffies(1 * MSEC_PER_SEC))) {
dev_err(ts->dev, "Error swreset timedout\n");
+ error = -ETIMEDOUT;
+ }
fsleep(USEC_PER_SEC);
- return ret ? 0 : -ETIMEDOUT;
+ return error;
}
static int axiom_u02_fillconfig(struct axiom_data *ts)
@@ -897,7 +897,7 @@ static int axiom_u02_enter_bootloader(struct axiom_data *ts)
struct axiom_u02_rev1_system_manager_msg msg = { };
struct device *dev = ts->dev;
unsigned int val;
- int ret;
+ int error;
if (!axiom_driver_supports_usage(ts, AXIOM_U02))
return -EINVAL;
@@ -908,57 +908,62 @@ static int axiom_u02_enter_bootloader(struct axiom_data *ts)
*/
msg.command = cpu_to_le16(AXIOM_U02_REV1_CMD_ENTERBOOTLOADER);
msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY1);
- ret = axiom_u02_send_msg(ts, &msg, false);
- if (ret) {
- dev_err(dev, "Failed to send bootloader-key1: %d\n", ret);
- return ret;
+ error = axiom_u02_send_msg(ts, &msg, false);
+ if (error) {
+ dev_err(dev, "Failed to send bootloader-key1: %d\n", error);
+ return error;
}
msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY2);
- ret = axiom_u02_send_msg(ts, &msg, false);
- if (ret) {
- dev_err(dev, "Failed to send bootloader-key2: %d\n", ret);
- return ret;
+ error = axiom_u02_send_msg(ts, &msg, false);
+ if (error) {
+ dev_err(dev, "Failed to send bootloader-key2: %d\n", error);
+ return error;
}
msg.parameters[0] = cpu_to_le16(AXIOM_U02_REV1_PARAM0_ENTERBOOLOADER_KEY3);
- ret = axiom_u02_send_msg(ts, &msg, false);
- if (ret) {
- dev_err(dev, "Failed to send bootloader-key3: %d\n", ret);
- return ret;
+ error = axiom_u02_send_msg(ts, &msg, false);
+ if (error) {
+ dev_err(dev, "Failed to send bootloader-key3: %d\n", error);
+ return error;
}
/* Sleep before the first read to give the device time */
fsleep(250 * USEC_PER_MSEC);
/* Wait till the device reports it is in bootloader mode */
- return regmap_read_poll_timeout(ts->regmap,
- AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, val,
- FIELD_GET(AXIOM_U31_REV1_MODE_MASK, val) ==
- AXIOM_U31_REV1_MODE_BLP, 250 * USEC_PER_MSEC,
- USEC_PER_SEC);
+ error = regmap_read_poll_timeout(ts->regmap,
+ AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, val,
+ FIELD_GET(AXIOM_U31_REV1_MODE_MASK, val) ==
+ AXIOM_U31_REV1_MODE_BLP,
+ 250 * USEC_PER_MSEC, USEC_PER_SEC);
+ if (error)
+ return error;
+
+ return 0;
}
-static int axiom_u04_get(struct axiom_data *ts, u8 **_buf)
+static u8 *axiom_u04_get(struct axiom_data *ts)
{
- u8 buf[AXIOM_U04_REV1_SIZE_BYTES];
unsigned int reg;
- int ret;
+ int error;
if (!axiom_driver_supports_usage(ts, AXIOM_U04))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
- reg = axiom_usage_baseaddr(ts, AXIOM_U04);
- ret = regmap_raw_read(ts->regmap, reg, buf, sizeof(buf));
- if (ret)
- return ret;
+ u8 *buf __free(kfree) = kzalloc(AXIOM_U04_REV1_SIZE_BYTES, GFP_KERNEL);
+ if (!buf)
+ return ERR_PTR(-ENOMEM);
- *_buf = kmemdup(buf, sizeof(buf), GFP_KERNEL);
+ reg = axiom_usage_baseaddr(ts, AXIOM_U04);
+ error = regmap_raw_read(ts->regmap, reg, buf, sizeof(buf));
+ if (error)
+ return ERR_PTR(error);
- return sizeof(buf);
+ return_ptr(buf);
}
-static int axiom_u04_set(struct axiom_data *ts, u8 *buf, unsigned int bufsize)
+static int axiom_u04_set(struct axiom_data *ts, u8 *buf)
{
unsigned int reg;
@@ -966,7 +971,7 @@ static int axiom_u04_set(struct axiom_data *ts, u8 *buf, unsigned int bufsize)
return -EINVAL;
reg = axiom_usage_baseaddr(ts, AXIOM_U04);
- return regmap_raw_write(ts->regmap, reg, buf, bufsize);
+ return regmap_raw_write(ts->regmap, reg, buf, AXIOM_U04_REV1_SIZE_BYTES);
}
/*
@@ -977,52 +982,52 @@ static int axiom_u31_parse_device_info(struct axiom_data *ts)
{
struct regmap *regmap = ts->regmap;
unsigned int id_low, id_high, val;
- int ret;
+ int error;
- ret = regmap_read(regmap, AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, &id_high);
- if (ret)
- return ret;
+ error = regmap_read(regmap, AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, &id_high);
+ if (error)
+ return error;
id_high = FIELD_GET(AXIOM_U31_REV1_DEVICE_ID_HIGH_MASK, id_high);
- ret = regmap_read(regmap, AXIOM_U31_REV1_DEVICE_ID_LOW_REG, &id_low);
- if (ret)
- return ret;
+ error = regmap_read(regmap, AXIOM_U31_REV1_DEVICE_ID_LOW_REG, &id_low);
+ if (error)
+ return error;
ts->device_id = id_high << 8 | id_low;
- ret = regmap_read(regmap, AXIOM_U31_REV1_RUNTIME_FW_MAJ_REG, &val);
- if (ret)
- return ret;
+ error = regmap_read(regmap, AXIOM_U31_REV1_RUNTIME_FW_MAJ_REG, &val);
+ if (error)
+ return error;
ts->fw_major = val;
- ret = regmap_read(regmap, AXIOM_U31_REV1_RUNTIME_FW_MIN_REG, &val);
- if (ret)
- return ret;
+ error = regmap_read(regmap, AXIOM_U31_REV1_RUNTIME_FW_MIN_REG, &val);
+ if (error)
+ return error;
ts->fw_minor = val;
/* All other fields are not allowed to be read in BLP mode */
if (axiom_get_runmode(ts) == AXIOM_BLP_MODE)
return 0;
- ret = regmap_read(regmap, AXIOM_U31_REV1_RUNTIME_FW_RC_REG, &val);
- if (ret)
- return ret;
+ error = regmap_read(regmap, AXIOM_U31_REV1_RUNTIME_FW_RC_REG, &val);
+ if (error)
+ return error;
ts->fw_rc = FIELD_GET(AXIOM_U31_REV1_RUNTIME_FW_RC_MASK, val);
ts->silicon_rev = FIELD_GET(AXIOM_U31_REV1_SILICON_REV_MASK, val);
- ret = regmap_read(regmap, AXIOM_U31_REV1_RUNTIME_FW_STATUS_REG, &val);
- if (ret)
- return ret;
+ error = regmap_read(regmap, AXIOM_U31_REV1_RUNTIME_FW_STATUS_REG, &val);
+ if (error)
+ return error;
ts->fw_status = FIELD_GET(AXIOM_U31_REV1_RUNTIME_FW_STATUS, val);
ts->fw_variant = FIELD_GET(AXIOM_U31_REV1_RUNTIME_FW_VARIANT, val);
- ret = regmap_read(regmap, AXIOM_U31_REV1_JEDEC_ID_HIGH_REG, &val);
- if (ret)
- return ret;
+ error = regmap_read(regmap, AXIOM_U31_REV1_JEDEC_ID_HIGH_REG, &val);
+ if (error)
+ return error;
ts->jedec_id = val << 8;
- ret = regmap_read(regmap, AXIOM_U31_REV1_JEDEC_ID_LOW_REG, &val);
- if (ret)
- return ret;
+ error = regmap_read(regmap, AXIOM_U31_REV1_JEDEC_ID_LOW_REG, &val);
+ if (error)
+ return error;
ts->jedec_id |= val;
return 0;
@@ -1032,20 +1037,19 @@ static int axiom_u33_read(struct axiom_data *ts, struct axiom_crc *crc);
static int axiom_u31_device_discover(struct axiom_data *ts)
{
- struct axiom_u31_usage_table_entry *u31_usage_table __free(kfree) = NULL;
struct axiom_u31_usage_table_entry *entry;
struct regmap *regmap = ts->regmap;
unsigned int mode, num_usages;
struct device *dev = ts->dev;
unsigned int i;
- int ret;
+ int error;
axiom_set_runmode(ts, AXIOM_DISCOVERY_MODE);
- ret = regmap_read(regmap, AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, &mode);
- if (ret) {
+ error = regmap_read(regmap, AXIOM_U31_REV1_DEVICE_ID_HIGH_REG, &mode);
+ if (error) {
dev_err(dev, "Failed to read MODE\n");
- return ret;
+ return error;
}
/* Abort if the device is in bootloader protocol mode */
@@ -1054,10 +1058,10 @@ static int axiom_u31_device_discover(struct axiom_data *ts)
axiom_set_runmode(ts, AXIOM_BLP_MODE);
/* Since we are not in bootloader mode we can parse the device info */
- ret = axiom_u31_parse_device_info(ts);
- if (ret) {
+ error = axiom_u31_parse_device_info(ts);
+ if (error) {
dev_err(dev, "Failed to parse device info\n");
- return ret;
+ return error;
}
/* All other fields are not allowed to be read in BLP mode */
@@ -1066,22 +1070,22 @@ static int axiom_u31_device_discover(struct axiom_data *ts)
return -EACCES;
}
- ret = regmap_read(regmap, AXIOM_U31_REV1_NUM_USAGES_REG, &num_usages);
- if (ret) {
+ error = regmap_read(regmap, AXIOM_U31_REV1_NUM_USAGES_REG, &num_usages);
+ if (error) {
dev_err(dev, "Failed to read NUM_USAGES\n");
- return ret;
+ return error;
}
- u31_usage_table = kcalloc(num_usages, sizeof(*u31_usage_table),
- GFP_KERNEL);
+ struct axiom_u31_usage_table_entry *u31_usage_table __free(kfree) =
+ kcalloc(num_usages, sizeof(*u31_usage_table), GFP_KERNEL);
if (!u31_usage_table)
return -ENOMEM;
- ret = regmap_raw_read(regmap, AXIOM_U31_REV1_PAGE1, u31_usage_table,
- array_size(num_usages, sizeof(*u31_usage_table)));
- if (ret) {
+ error = regmap_raw_read(regmap, AXIOM_U31_REV1_PAGE1, u31_usage_table,
+ array_size(num_usages, sizeof(*u31_usage_table)));
+ if (error) {
dev_err(dev, "Failed to read NUM_USAGES\n");
- return ret;
+ return error;
}
/*
@@ -1106,7 +1110,7 @@ static int axiom_u31_device_discover(struct axiom_data *ts)
if (IS_ERR(info)) {
dev_info(dev, "Required usage u%02X isn't supported for rev.%u\n",
entry->usage_num, entry->uifrevision);
- ret = -EACCES;
+ error = -EACCES;
}
size_bytes = axiom_get_usage_size_bytes(entry);
@@ -1121,20 +1125,24 @@ static int axiom_u31_device_discover(struct axiom_data *ts)
ts->max_report_byte_len = size_bytes;
}
- if (ret)
- return ret;
+ if (error)
+ return error;
/* From now on we are in TCP mode to include usage revision checks */
axiom_set_runmode(ts, AXIOM_TCP_MODE);
- return axiom_u33_read(ts, &ts->crc[AXIOM_CRC_CUR]);
+ error = axiom_u33_read(ts, &ts->crc[AXIOM_CRC_CUR]);
+ if (error)
+ return error;
+
+ return 0;
}
static int axiom_u33_read(struct axiom_data *ts, struct axiom_crc *crc)
{
struct device *dev = ts->dev;
unsigned int reg;
- int ret;
+ int error;
if (!axiom_driver_supports_usage(ts, AXIOM_U33))
return -EINVAL;
@@ -1143,10 +1151,10 @@ static int axiom_u33_read(struct axiom_data *ts, struct axiom_crc *crc)
struct axiom_u33_rev2 val;
reg = axiom_usage_baseaddr(ts, AXIOM_U33);
- ret = regmap_raw_read(ts->regmap, reg, &val, sizeof(val));
- if (ret) {
+ error = regmap_raw_read(ts->regmap, reg, &val, sizeof(val));
+ if (error) {
dev_err(dev, "Failed to read u33\n");
- return ret;
+ return error;
}
crc->runtime = le32_to_cpu(val.runtime_crc);
@@ -1160,10 +1168,10 @@ static int axiom_u33_read(struct axiom_data *ts, struct axiom_crc *crc)
struct axiom_u33_rev3 val;
reg = axiom_usage_baseaddr(ts, AXIOM_U33);
- ret = regmap_raw_read(ts->regmap, reg, &val, sizeof(val));
- if (ret) {
+ error = regmap_raw_read(ts->regmap, reg, &val, sizeof(val));
+ if (error) {
dev_err(dev, "Failed to read u33\n");
- return ret;
+ return error;
}
crc->runtime = le32_to_cpu(val.runtime_crc);
@@ -1195,31 +1203,22 @@ static bool axiom_u42_touch_enabled(struct axiom_data *ts, const u8 *buf,
}
}
-static void axiom_u42_get_touchslots(struct axiom_data *ts)
+static bool axiom_u42_get_touchslots(struct axiom_data *ts)
{
- u8 *buf __free(kfree) = NULL;
- struct device *dev = ts->dev;
unsigned int bufsize;
unsigned int reg;
- int ret, i;
-
- if (!axiom_driver_supports_usage(ts, AXIOM_U42)) {
- dev_warn(dev, "Use default touchslots num\n");
- goto fallback;
- }
+ int error, i;
bufsize = axiom_usage_size(ts, AXIOM_U42);
- buf = kzalloc(bufsize, GFP_KERNEL);
- if (!buf) {
- dev_warn(dev, "Failed to alloc u42 read buffer, use default value\n");
- goto fallback;
- }
+ u8 *buf __free(kfree) = kzalloc(bufsize, GFP_KERNEL);
+ if (!buf)
+ return false;
reg = axiom_usage_baseaddr(ts, AXIOM_U42);
- ret = regmap_raw_read(ts->regmap, reg, buf, bufsize);
- if (ret) {
- dev_warn(dev, "Failed to read u42, use default value\n");
- goto fallback;
+ error = regmap_raw_read(ts->regmap, reg, buf, bufsize);
+ if (error) {
+ dev_warn(ts->dev, "Failed to read u42\n");
+ return false;
}
ts->enabled_slots = 0;
@@ -1232,41 +1231,42 @@ static void axiom_u42_get_touchslots(struct axiom_data *ts)
}
}
- return;
+ return true;
+}
-fallback:
- ts->enabled_slots = AXIOM_MAX_TOUCHSLOTS_MASK;
- ts->num_slots = AXIOM_MAX_TOUCHSLOTS;
+static void axiom_get_touchslots(struct axiom_data *ts)
+{
+ if (!axiom_driver_supports_usage(ts, AXIOM_U42) ||
+ !axiom_u42_get_touchslots(ts)) {
+ dev_warn(ts->dev, "Use default touchslots num\n");
+ ts->enabled_slots = AXIOM_MAX_TOUCHSLOTS_MASK;
+ ts->num_slots = AXIOM_MAX_TOUCHSLOTS;
+ }
}
static void axiom_u64_cds_enabled(struct axiom_data *ts)
{
unsigned int reg, val;
- int ret;
-
- if (!axiom_driver_supports_usage(ts, AXIOM_U64))
- goto fallback_out;
-
- reg = axiom_usage_baseaddr(ts, AXIOM_U64);
- reg += AXIOM_U64_REV2_ENABLECDSPROCESSING_REG;
-
- ret = regmap_read(ts->regmap, reg, &val);
- if (ret)
- goto fallback_out;
+ int error;
- val = FIELD_GET(AXIOM_U64_REV2_ENABLECDSPROCESSING_MASK, val);
- ts->cds_enabled = val ? true : false;
+ ts->cds_enabled = false;
- return;
+ if (axiom_driver_supports_usage(ts, AXIOM_U64)) {
+ reg = axiom_usage_baseaddr(ts, AXIOM_U64);
+ reg += AXIOM_U64_REV2_ENABLECDSPROCESSING_REG;
-fallback_out:
- ts->cds_enabled = false;
+ error = regmap_read(ts->regmap, reg, &val);
+ if (!error) {
+ val = FIELD_GET(AXIOM_U64_REV2_ENABLECDSPROCESSING_MASK, val);
+ ts->cds_enabled = val ? true : false;
+ }
+ }
}
static int axiom_cdu_wait_idle(struct axiom_data *ts, u8 cdu_usage_num)
{
unsigned int reg;
- int ret, _ret;
+ int error, ret;
u16 cmd;
reg = axiom_usage_baseaddr(ts, cdu_usage_num);
@@ -1275,15 +1275,17 @@ static int axiom_cdu_wait_idle(struct axiom_data *ts, u8 cdu_usage_num)
* Missing regmap_raw_read_poll_timeout for now. RESP_SUCCESS means that
* the last command successfully completed and the device is idle.
*/
- ret = read_poll_timeout(regmap_raw_read, _ret,
- _ret || cmd == AXIOM_CDU_RESP_SUCCESS,
- 10 * USEC_PER_MSEC, 1 * USEC_PER_SEC, false,
- ts->regmap, reg, &cmd, 2);
- if (ret)
+ error = read_poll_timeout(regmap_raw_read, ret,
+ ret || cmd == AXIOM_CDU_RESP_SUCCESS,
+ 10 * USEC_PER_MSEC, 1 * USEC_PER_SEC, false,
+ ts->regmap, reg, &cmd, 2);
+ if (error) {
dev_err(ts->dev, "Poll CDU u%02X timedout with: %#x\n",
cdu_usage_num, cmd);
+ return error;
+ }
- return ret;
+ return 0;
}
/*********************** Report usage handling ********************************/
@@ -1312,8 +1314,8 @@ static int axiom_process_report(struct axiom_data *ts, unsigned char usage_num,
}
/* Make use of datasheet method 1 - single transfer read */
-static int
-axiom_u34_rev1_process_report(struct axiom_data *ts, const u8 *_buf, size_t bufsize)
+static int axiom_u34_rev1_process_report(struct axiom_data *ts,
+ const u8 *_buf, size_t bufsize)
{
unsigned int reg = axiom_usage_baseaddr(ts, AXIOM_U34);
struct regmap *regmap = ts->regmap;
@@ -1323,11 +1325,11 @@ axiom_u34_rev1_process_report(struct axiom_data *ts, const u8 *_buf, size_t bufs
u16 crc_report, crc_calc;
unsigned int len;
u8 *payload;
- int ret;
+ int error;
- ret = regmap_raw_read(regmap, reg, buf, ts->max_report_byte_len);
- if (ret)
- return ret;
+ error = regmap_raw_read(regmap, reg, buf, ts->max_report_byte_len);
+ if (error)
+ return error;
/* TODO: Add overflow statistics */
@@ -1373,8 +1375,8 @@ axiom_u34_rev1_process_report(struct axiom_data *ts, const u8 *_buf, size_t bufs
return 0;
}
-static void
-axiom_u41_rev2_decode_target(const u8 *buf, u8 id, u16 *x, u16 *y, s8 *z)
+static void axiom_u41_rev2_decode_target(const u8 *buf, u8 id,
+ u16 *x, u16 *y, s8 *z)
{
u16 val;
@@ -1389,8 +1391,8 @@ axiom_u41_rev2_decode_target(const u8 *buf, u8 id, u16 *x, u16 *y, s8 *z)
*z = buf[AXIOM_U41_REV2_Z_REG(id)];
}
-static int
-axiom_u41_rev2_process_report(struct axiom_data *ts, const u8 *buf, size_t bufsize)
+static int axiom_u41_rev2_process_report(struct axiom_data *ts,
+ const u8 *buf, size_t bufsize)
{
struct input_dev *input = ts->input;
unsigned char id;
@@ -1437,8 +1439,8 @@ axiom_u41_rev2_process_report(struct axiom_data *ts, const u8 *buf, size_t bufsi
return 0;
}
-static int
-axiom_u01_rev1_process_report(struct axiom_data *ts, const u8 *buf, size_t bufsize)
+static int axiom_u01_rev1_process_report(struct axiom_data *ts,
+ const u8 *buf, size_t bufsize)
{
switch (buf[AXIOM_U01_REV1_REPORTTYPE_REG]) {
case AXIOM_U01_REV1_REPORTTYPE_HELLO:
@@ -1509,13 +1511,11 @@ static int axiom_regmap_read(void *context, const void *reg_buf, size_t reg_size
xfer[1].len = val_size;
xfer[1].buf = val_buf;
- ret = i2c_transfer(i2c->adapter, xfer, 2);
- if (ret == 2)
+ ret = i2c_transfer(i2c->adapter, xfer, ARRAY_SIZE(xfer));
+ if (likely(ret == ARRAY_SIZE(xfer)))
return 0;
- else if (ret < 0)
- return ret;
- else
- return -EIO;
+
+ return ret < 0 ? ret : -EIO;
}
static int axiom_regmap_write(void *context, const void *data, size_t count)
@@ -1523,7 +1523,6 @@ static int axiom_regmap_write(void *context, const void *data, size_t count)
struct device *dev = context;
struct i2c_client *i2c = to_i2c_client(dev);
struct axiom_data *ts = i2c_get_clientdata(i2c);
- char *buf __free(kfree) = NULL;
struct axiom_cmd_header hdr;
u16 xferlen, addr, baseaddr;
size_t val_size, msg_size;
@@ -1548,7 +1547,7 @@ static int axiom_regmap_write(void *context, const void *data, size_t count)
return -EINVAL;
msg_size = sizeof(hdr) + val_size;
- buf = kzalloc(msg_size, GFP_KERNEL);
+ u8 *buf __free(kfree) = kzalloc(msg_size, GFP_KERNEL);
if (!buf)
return -ENOMEM;
@@ -1556,8 +1555,10 @@ static int axiom_regmap_write(void *context, const void *data, size_t count)
memcpy(&buf[sizeof(hdr)], &((char *)data)[2], val_size);
ret = i2c_master_send(i2c, buf, msg_size);
+ if (likely(ret == msg_size))
+ return 0;
- return ret == msg_size ? 0 : ret;
+ return ret < 0 ? ret : -EIO;
}
static const struct regmap_config axiom_i2c_regmap_config = {
@@ -1571,29 +1572,20 @@ static const struct regmap_config axiom_i2c_regmap_config = {
static int axiom_update_input_dev(struct axiom_data *ts);
-static enum fw_upload_err
-axiom_axfw_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
+static enum fw_upload_err __axiom_axfw_fw_prepare(struct axiom_data *ts,
+ struct axiom_firmware *afw,
+ const u8 *data, u32 size)
{
- struct axiom_data *ts = fw_upload->dd_handle;
- struct axiom_firmware *afw = &ts->fw[AXIOM_FW_AXFW];
u8 major_ver, minor_ver, rc_ver, status, variant;
u32 fw_file_crc32, crc32_calc;
struct device *dev = ts->dev;
unsigned int signature_len;
- enum fw_upload_err ret;
u16 fw_file_format_ver;
u16 fw_file_device_id;
- mutex_lock(&afw->lock);
- afw->cancel = false;
- mutex_unlock(&afw->lock);
-
- mutex_lock(&ts->fwupdate_lock);
-
if (size < sizeof(struct axiom_fw_axfw_hdr)) {
dev_err(dev, "Invalid AXFW file size\n");
- ret = FW_UPLOAD_ERR_INVALID_SIZE;
- goto out;
+ return FW_UPLOAD_ERR_INVALID_SIZE;
}
signature_len = strlen(AXIOM_FW_AXFW_SIGNATURE);
@@ -1603,8 +1595,7 @@ axiom_axfw_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
* ALC don't. Therefore the AXFW format is preferred.
*/
dev_warn(dev, "No AXFW signature, assume ALC firmware\n");
- ret = FW_UPLOAD_ERR_NONE;
- goto out;
+ return FW_UPLOAD_ERR_NONE;
}
fw_file_crc32 = get_unaligned_le32(&data[signature_len]);
@@ -1612,8 +1603,7 @@ axiom_axfw_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
if (fw_file_crc32 != crc32_calc) {
dev_err(dev, "AXFW CRC32 doesn't match (fw:%#x calc:%#x)\n",
fw_file_crc32, crc32_calc);
- ret = FW_UPLOAD_ERR_FW_INVALID;
- goto out;
+ return FW_UPLOAD_ERR_FW_INVALID;
}
data += signature_len + sizeof(fw_file_crc32);
@@ -1621,8 +1611,7 @@ axiom_axfw_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
if (fw_file_format_ver != AXIOM_FW_AXFW_FILE_FMT_VER) {
dev_err(dev, "Invalid AXFW file format version: %04x",
fw_file_format_ver);
- ret = FW_UPLOAD_ERR_FW_INVALID;
- goto out;
+ return FW_UPLOAD_ERR_FW_INVALID;
}
data += sizeof(fw_file_format_ver);
@@ -1630,8 +1619,7 @@ axiom_axfw_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
if (fw_file_device_id != ts->device_id) {
dev_err(dev, "Invalid AXFW target device (fw:%#04x dev:%#04x)\n",
fw_file_device_id, ts->device_id);
- ret = FW_UPLOAD_ERR_FW_INVALID;
- goto out;
+ return FW_UPLOAD_ERR_FW_INVALID;
}
/*
@@ -1640,7 +1628,7 @@ axiom_axfw_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
* * downloading the firmware failed in between, or
* * the following usage discovery failed.
*
- * All cases are crcitical and we need to use any firmware to
+ * All cases are critical and we need to use any firmware to
* bring the device back into a working state which is supported by the
* host.
*/
@@ -1657,19 +1645,32 @@ axiom_axfw_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
if (major_ver == ts->fw_major && minor_ver == ts->fw_minor &&
rc_ver == ts->fw_rc && status == ts->fw_status &&
variant == ts->fw_variant) {
- ret = FW_UPLOAD_ERR_DUPLICATE;
- goto out;
+ return FW_UPLOAD_ERR_DUPLICATE;
}
dev_info(dev, "Detected AXFW %02u.%02u.%02u (%s)\n",
major_ver, minor_ver, rc_ver,
status ? "production" : "engineering");
- mutex_lock(&afw->lock);
- ret = afw->cancel ? FW_UPLOAD_ERR_CANCELED : FW_UPLOAD_ERR_NONE;
- mutex_unlock(&afw->lock);
+ guard(mutex)(&afw->lock);
+ return afw->cancel ? FW_UPLOAD_ERR_CANCELED : FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err axiom_axfw_fw_prepare(struct fw_upload *fw_upload,
+ const u8 *data, u32 size)
+{
+ struct axiom_data *ts = fw_upload->dd_handle;
+ struct axiom_firmware *afw = &ts->fw[AXIOM_FW_AXFW];
+ enum fw_upload_err ret;
+
+ scoped_guard(mutex, &afw->lock) {
+ afw->cancel = false;
+ }
+
+ mutex_lock(&ts->fwupdate_lock);
+
+ ret = __axiom_axfw_fw_prepare(ts, afw, data, size);
-out:
/*
* In FW_UPLOAD_ERR_NONE case the complete handler will release the
* lock.
@@ -1683,28 +1684,26 @@ axiom_axfw_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
static int axiom_enter_bootloader_mode(struct axiom_data *ts)
{
struct device *dev = ts->dev;
- int ret;
+ int error;
axiom_set_runmode(ts, AXIOM_BLP_PRE_MODE);
- ret = axiom_u02_wait_idle(ts);
- if (ret)
+ error = axiom_u02_wait_idle(ts);
+ if (error)
goto err_out;
- ret = axiom_u02_enter_bootloader(ts);
- if (ret) {
+ error = axiom_u02_enter_bootloader(ts);
+ if (error) {
dev_err(dev, "Failed to enter bootloader mode\n");
goto err_out;
}
axiom_set_runmode(ts, AXIOM_BLP_MODE);
-
return 0;
err_out:
axiom_set_runmode(ts, AXIOM_TCP_MODE);
-
- return ret;
+ return error;
}
static int axoim_blp_wait_ready(struct axiom_data *ts)
@@ -1727,18 +1726,18 @@ static int axoim_blp_wait_ready(struct axiom_data *ts)
return ret;
}
-static int
-axiom_blp_write_chunk(struct axiom_data *ts, const u8 *data, u16 length)
+static int axiom_blp_write_chunk(struct axiom_data *ts,
+ const u8 *data, u16 length)
{
unsigned int chunk_size = AXIOM_U01_BLP_FIFO_CHK_SIZE_BYTES;
unsigned int reg = AXIOM_U01_BLP_FIFO_REG;
struct device *dev = ts->dev;
unsigned int pos = 0;
- int ret;
+ int error;
- ret = axoim_blp_wait_ready(ts);
- if (ret)
- return ret;
+ error = axoim_blp_wait_ready(ts);
+ if (error)
+ return error;
/*
* TODO: Downstream does this chunk transfers. Verify if this is
@@ -1751,16 +1750,16 @@ axiom_blp_write_chunk(struct axiom_data *ts, const u8 *data, u16 length)
if ((pos + chunk_size) > length)
len = length - pos;
- ret = regmap_raw_write(ts->regmap, reg, &data[pos], len);
- if (ret) {
- dev_err(dev, "Bootloader download AXFW chunk failed %d\n", ret);
- return ret;
+ error = regmap_raw_write(ts->regmap, reg, &data[pos], len);
+ if (error) {
+ dev_err(dev, "Bootloader download AXFW chunk failed %d\n", error);
+ return error;
}
pos += len;
- ret = axoim_blp_wait_ready(ts);
- if (ret)
- return ret;
+ error = axoim_blp_wait_ready(ts);
+ if (error)
+ return error;
}
return 0;
@@ -1773,11 +1772,11 @@ static int axiom_blp_reset(struct axiom_data *ts)
struct device *dev = ts->dev;
unsigned int attempts = 20;
unsigned int mode;
- int ret;
+ int error;
- ret = axoim_blp_wait_ready(ts);
- if (ret)
- return ret;
+ error = axoim_blp_wait_ready(ts);
+ if (error)
+ return error;
/*
* For some reason this write fail with -ENXIO. Skip checking the return
@@ -1787,17 +1786,17 @@ static int axiom_blp_reset(struct axiom_data *ts)
regmap_raw_write(ts->regmap, reg, &reset_cmd, sizeof(reset_cmd));
do {
- ret = regmap_read(ts->regmap, AXIOM_U31_REV1_DEVICE_ID_HIGH_REG,
- &mode);
- if (!ret)
+ error = regmap_read(ts->regmap, AXIOM_U31_REV1_DEVICE_ID_HIGH_REG,
+ &mode);
+ if (!error)
break;
fsleep(250 * USEC_PER_MSEC);
} while (attempts--);
- if (ret) {
- dev_err(dev, "Failed to read MODE after BLP reset: %d\n", ret);
- return ret;
+ if (error) {
+ dev_err(dev, "Failed to read MODE after BLP reset: %d\n", error);
+ return error;
}
mode = FIELD_GET(AXIOM_U31_REV1_MODE_MASK, mode);
@@ -1833,27 +1832,24 @@ static void axiom_unregister_input_dev(struct axiom_data *ts)
ts->input = NULL;
}
-static enum fw_upload_err
-axiom_axfw_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
- u32 size, u32 *written)
+static enum fw_upload_err axiom_axfw_fw_write(struct fw_upload *fw_upload,
+ const u8 *data, u32 offset,
+ u32 size, u32 *written)
{
struct axiom_data *ts = fw_upload->dd_handle;
struct axiom_firmware *afw = &ts->fw[AXIOM_FW_AXFW];
struct device *dev = ts->dev;
- bool cancel;
- int ret;
+ int error;
/* Done before cancel check due to cleanup based put */
- ret = pm_runtime_resume_and_get(ts->dev);
- if (ret)
+ error = pm_runtime_resume_and_get(ts->dev);
+ if (error)
return FW_UPLOAD_ERR_HW_ERROR;
- mutex_lock(&afw->lock);
- cancel = afw->cancel;
- mutex_unlock(&afw->lock);
-
- if (cancel)
- return FW_UPLOAD_ERR_CANCELED;
+ scoped_guard(mutex, &afw->lock) {
+ if (afw->cancel)
+ return FW_UPLOAD_ERR_CANCELED;
+ }
axiom_lock_input_device(ts);
@@ -1883,8 +1879,8 @@ axiom_axfw_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
* The bootlaoder FW can handle the complete chunk incl. the
* header.
*/
- ret = axiom_blp_write_chunk(ts, data, len);
- if (ret)
+ error = axiom_blp_write_chunk(ts, data, len);
+ if (error)
goto err;
size -= len;
@@ -1892,12 +1888,12 @@ axiom_axfw_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
data += len;
}
- ret = axiom_blp_reset(ts);
- if (ret)
+ error = axiom_blp_reset(ts);
+ if (error)
dev_warn(dev, "BLP reset failed\n");
- ret = axiom_u31_device_discover(ts);
- if (ret) {
+ error = axiom_u31_device_discover(ts);
+ if (error) {
/*
* This is critical and we need to avoid that the user-space can
* still use the input-dev.
@@ -1911,8 +1907,8 @@ axiom_axfw_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
/* Unlock before the input device gets unregistered */
axiom_unlock_input_device(ts);
- ret = axiom_update_input_dev(ts);
- if (ret) {
+ error = axiom_update_input_dev(ts);
+ if (error) {
dev_err(dev, "Input device update failed after AXFW/ALC firmware update\n");
return FW_UPLOAD_ERR_HW_ERROR;
}
@@ -1936,9 +1932,8 @@ static void axiom_axfw_fw_cancel(struct fw_upload *fw_upload)
struct axiom_data *ts = fw_upload->dd_handle;
struct axiom_firmware *afw = &ts->fw[AXIOM_FW_AXFW];
- mutex_lock(&afw->lock);
+ guard(mutex)(&afw->lock);
afw->cancel = true;
- mutex_unlock(&afw->lock);
}
static void axiom_axfw_fw_cleanup(struct fw_upload *fw_upload)
@@ -1958,8 +1953,8 @@ static const struct fw_upload_ops axiom_axfw_fw_upload_ops = {
.cleanup = axiom_axfw_fw_cleanup,
};
-static int
-axiom_set_new_crcs(struct axiom_data *ts, const struct axiom_fw_cfg_chunk *cfg)
+static int axiom_set_new_crcs(struct axiom_data *ts,
+ const struct axiom_fw_cfg_chunk *cfg)
{
struct axiom_crc *crc = &ts->crc[AXIOM_CRC_NEW];
const u32 *u33_data = (const u32 *)cfg->usage_content;
@@ -1987,8 +1982,8 @@ axiom_set_new_crcs(struct axiom_data *ts, const struct axiom_fw_cfg_chunk *cfg)
return 0;
}
-static unsigned int
-axiom_cfg_fw_prepare_chunk(struct axiom_fw_cfg_chunk *chunk, const u8 *data)
+static unsigned int axiom_cfg_fw_prepare_chunk(struct axiom_fw_cfg_chunk *chunk,
+ const u8 *data)
{
chunk->usage_num = data[0];
chunk->usage_rev = data[1];
@@ -2017,46 +2012,37 @@ static bool axiom_cfg_fw_update_required(struct axiom_data *ts)
!axiom_usage_crc_match(ts, AXIOM_U22, cur, new, u22_sequencedata) ||
!axiom_usage_crc_match(ts, AXIOM_U43, cur, new, u43_hotspots) ||
!axiom_usage_crc_match(ts, AXIOM_U93, cur, new, u93_profiles) ||
- !axiom_usage_crc_match(ts, AXIOM_U94, cur, new, u94_deltascalemap))
+ !axiom_usage_crc_match(ts, AXIOM_U94, cur, new, u94_deltascalemap)) {
return true;
+ }
return false;
}
-static enum fw_upload_err
-axiom_cfg_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
+static enum fw_upload_err __axiom_cfg_fw_prepare(struct axiom_data *ts,
+ struct axiom_firmware *afw,
+ const u8 *data, u32 size)
{
- struct axiom_data *ts = fw_upload->dd_handle;
- struct axiom_firmware *afw = &ts->fw[AXIOM_FW_CFG];
u32 cur_runtime_crc, fw_runtime_crc;
struct axiom_fw_cfg_chunk chunk;
struct device *dev = ts->dev;
- enum fw_upload_err ret;
u32 signature;
-
- mutex_lock(&afw->lock);
- afw->cancel = false;
- mutex_unlock(&afw->lock);
-
- mutex_lock(&ts->fwupdate_lock);
+ int error;
if (axiom_get_runmode(ts) != AXIOM_TCP_MODE) {
dev_err(dev, "Device not in TCP mode, abort TH2CFG update\n");
- ret = FW_UPLOAD_ERR_HW_ERROR;
- goto out;
+ return FW_UPLOAD_ERR_HW_ERROR;
}
if (size < sizeof(struct axiom_fw_cfg_hdr)) {
dev_err(dev, "Invalid TH2CFG file size\n");
- ret = FW_UPLOAD_ERR_INVALID_SIZE;
- goto out;
+ return FW_UPLOAD_ERR_INVALID_SIZE;
}
signature = get_unaligned_be32(data);
if (signature != AXIOM_FW_CFG_SIGNATURE) {
dev_err(dev, "Invalid TH2CFG signature\n");
- ret = FW_UPLOAD_ERR_FW_INVALID;
- goto out;
+ return FW_UPLOAD_ERR_FW_INVALID;
}
/* Skip to the first fw chunk */
@@ -2080,39 +2066,47 @@ axiom_cfg_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
if (size == 0) {
dev_err(dev, "Failed to find the u33 entry in TH2CFG\n");
- ret = FW_UPLOAD_ERR_FW_INVALID;
- goto out;
+ return FW_UPLOAD_ERR_FW_INVALID;
}
- ret = axiom_set_new_crcs(ts, &chunk);
- if (ret) {
- ret = FW_UPLOAD_ERR_FW_INVALID;
- goto out;
- }
+ error = axiom_set_new_crcs(ts, &chunk);
+ if (error)
+ return FW_UPLOAD_ERR_FW_INVALID;
/*
* Nothing to do if the CRCs are the same. TODO: Must be extended once
* the CDU update is added.
*/
- if (!axiom_cfg_fw_update_required(ts)) {
- ret = FW_UPLOAD_ERR_DUPLICATE;
- goto out;
- }
+ if (!axiom_cfg_fw_update_required(ts))
+ return FW_UPLOAD_ERR_DUPLICATE;
cur_runtime_crc = ts->crc[AXIOM_CRC_CUR].runtime;
fw_runtime_crc = ts->crc[AXIOM_CRC_NEW].runtime;
if (cur_runtime_crc != fw_runtime_crc) {
dev_err(dev, "TH2CFG and device runtime CRC doesn't match: %#x != %#x\n",
fw_runtime_crc, cur_runtime_crc);
- ret = FW_UPLOAD_ERR_FW_INVALID;
- goto out;
+ return FW_UPLOAD_ERR_FW_INVALID;
+ }
+
+ guard(mutex)(&afw->lock);
+ return afw->cancel ? FW_UPLOAD_ERR_CANCELED : FW_UPLOAD_ERR_NONE;
+}
+
+static enum fw_upload_err axiom_cfg_fw_prepare(struct fw_upload *fw_upload,
+ const u8 *data, u32 size)
+{
+ struct axiom_data *ts = fw_upload->dd_handle;
+ struct axiom_firmware *afw = &ts->fw[AXIOM_FW_CFG];
+ enum fw_upload_err ret;
+
+ scoped_guard(mutex, &afw->lock) {
+ afw->cancel = false;
}
- mutex_lock(&afw->lock);
- ret = afw->cancel ? FW_UPLOAD_ERR_CANCELED : FW_UPLOAD_ERR_NONE;
- mutex_unlock(&afw->lock);
+ mutex_lock(&ts->fwupdate_lock);
+
+ ret = __axiom_cfg_fw_prepare(ts, afw, data, size);
-out:
/*
* In FW_UPLOAD_ERR_NONE case the complete handler will release the
* lock.
@@ -2125,27 +2119,26 @@ axiom_cfg_fw_prepare(struct fw_upload *fw_upload, const u8 *data, u32 size)
static int axiom_zero_volatile_mem(struct axiom_data *ts)
{
- int ret, size;
- u8 *buf;
+ int error;
/* Zero out the volatile memory except for the user content in u04 */
- ret = axiom_u04_get(ts, &buf);
- if (ret < 0)
- return ret;
- size = ret;
+ u8 *buf __free(kfree) = axiom_u04_get(ts);
+ if (IS_ERR(buf))
+ return PTR_ERR(buf);
- ret = axiom_u02_fillconfig(ts);
- if (ret)
- goto out;
+ error = axiom_u02_fillconfig(ts);
+ if (error)
+ return error;
- ret = axiom_u04_set(ts, buf, size);
-out:
- kfree(buf);
- return ret;
+ error = axiom_u04_set(ts, buf);
+ if (error)
+ return error;
+
+ return 0;
}
-static bool
-axiom_skip_cfg_chunk(struct axiom_data *ts, const struct axiom_fw_cfg_chunk *chunk)
+static bool axiom_skip_cfg_chunk(struct axiom_data *ts,
+ const struct axiom_fw_cfg_chunk *chunk)
{
u8 usage_num = chunk->usage_num;
@@ -2156,21 +2149,22 @@ axiom_skip_cfg_chunk(struct axiom_data *ts, const struct axiom_fw_cfg_chunk *chu
/* Skip read-only usages */
if (ts->usage_table[usage_num].info &&
- ts->usage_table[usage_num].info->is_ro)
+ ts->usage_table[usage_num].info->is_ro) {
return true;
+ }
return false;
}
-static int
-axiom_write_cdu_usage(struct axiom_data *ts, const struct axiom_fw_cfg_chunk *chunk)
+static int axiom_write_cdu_usage(struct axiom_data *ts,
+ const struct axiom_fw_cfg_chunk *chunk)
{
struct axiom_cdu_usage cdu = { };
struct device *dev = ts->dev;
unsigned int remaining;
unsigned int reg;
unsigned int pos;
- int ret;
+ int error;
pos = 0;
remaining = chunk->usage_length;
@@ -2189,17 +2183,17 @@ axiom_write_cdu_usage(struct axiom_data *ts, const struct axiom_fw_cfg_chunk *ch
memset(cdu.data, 0, sizeof(cdu.data));
memcpy(cdu.data, &chunk->usage_content[pos], size);
- ret = regmap_raw_write(ts->regmap, reg, &cdu, sizeof(cdu));
- if (ret) {
+ error = regmap_raw_write(ts->regmap, reg, &cdu, sizeof(cdu));
+ if (error) {
dev_err(dev, "Failed to write CDU u%02X\n",
chunk->usage_num);
- return ret;
+ return error;
}
- ret = axiom_cdu_wait_idle(ts, chunk->usage_num);
- if (ret) {
+ error = axiom_cdu_wait_idle(ts, chunk->usage_num);
+ if (error) {
dev_err(dev, "CDU write wait-idle failed\n");
- return ret;
+ return error;
}
remaining -= size;
@@ -2215,59 +2209,65 @@ axiom_write_cdu_usage(struct axiom_data *ts, const struct axiom_fw_cfg_chunk *ch
cdu.parameters[0] = cpu_to_le16(AXIOM_CDU_PARAM0_COMMIT);
cdu.parameters[1] = cpu_to_le16(AXIOM_CDU_PARAM1_COMMIT);
- ret = regmap_raw_write(ts->regmap, reg, &cdu, sizeof(cdu));
- if (ret) {
+ error = regmap_raw_write(ts->regmap, reg, &cdu, sizeof(cdu));
+ if (error) {
dev_err(dev, "Failed to commit CDU u%02X to NVM\n",
chunk->usage_num);
- return ret;
+ return error;
}
- ret = axiom_wait_for_completion_timeout(ts, &ts->nvm_write,
- msecs_to_jiffies(5 * MSEC_PER_SEC));
- if (!ret) {
+ if (!axiom_wait_for_completion_timeout(ts, &ts->nvm_write,
+ msecs_to_jiffies(5 * MSEC_PER_SEC))) {
dev_err(ts->dev, "Error CDU u%02X commit timedout\n",
chunk->usage_num);
return -ETIMEDOUT;
}
- return axiom_cdu_wait_idle(ts, chunk->usage_num);
+ error = axiom_cdu_wait_idle(ts, chunk->usage_num);
+ if (error)
+ return error;
+
+ return 0;
}
-static int
-axiom_write_cfg_chunk(struct axiom_data *ts, const struct axiom_fw_cfg_chunk *chunk)
+static int axiom_write_cfg_chunk(struct axiom_data *ts,
+ const struct axiom_fw_cfg_chunk *chunk)
{
unsigned int reg;
- int ret;
+ int error;
if (ts->usage_table[chunk->usage_num].info &&
ts->usage_table[chunk->usage_num].info->is_cdu) {
- ret = axiom_write_cdu_usage(ts, chunk);
- if (ret)
- return ret;
- goto out;
+ error = axiom_write_cdu_usage(ts, chunk);
+ if (error)
+ return error;
+ } else {
+ reg = axiom_usage_baseaddr(ts, chunk->usage_num);
+ error = regmap_raw_write(ts->regmap, reg, chunk->usage_content,
+ chunk->usage_length);
+ if (error)
+ return error;
}
- reg = axiom_usage_baseaddr(ts, chunk->usage_num);
- ret = regmap_raw_write(ts->regmap, reg, chunk->usage_content, chunk->usage_length);
- if (ret)
- return ret;
+ error = axiom_u02_wait_idle(ts);
+ if (error)
+ return error;
-out:
- return axiom_u02_wait_idle(ts);
+ return 0;
}
static int axiom_verify_volatile_mem(struct axiom_data *ts)
{
- int ret;
+ int error;
- ret = axiom_u02_computecrc(ts);
- if (ret)
- return ret;
+ error = axiom_u02_computecrc(ts);
+ if (error)
+ return error;
/* Query the new CRCs after they are re-computed */
- ret = axiom_u33_read(ts, &ts->crc[AXIOM_CRC_CUR]);
- if (ret)
- return ret;
+ error = axiom_u33_read(ts, &ts->crc[AXIOM_CRC_CUR]);
+ if (error)
+ return error;
return ts->crc[AXIOM_CRC_CUR].vltusageconfig ==
ts->crc[AXIOM_CRC_NEW].vltusageconfig ? 0 : -EINVAL;
@@ -2310,27 +2310,24 @@ static int axiom_verify_crcs(struct axiom_data *ts)
return 0;
}
-static enum fw_upload_err
-axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
- u32 size, u32 *written)
+static enum fw_upload_err axiom_cfg_fw_write(struct fw_upload *fw_upload,
+ const u8 *data, u32 offset,
+ u32 size, u32 *written)
{
struct axiom_data *ts = fw_upload->dd_handle;
struct axiom_firmware *afw = &ts->fw[AXIOM_FW_CFG];
struct device *dev = ts->dev;
- bool cancel;
- int ret;
+ int error;
/* Done before cancel check due to cleanup based put */
- ret = pm_runtime_resume_and_get(ts->dev);
- if (ret)
+ error = pm_runtime_resume_and_get(ts->dev);
+ if (error)
return FW_UPLOAD_ERR_HW_ERROR;
- mutex_lock(&afw->lock);
- cancel = afw->cancel;
- mutex_unlock(&afw->lock);
-
- if (cancel)
- return FW_UPLOAD_ERR_CANCELED;
+ scoped_guard(mutex, &afw->lock) {
+ if (afw->cancel)
+ return FW_UPLOAD_ERR_CANCELED;
+ }
axiom_lock_input_device(ts);
@@ -2340,12 +2337,12 @@ axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
return FW_UPLOAD_ERR_HW_ERROR;
}
- ret = axiom_u02_stop(ts);
- if (ret)
+ error = axiom_u02_stop(ts);
+ if (error)
goto err_swreset;
- ret = axiom_zero_volatile_mem(ts);
- if (ret)
+ error = axiom_zero_volatile_mem(ts);
+ if (error)
goto err_swreset;
/* Skip to the first fw chunk */
@@ -2362,16 +2359,14 @@ axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
chunk_len = axiom_cfg_fw_prepare_chunk(&chunk, data);
if (axiom_skip_cfg_chunk(ts, &chunk)) {
dev_dbg(dev, "Skip TH2CFG usage u%02X\n", chunk.usage_num);
- goto next_chunk;
- }
-
- ret = axiom_write_cfg_chunk(ts, &chunk);
- if (ret) {
- axiom_set_runmode(ts, AXIOM_TCP_MODE);
- goto err_swreset;
+ } else {
+ error = axiom_write_cfg_chunk(ts, &chunk);
+ if (error) {
+ axiom_set_runmode(ts, AXIOM_TCP_MODE);
+ goto err_swreset;
+ }
}
-next_chunk:
data += chunk_len;
size -= chunk_len;
*written += chunk_len;
@@ -2380,28 +2375,28 @@ axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
axiom_set_runmode(ts, AXIOM_TCP_MODE);
/* Ensure that the chunks are written correctly */
- ret = axiom_verify_volatile_mem(ts);
- if (ret) {
+ error = axiom_verify_volatile_mem(ts);
+ if (error) {
dev_err(dev, "Failed to verify written config, abort\n");
goto err_swreset;
}
- ret = axiom_u02_save_config(ts);
- if (ret)
+ error = axiom_u02_save_config(ts);
+ if (error)
goto err_swreset;
/*
* TODO: Check if u02 start would be sufficient to load the new config
* values
*/
- ret = axiom_u02_swreset(ts);
- if (ret) {
+ error = axiom_u02_swreset(ts);
+ if (error) {
dev_err(dev, "Soft reset failed\n");
goto err_unlock;
}
- ret = axiom_u33_read(ts, &ts->crc[AXIOM_CRC_CUR]);
- if (ret)
+ error = axiom_u33_read(ts, &ts->crc[AXIOM_CRC_CUR]);
+ if (error)
goto err_unlock;
if (axiom_verify_crcs(ts))
@@ -2410,8 +2405,8 @@ axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
/* Unlock before the input device gets unregistered */
axiom_unlock_input_device(ts);
- ret = axiom_update_input_dev(ts);
- if (ret) {
+ error = axiom_update_input_dev(ts);
+ if (error) {
dev_err(dev, "Input device update failed after TH2CFG firmware update\n");
goto err_out;
}
@@ -2425,7 +2420,7 @@ axiom_cfg_fw_write(struct fw_upload *fw_upload, const u8 *data, u32 offset,
err_unlock:
axiom_unlock_input_device(ts);
err_out:
- return ret == -ETIMEDOUT ? FW_UPLOAD_ERR_TIMEOUT : FW_UPLOAD_ERR_HW_ERROR;
+ return error == -ETIMEDOUT ? FW_UPLOAD_ERR_TIMEOUT : FW_UPLOAD_ERR_HW_ERROR;
}
static void axiom_cfg_fw_cancel(struct fw_upload *fw_upload)
@@ -2433,9 +2428,8 @@ static void axiom_cfg_fw_cancel(struct fw_upload *fw_upload)
struct axiom_data *ts = fw_upload->dd_handle;
struct axiom_firmware *afw = &ts->fw[AXIOM_FW_CFG];
- mutex_lock(&afw->lock);
+ guard(mutex)(&afw->lock);
afw->cancel = true;
- mutex_unlock(&afw->lock);
}
static void axiom_cfg_fw_cleanup(struct fw_upload *fw_upload)
@@ -2474,7 +2468,7 @@ static int axiom_register_fwl(struct axiom_data *ts)
struct device *dev = ts->dev;
struct fw_upload *fwl;
char *fw_name;
- int ret;
+ int error;
if (!IS_ENABLED(CONFIG_FW_UPLOAD)) {
dev_dbg(dev, "axfw and th2cfgbin update disabled\n");
@@ -2490,9 +2484,9 @@ static int axiom_register_fwl(struct axiom_data *ts)
return dev_err_probe(dev, PTR_ERR(fwl),
"Failed to register firmware upload\n");
- ret = devm_add_action_or_reset(dev, axiom_remove_axfw_fwl_action, ts);
- if (ret)
- return ret;
+ error = devm_add_action_or_reset(dev, axiom_remove_axfw_fwl_action, ts);
+ if (error)
+ return error;
ts->fw[AXIOM_FW_AXFW].fwl = fwl;
@@ -2505,9 +2499,9 @@ static int axiom_register_fwl(struct axiom_data *ts)
return dev_err_probe(dev, PTR_ERR(fwl),
"Failed to register cfg firmware upload\n");
- ret = devm_add_action_or_reset(dev, axiom_remove_cfg_fwl_action, ts);
- if (ret)
- return ret;
+ error = devm_add_action_or_reset(dev, axiom_remove_cfg_fwl_action, ts);
+ if (error)
+ return error;
ts->fw[AXIOM_FW_CFG].fwl = fwl;
@@ -2643,7 +2637,7 @@ static int axiom_register_input_dev(struct axiom_data *ts,
struct device *dev = ts->dev;
struct i2c_client *client = to_i2c_client(dev);
struct input_dev *input;
- int ret;
+ int error;
input = input_allocate_device();
if (!input) {
@@ -2673,7 +2667,7 @@ static int axiom_register_input_dev(struct axiom_data *ts,
touchscreen_parse_properties(input, true, &ts->prop);
- axiom_u42_get_touchslots(ts);
+ axiom_get_touchslots(ts);
if (!ts->num_slots && update_in_process) {
input_free_device(input);
/*
@@ -2690,11 +2684,11 @@ static int axiom_register_input_dev(struct axiom_data *ts,
return -EINVAL;
}
- ret = input_mt_init_slots(input, ts->num_slots, INPUT_MT_DIRECT);
- if (ret) {
+ error = input_mt_init_slots(input, ts->num_slots, INPUT_MT_DIRECT);
+ if (error) {
input_free_device(input);
dev_err(dev, "Failed to init mt slots\n");
- return ret;
+ return error;
}
/*
@@ -2705,19 +2699,21 @@ static int axiom_register_input_dev(struct axiom_data *ts,
* always.
*/
if (!ts->irq_setup_done && client->irq) {
- ret = devm_request_threaded_irq(dev, client->irq, NULL, axiom_irq,
- IRQF_ONESHOT, dev_name(dev), ts);
- if (ret) {
+ error = devm_request_threaded_irq(dev, client->irq,
+ NULL, axiom_irq,
+ IRQF_ONESHOT,
+ dev_name(dev), ts);
+ if (error) {
dev_err(dev, "Failed to request IRQ\n");
- return ret;
+ return error;
}
ts->irq_setup_done = true;
} else {
- ret = input_setup_polling(input, axiom_poll);
- if (ret) {
+ error = input_setup_polling(input, axiom_poll);
+ if (error) {
input_free_device(input);
dev_err(dev, "Setup polling mode failed\n");
- return ret;
+ return error;
}
input_set_poll_interval(input, ts->poll_interval);
@@ -2726,14 +2722,14 @@ static int axiom_register_input_dev(struct axiom_data *ts,
input_set_drvdata(input, ts);
ts->input = input;
- ret = input_register_device(input);
- if (ret) {
+ error = input_register_device(input);
+ if (error) {
input_free_device(input);
ts->input = NULL;
dev_err(dev, "Failed to register input device\n");
};
- return ret;
+ return error;
}
static int axiom_update_input_dev(struct axiom_data *ts)
@@ -2747,15 +2743,15 @@ static int axiom_parse_firmware(struct axiom_data *ts)
{
struct device *dev = ts->dev;
struct gpio_desc *gpio;
- int ret;
+ int error;
ts->supplies[0].supply = "vddi";
ts->supplies[1].supply = "vdda";
ts->num_supplies = ARRAY_SIZE(ts->supplies);
- ret = devm_regulator_bulk_get(dev, ts->num_supplies, ts->supplies);
- if (ret)
- return dev_err_probe(dev, ret,
+ error = devm_regulator_bulk_get(dev, ts->num_supplies, ts->supplies);
+ if (error)
+ return dev_err_probe(dev, error,
"Failed to get power supplies\n");
gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
@@ -2770,20 +2766,15 @@ static int axiom_parse_firmware(struct axiom_data *ts)
return 0;
}
-static int axiom_power_device(struct axiom_data *ts, unsigned int enable)
+static int axiom_power_up_device(struct axiom_data *ts)
{
struct device *dev = ts->dev;
- int ret;
-
- if (!enable) {
- regulator_bulk_disable(ts->num_supplies, ts->supplies);
- return 0;
- }
+ int error;
- ret = regulator_bulk_enable(ts->num_supplies, ts->supplies);
- if (ret) {
+ error = regulator_bulk_enable(ts->num_supplies, ts->supplies);
+ if (error) {
dev_err(dev, "Failed to enable power supplies\n");
- return ret;
+ return error;
}
gpiod_set_value_cansleep(ts->reset_gpio, 1);
@@ -2795,6 +2786,11 @@ static int axiom_power_device(struct axiom_data *ts, unsigned int enable)
return 0;
}
+static void axiom_power_down_device(struct axiom_data *ts)
+{
+ regulator_bulk_disable(ts->num_supplies, ts->supplies);
+}
+
static int axiom_panel_prepared(struct drm_panel_follower *follower)
{
struct axiom_data *ts = container_of(follower, struct axiom_data,
@@ -2838,7 +2834,7 @@ static int axiom_i2c_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
struct axiom_data *ts;
- int ret;
+ int error;
ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
if (!ts)
@@ -2857,53 +2853,56 @@ static int axiom_i2c_probe(struct i2c_client *client)
init_completion(&ts->nvm_write.completion);
mutex_init(&ts->fwupdate_lock);
- ret = axiom_register_fwl(ts);
- if (ret)
- return ret;
+ error = axiom_register_fwl(ts);
+ if (error)
+ return error;
- ret = axiom_parse_firmware(ts);
- if (ret)
- return ret;
+ error = axiom_parse_firmware(ts);
+ if (error)
+ return error;
- ret = axiom_power_device(ts, 1);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to power-on device\n");
+ error = axiom_power_up_device(ts);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to power-on device\n");
pm_runtime_set_autosuspend_delay(dev, 10 * MSEC_PER_SEC);
pm_runtime_use_autosuspend(dev);
pm_runtime_set_active(dev);
pm_runtime_get_noresume(dev);
- ret = devm_pm_runtime_enable(dev);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to enable pm-runtime\n");
+ error = devm_pm_runtime_enable(dev);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to enable pm-runtime\n");
- ret = axiom_register_panel_follower(ts);
- if (ret)
- return dev_err_probe(dev, ret, "Failed to register panel follower\n");
+ error = axiom_register_panel_follower(ts);
+ if (error)
+ return dev_err_probe(dev, error, "Failed to register panel follower\n");
- ret = axiom_u31_device_discover(ts);
+ error = axiom_u31_device_discover(ts);
/*
* Register the device to allow FW updates in case that the current FW
* doesn't support the required driver usages or if the device is in
* bootloader mode.
*/
- if (ret && ret == -EACCES && IS_ENABLED(CONFIG_FW_UPLOAD)) {
- dev_warn(dev, "Device discovery failed, wait for user fw update\n");
- pm_runtime_mark_last_busy(dev);
- pm_runtime_put_sync_autosuspend(dev);
- return 0;
- } else if (ret) {
+ if (error) {
+ if (error == -EACCES && IS_ENABLED(CONFIG_FW_UPLOAD)) {
+ dev_warn(dev, "Device discovery failed, wait for user fw update\n");
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_sync_autosuspend(dev);
+ return 0;
+ }
pm_runtime_put_sync(dev);
- return dev_err_probe(dev, ret, "Device discovery failed\n");
+ return dev_err_probe(dev, error, "Device discovery failed\n");
}
- ret = axiom_register_input_dev(ts, false);
+ error = axiom_register_input_dev(ts, false);
pm_runtime_mark_last_busy(dev);
pm_runtime_put_sync_autosuspend(dev);
- if (ret && IS_ENABLED(CONFIG_FW_UPLOAD))
+ if (error) {
+ if (!IS_ENABLED(CONFIG_FW_UPLOAD))
+ return dev_err_probe(dev, error, "Failed to register input device\n");
+
dev_warn(dev, "Failed to register the input device, wait for user fw update\n");
- else if (ret)
- return dev_err_probe(dev, ret, "Failed to register input device\n");
+ }
return 0;
}
@@ -2923,18 +2922,20 @@ static int axiom_runtime_suspend(struct device *dev)
if (client->irq && ts->irq_setup_done)
disable_irq(client->irq);
- return axiom_power_device(ts, 0);
+ axiom_power_down_device(ts);
+
+ return 0;
}
static int axiom_runtime_resume(struct device *dev)
{
struct axiom_data *ts = dev_get_drvdata(dev);
struct i2c_client *client = to_i2c_client(dev);
- int ret;
+ int error;
- ret = axiom_power_device(ts, 1);
- if (ret)
- return ret;
+ error = axiom_power_up_device(ts);
+ if (error)
+ return error;
if (client->irq && ts->irq_setup_done)
enable_irq(client->irq);
^ permalink raw reply related
* Re: [PATCH v11 02/11] HID: asus: initialize additional endpoints only for legacy devices
From: Ilpo Järvinen @ 2026-01-20 13:56 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Denis Benato, platform-driver-x86, linux-input, LKML, Jiri Kosina,
Benjamin Tissoires, Corentin Chary, Luke D . Jones, Hans de Goede
In-Reply-To: <CAGwozwF--y3vsqhdqYFV_x1ff7jfSkYaQHeV1OV70yue7Huk=A@mail.gmail.com>
On Sat, 17 Jan 2026, Antheas Kapenekakis wrote:
> On Sat, 17 Jan 2026 at 18:05, Denis Benato <denis.benato@linux.dev> wrote:
> > On 1/17/26 17:16, Antheas Kapenekakis wrote:
> > > On Sat, 17 Jan 2026 at 17:12, Denis Benato <denis.benato@linux.dev> wrote:
> > >> On 1/17/26 17:10, Antheas Kapenekakis wrote:
> > >>> On Sat, 17 Jan 2026 at 16:13, Denis Benato <denis.benato@linux.dev> wrote:
> > >>>> On 1/17/26 16:07, Antheas Kapenekakis wrote:
> > >>>>> On Sat, 17 Jan 2026 at 14:51, Denis Benato <denis.benato@linux.dev> wrote:
> > >>>>>> On 1/17/26 00:10, Antheas Kapenekakis wrote:
> > >>>>>>> On Fri, 16 Jan 2026 at 21:44, Denis Benato <denis.benato@linux.dev> wrote:
> > >>>>>>>> On 1/16/26 14:31, Antheas Kapenekakis wrote:
> > >>>>>>>>
> > >>>>>>>>> Currently, ID1/ID2 initializations are performed for all NKEY devices.
> > >>>>>>>>> However, ID1 initializations are only required for RGB control and are
> > >>>>>>>>> only supported for RGB capable devices. ID2 initializations are only
> > >>>>>>>>> required for initializing the Anime display endpoint which is only
> > >>>>>>>>> supported on devices with an Anime display. Both of these
> > >>>>>>>>> initializations are out of scope for this driver (this is a brightness
> > >>>>>>>>> control and keyboard shortcut driver) and they should not be performed
> > >>>>>>>>> for devices that do not support them in any case.
> > >>>>>>>>>
> > >>>>>>>>> At the same time, there are older NKEY devices that have only been
> > >>>>>>>>> tested with these initializations in the kernel and it is not possible
> > >>>>>>>>> to recheck them. There is a possibility that especially with the ID1
> > >>>>>>>>> initialization, certain laptop models might have their shortcuts stop
> > >>>>>>>>> working (currently unproven).
> > >>>>>>>>>
> > >>>>>>>>> For an abundance of caution, only initialize ID1/ID2 for those older
> > >>>>>>>>> NKEY devices by introducing a quirk for them and replacing the NKEY
> > >>>>>>>>> quirk in the block that performs the inits with that.
> > >>>>>>>>>
> > >>>>>>>>> In addition, as these initializations might not be supported by the
> > >>>>>>>>> affected devices, change the function to not bail if they fail.
> > >>>>>>>>>
> > >>>>>>>>> Acked-by: Benjamin Tissoires <bentiss@kernel.org>
> > >>>>>>>>> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> > >>>>>>>>> ---
> > >>>>>>>>> drivers/hid/hid-asus.c | 16 ++++++----------
> > >>>>>>>>> 1 file changed, 6 insertions(+), 10 deletions(-)
> > >>>>>>>>>
> > >>>>>>>>> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > >>>>>>>>> index 323e6302bac5..dc7af12cf31a 100644
> > >>>>>>>>> --- a/drivers/hid/hid-asus.c
> > >>>>>>>>> +++ b/drivers/hid/hid-asus.c
> > >>>>>>>>> @@ -90,6 +90,7 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
> > >>>>>>>>> #define QUIRK_ROG_NKEY_KEYBOARD BIT(11)
> > >>>>>>>>> #define QUIRK_ROG_CLAYMORE_II_KEYBOARD BIT(12)
> > >>>>>>>>> #define QUIRK_ROG_ALLY_XPAD BIT(13)
> > >>>>>>>>> +#define QUIRK_ROG_NKEY_LEGACY BIT(14)
> > >>>>>>>> These past days I have taken a look at new 2025 models and they do make use of ID2,
> > >>>>>>>> and won't do harm sending ID1 either. I think you can safely remove the if and send regardless.
> > >>>>>>> Hi Denis,
> > >>>>>>> it is not the responsibility of this driver. ID2 is used by Anime
> > >>>>>>> models. It is a concession to make sure that we do not cause a
> > >>>>>>> regression that will cause warnings for a lot of users.
> > >>>>>> Who decided it is a concession?
> > >>>>> I would rather remove the extra calls unless they are shown to be
> > >>>>> needed, which they might be for these PIDs.
> > >>>> They are needed on older laptop and to not regress userspace.
> > >>>>
> > >>>> You just named _LEGACY an usb pid that is not legacy.
> > >>>>> The quirk is named legacy because we can't retest these devices. If we
> > >>>>> can, then we could remove the quirk and the inits if not needed.
> > >>>> We can't retest every device, and that pid is used in pre-2021 models,
> > >>>> and these are the unknown, I am criticizing the name of the quirk here,
> > >>>> not what it does.
> > >>> If you can test whether your device needs them that would be great.
> > >> That is pointless.
> > >>>> I am also questioning if the quirk is even needed since sending
> > >>>> those commands to (at least) recent hardware that doesn't use
> > >>>> those endpoints carries no downsides, while removing them
> > >>>> surely does.
> > >>> We have not found a device yet that needs them. I do not want to keep
> > >>> sending unneeded commands. It could cause obscure bugs or interfere
> > >>> with userspace software such as the one you maintain. So at least for
> > >>> new hardware that is possible to test we should remove them.
> > >> There is new hardware that needs them, as I said, including 2025 models.
> > > I was not aware of that. As far as I know they are not needed. Do you
> > > have a bug report with a specific laptop model I can look at?
> > There is current effort to integrate commands that requires those
> > initializations on 2025 laptop, why would I strip out a command
> > that I already know is required anyway?
>
> Hi,
> yes ID1 is required for RGB, I have a draft patch for it that would
> lazily do it if RGB is supported.
>
> I recall now a previous discussion about it being required for some
> laptop shortcuts but we never found a laptop that needs it so I forgot
>
> > No, this is not the way to go to knowingly and willingly cause
> > troubles (both known and unknown) to others just because
> > you think it's better this way.
> >
> > Change the name of _LEGACY to something else, have this accepted
> > and then if I see it's appropriate to remove the if and send those
> > regardless I will.
>
> Sure, up to you if you want to change the name. What would you like it
> be? I would like this series to merge
Can you name it e.g. something that should be neutral such as:
QUIRK_ROG_NKEY_ID1_ID2_INIT
I'm not sure if ROG NKEY should still be included into the name based on
what Denis mentioned about recent models but at least it gets rid of the
"legacy" connotation. If wider scope is necessary you could use just
QUIRK_ID1_ID2_INIT.
--
i.
^ 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