* [PATCH v5] HID: lg-g15 - Add support for Logitech G13. @ 2025-09-02 0:36 Leo L. Schwab 2025-09-02 9:14 ` Hans de Goede 2025-09-02 19:46 ` kernel test robot 0 siblings, 2 replies; 4+ messages in thread From: Leo L. Schwab @ 2025-09-02 0:36 UTC (permalink / raw) To: Hans de Goede Cc: Kate Hsuan, Leo L. Schwab, Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel The Logitech G13 is a gaming keypad with general-purpose macro keys, four LED-backlit macro preset keys, five "menu" keys, backlight toggle key, an analog thumbstick, RGB LED backlight, and a monochrome LCD display. Support input event generation for all keys and the thumbstick, and expose all LEDs. Signed-off-by: Leo L. Schwab <ewhac@ewhac.org> Reviewed-by: Hans de Goede <hansg@kernel.org> Tested-by: Kate Hsuan <hpa@redhat.com> --- Changes in v5: - None; resend v4 due to bounced email submission. Changes in v4: - Minor changes recommended by Hans de Goede <hansg@kernel.org>. Changes in v3: - Re-revise commit message. - Conditionally compile the section depending on CONFIG_LEDS_BRIGHTNESS_HW_CHANGED correctly this time. - Use led-class-multicolor facilities for the RGB backlight. - Changes recommended by Kate Hsuan <hpa@redhat.com>: - Use guard(mutex) construct. - Fix numerous style nits. Changes in v2: - Add `#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED` bracket around new code segment dependent on that feature (fixes test robot build error). - Use `guard(mutex)` construct in new code (existing code left unmodified). - Commit message revised. drivers/hid/hid-ids.h | 1 + drivers/hid/hid-lg-g15.c | 426 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 411 insertions(+), 16 deletions(-) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 33cc5820f2be..7ed1e402b80a 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -870,6 +870,7 @@ #define USB_DEVICE_ID_LOGITECH_DUAL_ACTION 0xc216 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2 0xc218 #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2 0xc219 +#define USB_DEVICE_ID_LOGITECH_G13 0xc21c #define USB_DEVICE_ID_LOGITECH_G15_LCD 0xc222 #define USB_DEVICE_ID_LOGITECH_G11 0xc225 #define USB_DEVICE_ID_LOGITECH_G15_V2_LCD 0xc227 diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c index f8605656257b..62cb795c2393 100644 --- a/drivers/hid/hid-lg-g15.c +++ b/drivers/hid/hid-lg-g15.c @@ -26,7 +26,11 @@ #define LG_G510_FEATURE_BACKLIGHT_RGB 0x05 #define LG_G510_FEATURE_POWER_ON_RGB 0x06 +#define LG_G13_FEATURE_M_KEYS_LEDS 0x05 +#define LG_G13_FEATURE_BACKLIGHT_RGB 0x07 + enum lg_g15_model { + LG_G13, LG_G15, LG_G15_V2, LG_G510, @@ -45,6 +49,12 @@ enum lg_g15_led_type { LG_G15_LED_MAX }; +struct g13_input_report { + u8 report_id; /* Report ID is always set to 1. */ + u8 joy_x, joy_y; + u8 keybits[5]; +}; + struct lg_g15_led { union { struct led_classdev cdev; @@ -63,12 +73,172 @@ struct lg_g15_data { struct mutex mutex; struct work_struct work; struct input_dev *input; + struct input_dev *input_js; /* Separate joystick device for G13. */ struct hid_device *hdev; enum lg_g15_model model; struct lg_g15_led leds[LG_G15_LED_MAX]; bool game_mode_enabled; }; +/********* G13 LED functions ***********/ +/* + * G13 retains no state across power cycles, and always powers up with the backlight on, + * color #5AFF6E, all macro key LEDs off. + */ +static int lg_g13_get_leds_state(struct lg_g15_data *g15) +{ + u8 * const tbuf = g15->transfer_buf; + int ret, high; + + /* RGB backlight. */ + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB, + tbuf, 5, + HID_FEATURE_REPORT, HID_REQ_GET_REPORT); + if (ret != 5) { + hid_err(g15->hdev, "Error getting backlight brightness: %d\n", ret); + return (ret < 0) ? ret : -EIO; + } + + /* Normalize RGB intensities against the highest component. */ + high = max3(tbuf[1], tbuf[2], tbuf[3]); + if (high) { + g15->leds[LG_G15_KBD_BRIGHTNESS].red = + DIV_ROUND_CLOSEST(tbuf[1] * 255, high); + g15->leds[LG_G15_KBD_BRIGHTNESS].green = + DIV_ROUND_CLOSEST(tbuf[2] * 255, high); + g15->leds[LG_G15_KBD_BRIGHTNESS].blue = + DIV_ROUND_CLOSEST(tbuf[3] * 255, high); + g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = high; + } else { + g15->leds[LG_G15_KBD_BRIGHTNESS].red = 255; + g15->leds[LG_G15_KBD_BRIGHTNESS].green = 255; + g15->leds[LG_G15_KBD_BRIGHTNESS].blue = 255; + g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = 0; + } + + /* Macro LEDs. */ + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS, + tbuf, 5, + HID_FEATURE_REPORT, HID_REQ_GET_REPORT); + if (ret != 5) { + hid_err(g15->hdev, "Error getting macro LED brightness: %d\n", ret); + return (ret < 0) ? ret : -EIO; + } + + for (int i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i) + g15->leds[i].brightness = tbuf[1] & (1 << (i - LG_G15_MACRO_PRESET1)); + + return 0; +} + +static int lg_g13_kbd_led_write(struct lg_g15_data *g15, + struct lg_g15_led *g15_led, + enum led_brightness brightness) +{ + struct mc_subled const * const subleds = g15_led->mcdev.subled_info; + u8 * const tbuf = g15->transfer_buf; + int ret; + + guard(mutex)(&g15->mutex); + + led_mc_calc_color_components(&g15_led->mcdev, brightness); + + tbuf[0] = 5; + tbuf[1] = subleds[0].brightness; + tbuf[2] = subleds[1].brightness; + tbuf[3] = subleds[2].brightness; + tbuf[4] = 0; + + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB, + tbuf, 5, + HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + if (ret != 5) { + hid_err(g15->hdev, "Error setting backlight brightness: %d\n", ret); + return (ret < 0) ? ret : -EIO; + } + + g15_led->brightness = brightness; + return 0; +} + +static int lg_g13_kbd_led_set(struct led_classdev *led_cdev, enum led_brightness brightness) +{ + struct led_classdev_mc *mc = lcdev_to_mccdev(led_cdev); + struct lg_g15_led *g15_led = + container_of(mc, struct lg_g15_led, mcdev); + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent); + + /* Ignore LED off on unregister / keyboard unplug */ + if (led_cdev->flags & LED_UNREGISTERING) + return 0; + + return lg_g13_kbd_led_write(g15, g15_led, brightness); +} + +static enum led_brightness lg_g13_kbd_led_get(struct led_classdev *led_cdev) +{ + struct led_classdev_mc const * const mc = lcdev_to_mccdev(led_cdev); + struct lg_g15_led const *g15_led = + container_of(mc, struct lg_g15_led, mcdev); + + return g15_led->brightness; +} + +static int lg_g13_mkey_led_set(struct led_classdev *led_cdev, enum led_brightness brightness) +{ + struct lg_g15_led *g15_led = + container_of(led_cdev, struct lg_g15_led, cdev); + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent); + int i, ret; + u8 * const tbuf = g15->transfer_buf; + u8 val, mask = 0; + + /* Ignore LED off on unregister / keyboard unplug */ + if (led_cdev->flags & LED_UNREGISTERING) + return 0; + + guard(mutex)(&g15->mutex); + + for (i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i) { + if (i == g15_led->led) + val = brightness; + else + val = g15->leds[i].brightness; + + if (val) + mask |= 1 << (i - LG_G15_MACRO_PRESET1); + } + + tbuf[0] = 5; + tbuf[1] = mask; + tbuf[2] = 0; + tbuf[3] = 0; + tbuf[4] = 0; + + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS, + tbuf, 5, + HID_FEATURE_REPORT, HID_REQ_SET_REPORT); + if (ret != 5) { + hid_err(g15->hdev, "Error setting LED brightness: %d\n", ret); + return (ret < 0) ? ret : -EIO; + } + + g15_led->brightness = brightness; + return 0; +} + +static enum led_brightness lg_g13_mkey_led_get(struct led_classdev *led_cdev) +{ + /* + * G13 doesn't change macro key LEDs behind our back, so they're + * whatever we last set them to. + */ + struct lg_g15_led *g15_led = + container_of(led_cdev, struct lg_g15_led, cdev); + + return g15_led->brightness; +} + /******** G15 and G15 v2 LED functions ********/ static int lg_g15_update_led_brightness(struct lg_g15_data *g15) @@ -390,6 +560,8 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15) int ret; switch (g15->model) { + case LG_G13: + return lg_g13_get_leds_state(g15); case LG_G15: case LG_G15_V2: return lg_g15_update_led_brightness(g15); @@ -417,6 +589,116 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15) /******** Input functions ********/ +/** + * g13_input_report.keybits[] is not 32-bit aligned, so we can't use the bitops macros. + * + * @ary: Pointer to array of u8s + * @b: Bit index into ary, LSB first. Not range checked. + */ +#define TEST_BIT(ary, b) ((1 << ((b) & 7)) & (ary)[(b) >> 3]) + +/* Table mapping keybits[] bit positions to event codes. */ +/* Note: Indices are discontinuous to aid readability. */ +static const u16 g13_keys_for_bits[] = { + /* Main keypad - keys G1 - G22 */ + [0] = KEY_MACRO1, + [1] = KEY_MACRO2, + [2] = KEY_MACRO3, + [3] = KEY_MACRO4, + [4] = KEY_MACRO5, + [5] = KEY_MACRO6, + [6] = KEY_MACRO7, + [7] = KEY_MACRO8, + [8] = KEY_MACRO9, + [9] = KEY_MACRO10, + [10] = KEY_MACRO11, + [11] = KEY_MACRO12, + [12] = KEY_MACRO13, + [13] = KEY_MACRO14, + [14] = KEY_MACRO15, + [15] = KEY_MACRO16, + [16] = KEY_MACRO17, + [17] = KEY_MACRO18, + [18] = KEY_MACRO19, + [19] = KEY_MACRO20, + [20] = KEY_MACRO21, + [21] = KEY_MACRO22, + + /* LCD menu buttons. */ + [24] = KEY_KBD_LCD_MENU5, /* "Next page" button */ + [25] = KEY_KBD_LCD_MENU1, /* Left-most */ + [26] = KEY_KBD_LCD_MENU2, + [27] = KEY_KBD_LCD_MENU3, + [28] = KEY_KBD_LCD_MENU4, /* Right-most */ + + /* Macro preset and record buttons; have red LEDs under them. */ + [29] = KEY_MACRO_PRESET1, + [30] = KEY_MACRO_PRESET2, + [31] = KEY_MACRO_PRESET3, + [32] = KEY_MACRO_RECORD_START, + + /* 33-35 handled by joystick device. */ + + /* Backlight toggle. */ + [37] = KEY_LIGHTS_TOGGLE, +}; + +#define G13_JS_KEYBITS_OFFSET 33 + +static const u16 g13_keys_for_bits_js[] = { + /* Joystick buttons */ + /* These keybits are at bit indices 33, 34, and 35. */ + BTN_BASE, /* Left side */ + BTN_BASE2, /* Bottom side */ + BTN_THUMB, /* Stick depress */ +}; + +static int lg_g13_event(struct lg_g15_data *g15, u8 const *data) +{ + struct g13_input_report const * const rep = (struct g13_input_report *) data; + int i, val; + + /* + * Main macropad and menu keys. + * Emit key events defined for each bit position. + */ + for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) { + if (g13_keys_for_bits[i]) { + val = TEST_BIT(rep->keybits, i); + input_report_key(g15->input, g13_keys_for_bits[i], val); + } + } + input_sync(g15->input); + + /* + * Joystick. + * Emit button and deflection events. + */ + for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) { + val = TEST_BIT(rep->keybits, i + G13_JS_KEYBITS_OFFSET); + input_report_key(g15->input_js, g13_keys_for_bits_js[i], val); + } + input_report_abs(g15->input_js, ABS_X, rep->joy_x); + input_report_abs(g15->input_js, ABS_Y, rep->joy_y); + input_sync(g15->input_js); + + if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) { + /* + * Bit 23 of keybits[] reports the current backlight on/off + * state. If it has changed from the last cached value, apply + * an update. + */ + bool hw_brightness_changed = (!!TEST_BIT(rep->keybits, 23)) + ^ (g15->leds[0].cdev.brightness_hw_changed > 0); + if (hw_brightness_changed) + led_classdev_notify_brightness_hw_changed( + &g15->leds[0].cdev, + TEST_BIT(rep->keybits, 23) ? LED_FULL : LED_OFF); + } + + return 0; +} + /* On the G15 Mark I Logitech has been quite creative with which bit is what */ static void lg_g15_handle_lcd_menu_keys(struct lg_g15_data *g15, u8 *data) { @@ -572,6 +854,10 @@ static int lg_g15_raw_event(struct hid_device *hdev, struct hid_report *report, return 0; switch (g15->model) { + case LG_G13: + if (data[0] == 0x01 && size == sizeof(struct g13_input_report)) + return lg_g13_event(g15, data); + break; case LG_G15: if (data[0] == 0x02 && size == 9) return lg_g15_event(g15, data); @@ -616,13 +902,22 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index) { int i; struct mc_subled *subled_info; - - g15->leds[index].mcdev.led_cdev.brightness_set_blocking = - lg_g510_kbd_led_set; - g15->leds[index].mcdev.led_cdev.brightness_get = - lg_g510_kbd_led_get; - g15->leds[index].mcdev.led_cdev.max_brightness = 255; - g15->leds[index].mcdev.num_colors = 3; + struct lg_g15_led * const gled = &g15->leds[index]; + + if (g15->model == LG_G13) { + gled->mcdev.led_cdev.brightness_set_blocking = + lg_g13_kbd_led_set; + gled->mcdev.led_cdev.brightness_get = + lg_g13_kbd_led_get; + gled->mcdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED; + } else { + gled->mcdev.led_cdev.brightness_set_blocking = + lg_g510_kbd_led_set; + gled->mcdev.led_cdev.brightness_get = + lg_g510_kbd_led_get; + } + gled->mcdev.led_cdev.max_brightness = 255; + gled->mcdev.num_colors = 3; subled_info = devm_kcalloc(&g15->hdev->dev, 3, sizeof(*subled_info), GFP_KERNEL); if (!subled_info) @@ -632,20 +927,20 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index) switch (i + 1) { case LED_COLOR_ID_RED: subled_info[i].color_index = LED_COLOR_ID_RED; - subled_info[i].intensity = g15->leds[index].red; + subled_info[i].intensity = gled->red; break; case LED_COLOR_ID_GREEN: subled_info[i].color_index = LED_COLOR_ID_GREEN; - subled_info[i].intensity = g15->leds[index].green; + subled_info[i].intensity = gled->green; break; case LED_COLOR_ID_BLUE: subled_info[i].color_index = LED_COLOR_ID_BLUE; - subled_info[i].intensity = g15->leds[index].blue; + subled_info[i].intensity = gled->blue; break; } subled_info[i].channel = i; } - g15->leds[index].mcdev.subled_info = subled_info; + gled->mcdev.subled_info = subled_info; } static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name) @@ -656,6 +951,23 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name) g15->leds[i].cdev.name = name; switch (g15->model) { + case LG_G13: + if (i < LG_G15_BRIGHTNESS_MAX) { + /* RGB backlight. */ + lg_g15_setup_led_rgb(g15, i); + ret = devm_led_classdev_multicolor_register_ext(&g15->hdev->dev, + &g15->leds[i].mcdev, + NULL); + } else { + /* Macro keys */ + g15->leds[i].cdev.brightness_set_blocking = lg_g13_mkey_led_set; + g15->leds[i].cdev.brightness_get = lg_g13_mkey_led_get; + g15->leds[i].cdev.max_brightness = 1; + + ret = devm_led_classdev_register(&g15->hdev->dev, + &g15->leds[i].cdev); + } + break; case LG_G15: case LG_G15_V2: g15->leds[i].cdev.brightness_get = lg_g15_led_get; @@ -702,11 +1014,9 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name) } /* Common input device init code shared between keyboards and Z-10 speaker handling */ -static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input, - const char *name) +static void lg_g15_init_input_dev_core(struct hid_device *hdev, struct input_dev *input, + char const *name) { - int i; - input->name = name; input->phys = hdev->phys; input->uniq = hdev->uniq; @@ -717,12 +1027,42 @@ static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *inp input->dev.parent = &hdev->dev; input->open = lg_g15_input_open; input->close = lg_g15_input_close; +} + +static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input, + const char *name) +{ + int i; + + lg_g15_init_input_dev_core(hdev, input, name); /* Keys below the LCD, intended for controlling a menu on the LCD */ for (i = 0; i < 5; i++) input_set_capability(input, EV_KEY, KEY_KBD_LCD_MENU1 + i); } +static void lg_g13_init_input_dev(struct hid_device *hdev, + struct input_dev *input, const char *name, + struct input_dev *input_js, const char *name_js) +{ + /* Macropad. */ + lg_g15_init_input_dev_core(hdev, input, name); + for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) { + if (g13_keys_for_bits[i]) + input_set_capability(input, EV_KEY, g13_keys_for_bits[i]); + } + + /* OBTW, we're a joystick, too... */ + lg_g15_init_input_dev_core(hdev, input_js, name_js); + for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) + input_set_capability(input_js, EV_KEY, g13_keys_for_bits_js[i]); + + input_set_capability(input_js, EV_ABS, ABS_X); + input_set_abs_params(input_js, ABS_X, 0, 255, 0, 0); + input_set_capability(input_js, EV_ABS, ABS_Y); + input_set_abs_params(input_js, ABS_Y, 0, 255, 0, 0); +} + static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) { static const char * const led_names[] = { @@ -739,7 +1079,7 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) unsigned int connect_mask = 0; bool has_ff000000 = false; struct lg_g15_data *g15; - struct input_dev *input; + struct input_dev *input, *input_js; struct hid_report *rep; int ret, i, gkeys = 0; @@ -778,6 +1118,25 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) hid_set_drvdata(hdev, (void *)g15); switch (g15->model) { + case LG_G13: + /* + * The G13 has an analog thumbstick with nearby buttons. Some + * libraries and applications are known to ignore devices that + * don't "look like" a joystick, and a device with two ABS axes + * and 25+ macro keys would confuse them. + * + * Create an additional input device dedicated to appear as a + * simplified joystick (two ABS axes, three BTN buttons). + */ + input_js = devm_input_allocate_device(&hdev->dev); + if (!input_js) + return -ENOMEM; + g15->input_js = input_js; + input_set_drvdata(input_js, hdev); + + connect_mask = HID_CONNECT_HIDRAW; + gkeys = 25; + break; case LG_G15: INIT_WORK(&g15->work, lg_g15_leds_changed_work); /* @@ -859,6 +1218,34 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) goto error_hw_stop; return 0; /* All done */ + } else if (g15->model == LG_G13) { + static char const * const g13_led_names[] = { + /* Backlight is shared between LCD and keys. */ + "g13:rgb:kbd_backlight", + NULL, /* Keep in sync with led_type enum */ + "g13:red:macro_preset_1", + "g13:red:macro_preset_2", + "g13:red:macro_preset_3", + "g13:red:macro_record", + }; + lg_g13_init_input_dev(hdev, + input, "Logitech G13 Gaming Keypad", + input_js, "Logitech G13 Thumbstick"); + ret = input_register_device(input); + if (ret) + goto error_hw_stop; + ret = input_register_device(input_js); + if (ret) + goto error_hw_stop; + + for (i = 0; i < ARRAY_SIZE(g13_led_names); ++i) { + if (g13_led_names[i]) { + ret = lg_g15_register_led(g15, i, g13_led_names[i]); + if (ret) + goto error_hw_stop; + } + } + return 0; } /* Setup and register input device */ @@ -903,6 +1290,13 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) } static const struct hid_device_id lg_g15_devices[] = { + /* + * The G13 is a macropad-only device with an LCD, LED backlighing, + * and joystick. + */ + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, + USB_DEVICE_ID_LOGITECH_G13), + .driver_data = LG_G13 }, /* The G11 is a G15 without the LCD, treat it as a G15 */ { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G11), -- 2.51.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v5] HID: lg-g15 - Add support for Logitech G13. 2025-09-02 0:36 [PATCH v5] HID: lg-g15 - Add support for Logitech G13 Leo L. Schwab @ 2025-09-02 9:14 ` Hans de Goede 2025-09-02 19:46 ` kernel test robot 1 sibling, 0 replies; 4+ messages in thread From: Hans de Goede @ 2025-09-02 9:14 UTC (permalink / raw) To: Leo L. Schwab Cc: Kate Hsuan, Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel Hi All, On 2-Sep-25 2:36 AM, Leo L. Schwab wrote: > The Logitech G13 is a gaming keypad with general-purpose macro keys, > four LED-backlit macro preset keys, five "menu" keys, backlight toggle > key, an analog thumbstick, RGB LED backlight, and a monochrome LCD > display. > > Support input event generation for all keys and the thumbstick, and > expose all LEDs. > > Signed-off-by: Leo L. Schwab <ewhac@ewhac.org> > Reviewed-by: Hans de Goede <hansg@kernel.org> > Tested-by: Kate Hsuan <hpa@redhat.com> > --- > Changes in v5: > - None; resend v4 due to bounced email submission. Note to the HID maintainers, the brightness_hw_changed handling needs some work here, see the v3 thread discussion, so please do not merge this yet. Regards, Hans > Changes in v4: > - Minor changes recommended by Hans de Goede <hansg@kernel.org>. > Changes in v3: > - Re-revise commit message. > - Conditionally compile the section depending on > CONFIG_LEDS_BRIGHTNESS_HW_CHANGED correctly this time. > - Use led-class-multicolor facilities for the RGB backlight. > - Changes recommended by Kate Hsuan <hpa@redhat.com>: > - Use guard(mutex) construct. > - Fix numerous style nits. > Changes in v2: > - Add `#ifdef CONFIG_LEDS_BRIGHTNESS_HW_CHANGED` bracket around new > code segment dependent on that feature (fixes test robot build > error). > - Use `guard(mutex)` construct in new code (existing code left > unmodified). > - Commit message revised. > > drivers/hid/hid-ids.h | 1 + > drivers/hid/hid-lg-g15.c | 426 +++++++++++++++++++++++++++++++++++++-- > 2 files changed, 411 insertions(+), 16 deletions(-) > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 33cc5820f2be..7ed1e402b80a 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -870,6 +870,7 @@ > #define USB_DEVICE_ID_LOGITECH_DUAL_ACTION 0xc216 > #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2 0xc218 > #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2 0xc219 > +#define USB_DEVICE_ID_LOGITECH_G13 0xc21c > #define USB_DEVICE_ID_LOGITECH_G15_LCD 0xc222 > #define USB_DEVICE_ID_LOGITECH_G11 0xc225 > #define USB_DEVICE_ID_LOGITECH_G15_V2_LCD 0xc227 > diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c > index f8605656257b..62cb795c2393 100644 > --- a/drivers/hid/hid-lg-g15.c > +++ b/drivers/hid/hid-lg-g15.c > @@ -26,7 +26,11 @@ > #define LG_G510_FEATURE_BACKLIGHT_RGB 0x05 > #define LG_G510_FEATURE_POWER_ON_RGB 0x06 > > +#define LG_G13_FEATURE_M_KEYS_LEDS 0x05 > +#define LG_G13_FEATURE_BACKLIGHT_RGB 0x07 > + > enum lg_g15_model { > + LG_G13, > LG_G15, > LG_G15_V2, > LG_G510, > @@ -45,6 +49,12 @@ enum lg_g15_led_type { > LG_G15_LED_MAX > }; > > +struct g13_input_report { > + u8 report_id; /* Report ID is always set to 1. */ > + u8 joy_x, joy_y; > + u8 keybits[5]; > +}; > + > struct lg_g15_led { > union { > struct led_classdev cdev; > @@ -63,12 +73,172 @@ struct lg_g15_data { > struct mutex mutex; > struct work_struct work; > struct input_dev *input; > + struct input_dev *input_js; /* Separate joystick device for G13. */ > struct hid_device *hdev; > enum lg_g15_model model; > struct lg_g15_led leds[LG_G15_LED_MAX]; > bool game_mode_enabled; > }; > > +/********* G13 LED functions ***********/ > +/* > + * G13 retains no state across power cycles, and always powers up with the backlight on, > + * color #5AFF6E, all macro key LEDs off. > + */ > +static int lg_g13_get_leds_state(struct lg_g15_data *g15) > +{ > + u8 * const tbuf = g15->transfer_buf; > + int ret, high; > + > + /* RGB backlight. */ > + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB, > + tbuf, 5, > + HID_FEATURE_REPORT, HID_REQ_GET_REPORT); > + if (ret != 5) { > + hid_err(g15->hdev, "Error getting backlight brightness: %d\n", ret); > + return (ret < 0) ? ret : -EIO; > + } > + > + /* Normalize RGB intensities against the highest component. */ > + high = max3(tbuf[1], tbuf[2], tbuf[3]); > + if (high) { > + g15->leds[LG_G15_KBD_BRIGHTNESS].red = > + DIV_ROUND_CLOSEST(tbuf[1] * 255, high); > + g15->leds[LG_G15_KBD_BRIGHTNESS].green = > + DIV_ROUND_CLOSEST(tbuf[2] * 255, high); > + g15->leds[LG_G15_KBD_BRIGHTNESS].blue = > + DIV_ROUND_CLOSEST(tbuf[3] * 255, high); > + g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = high; > + } else { > + g15->leds[LG_G15_KBD_BRIGHTNESS].red = 255; > + g15->leds[LG_G15_KBD_BRIGHTNESS].green = 255; > + g15->leds[LG_G15_KBD_BRIGHTNESS].blue = 255; > + g15->leds[LG_G15_KBD_BRIGHTNESS].brightness = 0; > + } > + > + /* Macro LEDs. */ > + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS, > + tbuf, 5, > + HID_FEATURE_REPORT, HID_REQ_GET_REPORT); > + if (ret != 5) { > + hid_err(g15->hdev, "Error getting macro LED brightness: %d\n", ret); > + return (ret < 0) ? ret : -EIO; > + } > + > + for (int i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i) > + g15->leds[i].brightness = tbuf[1] & (1 << (i - LG_G15_MACRO_PRESET1)); > + > + return 0; > +} > + > +static int lg_g13_kbd_led_write(struct lg_g15_data *g15, > + struct lg_g15_led *g15_led, > + enum led_brightness brightness) > +{ > + struct mc_subled const * const subleds = g15_led->mcdev.subled_info; > + u8 * const tbuf = g15->transfer_buf; > + int ret; > + > + guard(mutex)(&g15->mutex); > + > + led_mc_calc_color_components(&g15_led->mcdev, brightness); > + > + tbuf[0] = 5; > + tbuf[1] = subleds[0].brightness; > + tbuf[2] = subleds[1].brightness; > + tbuf[3] = subleds[2].brightness; > + tbuf[4] = 0; > + > + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_BACKLIGHT_RGB, > + tbuf, 5, > + HID_FEATURE_REPORT, HID_REQ_SET_REPORT); > + if (ret != 5) { > + hid_err(g15->hdev, "Error setting backlight brightness: %d\n", ret); > + return (ret < 0) ? ret : -EIO; > + } > + > + g15_led->brightness = brightness; > + return 0; > +} > + > +static int lg_g13_kbd_led_set(struct led_classdev *led_cdev, enum led_brightness brightness) > +{ > + struct led_classdev_mc *mc = lcdev_to_mccdev(led_cdev); > + struct lg_g15_led *g15_led = > + container_of(mc, struct lg_g15_led, mcdev); > + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent); > + > + /* Ignore LED off on unregister / keyboard unplug */ > + if (led_cdev->flags & LED_UNREGISTERING) > + return 0; > + > + return lg_g13_kbd_led_write(g15, g15_led, brightness); > +} > + > +static enum led_brightness lg_g13_kbd_led_get(struct led_classdev *led_cdev) > +{ > + struct led_classdev_mc const * const mc = lcdev_to_mccdev(led_cdev); > + struct lg_g15_led const *g15_led = > + container_of(mc, struct lg_g15_led, mcdev); > + > + return g15_led->brightness; > +} > + > +static int lg_g13_mkey_led_set(struct led_classdev *led_cdev, enum led_brightness brightness) > +{ > + struct lg_g15_led *g15_led = > + container_of(led_cdev, struct lg_g15_led, cdev); > + struct lg_g15_data *g15 = dev_get_drvdata(led_cdev->dev->parent); > + int i, ret; > + u8 * const tbuf = g15->transfer_buf; > + u8 val, mask = 0; > + > + /* Ignore LED off on unregister / keyboard unplug */ > + if (led_cdev->flags & LED_UNREGISTERING) > + return 0; > + > + guard(mutex)(&g15->mutex); > + > + for (i = LG_G15_MACRO_PRESET1; i < LG_G15_LED_MAX; ++i) { > + if (i == g15_led->led) > + val = brightness; > + else > + val = g15->leds[i].brightness; > + > + if (val) > + mask |= 1 << (i - LG_G15_MACRO_PRESET1); > + } > + > + tbuf[0] = 5; > + tbuf[1] = mask; > + tbuf[2] = 0; > + tbuf[3] = 0; > + tbuf[4] = 0; > + > + ret = hid_hw_raw_request(g15->hdev, LG_G13_FEATURE_M_KEYS_LEDS, > + tbuf, 5, > + HID_FEATURE_REPORT, HID_REQ_SET_REPORT); > + if (ret != 5) { > + hid_err(g15->hdev, "Error setting LED brightness: %d\n", ret); > + return (ret < 0) ? ret : -EIO; > + } > + > + g15_led->brightness = brightness; > + return 0; > +} > + > +static enum led_brightness lg_g13_mkey_led_get(struct led_classdev *led_cdev) > +{ > + /* > + * G13 doesn't change macro key LEDs behind our back, so they're > + * whatever we last set them to. > + */ > + struct lg_g15_led *g15_led = > + container_of(led_cdev, struct lg_g15_led, cdev); > + > + return g15_led->brightness; > +} > + > /******** G15 and G15 v2 LED functions ********/ > > static int lg_g15_update_led_brightness(struct lg_g15_data *g15) > @@ -390,6 +560,8 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15) > int ret; > > switch (g15->model) { > + case LG_G13: > + return lg_g13_get_leds_state(g15); > case LG_G15: > case LG_G15_V2: > return lg_g15_update_led_brightness(g15); > @@ -417,6 +589,116 @@ static int lg_g15_get_initial_led_brightness(struct lg_g15_data *g15) > > /******** Input functions ********/ > > +/** > + * g13_input_report.keybits[] is not 32-bit aligned, so we can't use the bitops macros. > + * > + * @ary: Pointer to array of u8s > + * @b: Bit index into ary, LSB first. Not range checked. > + */ > +#define TEST_BIT(ary, b) ((1 << ((b) & 7)) & (ary)[(b) >> 3]) > + > +/* Table mapping keybits[] bit positions to event codes. */ > +/* Note: Indices are discontinuous to aid readability. */ > +static const u16 g13_keys_for_bits[] = { > + /* Main keypad - keys G1 - G22 */ > + [0] = KEY_MACRO1, > + [1] = KEY_MACRO2, > + [2] = KEY_MACRO3, > + [3] = KEY_MACRO4, > + [4] = KEY_MACRO5, > + [5] = KEY_MACRO6, > + [6] = KEY_MACRO7, > + [7] = KEY_MACRO8, > + [8] = KEY_MACRO9, > + [9] = KEY_MACRO10, > + [10] = KEY_MACRO11, > + [11] = KEY_MACRO12, > + [12] = KEY_MACRO13, > + [13] = KEY_MACRO14, > + [14] = KEY_MACRO15, > + [15] = KEY_MACRO16, > + [16] = KEY_MACRO17, > + [17] = KEY_MACRO18, > + [18] = KEY_MACRO19, > + [19] = KEY_MACRO20, > + [20] = KEY_MACRO21, > + [21] = KEY_MACRO22, > + > + /* LCD menu buttons. */ > + [24] = KEY_KBD_LCD_MENU5, /* "Next page" button */ > + [25] = KEY_KBD_LCD_MENU1, /* Left-most */ > + [26] = KEY_KBD_LCD_MENU2, > + [27] = KEY_KBD_LCD_MENU3, > + [28] = KEY_KBD_LCD_MENU4, /* Right-most */ > + > + /* Macro preset and record buttons; have red LEDs under them. */ > + [29] = KEY_MACRO_PRESET1, > + [30] = KEY_MACRO_PRESET2, > + [31] = KEY_MACRO_PRESET3, > + [32] = KEY_MACRO_RECORD_START, > + > + /* 33-35 handled by joystick device. */ > + > + /* Backlight toggle. */ > + [37] = KEY_LIGHTS_TOGGLE, > +}; > + > +#define G13_JS_KEYBITS_OFFSET 33 > + > +static const u16 g13_keys_for_bits_js[] = { > + /* Joystick buttons */ > + /* These keybits are at bit indices 33, 34, and 35. */ > + BTN_BASE, /* Left side */ > + BTN_BASE2, /* Bottom side */ > + BTN_THUMB, /* Stick depress */ > +}; > + > +static int lg_g13_event(struct lg_g15_data *g15, u8 const *data) > +{ > + struct g13_input_report const * const rep = (struct g13_input_report *) data; > + int i, val; > + > + /* > + * Main macropad and menu keys. > + * Emit key events defined for each bit position. > + */ > + for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) { > + if (g13_keys_for_bits[i]) { > + val = TEST_BIT(rep->keybits, i); > + input_report_key(g15->input, g13_keys_for_bits[i], val); > + } > + } > + input_sync(g15->input); > + > + /* > + * Joystick. > + * Emit button and deflection events. > + */ > + for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) { > + val = TEST_BIT(rep->keybits, i + G13_JS_KEYBITS_OFFSET); > + input_report_key(g15->input_js, g13_keys_for_bits_js[i], val); > + } > + input_report_abs(g15->input_js, ABS_X, rep->joy_x); > + input_report_abs(g15->input_js, ABS_Y, rep->joy_y); > + input_sync(g15->input_js); > + > + if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) { > + /* > + * Bit 23 of keybits[] reports the current backlight on/off > + * state. If it has changed from the last cached value, apply > + * an update. > + */ > + bool hw_brightness_changed = (!!TEST_BIT(rep->keybits, 23)) > + ^ (g15->leds[0].cdev.brightness_hw_changed > 0); > + if (hw_brightness_changed) > + led_classdev_notify_brightness_hw_changed( > + &g15->leds[0].cdev, > + TEST_BIT(rep->keybits, 23) ? LED_FULL : LED_OFF); > + } > + > + return 0; > +} > + > /* On the G15 Mark I Logitech has been quite creative with which bit is what */ > static void lg_g15_handle_lcd_menu_keys(struct lg_g15_data *g15, u8 *data) > { > @@ -572,6 +854,10 @@ static int lg_g15_raw_event(struct hid_device *hdev, struct hid_report *report, > return 0; > > switch (g15->model) { > + case LG_G13: > + if (data[0] == 0x01 && size == sizeof(struct g13_input_report)) > + return lg_g13_event(g15, data); > + break; > case LG_G15: > if (data[0] == 0x02 && size == 9) > return lg_g15_event(g15, data); > @@ -616,13 +902,22 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index) > { > int i; > struct mc_subled *subled_info; > - > - g15->leds[index].mcdev.led_cdev.brightness_set_blocking = > - lg_g510_kbd_led_set; > - g15->leds[index].mcdev.led_cdev.brightness_get = > - lg_g510_kbd_led_get; > - g15->leds[index].mcdev.led_cdev.max_brightness = 255; > - g15->leds[index].mcdev.num_colors = 3; > + struct lg_g15_led * const gled = &g15->leds[index]; > + > + if (g15->model == LG_G13) { > + gled->mcdev.led_cdev.brightness_set_blocking = > + lg_g13_kbd_led_set; > + gled->mcdev.led_cdev.brightness_get = > + lg_g13_kbd_led_get; > + gled->mcdev.led_cdev.flags = LED_BRIGHT_HW_CHANGED; > + } else { > + gled->mcdev.led_cdev.brightness_set_blocking = > + lg_g510_kbd_led_set; > + gled->mcdev.led_cdev.brightness_get = > + lg_g510_kbd_led_get; > + } > + gled->mcdev.led_cdev.max_brightness = 255; > + gled->mcdev.num_colors = 3; > > subled_info = devm_kcalloc(&g15->hdev->dev, 3, sizeof(*subled_info), GFP_KERNEL); > if (!subled_info) > @@ -632,20 +927,20 @@ static void lg_g15_setup_led_rgb(struct lg_g15_data *g15, int index) > switch (i + 1) { > case LED_COLOR_ID_RED: > subled_info[i].color_index = LED_COLOR_ID_RED; > - subled_info[i].intensity = g15->leds[index].red; > + subled_info[i].intensity = gled->red; > break; > case LED_COLOR_ID_GREEN: > subled_info[i].color_index = LED_COLOR_ID_GREEN; > - subled_info[i].intensity = g15->leds[index].green; > + subled_info[i].intensity = gled->green; > break; > case LED_COLOR_ID_BLUE: > subled_info[i].color_index = LED_COLOR_ID_BLUE; > - subled_info[i].intensity = g15->leds[index].blue; > + subled_info[i].intensity = gled->blue; > break; > } > subled_info[i].channel = i; > } > - g15->leds[index].mcdev.subled_info = subled_info; > + gled->mcdev.subled_info = subled_info; > } > > static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name) > @@ -656,6 +951,23 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name) > g15->leds[i].cdev.name = name; > > switch (g15->model) { > + case LG_G13: > + if (i < LG_G15_BRIGHTNESS_MAX) { > + /* RGB backlight. */ > + lg_g15_setup_led_rgb(g15, i); > + ret = devm_led_classdev_multicolor_register_ext(&g15->hdev->dev, > + &g15->leds[i].mcdev, > + NULL); > + } else { > + /* Macro keys */ > + g15->leds[i].cdev.brightness_set_blocking = lg_g13_mkey_led_set; > + g15->leds[i].cdev.brightness_get = lg_g13_mkey_led_get; > + g15->leds[i].cdev.max_brightness = 1; > + > + ret = devm_led_classdev_register(&g15->hdev->dev, > + &g15->leds[i].cdev); > + } > + break; > case LG_G15: > case LG_G15_V2: > g15->leds[i].cdev.brightness_get = lg_g15_led_get; > @@ -702,11 +1014,9 @@ static int lg_g15_register_led(struct lg_g15_data *g15, int i, const char *name) > } > > /* Common input device init code shared between keyboards and Z-10 speaker handling */ > -static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input, > - const char *name) > +static void lg_g15_init_input_dev_core(struct hid_device *hdev, struct input_dev *input, > + char const *name) > { > - int i; > - > input->name = name; > input->phys = hdev->phys; > input->uniq = hdev->uniq; > @@ -717,12 +1027,42 @@ static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *inp > input->dev.parent = &hdev->dev; > input->open = lg_g15_input_open; > input->close = lg_g15_input_close; > +} > + > +static void lg_g15_init_input_dev(struct hid_device *hdev, struct input_dev *input, > + const char *name) > +{ > + int i; > + > + lg_g15_init_input_dev_core(hdev, input, name); > > /* Keys below the LCD, intended for controlling a menu on the LCD */ > for (i = 0; i < 5; i++) > input_set_capability(input, EV_KEY, KEY_KBD_LCD_MENU1 + i); > } > > +static void lg_g13_init_input_dev(struct hid_device *hdev, > + struct input_dev *input, const char *name, > + struct input_dev *input_js, const char *name_js) > +{ > + /* Macropad. */ > + lg_g15_init_input_dev_core(hdev, input, name); > + for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) { > + if (g13_keys_for_bits[i]) > + input_set_capability(input, EV_KEY, g13_keys_for_bits[i]); > + } > + > + /* OBTW, we're a joystick, too... */ > + lg_g15_init_input_dev_core(hdev, input_js, name_js); > + for (int i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) > + input_set_capability(input_js, EV_KEY, g13_keys_for_bits_js[i]); > + > + input_set_capability(input_js, EV_ABS, ABS_X); > + input_set_abs_params(input_js, ABS_X, 0, 255, 0, 0); > + input_set_capability(input_js, EV_ABS, ABS_Y); > + input_set_abs_params(input_js, ABS_Y, 0, 255, 0, 0); > +} > + > static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) > { > static const char * const led_names[] = { > @@ -739,7 +1079,7 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) > unsigned int connect_mask = 0; > bool has_ff000000 = false; > struct lg_g15_data *g15; > - struct input_dev *input; > + struct input_dev *input, *input_js; > struct hid_report *rep; > int ret, i, gkeys = 0; > > @@ -778,6 +1118,25 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) > hid_set_drvdata(hdev, (void *)g15); > > switch (g15->model) { > + case LG_G13: > + /* > + * The G13 has an analog thumbstick with nearby buttons. Some > + * libraries and applications are known to ignore devices that > + * don't "look like" a joystick, and a device with two ABS axes > + * and 25+ macro keys would confuse them. > + * > + * Create an additional input device dedicated to appear as a > + * simplified joystick (two ABS axes, three BTN buttons). > + */ > + input_js = devm_input_allocate_device(&hdev->dev); > + if (!input_js) > + return -ENOMEM; > + g15->input_js = input_js; > + input_set_drvdata(input_js, hdev); > + > + connect_mask = HID_CONNECT_HIDRAW; > + gkeys = 25; > + break; > case LG_G15: > INIT_WORK(&g15->work, lg_g15_leds_changed_work); > /* > @@ -859,6 +1218,34 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) > goto error_hw_stop; > > return 0; /* All done */ > + } else if (g15->model == LG_G13) { > + static char const * const g13_led_names[] = { > + /* Backlight is shared between LCD and keys. */ > + "g13:rgb:kbd_backlight", > + NULL, /* Keep in sync with led_type enum */ > + "g13:red:macro_preset_1", > + "g13:red:macro_preset_2", > + "g13:red:macro_preset_3", > + "g13:red:macro_record", > + }; > + lg_g13_init_input_dev(hdev, > + input, "Logitech G13 Gaming Keypad", > + input_js, "Logitech G13 Thumbstick"); > + ret = input_register_device(input); > + if (ret) > + goto error_hw_stop; > + ret = input_register_device(input_js); > + if (ret) > + goto error_hw_stop; > + > + for (i = 0; i < ARRAY_SIZE(g13_led_names); ++i) { > + if (g13_led_names[i]) { > + ret = lg_g15_register_led(g15, i, g13_led_names[i]); > + if (ret) > + goto error_hw_stop; > + } > + } > + return 0; > } > > /* Setup and register input device */ > @@ -903,6 +1290,13 @@ static int lg_g15_probe(struct hid_device *hdev, const struct hid_device_id *id) > } > > static const struct hid_device_id lg_g15_devices[] = { > + /* > + * The G13 is a macropad-only device with an LCD, LED backlighing, > + * and joystick. > + */ > + { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, > + USB_DEVICE_ID_LOGITECH_G13), > + .driver_data = LG_G13 }, > /* The G11 is a G15 without the LCD, treat it as a G15 */ > { HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, > USB_DEVICE_ID_LOGITECH_G11), ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5] HID: lg-g15 - Add support for Logitech G13. 2025-09-02 0:36 [PATCH v5] HID: lg-g15 - Add support for Logitech G13 Leo L. Schwab 2025-09-02 9:14 ` Hans de Goede @ 2025-09-02 19:46 ` kernel test robot 2025-09-02 20:47 ` Hans de Goede 1 sibling, 1 reply; 4+ messages in thread From: kernel test robot @ 2025-09-02 19:46 UTC (permalink / raw) To: Leo L. Schwab, Hans de Goede Cc: llvm, oe-kbuild-all, Kate Hsuan, Leo L. Schwab, Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel Hi Leo, kernel test robot noticed the following build errors: [auto build test ERROR on hid/for-next] [also build test ERROR on linus/master v6.17-rc4 next-20250902] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Leo-L-Schwab/HID-lg-g15-Add-support-for-Logitech-G13/20250902-091504 base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next patch link: https://lore.kernel.org/r/20250902003659.361934-2-ewhac%40ewhac.org patch subject: [PATCH v5] HID: lg-g15 - Add support for Logitech G13. config: i386-buildonly-randconfig-006-20250902 (https://download.01.org/0day-ci/archive/20250903/202509030200.ITZPZGmG-lkp@intel.com/config) compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250903/202509030200.ITZPZGmG-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202509030200.ITZPZGmG-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/hid/hid-lg-g15.c:692:30: error: no member named 'brightness_hw_changed' in 'struct led_classdev' 692 | ^ (g15->leds[0].cdev.brightness_hw_changed > 0); | ~~~~~~~~~~~~~~~~~ ^ 1 error generated. vim +692 drivers/hid/hid-lg-g15.c 655 656 static int lg_g13_event(struct lg_g15_data *g15, u8 const *data) 657 { 658 struct g13_input_report const * const rep = (struct g13_input_report *) data; 659 int i, val; 660 661 /* 662 * Main macropad and menu keys. 663 * Emit key events defined for each bit position. 664 */ 665 for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) { 666 if (g13_keys_for_bits[i]) { 667 val = TEST_BIT(rep->keybits, i); 668 input_report_key(g15->input, g13_keys_for_bits[i], val); 669 } 670 } 671 input_sync(g15->input); 672 673 /* 674 * Joystick. 675 * Emit button and deflection events. 676 */ 677 for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) { 678 val = TEST_BIT(rep->keybits, i + G13_JS_KEYBITS_OFFSET); 679 input_report_key(g15->input_js, g13_keys_for_bits_js[i], val); 680 } 681 input_report_abs(g15->input_js, ABS_X, rep->joy_x); 682 input_report_abs(g15->input_js, ABS_Y, rep->joy_y); 683 input_sync(g15->input_js); 684 685 if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) { 686 /* 687 * Bit 23 of keybits[] reports the current backlight on/off 688 * state. If it has changed from the last cached value, apply 689 * an update. 690 */ 691 bool hw_brightness_changed = (!!TEST_BIT(rep->keybits, 23)) > 692 ^ (g15->leds[0].cdev.brightness_hw_changed > 0); 693 if (hw_brightness_changed) 694 led_classdev_notify_brightness_hw_changed( 695 &g15->leds[0].cdev, 696 TEST_BIT(rep->keybits, 23) ? LED_FULL : LED_OFF); 697 } 698 699 return 0; 700 } 701 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5] HID: lg-g15 - Add support for Logitech G13. 2025-09-02 19:46 ` kernel test robot @ 2025-09-02 20:47 ` Hans de Goede 0 siblings, 0 replies; 4+ messages in thread From: Hans de Goede @ 2025-09-02 20:47 UTC (permalink / raw) To: kernel test robot, Leo L. Schwab Cc: llvm, oe-kbuild-all, Kate Hsuan, Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel Hi, On 2-Sep-25 21:46, kernel test robot wrote: > Hi Leo, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on hid/for-next] > [also build test ERROR on linus/master v6.17-rc4 next-20250902] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Leo-L-Schwab/HID-lg-g15-Add-support-for-Logitech-G13/20250902-091504 > base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next > patch link: https://lore.kernel.org/r/20250902003659.361934-2-ewhac%40ewhac.org > patch subject: [PATCH v5] HID: lg-g15 - Add support for Logitech G13. > config: i386-buildonly-randconfig-006-20250902 (https://download.01.org/0day-ci/archive/20250903/202509030200.ITZPZGmG-lkp@intel.com/config) > compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261) > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250903/202509030200.ITZPZGmG-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202509030200.ITZPZGmG-lkp@intel.com/ > > All errors (new ones prefixed by >>): > >>> drivers/hid/hid-lg-g15.c:692:30: error: no member named 'brightness_hw_changed' in 'struct led_classdev' > 692 | ^ (g15->leds[0].cdev.brightness_hw_changed > 0); > | ~~~~~~~~~~~~~~~~~ ^ > 1 error generated. Ah right, IS_ENABLED() only helps to avoid errors about references which are resolved link-time and this is a compile-time issue. But as discussed in the v3 thread we don't want to use cdev.brightness_hw_changed anyways. Regards, Hans > > vim +692 drivers/hid/hid-lg-g15.c > > 655 > 656 static int lg_g13_event(struct lg_g15_data *g15, u8 const *data) > 657 { > 658 struct g13_input_report const * const rep = (struct g13_input_report *) data; > 659 int i, val; > 660 > 661 /* > 662 * Main macropad and menu keys. > 663 * Emit key events defined for each bit position. > 664 */ > 665 for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits); ++i) { > 666 if (g13_keys_for_bits[i]) { > 667 val = TEST_BIT(rep->keybits, i); > 668 input_report_key(g15->input, g13_keys_for_bits[i], val); > 669 } > 670 } > 671 input_sync(g15->input); > 672 > 673 /* > 674 * Joystick. > 675 * Emit button and deflection events. > 676 */ > 677 for (i = 0; i < ARRAY_SIZE(g13_keys_for_bits_js); ++i) { > 678 val = TEST_BIT(rep->keybits, i + G13_JS_KEYBITS_OFFSET); > 679 input_report_key(g15->input_js, g13_keys_for_bits_js[i], val); > 680 } > 681 input_report_abs(g15->input_js, ABS_X, rep->joy_x); > 682 input_report_abs(g15->input_js, ABS_Y, rep->joy_y); > 683 input_sync(g15->input_js); > 684 > 685 if (IS_ENABLED(CONFIG_LEDS_BRIGHTNESS_HW_CHANGED)) { > 686 /* > 687 * Bit 23 of keybits[] reports the current backlight on/off > 688 * state. If it has changed from the last cached value, apply > 689 * an update. > 690 */ > 691 bool hw_brightness_changed = (!!TEST_BIT(rep->keybits, 23)) > > 692 ^ (g15->leds[0].cdev.brightness_hw_changed > 0); > 693 if (hw_brightness_changed) > 694 led_classdev_notify_brightness_hw_changed( > 695 &g15->leds[0].cdev, > 696 TEST_BIT(rep->keybits, 23) ? LED_FULL : LED_OFF); > 697 } > 698 > 699 return 0; > 700 } > 701 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-02 20:47 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-02 0:36 [PATCH v5] HID: lg-g15 - Add support for Logitech G13 Leo L. Schwab 2025-09-02 9:14 ` Hans de Goede 2025-09-02 19:46 ` kernel test robot 2025-09-02 20:47 ` Hans de Goede
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).