* [PATCH 0/3] HID: steelseries: add SteelSeries Arctis 9 support
@ 2025-01-01 15:11 Christian Mayer
2025-01-01 15:11 ` [PATCH 1/3] " Christian Mayer
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Christian Mayer @ 2025-01-01 15:11 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Bastien Nocera
Hi,
i added support for the SteelSeries Arctis 9 headset.
HID: steelseries: add SteelSeries Arctis 9 support
HID: steelseries: export charging state for the SteelSeries Arctis 9 headset
HID: steelseries: export model and manufacturer
drivers/hid/hid-steelseries.c | 138 ++++++++++++++++++++++++++++++++++++------
1 file changed, 120 insertions(+), 18 deletions(-)
Christian
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/3] HID: steelseries: add SteelSeries Arctis 9 support 2025-01-01 15:11 [PATCH 0/3] HID: steelseries: add SteelSeries Arctis 9 support Christian Mayer @ 2025-01-01 15:11 ` Christian Mayer 2025-01-04 11:58 ` Bastien Nocera 2025-01-04 12:45 ` Christophe JAILLET 2025-01-01 15:11 ` [PATCH 2/3] HID: steelseries: export charging state for the SteelSeries Arctis 9 headset Christian Mayer 2025-01-01 15:11 ` [PATCH 3/3] HID: steelseries: export model and manufacturer Christian Mayer 2 siblings, 2 replies; 12+ messages in thread From: Christian Mayer @ 2025-01-01 15:11 UTC (permalink / raw) To: linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Bastien Nocera, Christian Mayer Add support for the SteelSeries Arctis 9 headset. This driver will export the battery information like it already does for the Arcits 1 headset. Signed-off-by: Christian Mayer <git@mayer-bgk.de> --- drivers/hid/hid-steelseries.c | 105 +++++++++++++++++++++++++++++----- 1 file changed, 90 insertions(+), 15 deletions(-) diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c index f9ff5be94309..f102045a9b6b 100644 --- a/drivers/hid/hid-steelseries.c +++ b/drivers/hid/hid-steelseries.c @@ -18,7 +18,8 @@ #include "hid-ids.h" #define STEELSERIES_SRWS1 BIT(0) -#define STEELSERIES_ARCTIS_1 BIT(1) +#define STEELSERIES_ARCTIS_1 BIT(1) +#define STEELSERIES_ARCTIS_9 BIT(2) struct steelseries_device { struct hid_device *hdev; @@ -35,7 +36,7 @@ struct steelseries_device { }; #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) #define SRWS1_NUMBER_LEDS 15 struct steelseries_srws1_data { __u16 led_state; @@ -95,7 +96,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { 0x29, 0x11, /* Usage Maximum (11h), */ 0x95, 0x11, /* Report Count (17), */ 0x81, 0x02, /* Input (Variable), */ - /* ---- Dial patch starts here ---- */ + /* ---- Dial patch starts here ---- */ 0x05, 0x01, /* Usage Page (Desktop), */ 0x09, 0x33, /* Usage (RX), */ 0x75, 0x04, /* Report Size (4), */ @@ -108,7 +109,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { 0x95, 0x01, /* Report Count (1), */ 0x25, 0x03, /* Logical Maximum (3), */ 0x81, 0x02, /* Input (Variable), */ - /* ---- Dial patch ends here ---- */ + /* ---- Dial patch ends here ---- */ 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */ 0x09, 0x01, /* Usage (01h), */ 0x75, 0x04, /* Changed Report Size (4), */ @@ -125,7 +126,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { }; #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) static void steelseries_srws1_set_leds(struct hid_device *hdev, __u16 leds) { struct list_head *report_list = &hdev->report_enum[HID_OUTPUT_REPORT].report_list; @@ -368,32 +369,36 @@ static void steelseries_srws1_remove(struct hid_device *hdev) hid_hw_stop(hdev); kfree(drv_data); - return; } #endif #define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000 #define ARCTIS_1_BATTERY_RESPONSE_LEN 8 +#define ARCTIS_9_BATTERY_RESPONSE_LEN 64 static const char arctis_1_battery_request[] = { 0x06, 0x12 }; +static const char arctis_9_battery_request[] = { 0x00, 0x20 }; -static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev) +static int steelseries_headset_request_battery(struct hid_device *hdev, + const char *request, size_t len) { u8 *write_buf; int ret; /* Request battery information */ - write_buf = kmemdup(arctis_1_battery_request, sizeof(arctis_1_battery_request), GFP_KERNEL); + write_buf = kmemdup(request, len, GFP_KERNEL); if (!write_buf) return -ENOMEM; - ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0], - write_buf, sizeof(arctis_1_battery_request), + hid_dbg(hdev, "Sending battery request report"); + ret = hid_hw_raw_request(hdev, request[0], + write_buf, len, HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); - if (ret < (int)sizeof(arctis_1_battery_request)) { + if (ret < (int)len) { hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); ret = -ENODATA; } + kfree(write_buf); return ret; } @@ -404,7 +409,11 @@ static void steelseries_headset_fetch_battery(struct hid_device *hdev) int ret = 0; if (sd->quirks & STEELSERIES_ARCTIS_1) - ret = steelseries_headset_arctis_1_fetch_battery(hdev); + ret = steelseries_headset_request_battery(hdev, + arctis_1_battery_request, sizeof(arctis_1_battery_request)); + else if (sd->quirks & STEELSERIES_ARCTIS_9) + ret = steelseries_headset_request_battery(hdev, + arctis_9_battery_request, sizeof(arctis_9_battery_request)); if (ret < 0) hid_dbg(hdev, @@ -520,9 +529,22 @@ static int steelseries_headset_battery_register(struct steelseries_device *sd) INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick); steelseries_headset_fetch_battery(sd->hdev); + if (sd->quirks & STEELSERIES_ARCTIS_9) { + // The first fetch_battery request can remain unanswered in some cases + schedule_delayed_work(&sd->battery_work, + msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS)); + } + return 0; } +static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t usagePage) +{ + return hdev->rdesc[0] == 0x06 && + hdev->rdesc[1] == usagePage && + hdev->rdesc[2] == 0xff; +} + static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id) { struct steelseries_device *sd; @@ -537,7 +559,7 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id if (sd->quirks & STEELSERIES_SRWS1) { #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) return steelseries_srws1_probe(hdev, id); #else return -ENODEV; @@ -548,12 +570,22 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id if (ret) return ret; + if (sd->quirks & STEELSERIES_ARCTIS_9 && + !steelseries_is_vendor_usage_page(hdev, 0xc0)) + return -ENODEV; + spin_lock_init(&sd->lock); ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT); if (ret) return ret; + if (sd->quirks & STEELSERIES_ARCTIS_9) { + ret = hid_hw_open(hdev); + if (ret) + return ret; + } + if (steelseries_headset_battery_register(sd) < 0) hid_err(sd->hdev, "Failed to register battery for headset\n"); @@ -568,7 +600,7 @@ static void steelseries_remove(struct hid_device *hdev) if (sd->quirks & STEELSERIES_SRWS1) { #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) steelseries_srws1_remove(hdev); #endif return; @@ -580,6 +612,9 @@ static void steelseries_remove(struct hid_device *hdev) cancel_delayed_work_sync(&sd->battery_work); + if (sd->quirks & STEELSERIES_ARCTIS_9) + hid_hw_close(hdev); + hid_hw_stop(hdev); } @@ -599,6 +634,15 @@ static const __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, return rdesc; } +static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in, uint8_t max_in) +{ + if (capacity > max_in) + return 100; + if (capacity < min_in) + return 0; + return (capacity - min_in) * 100 / (max_in - min_in); +} + static int steelseries_headset_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *read_buf, int size) @@ -630,6 +674,32 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, } } + if (sd->quirks & STEELSERIES_ARCTIS_9) { + hid_dbg(sd->hdev, + "Parsing raw event for Arctis 9 headset (%*ph)\n", size, read_buf); + if (size < ARCTIS_9_BATTERY_RESPONSE_LEN) { + if (!delayed_work_pending(&sd->battery_work)) + goto request_battery; + return 0; + } + + if (read_buf[0] == 0xaa && read_buf[1] == 0x01) { + connected = true; + + /* + * Found no official documentation about min and max. + * Values defined by testing. + */ + capacity = steelseries_headset_map_capacity(read_buf[3], 0x68, 0x9d); + } else { + /* + * Device is off and sends the last known status read_buf[1] == 0x03 or + * there is no known status of the device read_buf[0] == 0x55 + */ + connected = false; + } + } + if (connected != sd->headset_connected) { hid_dbg(sd->hdev, "Connected status changed from %sconnected to %sconnected\n", @@ -663,7 +733,11 @@ static const struct hid_device_id steelseries_devices[] = { { /* SteelSeries Arctis 1 Wireless for XBox */ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6), - .driver_data = STEELSERIES_ARCTIS_1 }, + .driver_data = STEELSERIES_ARCTIS_1 }, + + { /* SteelSeries Arctis 9 Wireless for XBox */ + HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12c2), + .driver_data = STEELSERIES_ARCTIS_9 }, { } }; @@ -683,3 +757,4 @@ MODULE_DESCRIPTION("HID driver for Steelseries devices"); MODULE_LICENSE("GPL"); MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>"); MODULE_AUTHOR("Simon Wood <simon@mungewell.org>"); +MODULE_AUTHOR("Christian Mayer <git@mayer-bgk.de>"); -- 2.47.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] HID: steelseries: add SteelSeries Arctis 9 support 2025-01-01 15:11 ` [PATCH 1/3] " Christian Mayer @ 2025-01-04 11:58 ` Bastien Nocera 2025-01-05 10:16 ` Christian Mayer 2025-01-04 12:45 ` Christophe JAILLET 1 sibling, 1 reply; 12+ messages in thread From: Bastien Nocera @ 2025-01-04 11:58 UTC (permalink / raw) To: Christian Mayer, linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires On Wed, 2025-01-01 at 15:11 +0000, Christian Mayer wrote: > Add support for the SteelSeries Arctis 9 headset. This driver > will export the battery information like it already does for > the Arcits 1 headset. > > Signed-off-by: Christian Mayer <git@mayer-bgk.de> Please remove all the whitespace changes, and split off the steelseries_headset_request_battery() refactoring into its own commit. Detailed review comments below. > --- > drivers/hid/hid-steelseries.c | 105 +++++++++++++++++++++++++++++----- > 1 file changed, 90 insertions(+), 15 deletions(-) > > diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c > index f9ff5be94309..f102045a9b6b 100644 > --- a/drivers/hid/hid-steelseries.c > +++ b/drivers/hid/hid-steelseries.c > @@ -18,7 +18,8 @@ > #include "hid-ids.h" > > #define STEELSERIES_SRWS1 BIT(0) > -#define STEELSERIES_ARCTIS_1 BIT(1) > +#define STEELSERIES_ARCTIS_1 BIT(1) Whitespace change > +#define STEELSERIES_ARCTIS_9 BIT(2) > > struct steelseries_device { > struct hid_device *hdev; > @@ -35,7 +36,7 @@ struct steelseries_device { > }; > > #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ > - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) > + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) Whitespace change > #define SRWS1_NUMBER_LEDS 15 > struct steelseries_srws1_data { > __u16 led_state; > @@ -95,7 +96,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { > 0x29, 0x11, /* Usage Maximum (11h), */ > 0x95, 0x11, /* Report Count (17), */ > 0x81, 0x02, /* Input (Variable), */ > - /* ---- Dial patch starts here ---- */ > + /* ---- Dial patch starts here ---- */ Whitespace change > 0x05, 0x01, /* Usage Page (Desktop), */ > 0x09, 0x33, /* Usage (RX), */ > 0x75, 0x04, /* Report Size (4), */ > @@ -108,7 +109,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { > 0x95, 0x01, /* Report Count (1), */ > 0x25, 0x03, /* Logical Maximum (3), */ > 0x81, 0x02, /* Input (Variable), */ > - /* ---- Dial patch ends here ---- */ > + /* ---- Dial patch ends here ---- */ Whitespace change > 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */ > 0x09, 0x01, /* Usage (01h), */ > 0x75, 0x04, /* Changed Report Size (4), */ > @@ -125,7 +126,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { > }; > > #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ > - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) > + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) Whitespace change > static void steelseries_srws1_set_leds(struct hid_device *hdev, __u16 leds) > { > struct list_head *report_list = &hdev->report_enum[HID_OUTPUT_REPORT].report_list; > @@ -368,32 +369,36 @@ static void steelseries_srws1_remove(struct hid_device *hdev) > > hid_hw_stop(hdev); > kfree(drv_data); > - return; Should be a separate commit. > } > #endif > > #define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000 > > #define ARCTIS_1_BATTERY_RESPONSE_LEN 8 > +#define ARCTIS_9_BATTERY_RESPONSE_LEN 64 > static const char arctis_1_battery_request[] = { 0x06, 0x12 }; > +static const char arctis_9_battery_request[] = { 0x00, 0x20 }; > > -static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev) > +static int steelseries_headset_request_battery(struct hid_device *hdev, > + const char *request, size_t len) This change should be a two-step process (so two separate commits), first factor out the battery request function so it take the request and len as you've done, then add support for that new device. > { > u8 *write_buf; > int ret; > > /* Request battery information */ > - write_buf = kmemdup(arctis_1_battery_request, sizeof(arctis_1_battery_request), GFP_KERNEL); > + write_buf = kmemdup(request, len, GFP_KERNEL); > if (!write_buf) > return -ENOMEM; > > - ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0], > - write_buf, sizeof(arctis_1_battery_request), > + hid_dbg(hdev, "Sending battery request report"); > + ret = hid_hw_raw_request(hdev, request[0], > + write_buf, len, > HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); > - if (ret < (int)sizeof(arctis_1_battery_request)) { > + if (ret < (int)len) { > hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); > ret = -ENODATA; > } > + > kfree(write_buf); > return ret; > } > @@ -404,7 +409,11 @@ static void steelseries_headset_fetch_battery(struct hid_device *hdev) > int ret = 0; > > if (sd->quirks & STEELSERIES_ARCTIS_1) > - ret = steelseries_headset_arctis_1_fetch_battery(hdev); > + ret = steelseries_headset_request_battery(hdev, > + arctis_1_battery_request, sizeof(arctis_1_battery_request)); > + else if (sd->quirks & STEELSERIES_ARCTIS_9) > + ret = steelseries_headset_request_battery(hdev, > + arctis_9_battery_request, sizeof(arctis_9_battery_request)); > > if (ret < 0) > hid_dbg(hdev, > @@ -520,9 +529,22 @@ static int steelseries_headset_battery_register(struct steelseries_device *sd) > INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick); > steelseries_headset_fetch_battery(sd->hdev); > > + if (sd->quirks & STEELSERIES_ARCTIS_9) { > + // The first fetch_battery request can remain unanswered in some cases No C++-style comments. > + schedule_delayed_work(&sd->battery_work, > + msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS)); > + } > + > return 0; > } > > +static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t usagePage) > +{ > + return hdev->rdesc[0] == 0x06 && > + hdev->rdesc[1] == usagePage && > + hdev->rdesc[2] == 0xff; > +} Should probably be usage_page. > + > static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id) > { > struct steelseries_device *sd; > @@ -537,7 +559,7 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id > > if (sd->quirks & STEELSERIES_SRWS1) { > #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ > - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) > + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) > return steelseries_srws1_probe(hdev, id); > #else > return -ENODEV; > @@ -548,12 +570,22 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id > if (ret) > return ret; > > + if (sd->quirks & STEELSERIES_ARCTIS_9 && > + !steelseries_is_vendor_usage_page(hdev, 0xc0)) > + return -ENODEV; > + > spin_lock_init(&sd->lock); > > ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT); > if (ret) > return ret; > > + if (sd->quirks & STEELSERIES_ARCTIS_9) { > + ret = hid_hw_open(hdev); Is this needed? If so, this probably needs to be added as a separate commit, for all headsets rather than just this one. > + if (ret) > + return ret; > + } > + > if (steelseries_headset_battery_register(sd) < 0) > hid_err(sd->hdev, > "Failed to register battery for headset\n"); > @@ -568,7 +600,7 @@ static void steelseries_remove(struct hid_device *hdev) > > if (sd->quirks & STEELSERIES_SRWS1) { > #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ > - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) > + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) > steelseries_srws1_remove(hdev); > #endif > return; > @@ -580,6 +612,9 @@ static void steelseries_remove(struct hid_device *hdev) > > cancel_delayed_work_sync(&sd->battery_work); > > + if (sd->quirks & STEELSERIES_ARCTIS_9) > + hid_hw_close(hdev); > + > hid_hw_stop(hdev); > } > > @@ -599,6 +634,15 @@ static const __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, > return rdesc; > } > > +static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in, uint8_t max_in) > +{ > + if (capacity > max_in) > + return 100; > + if (capacity < min_in) > + return 0; > + return (capacity - min_in) * 100 / (max_in - min_in); > +} > + > static int steelseries_headset_raw_event(struct hid_device *hdev, > struct hid_report *report, u8 *read_buf, > int size) > @@ -630,6 +674,32 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, > } > } > > + if (sd->quirks & STEELSERIES_ARCTIS_9) { > + hid_dbg(sd->hdev, > + "Parsing raw event for Arctis 9 headset (%*ph)\n", size, read_buf); > + if (size < ARCTIS_9_BATTERY_RESPONSE_LEN) { > + if (!delayed_work_pending(&sd->battery_work)) > + goto request_battery; > + return 0; > + } > + > + if (read_buf[0] == 0xaa && read_buf[1] == 0x01) { > + connected = true; > + > + /* > + * Found no official documentation about min and max. > + * Values defined by testing. > + */ > + capacity = steelseries_headset_map_capacity(read_buf[3], 0x68, 0x9d); > + } else { > + /* > + * Device is off and sends the last known status read_buf[1] == 0x03 or > + * there is no known status of the device read_buf[0] == 0x55 > + */ > + connected = false; > + } > + } > + > if (connected != sd->headset_connected) { > hid_dbg(sd->hdev, > "Connected status changed from %sconnected to %sconnected\n", > @@ -663,7 +733,11 @@ static const struct hid_device_id steelseries_devices[] = { > > { /* SteelSeries Arctis 1 Wireless for XBox */ > HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6), > - .driver_data = STEELSERIES_ARCTIS_1 }, > + .driver_data = STEELSERIES_ARCTIS_1 }, Whitespace change. > + > + { /* SteelSeries Arctis 9 Wireless for XBox */ > + HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12c2), > + .driver_data = STEELSERIES_ARCTIS_9 }, > > { } > }; > @@ -683,3 +757,4 @@ MODULE_DESCRIPTION("HID driver for Steelseries devices"); > MODULE_LICENSE("GPL"); > MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>"); > MODULE_AUTHOR("Simon Wood <simon@mungewell.org>"); > +MODULE_AUTHOR("Christian Mayer <git@mayer-bgk.de>"); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] HID: steelseries: add SteelSeries Arctis 9 support 2025-01-04 11:58 ` Bastien Nocera @ 2025-01-05 10:16 ` Christian Mayer 2025-01-09 13:35 ` Bastien Nocera 0 siblings, 1 reply; 12+ messages in thread From: Christian Mayer @ 2025-01-05 10:16 UTC (permalink / raw) To: Bastien Nocera, Christian Mayer, linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires > On Wed, 2025-01-01 at 15:11 +0000, Christian Mayer wrote: >> Add support for the SteelSeries Arctis 9 headset. This driver >> will export the battery information like it already does for >> the Arcits 1 headset. >> >> Signed-off-by: Christian Mayer <git@mayer-bgk.de> > > > Please remove all the whitespace changes, and split off the > steelseries_headset_request_battery() refactoring into its own commit. > > Detailed review comments below. Thank you for your feedback Bastien. I will repost my patches with the fixes within the next days. Please see one follow-up question about hid_hw_open below. > >> --- >> drivers/hid/hid-steelseries.c | 105 +++++++++++++++++++++++++++++----- >> 1 file changed, 90 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c >> index f9ff5be94309..f102045a9b6b 100644 >> --- a/drivers/hid/hid-steelseries.c >> +++ b/drivers/hid/hid-steelseries.c >> @@ -18,7 +18,8 @@ >> #include "hid-ids.h" >> >> #define STEELSERIES_SRWS1 BIT(0) >> -#define STEELSERIES_ARCTIS_1 BIT(1) >> +#define STEELSERIES_ARCTIS_1 BIT(1) > > Whitespace change > >> +#define STEELSERIES_ARCTIS_9 BIT(2) >> >> struct steelseries_device { >> struct hid_device *hdev; >> @@ -35,7 +36,7 @@ struct steelseries_device { >> }; >> >> #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ >> - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) >> + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) > > Whitespace change > >> #define SRWS1_NUMBER_LEDS 15 >> struct steelseries_srws1_data { >> __u16 led_state; >> @@ -95,7 +96,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { >> 0x29, 0x11, /* Usage Maximum (11h), */ >> 0x95, 0x11, /* Report Count (17), */ >> 0x81, 0x02, /* Input (Variable), */ >> - /* ---- Dial patch starts here ---- */ >> + /* ---- Dial patch starts here ---- */ > > Whitespace change > >> 0x05, 0x01, /* Usage Page (Desktop), */ >> 0x09, 0x33, /* Usage (RX), */ >> 0x75, 0x04, /* Report Size (4), */ >> @@ -108,7 +109,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { >> 0x95, 0x01, /* Report Count (1), */ >> 0x25, 0x03, /* Logical Maximum (3), */ >> 0x81, 0x02, /* Input (Variable), */ >> - /* ---- Dial patch ends here ---- */ >> + /* ---- Dial patch ends here ---- */ > > Whitespace change > >> 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */ >> 0x09, 0x01, /* Usage (01h), */ >> 0x75, 0x04, /* Changed Report Size (4), */ >> @@ -125,7 +126,7 @@ static const __u8 steelseries_srws1_rdesc_fixed[] = { >> }; >> >> #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ >> - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) >> + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) > > Whitespace change > >> static void steelseries_srws1_set_leds(struct hid_device *hdev, __u16 leds) >> { >> struct list_head *report_list = &hdev->report_enum[HID_OUTPUT_REPORT].report_list; >> @@ -368,32 +369,36 @@ static void steelseries_srws1_remove(struct hid_device *hdev) >> >> hid_hw_stop(hdev); >> kfree(drv_data); >> - return; > > Should be a separate commit. > >> } >> #endif >> >> #define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000 >> >> #define ARCTIS_1_BATTERY_RESPONSE_LEN 8 >> +#define ARCTIS_9_BATTERY_RESPONSE_LEN 64 >> static const char arctis_1_battery_request[] = { 0x06, 0x12 }; >> +static const char arctis_9_battery_request[] = { 0x00, 0x20 }; >> >> -static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev) >> +static int steelseries_headset_request_battery(struct hid_device *hdev, >> + const char *request, size_t len) > > This change should be a two-step process (so two separate commits), > first factor out the battery request function so it take the request > and len as you've done, then add support for that new device. > >> { >> u8 *write_buf; >> int ret; >> >> /* Request battery information */ >> - write_buf = kmemdup(arctis_1_battery_request, sizeof(arctis_1_battery_request), GFP_KERNEL); >> + write_buf = kmemdup(request, len, GFP_KERNEL); >> if (!write_buf) >> return -ENOMEM; >> >> - ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0], >> - write_buf, sizeof(arctis_1_battery_request), >> + hid_dbg(hdev, "Sending battery request report"); >> + ret = hid_hw_raw_request(hdev, request[0], >> + write_buf, len, >> HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); >> - if (ret < (int)sizeof(arctis_1_battery_request)) { >> + if (ret < (int)len) { >> hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); >> ret = -ENODATA; >> } >> + >> kfree(write_buf); >> return ret; >> } >> @@ -404,7 +409,11 @@ static void steelseries_headset_fetch_battery(struct hid_device *hdev) >> int ret = 0; >> >> if (sd->quirks & STEELSERIES_ARCTIS_1) >> - ret = steelseries_headset_arctis_1_fetch_battery(hdev); >> + ret = steelseries_headset_request_battery(hdev, >> + arctis_1_battery_request, sizeof(arctis_1_battery_request)); >> + else if (sd->quirks & STEELSERIES_ARCTIS_9) >> + ret = steelseries_headset_request_battery(hdev, >> + arctis_9_battery_request, sizeof(arctis_9_battery_request)); >> >> if (ret < 0) >> hid_dbg(hdev, >> @@ -520,9 +529,22 @@ static int steelseries_headset_battery_register(struct steelseries_device *sd) >> INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick); >> steelseries_headset_fetch_battery(sd->hdev); >> >> + if (sd->quirks & STEELSERIES_ARCTIS_9) { >> + // The first fetch_battery request can remain unanswered in some cases > > No C++-style comments. > >> + schedule_delayed_work(&sd->battery_work, >> + msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS)); >> + } >> + >> return 0; >> } >> >> +static bool steelseries_is_vendor_usage_page(struct hid_device *hdev, uint8_t usagePage) >> +{ >> + return hdev->rdesc[0] == 0x06 && >> + hdev->rdesc[1] == usagePage && >> + hdev->rdesc[2] == 0xff; >> +} > > Should probably be usage_page. > >> + >> static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id) >> { >> struct steelseries_device *sd; >> @@ -537,7 +559,7 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id >> >> if (sd->quirks & STEELSERIES_SRWS1) { >> #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ >> - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) >> + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) >> return steelseries_srws1_probe(hdev, id); >> #else >> return -ENODEV; >> @@ -548,12 +570,22 @@ static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id >> if (ret) >> return ret; >> >> + if (sd->quirks & STEELSERIES_ARCTIS_9 && >> + !steelseries_is_vendor_usage_page(hdev, 0xc0)) >> + return -ENODEV; >> + >> spin_lock_init(&sd->lock); >> >> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT); >> if (ret) >> return ret; >> >> + if (sd->quirks & STEELSERIES_ARCTIS_9) { >> + ret = hid_hw_open(hdev); > > Is this needed? If so, this probably needs to be added as a separate > commit, for all headsets rather than just this one. Yes that's mandatory to get raw_events from the device. I thought about adding this to all headsets, but i was not sure if this breaks anything for the Arctis 1 headset. But i can add this for all headsets, that's fine with me. Would you like me to create a completely separate patch for this or should i submit this with the preparation patch which refactors steelseries_headset_fetch_battery? > >> + if (ret) >> + return ret; >> + } >> + >> if (steelseries_headset_battery_register(sd) < 0) >> hid_err(sd->hdev, >> "Failed to register battery for headset\n"); >> @@ -568,7 +600,7 @@ static void steelseries_remove(struct hid_device *hdev) >> >> if (sd->quirks & STEELSERIES_SRWS1) { >> #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ >> - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) >> + (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES)) >> steelseries_srws1_remove(hdev); >> #endif >> return; >> @@ -580,6 +612,9 @@ static void steelseries_remove(struct hid_device *hdev) >> >> cancel_delayed_work_sync(&sd->battery_work); >> >> + if (sd->quirks & STEELSERIES_ARCTIS_9) >> + hid_hw_close(hdev); >> + >> hid_hw_stop(hdev); >> } >> >> @@ -599,6 +634,15 @@ static const __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, >> return rdesc; >> } >> >> +static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in, uint8_t max_in) >> +{ >> + if (capacity > max_in) >> + return 100; >> + if (capacity < min_in) >> + return 0; >> + return (capacity - min_in) * 100 / (max_in - min_in); >> +} >> + >> static int steelseries_headset_raw_event(struct hid_device *hdev, >> struct hid_report *report, u8 *read_buf, >> int size) >> @@ -630,6 +674,32 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, >> } >> } >> >> + if (sd->quirks & STEELSERIES_ARCTIS_9) { >> + hid_dbg(sd->hdev, >> + "Parsing raw event for Arctis 9 headset (%*ph)\n", size, read_buf); >> + if (size < ARCTIS_9_BATTERY_RESPONSE_LEN) { >> + if (!delayed_work_pending(&sd->battery_work)) >> + goto request_battery; >> + return 0; >> + } >> + >> + if (read_buf[0] == 0xaa && read_buf[1] == 0x01) { >> + connected = true; >> + >> + /* >> + * Found no official documentation about min and max. >> + * Values defined by testing. >> + */ >> + capacity = steelseries_headset_map_capacity(read_buf[3], 0x68, 0x9d); >> + } else { >> + /* >> + * Device is off and sends the last known status read_buf[1] == 0x03 or >> + * there is no known status of the device read_buf[0] == 0x55 >> + */ >> + connected = false; >> + } >> + } >> + >> if (connected != sd->headset_connected) { >> hid_dbg(sd->hdev, >> "Connected status changed from %sconnected to %sconnected\n", >> @@ -663,7 +733,11 @@ static const struct hid_device_id steelseries_devices[] = { >> >> { /* SteelSeries Arctis 1 Wireless for XBox */ >> HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6), >> - .driver_data = STEELSERIES_ARCTIS_1 }, >> + .driver_data = STEELSERIES_ARCTIS_1 }, > > Whitespace change. > >> + >> + { /* SteelSeries Arctis 9 Wireless for XBox */ >> + HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12c2), >> + .driver_data = STEELSERIES_ARCTIS_9 }, >> >> { } >> }; >> @@ -683,3 +757,4 @@ MODULE_DESCRIPTION("HID driver for Steelseries devices"); >> MODULE_LICENSE("GPL"); >> MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>"); >> MODULE_AUTHOR("Simon Wood <simon@mungewell.org>"); >> +MODULE_AUTHOR("Christian Mayer <git@mayer-bgk.de>"); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] HID: steelseries: add SteelSeries Arctis 9 support 2025-01-05 10:16 ` Christian Mayer @ 2025-01-09 13:35 ` Bastien Nocera 2025-01-12 11:49 ` Christian Mayer 0 siblings, 1 reply; 12+ messages in thread From: Bastien Nocera @ 2025-01-09 13:35 UTC (permalink / raw) To: Christian Mayer, Christian Mayer, linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires On Sun, 2025-01-05 at 11:16 +0100, Christian Mayer wrote: > > > On Wed, 2025-01-01 at 15:11 +0000, Christian Mayer wrote: <snip> > > > > > + if (sd->quirks & STEELSERIES_ARCTIS_9) { > > > + ret = hid_hw_open(hdev); > > > > Is this needed? If so, this probably needs to be added as a > > separate > > commit, for all headsets rather than just this one. > Yes that's mandatory to get raw_events from the device. > I thought about adding this to all headsets, but i was not sure if > this > breaks anything for the Arctis 1 headset. > > But i can add this for all headsets, that's fine with me. > Would you like me to create a completely separate patch for this or > should i submit this with the preparation patch which refactors > steelseries_headset_fetch_battery? Please send a patch that does it for both headsets, and I'll test it locally. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] HID: steelseries: add SteelSeries Arctis 9 support 2025-01-09 13:35 ` Bastien Nocera @ 2025-01-12 11:49 ` Christian Mayer 0 siblings, 0 replies; 12+ messages in thread From: Christian Mayer @ 2025-01-12 11:49 UTC (permalink / raw) To: Bastien Nocera, Christian Mayer, linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires Am 09.01.25 um 14:35 schrieb Bastien Nocera: > On Sun, 2025-01-05 at 11:16 +0100, Christian Mayer wrote: >> >>> On Wed, 2025-01-01 at 15:11 +0000, Christian Mayer wrote: > <snip> >>> >>>> + if (sd->quirks & STEELSERIES_ARCTIS_9) { >>>> + ret = hid_hw_open(hdev); >>> >>> Is this needed? If so, this probably needs to be added as a >>> separate >>> commit, for all headsets rather than just this one. >> Yes that's mandatory to get raw_events from the device. >> I thought about adding this to all headsets, but i was not sure if >> this >> breaks anything for the Arctis 1 headset. >> >> But i can add this for all headsets, that's fine with me. >> Would you like me to create a completely separate patch for this or >> should i submit this with the preparation patch which refactors >> steelseries_headset_fetch_battery? > > Please send a patch that does it for both headsets, and I'll test it > locally. I resent the patches with the requested changes (v2) Please use v2-0001-HID-steelseries-preparation-for-adding-SteelSerie.patch for testing. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] HID: steelseries: add SteelSeries Arctis 9 support 2025-01-01 15:11 ` [PATCH 1/3] " Christian Mayer 2025-01-04 11:58 ` Bastien Nocera @ 2025-01-04 12:45 ` Christophe JAILLET 2025-01-05 10:33 ` Christian Mayer 1 sibling, 1 reply; 12+ messages in thread From: Christophe JAILLET @ 2025-01-04 12:45 UTC (permalink / raw) To: Christian Mayer, linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Bastien Nocera Le 01/01/2025 à 16:11, Christian Mayer a écrit : > Add support for the SteelSeries Arctis 9 headset. This driver > will export the battery information like it already does for > the Arcits 1 headset. > > Signed-off-by: Christian Mayer <git@mayer-bgk.de> ... > -static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev) > +static int steelseries_headset_request_battery(struct hid_device *hdev, > + const char *request, size_t len) > { > u8 *write_buf; > int ret; > > /* Request battery information */ > - write_buf = kmemdup(arctis_1_battery_request, sizeof(arctis_1_battery_request), GFP_KERNEL); > + write_buf = kmemdup(request, len, GFP_KERNEL); > if (!write_buf) > return -ENOMEM; > > - ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0], > - write_buf, sizeof(arctis_1_battery_request), > + hid_dbg(hdev, "Sending battery request report"); > + ret = hid_hw_raw_request(hdev, request[0], > + write_buf, len, This could be on the same line. > HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); > - if (ret < (int)sizeof(arctis_1_battery_request)) { > + if (ret < (int)len) { > hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); > ret = -ENODATA; > } ... > +static uint8_t steelseries_headset_map_capacity(uint8_t capacity, uint8_t min_in, uint8_t max_in) > +{ > + if (capacity > max_in) >= ? > + return 100; > + if (capacity < min_in) <= ? > + return 0; > + return (capacity - min_in) * 100 / (max_in - min_in); > +} ... CJ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/3] HID: steelseries: add SteelSeries Arctis 9 support 2025-01-04 12:45 ` Christophe JAILLET @ 2025-01-05 10:33 ` Christian Mayer 0 siblings, 0 replies; 12+ messages in thread From: Christian Mayer @ 2025-01-05 10:33 UTC (permalink / raw) To: Christophe JAILLET, Christian Mayer, linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Bastien Nocera Am 04.01.25 um 13:45 schrieb Christophe JAILLET: > Le 01/01/2025 à 16:11, Christian Mayer a écrit : >> Add support for the SteelSeries Arctis 9 headset. This driver >> will export the battery information like it already does for >> the Arcits 1 headset. >> >> Signed-off-by: Christian Mayer <git@mayer-bgk.de> > > ... Thanks for your feedback Christophe. I will repost my patches with the fixes for your comments together with the fixes for Bastiens comment. > >> -static int steelseries_headset_arctis_1_fetch_battery(struct >> hid_device *hdev) >> +static int steelseries_headset_request_battery(struct hid_device *hdev, >> + const char *request, size_t len) >> { >> u8 *write_buf; >> int ret; >> /* Request battery information */ >> - write_buf = kmemdup(arctis_1_battery_request, >> sizeof(arctis_1_battery_request), GFP_KERNEL); >> + write_buf = kmemdup(request, len, GFP_KERNEL); >> if (!write_buf) >> return -ENOMEM; >> - ret = hid_hw_raw_request(hdev, arctis_1_battery_request[0], >> - write_buf, sizeof(arctis_1_battery_request), >> + hid_dbg(hdev, "Sending battery request report"); >> + ret = hid_hw_raw_request(hdev, request[0], >> + write_buf, len, > > This could be on the same line. > >> HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); >> - if (ret < (int)sizeof(arctis_1_battery_request)) { >> + if (ret < (int)len) { >> hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); >> ret = -ENODATA; >> } > > ... > >> +static uint8_t steelseries_headset_map_capacity(uint8_t capacity, >> uint8_t min_in, uint8_t max_in) >> +{ >> + if (capacity > max_in) > > >= ? > >> + return 100; >> + if (capacity < min_in) > > <= ? > >> + return 0; >> + return (capacity - min_in) * 100 / (max_in - min_in); >> +} > > ... > > CJ ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/3] HID: steelseries: export charging state for the SteelSeries Arctis 9 headset 2025-01-01 15:11 [PATCH 0/3] HID: steelseries: add SteelSeries Arctis 9 support Christian Mayer 2025-01-01 15:11 ` [PATCH 1/3] " Christian Mayer @ 2025-01-01 15:11 ` Christian Mayer 2025-01-04 12:02 ` Bastien Nocera 2025-01-01 15:11 ` [PATCH 3/3] HID: steelseries: export model and manufacturer Christian Mayer 2 siblings, 1 reply; 12+ messages in thread From: Christian Mayer @ 2025-01-01 15:11 UTC (permalink / raw) To: linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Bastien Nocera, Christian Mayer The Arctis 9 headset provides the information if the power cable is plugged in and charging via the battery report. This information can be exported. Signed-off-by: Christian Mayer <git@mayer-bgk.de> --- drivers/hid/hid-steelseries.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c index f102045a9b6b..d11296bc0e1e 100644 --- a/drivers/hid/hid-steelseries.c +++ b/drivers/hid/hid-steelseries.c @@ -33,6 +33,7 @@ struct steelseries_device { struct power_supply *battery; uint8_t battery_capacity; bool headset_connected; + bool battery_charging; }; #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ @@ -450,9 +451,12 @@ static int steelseries_headset_battery_get_property(struct power_supply *psy, val->intval = 1; break; case POWER_SUPPLY_PROP_STATUS: - val->intval = sd->headset_connected ? - POWER_SUPPLY_STATUS_DISCHARGING : - POWER_SUPPLY_STATUS_UNKNOWN; + if (sd->headset_connected) { + val->intval = sd->battery_charging ? + POWER_SUPPLY_STATUS_CHARGING : + POWER_SUPPLY_STATUS_DISCHARGING; + } else + val->intval = POWER_SUPPLY_STATUS_UNKNOWN; break; case POWER_SUPPLY_PROP_SCOPE: val->intval = POWER_SUPPLY_SCOPE_DEVICE; @@ -514,6 +518,7 @@ static int steelseries_headset_battery_register(struct steelseries_device *sd) /* avoid the warning of 0% battery while waiting for the first info */ steelseries_headset_set_wireless_status(sd->hdev, false); sd->battery_capacity = 100; + sd->battery_charging = false; sd->battery = devm_power_supply_register(&sd->hdev->dev, &sd->battery_desc, &battery_cfg); @@ -650,6 +655,7 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, struct steelseries_device *sd = hid_get_drvdata(hdev); int capacity = sd->battery_capacity; bool connected = sd->headset_connected; + bool charging = sd->battery_charging; unsigned long flags; /* Not a headset */ @@ -685,6 +691,7 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, if (read_buf[0] == 0xaa && read_buf[1] == 0x01) { connected = true; + charging = read_buf[4] == 0x01; /* * Found no official documentation about min and max. @@ -697,6 +704,7 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, * there is no known status of the device read_buf[0] == 0x55 */ connected = false; + charging = false; } } @@ -717,6 +725,15 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, power_supply_changed(sd->battery); } + if (charging != sd->battery_charging) { + hid_dbg(sd->hdev, + "Battery charging status changed from %scharging to %scharging\n", + sd->battery_charging ? "" : "not ", + charging ? "" : "not "); + sd->battery_charging = charging; + power_supply_changed(sd->battery); + } + request_battery: spin_lock_irqsave(&sd->lock, flags); if (!sd->removed) -- 2.47.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/3] HID: steelseries: export charging state for the SteelSeries Arctis 9 headset 2025-01-01 15:11 ` [PATCH 2/3] HID: steelseries: export charging state for the SteelSeries Arctis 9 headset Christian Mayer @ 2025-01-04 12:02 ` Bastien Nocera 0 siblings, 0 replies; 12+ messages in thread From: Bastien Nocera @ 2025-01-04 12:02 UTC (permalink / raw) To: Christian Mayer, linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires On Wed, 2025-01-01 at 15:11 +0000, Christian Mayer wrote: > The Arctis 9 headset provides the information if > the power cable is plugged in and charging via the battery report. > This information can be exported. > > Signed-off-by: Christian Mayer <git@mayer-bgk.de> Reviewed-by: Bastien Nocera <hadess@hadess.net> > --- > drivers/hid/hid-steelseries.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid- > steelseries.c > index f102045a9b6b..d11296bc0e1e 100644 > --- a/drivers/hid/hid-steelseries.c > +++ b/drivers/hid/hid-steelseries.c > @@ -33,6 +33,7 @@ struct steelseries_device { > struct power_supply *battery; > uint8_t battery_capacity; > bool headset_connected; > + bool battery_charging; > }; > > #if IS_BUILTIN(CONFIG_LEDS_CLASS) || \ > @@ -450,9 +451,12 @@ static int > steelseries_headset_battery_get_property(struct power_supply *psy, > val->intval = 1; > break; > case POWER_SUPPLY_PROP_STATUS: > - val->intval = sd->headset_connected ? > - POWER_SUPPLY_STATUS_DISCHARGING : > - POWER_SUPPLY_STATUS_UNKNOWN; > + if (sd->headset_connected) { > + val->intval = sd->battery_charging ? > + POWER_SUPPLY_STATUS_CHARGING : > + POWER_SUPPLY_STATUS_DISCHARGING; > + } else > + val->intval = POWER_SUPPLY_STATUS_UNKNOWN; > break; > case POWER_SUPPLY_PROP_SCOPE: > val->intval = POWER_SUPPLY_SCOPE_DEVICE; > @@ -514,6 +518,7 @@ static int > steelseries_headset_battery_register(struct steelseries_device *sd) > /* avoid the warning of 0% battery while waiting for the > first info */ > steelseries_headset_set_wireless_status(sd->hdev, false); > sd->battery_capacity = 100; > + sd->battery_charging = false; > > sd->battery = devm_power_supply_register(&sd->hdev->dev, > &sd->battery_desc, &battery_cfg); > @@ -650,6 +655,7 @@ static int steelseries_headset_raw_event(struct > hid_device *hdev, > struct steelseries_device *sd = hid_get_drvdata(hdev); > int capacity = sd->battery_capacity; > bool connected = sd->headset_connected; > + bool charging = sd->battery_charging; > unsigned long flags; > > /* Not a headset */ > @@ -685,6 +691,7 @@ static int steelseries_headset_raw_event(struct > hid_device *hdev, > > if (read_buf[0] == 0xaa && read_buf[1] == 0x01) { > connected = true; > + charging = read_buf[4] == 0x01; > > /* > * Found no official documentation about min > and max. > @@ -697,6 +704,7 @@ static int steelseries_headset_raw_event(struct > hid_device *hdev, > * there is no known status of the device > read_buf[0] == 0x55 > */ > connected = false; > + charging = false; > } > } > > @@ -717,6 +725,15 @@ static int steelseries_headset_raw_event(struct > hid_device *hdev, > power_supply_changed(sd->battery); > } > > + if (charging != sd->battery_charging) { > + hid_dbg(sd->hdev, > + "Battery charging status changed from > %scharging to %scharging\n", > + sd->battery_charging ? "" : "not ", > + charging ? "" : "not "); > + sd->battery_charging = charging; > + power_supply_changed(sd->battery); > + } > + > request_battery: > spin_lock_irqsave(&sd->lock, flags); > if (!sd->removed) ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] HID: steelseries: export model and manufacturer 2025-01-01 15:11 [PATCH 0/3] HID: steelseries: add SteelSeries Arctis 9 support Christian Mayer 2025-01-01 15:11 ` [PATCH 1/3] " Christian Mayer 2025-01-01 15:11 ` [PATCH 2/3] HID: steelseries: export charging state for the SteelSeries Arctis 9 headset Christian Mayer @ 2025-01-01 15:11 ` Christian Mayer 2025-01-04 12:01 ` Bastien Nocera 2 siblings, 1 reply; 12+ messages in thread From: Christian Mayer @ 2025-01-01 15:11 UTC (permalink / raw) To: linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Bastien Nocera, Christian Mayer Export model and manufacturer with the power supply properties. This helps identifing the device in the battery overview. In the case of the Arctis 9 headset, the manufacturer is prefixed twice in the device name. Signed-off-by: Christian Mayer <git@mayer-bgk.de> --- drivers/hid/hid-steelseries.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c index d11296bc0e1e..32e559376be4 100644 --- a/drivers/hid/hid-steelseries.c +++ b/drivers/hid/hid-steelseries.c @@ -447,6 +447,14 @@ static int steelseries_headset_battery_get_property(struct power_supply *psy, int ret = 0; switch (psp) { + case POWER_SUPPLY_PROP_MODEL_NAME: + val->strval = sd->hdev->name; + while (!strncmp(val->strval, "SteelSeries ", 12)) + val->strval += 12; + break; + case POWER_SUPPLY_PROP_MANUFACTURER: + val->strval = "SteelSeries"; + break; case POWER_SUPPLY_PROP_PRESENT: val->intval = 1; break; @@ -490,6 +498,8 @@ steelseries_headset_set_wireless_status(struct hid_device *hdev, } static enum power_supply_property steelseries_headset_battery_props[] = { + POWER_SUPPLY_PROP_MODEL_NAME, + POWER_SUPPLY_PROP_MANUFACTURER, POWER_SUPPLY_PROP_PRESENT, POWER_SUPPLY_PROP_STATUS, POWER_SUPPLY_PROP_SCOPE, -- 2.47.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] HID: steelseries: export model and manufacturer 2025-01-01 15:11 ` [PATCH 3/3] HID: steelseries: export model and manufacturer Christian Mayer @ 2025-01-04 12:01 ` Bastien Nocera 0 siblings, 0 replies; 12+ messages in thread From: Bastien Nocera @ 2025-01-04 12:01 UTC (permalink / raw) To: Christian Mayer, linux-input Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires On Wed, 2025-01-01 at 15:11 +0000, Christian Mayer wrote: > Export model and manufacturer with the power supply properties. > This helps identifing the device in the battery overview. > In the case of the Arctis 9 headset, the manufacturer is prefixed > twice in > the device name. > > Signed-off-by: Christian Mayer <git@mayer-bgk.de> > --- > drivers/hid/hid-steelseries.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid- > steelseries.c > index d11296bc0e1e..32e559376be4 100644 > --- a/drivers/hid/hid-steelseries.c > +++ b/drivers/hid/hid-steelseries.c > @@ -447,6 +447,14 @@ static int > steelseries_headset_battery_get_property(struct power_supply *psy, > int ret = 0; > > switch (psp) { > + case POWER_SUPPLY_PROP_MODEL_NAME: > + val->strval = sd->hdev->name; > + while (!strncmp(val->strval, "SteelSeries ", 12)) > + val->strval += 12; Please use constants instead of magic numbers: #define STEELSERIES_PREFIX "SteelSeries " #define STEELSERIES_PREFIX_LEN strlen(STEELSERIES_PREFIX) > + break; > + case POWER_SUPPLY_PROP_MANUFACTURER: > + val->strval = "SteelSeries"; > + break; > case POWER_SUPPLY_PROP_PRESENT: > val->intval = 1; > break; > @@ -490,6 +498,8 @@ steelseries_headset_set_wireless_status(struct > hid_device *hdev, > } > > static enum power_supply_property > steelseries_headset_battery_props[] = { > + POWER_SUPPLY_PROP_MODEL_NAME, > + POWER_SUPPLY_PROP_MANUFACTURER, > POWER_SUPPLY_PROP_PRESENT, > POWER_SUPPLY_PROP_STATUS, > POWER_SUPPLY_PROP_SCOPE, ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-01-12 11:49 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-01 15:11 [PATCH 0/3] HID: steelseries: add SteelSeries Arctis 9 support Christian Mayer 2025-01-01 15:11 ` [PATCH 1/3] " Christian Mayer 2025-01-04 11:58 ` Bastien Nocera 2025-01-05 10:16 ` Christian Mayer 2025-01-09 13:35 ` Bastien Nocera 2025-01-12 11:49 ` Christian Mayer 2025-01-04 12:45 ` Christophe JAILLET 2025-01-05 10:33 ` Christian Mayer 2025-01-01 15:11 ` [PATCH 2/3] HID: steelseries: export charging state for the SteelSeries Arctis 9 headset Christian Mayer 2025-01-04 12:02 ` Bastien Nocera 2025-01-01 15:11 ` [PATCH 3/3] HID: steelseries: export model and manufacturer Christian Mayer 2025-01-04 12:01 ` Bastien Nocera
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).