* [PATCH 00/15] HID: hid-oxp: fix and extend X2-family controller support @ 2026-09-10 3:21 Andrei Aldea 2026-09-10 3:21 ` [PATCH 15/15] HID: hid-oxp: add X2 auxiliary RGB zones Andrei Aldea 2026-09-10 20:24 ` [PATCH 00/15] HID: hid-oxp: fix and extend X2-family controller support Derek J. Clark 0 siblings, 2 replies; 4+ messages in thread From: Andrei Aldea @ 2026-09-10 3:21 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, Derek J. Clark Cc: linux-input, linux-kernel, Lee Jones, Pavel Machek, linux-leds, linux-api This 15-patch series fixes existing hid-oxp configuration and RGB issues and adds controller and lighting support for ONEXPLAYER X2-family devices, including the ONEXPLAYER 3 and X2 Mini Pro. The fixes correct the M1/M2 defaults, validate input reports and RGB state, correct brightness and color scaling, serialize RGB updates, and make configuration state and delayed-work lifetime local to each HID interface. They also handle controller reinitialization across system suspend. The X2 support selects configuration interface 2, supplies the required three-page button map, and adds the Gen3 protocol for joystick-ring zones 1, 2 and 7. The Guide button and rear logo receive independent multicolor LED devices using zones 5 and 6, with monocolor and breathing effects. The final patch documents the controller and lighting sysfs ABI. Patches 1-10 contain behavioral fixes. Patch 11 groups existing declarations. Patch 12 adds X2 button maps and interface selection; patch 13 provides the shared LED representation; patches 14-15 add X2 lighting support. This series is based on mainline commit 50d05c7c76c96b90462f24debacca971d2e86713. Testing: I tested this series on an ONEXPLAYER 3 running Bazzite 44 with kernel 7.2.0-ogc6.1.fc44.x86_64. Testing covered front controls, rear paddles, Steam and Quick Access behavior, independent RGB effects, brightness/off controls, and suspend/resume restoration. I also built the module with W=1. X2 Mini Pro hardware testing remains outstanding. All 15 patches apply cleanly to the stated base and pass git diff --check and checkpatch.pl with zero errors and zero warnings. Development and review: https://github.com/OpenGamingCollective/linux-unstable/pull/13 I used LLM assistance for protocol analysis, implementation, documentation, code review, and test orchestration. Andrei Aldea (15): HID: hid-oxp: fix default M1 and M2 key mappings HID: hid-oxp: validate input report lengths before decoding HID: hid-oxp: retain fractional brightness when reading RGB status HID: hid-oxp: reject invalid Gen2 RGB status values HID: hid-oxp: fix multicolor LED intensity scaling HID: hid-oxp: serialize complete RGB updates HID: hid-oxp: select brightness policy for the new RGB effect HID: hid-oxp: stop configuration work during teardown HID: hid-oxp: keep configuration state per HID interface HID: hid-oxp: handle controller reinitialization across suspend HID: hid-oxp: group declarations and protocol definitions HID: hid-oxp: support three-page button maps on X2 controllers HID: hid-oxp: represent RGB LEDs with a common array HID: hid-oxp: add Gen3 joystick ring RGB support HID: hid-oxp: add X2 auxiliary RGB zones .../ABI/testing/sysfs-driver-hid-oxp | 196 ++ MAINTAINERS | 1 + drivers/hid/hid-oxp.c | 1743 ++++++++++++++--- 3 files changed, 1623 insertions(+), 317 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-oxp base-commit: 50d05c7c76c96b90462f24debacca971d2e86713 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 15/15] HID: hid-oxp: add X2 auxiliary RGB zones 2026-09-10 3:21 [PATCH 00/15] HID: hid-oxp: fix and extend X2-family controller support Andrei Aldea @ 2026-09-10 3:21 ` Andrei Aldea 2026-09-10 20:20 ` Derek J. Clark 2026-09-10 20:24 ` [PATCH 00/15] HID: hid-oxp: fix and extend X2-family controller support Derek J. Clark 1 sibling, 1 reply; 4+ messages in thread From: Andrei Aldea @ 2026-09-10 3:21 UTC (permalink / raw) To: Jiri Kosina, Benjamin Tissoires, Derek J. Clark Cc: linux-input, linux-kernel, Lee Jones, Pavel Machek, linux-leds, linux-api Add separate multicolor LED devices for the X2 guide button and rear logo using Gen3 zones 5 and 6. Each LED retains independent color, brightness, and monocolor or breathing effect state. Use the common LED array and type-tagged state introduced earlier. Protect auxiliary state snapshots with scoped spinlocks, serialize output with other RGB operations, and retry a failed command once. Do not alter an auxiliary zone until userspace has supplied valid state, and restore valid cached settings after resume. Expose these zones only on the matched ONEXPLAYER 3 and X2 Mini Pro. Physical validation of the latter remains outstanding. Document the auxiliary effects together with the existing joystick-ring effects and add the previously omitted controller, mapping, rumble, and ring-lighting ABI. Add the ABI file to the ONEXPLAYER HID maintainer pattern. Assisted-by: LLM Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com> Signed-off-by: Andrei Aldea <andrei1998@gmail.com> --- .../ABI/testing/sysfs-driver-hid-oxp | 196 +++++++++++ MAINTAINERS | 1 + drivers/hid/hid-oxp.c | 305 +++++++++++++++++- 3 files changed, 497 insertions(+), 5 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-oxp diff --git a/Documentation/ABI/testing/sysfs-driver-hid-oxp b/Documentation/ABI/testing/sysfs-driver-hid-oxp new file mode 100644 index 0000000..7c97f28 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-hid-oxp @@ -0,0 +1,196 @@ +What: /sys/class/leds/oxp:rgb:joystick_rings/effect +What: /sys/class/leds/oxp:rgb:guide_button/effect +What: /sys/class/leds/oxp:rgb:rear_logo/effect +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read/write attribute selecting the lighting effect for the LED. + + The joystick_rings LED supports the following effect names: + + ``aurora``, ``birthday_cake``, ``flowing_light``, + ``chroma_popping``, ``neon``, ``chroma_breathing``, ``dreamy``, + ``warm_sun``, ``cyberpunk``, ``sea_foam``, ``sunset_afterglow``, + ``colorful``, ``monster_woke``, ``green_breathing``, + ``blue_breathing``, ``yellow_breathing``, ``teal_breathing``, + ``purple_breathing``, ``foggy_haze``, and ``monocolor``. + + The guide_button and rear_logo LEDs support ``monocolor`` and + ``breathing``. These two LEDs were added in August 2026 and are + available on the ONEXPLAYER 3 and ONEXPLAYER X2 Mini Pro. + + The ``monocolor`` effect displays the color selected through + the standard multicolor LED ``multi_intensity`` attribute. + The auxiliary ``breathing`` effect pulses the selected color. + Other joystick ring effects select firmware-defined presets. + The standard brightness and multicolor attributes are documented + in Documentation/ABI/testing/sysfs-class-led and + Documentation/ABI/testing/sysfs-class-led-multicolor. + + Read returns the selected effect name followed by a newline. + The value may reflect cached driver state rather than a live + hardware query. The joystick_rings LED may report ``unknown`` + when no known effect has been recorded; this is not a selectable + effect. Auxiliary effect writes queue the update asynchronously. + +What: /sys/class/leds/oxp:rgb:joystick_rings/effect_index +What: /sys/class/leds/oxp:rgb:guide_button/effect_index +What: /sys/class/leds/oxp:rgb:rear_logo/effect_index +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read-only list of effect names supported by the corresponding + LED, separated by spaces and terminated by a newline. + + The supported names are described in the effect entry above. + The list is specific to each LED and does not include ``unknown``. + The guide_button and rear_logo attributes were added in August + 2026 and are available on the ONEXPLAYER 3 and ONEXPLAYER X2 + Mini Pro. + +What: /sys/class/leds/oxp:rgb:joystick_rings/enabled +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read/write enable state for the joystick ring lighting. + + Write ``true`` to enable lighting or ``false`` to disable it. + Only these literal names are accepted, not numeric boolean + values. Read requests lighting status and returns the driver's + enable state as one of these names followed by a newline. + + This attribute is not exposed for guide_button or rear_logo. + +What: /sys/class/leds/oxp:rgb:joystick_rings/enabled_index +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read-only list of names accepted by enabled. Returns + ``false true`` followed by a newline. + +What: /sys/class/leds/oxp:rgb:joystick_rings/speed +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read/write firmware animation-speed setting for the joystick + ring lighting. Write a decimal integer from 0 through 9. + These are firmware levels, not a duration or frequency. + + Read requests lighting status and returns the driver's speed + setting as a decimal integer followed by a newline. + This attribute is not exposed for guide_button or rear_logo. + +What: /sys/class/leds/oxp:rgb:joystick_rings/speed_range +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read-only range of values accepted by speed. Returns ``0-9`` + followed by a newline; both endpoints are inclusive. + +What: /sys/bus/hid/devices/.../gamepad_mode +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read/write controller mode on the HID configuration interface + with usage page 0xff00 (shared by Gen2 and X2/Gen3 devices). Write one of the following names: + + * ``xinput``: normal gamepad input. + * ``debug``: route controller events to a hidraw interface for + input-mapping diagnostics instead of the xpad evdev interface. + + Read returns the driver's cached mode name followed by a newline, + not a firmware query. The initial cached mode is ``xinput``. + Switching to ``xinput`` also reapplies the cached rumble intensity. + + This attribute and the other controller configuration attributes + below are attached directly to the selected HID configuration device's + sysfs directory, not an input device or LED directory. They are + not exposed on Gen1 configuration interfaces. + +What: /sys/bus/hid/devices/.../gamepad_mode_index +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read-only list of supported controller modes. Returns + ``xinput debug`` followed by a newline. + +What: /sys/bus/hid/devices/.../button_a +What: /sys/bus/hid/devices/.../button_b +What: /sys/bus/hid/devices/.../button_x +What: /sys/bus/hid/devices/.../button_y +What: /sys/bus/hid/devices/.../button_lb +What: /sys/bus/hid/devices/.../button_rb +What: /sys/bus/hid/devices/.../button_lt +What: /sys/bus/hid/devices/.../button_rt +What: /sys/bus/hid/devices/.../button_start +What: /sys/bus/hid/devices/.../button_select +What: /sys/bus/hid/devices/.../button_l3 +What: /sys/bus/hid/devices/.../button_r3 +What: /sys/bus/hid/devices/.../button_d_up +What: /sys/bus/hid/devices/.../button_d_down +What: /sys/bus/hid/devices/.../button_d_left +What: /sys/bus/hid/devices/.../button_d_right +What: /sys/bus/hid/devices/.../button_m1 +What: /sys/bus/hid/devices/.../button_m2 +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read/write output mapping for the named physical controller + button on a HID configuration device with usage page 0xff00. + + Write one exact, case-sensitive mapping name from + button_mapping_options, such as ``BTN_A`` or ``KEY_F15``. + Numeric key codes, multiple names, and arbitrary key names + are not accepted. + + Read returns the driver's cached mapping name followed by a + newline. Writes update that cache and queue the firmware update + asynchronously; successful writes do not confirm that the + hardware has already applied the mapping. + +What: /sys/bus/hid/devices/.../button_mapping_options +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read-only list of output mapping names accepted by the + controller button attributes, separated by spaces and terminated by a + newline. All 18 button attributes accept the same list. + + The gamepad mapping names are: + + ``BTN_A``, ``BTN_B``, ``BTN_X``, ``BTN_Y``, ``BTN_LB``, + ``BTN_RB``, ``BTN_LT``, ``BTN_RT``, ``BTN_START``, ``BTN_SELECT``, + ``BTN_L3``, ``BTN_R3``, ``DPAD_UP``, ``DPAD_DOWN``, + ``DPAD_LEFT``, ``DPAD_RIGHT``, ``JOY_L_UP``, ``JOY_L_UP_RIGHT``, + ``JOY_L_RIGHT``, ``JOY_L_DOWN_RIGHT``, ``JOY_L_DOWN``, + ``JOY_L_DOWN_LEFT``, ``JOY_L_LEFT``, ``JOY_L_UP_LEFT``, + ``JOY_R_UP``, ``JOY_R_UP_RIGHT``, ``JOY_R_RIGHT``, + ``JOY_R_DOWN_RIGHT``, ``JOY_R_DOWN``, ``JOY_R_DOWN_LEFT``, + ``JOY_R_LEFT``, ``JOY_R_UP_LEFT``, and ``BTN_GUIDE``. + + Keyboard mapping names are ``KEY_F1`` through ``KEY_F24``. + There are no ``BTN_M1`` or ``BTN_M2`` output mapping names; + button_m1 and button_m2 select from the same options as the + other buttons. + +What: /sys/bus/hid/devices/.../reset_buttons +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Write-only reset of the controller's button mappings. + Write the decimal value ``1`` to restore the driver's default + mappings and send them to the controller. Other values are + rejected. + + Main controls map to their corresponding gamepad functions; + M1 maps to ``KEY_F15`` and M2 to ``KEY_F16``. This resets button + mappings only, not lighting, rumble, or other device settings. + It is not a factory reset. Cached mappings are reset before + the firmware update and are not rolled back if that update fails. + +What: /sys/bus/hid/devices/.../rumble_intensity +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read/write rumble-intensity level on the HID configuration + device with usage page 0xff00. Write a decimal integer from 0 through 5. These are + firmware levels, not percentages or values in physical units. + + Read returns the driver's cached level as a decimal integer + followed by a newline. The initial cached value is 5. A write + sends the firmware command before updating the cached level. + +What: /sys/bus/hid/devices/.../rumble_intensity_range +Date: April 2026 +Contact: Derek J. Clark <derekjohn.clark@gmail.com> +Description: Read-only range of values accepted by the rumble_intensity + attribute. Returns ``0-5`` followed by a newline; both endpoints + are inclusive. diff --git a/MAINTAINERS b/MAINTAINERS index c392ded..fe5a2f6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20381,6 +20381,7 @@ ONEXPLAYER HID DRIVER M: Derek J. Clark <derekjohn.clark@gmail.com> L: linux-input@vger.kernel.org S: Maintained +F: Documentation/ABI/testing/sysfs-driver-hid-oxp F: drivers/hid/hid-oxp.c ONEXPLAYER PLATFORM EC DRIVER diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c index 9a46602..757cef8 100644 --- a/drivers/hid/hid-oxp.c +++ b/drivers/hid/hid-oxp.c @@ -45,6 +45,7 @@ #define OXP_GET_PROPERTY 0xfc #define OXP_SET_PROPERTY 0xfd #define OXP_EFFECT_MONO_TRUE 0xfe /* actual index for monocolor */ +#define OXP_EFFECT_BREATHING_TRUE 0xf0 #define OXP_DEVICE_ATTR_RW(_name, _group) \ static ssize_t _name##_store(struct device *dev, \ @@ -358,6 +359,17 @@ struct oxp_gen_3_rgb_color_report { enum oxp_rgb_type { OXP_RGB_FULL, + OXP_RGB_AUX, +}; + +enum oxp_rgb_aux_effect { + OXP_RGB_AUX_EFFECT_MONOCOLOR, + OXP_RGB_AUX_EFFECT_BREATHING, +}; + +static const char *const oxp_rgb_aux_effect_text[] = { + [OXP_RGB_AUX_EFFECT_MONOCOLOR] = "monocolor", + [OXP_RGB_AUX_EFFECT_BREATHING] = "breathing", }; struct oxp_rgb_full_state { @@ -367,18 +379,30 @@ struct oxp_rgb_full_state { u8 speed; }; +struct oxp_rgb_aux_state { + enum led_brightness brightness; + bool valid; + u8 effect; + u8 green; + u8 blue; + u8 red; +}; + struct oxp_rgb_led { struct mc_subled subled_info[3]; struct led_classdev_mc mc_cdev; struct delayed_work work; struct oxp_hid_cfg *cfg; + spinlock_t state_lock; /* protects auxiliary RGB state */ void *state; u8 type; + u8 zone; }; struct oxp_rgb_led_desc { const char *name; u8 type; + u8 zone; }; struct oxp_attr { @@ -417,6 +441,23 @@ static struct oxp_rgb_full_state *oxp_rgb_full_state(struct oxp_rgb_led *led) switch (led->type) { case OXP_RGB_FULL: return led->state; + case OXP_RGB_AUX: + return NULL; + } + + return NULL; +} + +static struct oxp_rgb_aux_state *oxp_rgb_aux_state(struct oxp_rgb_led *led) +{ + if (!led) + return NULL; + + switch (led->type) { + case OXP_RGB_FULL: + return NULL; + case OXP_RGB_AUX: + return led->state; } return NULL; @@ -473,6 +514,8 @@ static int oxp_rgb_status_store(struct oxp_rgb_led *led, u8 enabled, u8 speed, u8 brightness); static int oxp_rgb_effect_set(struct oxp_rgb_led *led, u8 effect); +static void oxp_rgb_aux_restore_locked(struct oxp_rgb_led *led); + static void oxp_rgb_restore(struct oxp_hid_cfg *cfg) { struct oxp_rgb_full_state *state; @@ -510,6 +553,9 @@ static void oxp_rgb_restore(struct oxp_hid_cfg *cfg) "Error: Failed to restore RGB effect: %i\n", ret); break; + case OXP_RGB_AUX: + oxp_rgb_aux_restore_locked(led); + break; } } } @@ -842,6 +888,19 @@ static int oxp_gen_3_rgb_property_out(struct oxp_hid_cfg *cfg, u8 *data, return first_err; } +static int oxp_gen_3_rgb_zone_property_out(struct oxp_hid_cfg *cfg, + u8 *data, u8 data_size) +{ + int ret; + + ret = oxp_gen_3_property_out(cfg, data, data_size); + /* Retry once if the write or its acknowledgment failed. */ + if (ret) + ret = oxp_gen_3_property_out(cfg, data, data_size); + + return ret; +} + static void oxp_gen_3_rgb_fill_color(struct oxp_gen_3_rgb_color_report *report, u8 command, u8 zone, u8 red, u8 green, u8 blue) @@ -858,6 +917,85 @@ static void oxp_gen_3_rgb_fill_color(struct oxp_gen_3_rgb_color_report *report, report->final_green = green; } +static bool oxp_rgb_aux_snapshot(struct oxp_rgb_led *led, + struct oxp_rgb_aux_state *state) +{ + struct oxp_rgb_aux_state *cached = oxp_rgb_aux_state(led); + + if (!cached) + return false; + + scoped_guard(spinlock_irqsave, &led->state_lock) { + *state = *cached; + } + + return state->valid; +} + +static int oxp_rgb_aux_apply(struct oxp_rgb_led *led, + const struct oxp_rgb_aux_state *state) +{ + struct oxp_hid_cfg *cfg = led->cfg; + unsigned int max_brightness = led->mc_cdev.led_cdev.max_brightness; + unsigned int brightness = min_t(unsigned int, state->brightness, + max_brightness); + u8 status_data[6] = { OXP_SET_PROPERTY, led->zone, 0x02, + OXP_FEAT_DISABLED, 0x05, 0x04 }; + struct oxp_gen_3_rgb_color_report color_report; + u8 command; + u8 green; + u8 blue; + u8 red; + int ret; + + if (brightness) + status_data[3] = OXP_FEAT_ENABLED; + + ret = oxp_gen_3_rgb_zone_property_out(cfg, status_data, + sizeof(status_data)); + if (ret || !brightness) + return ret; + + /* Scale each RGB channel by brightness, rounding to the nearest integer. */ + red = ((unsigned int)state->red * brightness + max_brightness / 2) / + max_brightness; + green = ((unsigned int)state->green * brightness + max_brightness / 2) / + max_brightness; + blue = ((unsigned int)state->blue * brightness + max_brightness / 2) / + max_brightness; + + switch (state->effect) { + case OXP_RGB_AUX_EFFECT_MONOCOLOR: + command = OXP_EFFECT_MONO_TRUE; + break; + case OXP_RGB_AUX_EFFECT_BREATHING: + command = OXP_EFFECT_BREATHING_TRUE; + break; + default: + return -EINVAL; + } + + oxp_gen_3_rgb_fill_color(&color_report, command, led->zone, red, green, + blue); + return oxp_gen_3_rgb_zone_property_out(cfg, (u8 *)&color_report, + sizeof(color_report)); +} + +static void oxp_rgb_aux_restore_locked(struct oxp_rgb_led *led) +{ + struct oxp_hid_cfg *cfg = led->cfg; + struct oxp_rgb_aux_state state; + int ret; + + if (!oxp_rgb_aux_snapshot(led, &state)) + return; + + ret = oxp_rgb_aux_apply(led, &state); + if (ret) + dev_err(&cfg->hdev->dev, + "Failed to restore RGB zone %#04x: %i\n", led->zone, ret); +} + static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) @@ -1913,6 +2051,25 @@ static void oxp_gen_3_rgb_queue(struct oxp_rgb_led *led, "Error: Failed to write RGB color: %i\n", ret); } +static void oxp_rgb_aux_queue(struct oxp_rgb_led *led) +{ + struct oxp_hid_cfg *cfg = led->cfg; + struct oxp_rgb_aux_state state; + int ret; + + if (!oxp_rgb_aux_snapshot(led, &state)) + return; + + guard(mutex)(&cfg->rgb_mutex); + if (READ_ONCE(cfg->suspended) || READ_ONCE(cfg->removing)) + return; + + ret = oxp_rgb_aux_apply(led, &state); + if (ret) + dev_err(led->mc_cdev.led_cdev.dev, + "Failed to write RGB zone %#04x: %i\n", led->zone, ret); +} + static void oxp_rgb_queue_fn(struct work_struct *work) { struct oxp_rgb_led *led = container_of(to_delayed_work(work), @@ -1929,6 +2086,9 @@ static void oxp_rgb_queue_fn(struct work_struct *work) else oxp_rgb_full_queue(led, led->state); break; + case OXP_RGB_AUX: + oxp_rgb_aux_queue(led); + break; } } @@ -1937,16 +2097,112 @@ static void oxp_rgb_brightness_set(struct led_classdev *led_cdev, { struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev); struct oxp_rgb_led *led = container_of(mc_cdev, struct oxp_rgb_led, - mc_cdev); + mc_cdev); struct oxp_hid_cfg *cfg = led->cfg; + struct oxp_rgb_aux_state *state; - if (READ_ONCE(cfg->suspended) || READ_ONCE(cfg->removing)) + if (READ_ONCE(cfg->removing)) + return; + + switch (led->type) { + case OXP_RGB_FULL: + if (READ_ONCE(cfg->suspended)) + return; + led_cdev->brightness = brightness; + break; + case OXP_RGB_AUX: + state = led->state; + led_cdev->brightness = brightness; + scoped_guard(spinlock_irqsave, &led->state_lock) { + state->brightness = brightness; + state->red = led->subled_info[0].intensity; + state->green = led->subled_info[1].intensity; + state->blue = led->subled_info[2].intensity; + state->valid = true; + } + if (READ_ONCE(cfg->suspended)) + return; + break; + default: return; + } - led_cdev->brightness = brightness; mod_delayed_work(system_dfl_wq, &led->work, msecs_to_jiffies(50)); } +static ssize_t oxp_rgb_aux_effect_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct oxp_rgb_led *led = oxp_rgb_led_from_dev(dev); + struct oxp_hid_cfg *cfg = led->cfg; + struct oxp_rgb_aux_state *state = oxp_rgb_aux_state(led); + int ret; + + if (!state || READ_ONCE(cfg->removing)) + return -ENODEV; + + ret = sysfs_match_string(oxp_rgb_aux_effect_text, buf); + if (ret < 0) + return ret; + + scoped_guard(spinlock_irqsave, &led->state_lock) { + state->effect = ret; + state->valid = true; + } + + if (!READ_ONCE(cfg->suspended) && !READ_ONCE(cfg->removing)) + mod_delayed_work(system_dfl_wq, &led->work, + msecs_to_jiffies(50)); + + return count; +} + +static ssize_t oxp_rgb_aux_effect_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct oxp_rgb_led *led = oxp_rgb_led_from_dev(dev); + struct oxp_hid_cfg *cfg = led->cfg; + struct oxp_rgb_aux_state state; + + if (!oxp_rgb_aux_state(led) || READ_ONCE(cfg->removing)) + return -ENODEV; + + oxp_rgb_aux_snapshot(led, &state); + if (state.effect >= ARRAY_SIZE(oxp_rgb_aux_effect_text)) + return -EINVAL; + + return sysfs_emit(buf, "%s\n", oxp_rgb_aux_effect_text[state.effect]); +} +static DEVICE_ATTR_RW_NAMED(oxp_rgb_aux_effect, "effect"); + +static ssize_t oxp_rgb_aux_effect_index_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + int count = 0; + int i; + + for (i = 0; i < ARRAY_SIZE(oxp_rgb_aux_effect_text); i++) + count += sysfs_emit_at(buf, count, "%s ", + oxp_rgb_aux_effect_text[i]); + if (count) + buf[count - 1] = '\n'; + + return count; +} +static DEVICE_ATTR_RO_NAMED(oxp_rgb_aux_effect_index, "effect_index"); + +static struct attribute *oxp_rgb_aux_attrs[] = { + &dev_attr_oxp_rgb_aux_effect.attr, + &dev_attr_oxp_rgb_aux_effect_index.attr, + NULL, +}; + +static const struct attribute_group oxp_rgb_aux_attr_group = { + .attrs = oxp_rgb_aux_attrs, +}; + static struct attribute *oxp_rgb_attrs[] = { &dev_attr_effect.attr, &dev_attr_effect_index.attr, @@ -1966,12 +2222,23 @@ static const struct oxp_rgb_led_desc oxp_rgb_led_descs[] = { .name = "oxp:rgb:joystick_rings", .type = OXP_RGB_FULL, }, + { + .name = "oxp:rgb:guide_button", + .type = OXP_RGB_AUX, + .zone = 0x05, + }, + { + .name = "oxp:rgb:rear_logo", + .type = OXP_RGB_AUX, + .zone = 0x06, + }, }; static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, const struct oxp_rgb_led_desc *desc) { struct oxp_rgb_full_state *full_state; + struct oxp_rgb_aux_state *aux_state; struct hid_device *hdev = cfg->hdev; u8 green; u8 blue; @@ -1979,6 +2246,7 @@ static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, led->cfg = cfg; led->type = desc->type; + led->zone = desc->zone; switch (led->type) { case OXP_RGB_FULL: @@ -1992,6 +2260,20 @@ static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, green = 0x22; blue = 0x99; break; + case OXP_RGB_AUX: + aux_state = devm_kzalloc(&hdev->dev, sizeof(*aux_state), + GFP_KERNEL); + if (!aux_state) + return -ENOMEM; + aux_state->red = 0xff; + aux_state->green = 0xff; + aux_state->blue = 0xff; + aux_state->effect = OXP_RGB_AUX_EFFECT_MONOCOLOR; + led->state = aux_state; + red = 0xff; + green = 0xff; + blue = 0xff; + break; default: return -EINVAL; } @@ -2020,6 +2302,7 @@ static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, led->mc_cdev.led_cdev.brightness_set = oxp_rgb_brightness_set; led->mc_cdev.num_colors = ARRAY_SIZE(led->subled_info); led->mc_cdev.subled_info = led->subled_info; + spin_lock_init(&led->state_lock); INIT_DELAYED_WORK(&led->work, oxp_rgb_queue_fn); return 0; @@ -2027,7 +2310,8 @@ static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, static int oxp_rgb_leds_register(struct oxp_hid_cfg *cfg) { - int led_count = ARRAY_SIZE(oxp_rgb_led_descs); + int led_count = cfg->x2_rgb ? ARRAY_SIZE(oxp_rgb_led_descs) : 1; + const struct attribute_group *attr_group; struct hid_device *hdev = cfg->hdev; struct oxp_rgb_led *led; int ret; @@ -2051,8 +2335,19 @@ static int oxp_rgb_leds_register(struct oxp_hid_cfg *cfg) return dev_err_probe(&hdev->dev, ret, "Failed to create RGB device\n"); + switch (led->type) { + case OXP_RGB_FULL: + attr_group = &oxp_rgb_attr_group; + break; + case OXP_RGB_AUX: + attr_group = &oxp_rgb_aux_attr_group; + break; + default: + return -EINVAL; + } + ret = devm_device_add_group(led->mc_cdev.led_cdev.dev, - &oxp_rgb_attr_group); + attr_group); if (ret) return dev_err_probe(led->mc_cdev.led_cdev.dev, ret, "Failed to create RGB configuration attributes\n"); ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 15/15] HID: hid-oxp: add X2 auxiliary RGB zones 2026-09-10 3:21 ` [PATCH 15/15] HID: hid-oxp: add X2 auxiliary RGB zones Andrei Aldea @ 2026-09-10 20:20 ` Derek J. Clark 0 siblings, 0 replies; 4+ messages in thread From: Derek J. Clark @ 2026-09-10 20:20 UTC (permalink / raw) To: Andrei Aldea, Jiri Kosina, Benjamin Tissoires Cc: linux-input, linux-kernel, Lee Jones, Pavel Machek, linux-leds, linux-api On 9/9/26 20:21, Andrei Aldea wrote: > Add separate multicolor LED devices for the X2 guide button and rear logo > using Gen3 zones 5 and 6. Each LED retains independent color, brightness, > and monocolor or breathing effect state. > > Use the common LED array and type-tagged state introduced earlier. > Protect auxiliary state snapshots with scoped spinlocks, serialize output > with other RGB operations, and retry a failed command once. Do not alter > an auxiliary zone until userspace has supplied valid state, and restore > valid cached settings after resume. > > Expose these zones only on the matched ONEXPLAYER 3 and X2 Mini Pro. > Physical validation of the latter remains outstanding. > > Document the auxiliary effects together with the existing joystick-ring > effects and add the previously omitted controller, mapping, rumble, and > ring-lighting ABI. Add the ABI file to the ONEXPLAYER HID maintainer > pattern. > > Assisted-by: LLM > Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com> > Signed-off-by: Andrei Aldea <andrei1998@gmail.com> > --- > .../ABI/testing/sysfs-driver-hid-oxp | 196 +++++++++++ > MAINTAINERS | 1 + > drivers/hid/hid-oxp.c | 305 +++++++++++++++++- > 3 files changed, 497 insertions(+), 5 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-oxp > > diff --git a/Documentation/ABI/testing/sysfs-driver-hid-oxp b/Documentation/ABI/testing/sysfs-driver-hid-oxp > new file mode 100644 > index 0000000..7c97f28 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-hid-oxp > @@ -0,0 +1,196 @@ > +What: /sys/class/leds/oxp:rgb:joystick_rings/effect > +What: /sys/class/leds/oxp:rgb:guide_button/effect > +What: /sys/class/leds/oxp:rgb:rear_logo/effect > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read/write attribute selecting the lighting effect for the LED. > + > + The joystick_rings LED supports the following effect names: > + > + ``aurora``, ``birthday_cake``, ``flowing_light``, > + ``chroma_popping``, ``neon``, ``chroma_breathing``, ``dreamy``, > + ``warm_sun``, ``cyberpunk``, ``sea_foam``, ``sunset_afterglow``, > + ``colorful``, ``monster_woke``, ``green_breathing``, > + ``blue_breathing``, ``yellow_breathing``, ``teal_breathing``, > + ``purple_breathing``, ``foggy_haze``, and ``monocolor``. > + > + The guide_button and rear_logo LEDs support ``monocolor`` and > + ``breathing``. These two LEDs were added in August 2026 and are > + available on the ONEXPLAYER 3 and ONEXPLAYER X2 Mini Pro. > + > + The ``monocolor`` effect displays the color selected through > + the standard multicolor LED ``multi_intensity`` attribute. > + The auxiliary ``breathing`` effect pulses the selected color. > + Other joystick ring effects select firmware-defined presets. > + The standard brightness and multicolor attributes are documented > + in Documentation/ABI/testing/sysfs-class-led and > + Documentation/ABI/testing/sysfs-class-led-multicolor. > + > + Read returns the selected effect name followed by a newline. > + The value may reflect cached driver state rather than a live > + hardware query. The joystick_rings LED may report ``unknown`` > + when no known effect has been recorded; this is not a selectable > + effect. Auxiliary effect writes queue the update asynchronously. > + > +What: /sys/class/leds/oxp:rgb:joystick_rings/effect_index > +What: /sys/class/leds/oxp:rgb:guide_button/effect_index > +What: /sys/class/leds/oxp:rgb:rear_logo/effect_index > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read-only list of effect names supported by the corresponding > + LED, separated by spaces and terminated by a newline. > + > + The supported names are described in the effect entry above. > + The list is specific to each LED and does not include ``unknown``. > + The guide_button and rear_logo attributes were added in August > + 2026 and are available on the ONEXPLAYER 3 and ONEXPLAYER X2 > + Mini Pro. > + > +What: /sys/class/leds/oxp:rgb:joystick_rings/enabled > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read/write enable state for the joystick ring lighting. > + > + Write ``true`` to enable lighting or ``false`` to disable it. > + Only these literal names are accepted, not numeric boolean > + values. Read requests lighting status and returns the driver's > + enable state as one of these names followed by a newline. > + > + This attribute is not exposed for guide_button or rear_logo. > + > +What: /sys/class/leds/oxp:rgb:joystick_rings/enabled_index > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read-only list of names accepted by enabled. Returns > + ``false true`` followed by a newline. > + > +What: /sys/class/leds/oxp:rgb:joystick_rings/speed > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read/write firmware animation-speed setting for the joystick > + ring lighting. Write a decimal integer from 0 through 9. > + These are firmware levels, not a duration or frequency. > + > + Read requests lighting status and returns the driver's speed > + setting as a decimal integer followed by a newline. > + This attribute is not exposed for guide_button or rear_logo. > + > +What: /sys/class/leds/oxp:rgb:joystick_rings/speed_range > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read-only range of values accepted by speed. Returns ``0-9`` > + followed by a newline; both endpoints are inclusive. > + > +What: /sys/bus/hid/devices/.../gamepad_mode > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read/write controller mode on the HID configuration interface > + with usage page 0xff00 (shared by Gen2 and X2/Gen3 devices). Write one of the following names: > + > + * ``xinput``: normal gamepad input. > + * ``debug``: route controller events to a hidraw interface for > + input-mapping diagnostics instead of the xpad evdev interface. > + > + Read returns the driver's cached mode name followed by a newline, > + not a firmware query. The initial cached mode is ``xinput``. > + Switching to ``xinput`` also reapplies the cached rumble intensity. > + > + This attribute and the other controller configuration attributes > + below are attached directly to the selected HID configuration device's > + sysfs directory, not an input device or LED directory. They are > + not exposed on Gen1 configuration interfaces. > + > +What: /sys/bus/hid/devices/.../gamepad_mode_index > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read-only list of supported controller modes. Returns > + ``xinput debug`` followed by a newline. > + > +What: /sys/bus/hid/devices/.../button_a > +What: /sys/bus/hid/devices/.../button_b > +What: /sys/bus/hid/devices/.../button_x > +What: /sys/bus/hid/devices/.../button_y > +What: /sys/bus/hid/devices/.../button_lb > +What: /sys/bus/hid/devices/.../button_rb > +What: /sys/bus/hid/devices/.../button_lt > +What: /sys/bus/hid/devices/.../button_rt > +What: /sys/bus/hid/devices/.../button_start > +What: /sys/bus/hid/devices/.../button_select > +What: /sys/bus/hid/devices/.../button_l3 > +What: /sys/bus/hid/devices/.../button_r3 > +What: /sys/bus/hid/devices/.../button_d_up > +What: /sys/bus/hid/devices/.../button_d_down > +What: /sys/bus/hid/devices/.../button_d_left > +What: /sys/bus/hid/devices/.../button_d_right > +What: /sys/bus/hid/devices/.../button_m1 > +What: /sys/bus/hid/devices/.../button_m2 > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read/write output mapping for the named physical controller > + button on a HID configuration device with usage page 0xff00. > + > + Write one exact, case-sensitive mapping name from > + button_mapping_options, such as ``BTN_A`` or ``KEY_F15``. > + Numeric key codes, multiple names, and arbitrary key names > + are not accepted. > + > + Read returns the driver's cached mapping name followed by a > + newline. Writes update that cache and queue the firmware update > + asynchronously; successful writes do not confirm that the > + hardware has already applied the mapping. > + > +What: /sys/bus/hid/devices/.../button_mapping_options > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read-only list of output mapping names accepted by the > + controller button attributes, separated by spaces and terminated by a > + newline. All 18 button attributes accept the same list. > + > + The gamepad mapping names are: > + > + ``BTN_A``, ``BTN_B``, ``BTN_X``, ``BTN_Y``, ``BTN_LB``, > + ``BTN_RB``, ``BTN_LT``, ``BTN_RT``, ``BTN_START``, ``BTN_SELECT``, > + ``BTN_L3``, ``BTN_R3``, ``DPAD_UP``, ``DPAD_DOWN``, > + ``DPAD_LEFT``, ``DPAD_RIGHT``, ``JOY_L_UP``, ``JOY_L_UP_RIGHT``, > + ``JOY_L_RIGHT``, ``JOY_L_DOWN_RIGHT``, ``JOY_L_DOWN``, > + ``JOY_L_DOWN_LEFT``, ``JOY_L_LEFT``, ``JOY_L_UP_LEFT``, > + ``JOY_R_UP``, ``JOY_R_UP_RIGHT``, ``JOY_R_RIGHT``, > + ``JOY_R_DOWN_RIGHT``, ``JOY_R_DOWN``, ``JOY_R_DOWN_LEFT``, > + ``JOY_R_LEFT``, ``JOY_R_UP_LEFT``, and ``BTN_GUIDE``. > + > + Keyboard mapping names are ``KEY_F1`` through ``KEY_F24``. > + There are no ``BTN_M1`` or ``BTN_M2`` output mapping names; > + button_m1 and button_m2 select from the same options as the > + other buttons. > + > +What: /sys/bus/hid/devices/.../reset_buttons > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Write-only reset of the controller's button mappings. > + Write the decimal value ``1`` to restore the driver's default > + mappings and send them to the controller. Other values are > + rejected. > + > + Main controls map to their corresponding gamepad functions; > + M1 maps to ``KEY_F15`` and M2 to ``KEY_F16``. This resets button > + mappings only, not lighting, rumble, or other device settings. > + It is not a factory reset. Cached mappings are reset before > + the firmware update and are not rolled back if that update fails. > + > +What: /sys/bus/hid/devices/.../rumble_intensity > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read/write rumble-intensity level on the HID configuration > + device with usage page 0xff00. Write a decimal integer from 0 through 5. These are > + firmware levels, not percentages or values in physical units. > + > + Read returns the driver's cached level as a decimal integer > + followed by a newline. The initial cached value is 5. A write > + sends the firmware command before updating the cached level. > + > +What: /sys/bus/hid/devices/.../rumble_intensity_range > +Date: April 2026 > +Contact: Derek J. Clark <derekjohn.clark@gmail.com> > +Description: Read-only range of values accepted by the rumble_intensity > + attribute. Returns ``0-5`` followed by a newline; both endpoints > + are inclusive. > diff --git a/MAINTAINERS b/MAINTAINERS > index c392ded..fe5a2f6 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -20381,6 +20381,7 @@ ONEXPLAYER HID DRIVER > M: Derek J. Clark <derekjohn.clark@gmail.com> > L: linux-input@vger.kernel.org > S: Maintained > +F: Documentation/ABI/testing/sysfs-driver-hid-oxp > F: drivers/hid/hid-oxp.c > > ONEXPLAYER PLATFORM EC DRIVER > diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c > index 9a46602..757cef8 100644 > --- a/drivers/hid/hid-oxp.c > +++ b/drivers/hid/hid-oxp.c > @@ -45,6 +45,7 @@ > #define OXP_GET_PROPERTY 0xfc > #define OXP_SET_PROPERTY 0xfd > #define OXP_EFFECT_MONO_TRUE 0xfe /* actual index for monocolor */ > +#define OXP_EFFECT_BREATHING_TRUE 0xf0 > > #define OXP_DEVICE_ATTR_RW(_name, _group) \ > static ssize_t _name##_store(struct device *dev, \ > @@ -358,6 +359,17 @@ struct oxp_gen_3_rgb_color_report { > > enum oxp_rgb_type { > OXP_RGB_FULL, > + OXP_RGB_AUX, > +}; > + > +enum oxp_rgb_aux_effect { > + OXP_RGB_AUX_EFFECT_MONOCOLOR, > + OXP_RGB_AUX_EFFECT_BREATHING, > +}; > + > +static const char *const oxp_rgb_aux_effect_text[] = { > + [OXP_RGB_AUX_EFFECT_MONOCOLOR] = "monocolor", > + [OXP_RGB_AUX_EFFECT_BREATHING] = "breathing", > }; > > struct oxp_rgb_full_state { > @@ -367,18 +379,30 @@ struct oxp_rgb_full_state { > u8 speed; > }; > > +struct oxp_rgb_aux_state { > + enum led_brightness brightness; > + bool valid; > + u8 effect; > + u8 green; > + u8 blue; > + u8 red; > +}; > + > struct oxp_rgb_led { > struct mc_subled subled_info[3]; > struct led_classdev_mc mc_cdev; > struct delayed_work work; > struct oxp_hid_cfg *cfg; > + spinlock_t state_lock; /* protects auxiliary RGB state */ > void *state; > u8 type; > + u8 zone; > }; > > struct oxp_rgb_led_desc { > const char *name; > u8 type; > + u8 zone; > }; > > struct oxp_attr { > @@ -417,6 +441,23 @@ static struct oxp_rgb_full_state *oxp_rgb_full_state(struct oxp_rgb_led *led) > switch (led->type) { > case OXP_RGB_FULL: > return led->state; > + case OXP_RGB_AUX: > + return NULL; > + } > + > + return NULL; > +} > + > +static struct oxp_rgb_aux_state *oxp_rgb_aux_state(struct oxp_rgb_led *led) > +{ > + if (!led) > + return NULL; > + > + switch (led->type) { > + case OXP_RGB_FULL: > + return NULL; > + case OXP_RGB_AUX: > + return led->state; > } > > return NULL; > @@ -473,6 +514,8 @@ static int oxp_rgb_status_store(struct oxp_rgb_led *led, u8 enabled, u8 speed, > u8 brightness); > static int oxp_rgb_effect_set(struct oxp_rgb_led *led, u8 effect); > > +static void oxp_rgb_aux_restore_locked(struct oxp_rgb_led *led); > + > static void oxp_rgb_restore(struct oxp_hid_cfg *cfg) > { > struct oxp_rgb_full_state *state; > @@ -510,6 +553,9 @@ static void oxp_rgb_restore(struct oxp_hid_cfg *cfg) > "Error: Failed to restore RGB effect: %i\n", > ret); > break; > + case OXP_RGB_AUX: > + oxp_rgb_aux_restore_locked(led); > + break; > } > } > } > @@ -842,6 +888,19 @@ static int oxp_gen_3_rgb_property_out(struct oxp_hid_cfg *cfg, u8 *data, > return first_err; > } > > +static int oxp_gen_3_rgb_zone_property_out(struct oxp_hid_cfg *cfg, > + u8 *data, u8 data_size) > +{ > + int ret; > + > + ret = oxp_gen_3_property_out(cfg, data, data_size); > + /* Retry once if the write or its acknowledgment failed. */ > + if (ret) > + ret = oxp_gen_3_property_out(cfg, data, data_size); > + > + return ret; > +} > + > static void oxp_gen_3_rgb_fill_color(struct oxp_gen_3_rgb_color_report *report, > u8 command, u8 zone, u8 red, u8 green, > u8 blue) > @@ -858,6 +917,85 @@ static void oxp_gen_3_rgb_fill_color(struct oxp_gen_3_rgb_color_report *report, > report->final_green = green; > } > > +static bool oxp_rgb_aux_snapshot(struct oxp_rgb_led *led, > + struct oxp_rgb_aux_state *state) > +{ > + struct oxp_rgb_aux_state *cached = oxp_rgb_aux_state(led); > + > + if (!cached) > + return false; > + > + scoped_guard(spinlock_irqsave, &led->state_lock) { > + *state = *cached; > + } > + > + return state->valid; > +} > + > +static int oxp_rgb_aux_apply(struct oxp_rgb_led *led, > + const struct oxp_rgb_aux_state *state) > +{ > + struct oxp_hid_cfg *cfg = led->cfg; > + unsigned int max_brightness = led->mc_cdev.led_cdev.max_brightness; > + unsigned int brightness = min_t(unsigned int, state->brightness, > + max_brightness); > + u8 status_data[6] = { OXP_SET_PROPERTY, led->zone, 0x02, > + OXP_FEAT_DISABLED, 0x05, 0x04 }; > + struct oxp_gen_3_rgb_color_report color_report; > + u8 command; > + u8 green; > + u8 blue; > + u8 red; > + int ret; > + > + if (brightness) > + status_data[3] = OXP_FEAT_ENABLED; > + > + ret = oxp_gen_3_rgb_zone_property_out(cfg, status_data, > + sizeof(status_data)); > + if (ret || !brightness) > + return ret; > + > + /* Scale each RGB channel by brightness, rounding to the nearest integer. */ > + red = ((unsigned int)state->red * brightness + max_brightness / 2) / > + max_brightness; > + green = ((unsigned int)state->green * brightness + max_brightness / 2) / > + max_brightness; > + blue = ((unsigned int)state->blue * brightness + max_brightness / 2) / > + max_brightness; > + > + switch (state->effect) { > + case OXP_RGB_AUX_EFFECT_MONOCOLOR: > + command = OXP_EFFECT_MONO_TRUE; > + break; > + case OXP_RGB_AUX_EFFECT_BREATHING: > + command = OXP_EFFECT_BREATHING_TRUE; > + break; > + default: > + return -EINVAL; > + } > + > + oxp_gen_3_rgb_fill_color(&color_report, command, led->zone, red, green, > + blue); > + return oxp_gen_3_rgb_zone_property_out(cfg, (u8 *)&color_report, > + sizeof(color_report)); > +} > + > +static void oxp_rgb_aux_restore_locked(struct oxp_rgb_led *led) > +{ > + struct oxp_hid_cfg *cfg = led->cfg; > + struct oxp_rgb_aux_state state; > + int ret; > + > + if (!oxp_rgb_aux_snapshot(led, &state)) > + return; > + > + ret = oxp_rgb_aux_apply(led, &state); > + if (ret) > + dev_err(&cfg->hdev->dev, > + "Failed to restore RGB zone %#04x: %i\n", led->zone, ret); > +} > + > static ssize_t gamepad_mode_store(struct device *dev, > struct device_attribute *attr, const char *buf, > size_t count) > @@ -1913,6 +2051,25 @@ static void oxp_gen_3_rgb_queue(struct oxp_rgb_led *led, > "Error: Failed to write RGB color: %i\n", ret); > } > > +static void oxp_rgb_aux_queue(struct oxp_rgb_led *led) > +{ > + struct oxp_hid_cfg *cfg = led->cfg; > + struct oxp_rgb_aux_state state; > + int ret; > + > + if (!oxp_rgb_aux_snapshot(led, &state)) > + return; > + > + guard(mutex)(&cfg->rgb_mutex); > + if (READ_ONCE(cfg->suspended) || READ_ONCE(cfg->removing)) > + return; > + > + ret = oxp_rgb_aux_apply(led, &state); > + if (ret) > + dev_err(led->mc_cdev.led_cdev.dev, > + "Failed to write RGB zone %#04x: %i\n", led->zone, ret); > +} > + > static void oxp_rgb_queue_fn(struct work_struct *work) > { > struct oxp_rgb_led *led = container_of(to_delayed_work(work), > @@ -1929,6 +2086,9 @@ static void oxp_rgb_queue_fn(struct work_struct *work) > else > oxp_rgb_full_queue(led, led->state); > break; > + case OXP_RGB_AUX: > + oxp_rgb_aux_queue(led); > + break; > } > } > > @@ -1937,16 +2097,112 @@ static void oxp_rgb_brightness_set(struct led_classdev *led_cdev, > { > struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev); > struct oxp_rgb_led *led = container_of(mc_cdev, struct oxp_rgb_led, > - mc_cdev); > + mc_cdev); > struct oxp_hid_cfg *cfg = led->cfg; > + struct oxp_rgb_aux_state *state; > > - if (READ_ONCE(cfg->suspended) || READ_ONCE(cfg->removing)) > + if (READ_ONCE(cfg->removing)) > + return; > + > + switch (led->type) { > + case OXP_RGB_FULL: > + if (READ_ONCE(cfg->suspended)) > + return; > + led_cdev->brightness = brightness; > + break; > + case OXP_RGB_AUX: > + state = led->state; > + led_cdev->brightness = brightness; > + scoped_guard(spinlock_irqsave, &led->state_lock) { > + state->brightness = brightness; > + state->red = led->subled_info[0].intensity; > + state->green = led->subled_info[1].intensity; > + state->blue = led->subled_info[2].intensity; > + state->valid = true; > + } > + if (READ_ONCE(cfg->suspended)) > + return; > + break; > + default: > return; > + } > > - led_cdev->brightness = brightness; > mod_delayed_work(system_dfl_wq, &led->work, msecs_to_jiffies(50)); > } > > +static ssize_t oxp_rgb_aux_effect_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct oxp_rgb_led *led = oxp_rgb_led_from_dev(dev); > + struct oxp_hid_cfg *cfg = led->cfg; > + struct oxp_rgb_aux_state *state = oxp_rgb_aux_state(led); > + int ret; > + > + if (!state || READ_ONCE(cfg->removing)) > + return -ENODEV; > + > + ret = sysfs_match_string(oxp_rgb_aux_effect_text, buf); > + if (ret < 0) > + return ret; > + > + scoped_guard(spinlock_irqsave, &led->state_lock) { > + state->effect = ret; > + state->valid = true; > + } > + > + if (!READ_ONCE(cfg->suspended) && !READ_ONCE(cfg->removing)) > + mod_delayed_work(system_dfl_wq, &led->work, > + msecs_to_jiffies(50)); > + > + return count; > +} > + > +static ssize_t oxp_rgb_aux_effect_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct oxp_rgb_led *led = oxp_rgb_led_from_dev(dev); > + struct oxp_hid_cfg *cfg = led->cfg; > + struct oxp_rgb_aux_state state; > + > + if (!oxp_rgb_aux_state(led) || READ_ONCE(cfg->removing)) > + return -ENODEV; > + > + oxp_rgb_aux_snapshot(led, &state); > + if (state.effect >= ARRAY_SIZE(oxp_rgb_aux_effect_text)) > + return -EINVAL; > + > + return sysfs_emit(buf, "%s\n", oxp_rgb_aux_effect_text[state.effect]); > +} > +static DEVICE_ATTR_RW_NAMED(oxp_rgb_aux_effect, "effect"); > + > +static ssize_t oxp_rgb_aux_effect_index_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + int count = 0; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(oxp_rgb_aux_effect_text); i++) > + count += sysfs_emit_at(buf, count, "%s ", > + oxp_rgb_aux_effect_text[i]); > + if (count) > + buf[count - 1] = '\n'; > + > + return count; > +} > +static DEVICE_ATTR_RO_NAMED(oxp_rgb_aux_effect_index, "effect_index"); > + > +static struct attribute *oxp_rgb_aux_attrs[] = { > + &dev_attr_oxp_rgb_aux_effect.attr, > + &dev_attr_oxp_rgb_aux_effect_index.attr, > + NULL, > +}; > + > +static const struct attribute_group oxp_rgb_aux_attr_group = { > + .attrs = oxp_rgb_aux_attrs, > +}; > + > static struct attribute *oxp_rgb_attrs[] = { > &dev_attr_effect.attr, > &dev_attr_effect_index.attr, > @@ -1966,12 +2222,23 @@ static const struct oxp_rgb_led_desc oxp_rgb_led_descs[] = { > .name = "oxp:rgb:joystick_rings", > .type = OXP_RGB_FULL, > }, > + { > + .name = "oxp:rgb:guide_button", > + .type = OXP_RGB_AUX, > + .zone = 0x05, > + }, > + { > + .name = "oxp:rgb:rear_logo", > + .type = OXP_RGB_AUX, > + .zone = 0x06, > + }, > }; > > static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, > const struct oxp_rgb_led_desc *desc) > { > struct oxp_rgb_full_state *full_state; > + struct oxp_rgb_aux_state *aux_state; > struct hid_device *hdev = cfg->hdev; > u8 green; > u8 blue; > @@ -1979,6 +2246,7 @@ static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, > > led->cfg = cfg; > led->type = desc->type; > + led->zone = desc->zone; > > switch (led->type) { > case OXP_RGB_FULL: > @@ -1992,6 +2260,20 @@ static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, > green = 0x22; > blue = 0x99; > break; > + case OXP_RGB_AUX: > + aux_state = devm_kzalloc(&hdev->dev, sizeof(*aux_state), > + GFP_KERNEL); > + if (!aux_state) > + return -ENOMEM; > + aux_state->red = 0xff; > + aux_state->green = 0xff; > + aux_state->blue = 0xff; > + aux_state->effect = OXP_RGB_AUX_EFFECT_MONOCOLOR; > + led->state = aux_state; > + red = 0xff; > + green = 0xff; > + blue = 0xff; > + break; > default: > return -EINVAL; > } > @@ -2020,6 +2302,7 @@ static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, > led->mc_cdev.led_cdev.brightness_set = oxp_rgb_brightness_set; > led->mc_cdev.num_colors = ARRAY_SIZE(led->subled_info); > led->mc_cdev.subled_info = led->subled_info; > + spin_lock_init(&led->state_lock); > INIT_DELAYED_WORK(&led->work, oxp_rgb_queue_fn); > > return 0; > @@ -2027,7 +2310,8 @@ static int oxp_rgb_led_init(struct oxp_hid_cfg *cfg, struct oxp_rgb_led *led, > > static int oxp_rgb_leds_register(struct oxp_hid_cfg *cfg) > { > - int led_count = ARRAY_SIZE(oxp_rgb_led_descs); > + int led_count = cfg->x2_rgb ? ARRAY_SIZE(oxp_rgb_led_descs) : 1; > + const struct attribute_group *attr_group; > struct hid_device *hdev = cfg->hdev; > struct oxp_rgb_led *led; > int ret; > @@ -2051,8 +2335,19 @@ static int oxp_rgb_leds_register(struct oxp_hid_cfg *cfg) > return dev_err_probe(&hdev->dev, ret, > "Failed to create RGB device\n"); > > + switch (led->type) { > + case OXP_RGB_FULL: > + attr_group = &oxp_rgb_attr_group; > + break; > + case OXP_RGB_AUX: > + attr_group = &oxp_rgb_aux_attr_group; > + break; > + default: > + return -EINVAL; > + } > + > ret = devm_device_add_group(led->mc_cdev.led_cdev.dev, > - &oxp_rgb_attr_group); > + attr_group); > if (ret) > return dev_err_probe(led->mc_cdev.led_cdev.dev, ret, > "Failed to create RGB configuration attributes\n"); I'm unable to test as these features are not available on my OneXPlayer F1 Pro. Reviewed-by: Derek J. Clark <derekjohn.clark@gmail.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 00/15] HID: hid-oxp: fix and extend X2-family controller support 2026-09-10 3:21 [PATCH 00/15] HID: hid-oxp: fix and extend X2-family controller support Andrei Aldea 2026-09-10 3:21 ` [PATCH 15/15] HID: hid-oxp: add X2 auxiliary RGB zones Andrei Aldea @ 2026-09-10 20:24 ` Derek J. Clark 1 sibling, 0 replies; 4+ messages in thread From: Derek J. Clark @ 2026-09-10 20:24 UTC (permalink / raw) To: Andrei Aldea, Jiri Kosina, Benjamin Tissoires Cc: linux-input, linux-kernel, Lee Jones, Pavel Machek, linux-leds, linux-api On 9/9/26 20:21, Andrei Aldea wrote: > This 15-patch series fixes existing hid-oxp configuration and RGB issues > and adds controller and lighting support for ONEXPLAYER X2-family devices, > including the ONEXPLAYER 3 and X2 Mini Pro. > > The fixes correct the M1/M2 defaults, validate input reports and RGB state, > correct brightness and color scaling, serialize RGB updates, and make > configuration state and delayed-work lifetime local to each HID interface. > They also handle controller reinitialization across system suspend. > > The X2 support selects configuration interface 2, supplies the required > three-page button map, and adds the Gen3 protocol for joystick-ring zones > 1, 2 and 7. The Guide button and rear logo receive independent multicolor > LED devices using zones 5 and 6, with monocolor and breathing effects. > The final patch documents the controller and lighting sysfs ABI. > > Patches 1-10 contain behavioral fixes. Patch 11 groups existing declarations. > Patch 12 adds X2 button maps and interface selection; patch 13 provides the > shared LED representation; patches 14-15 add X2 lighting support. > > This series is based on mainline commit > 50d05c7c76c96b90462f24debacca971d2e86713. > > Testing: > > I tested this series on an ONEXPLAYER 3 running Bazzite 44 with kernel > 7.2.0-ogc6.1.fc44.x86_64. Testing covered front controls, rear paddles, > Steam and Quick Access behavior, independent RGB effects, brightness/off > controls, and suspend/resume restoration. I also built the module with > W=1. X2 Mini Pro hardware testing remains outstanding. > > All 15 patches apply cleanly to the stated base and pass git diff --check > and checkpatch.pl with zero errors and zero warnings. > > Development and review: > https://github.com/OpenGamingCollective/linux-unstable/pull/13 > > I used LLM assistance for protocol analysis, implementation, > documentation, code review, and test orchestration. Hi Andrei, Thank you for this, a lot of really good and needed changes are present here that will greatly improve the driver and it takes a load off my plate. For the patches with Fixes tags please add Cc: stable@vger.kernel.org in v2. I'd also revisit which patch some of them fix and make sure the tags are correct, RGB and buttons were added in separate patches. Thanks, Derek. > Andrei Aldea (15): > HID: hid-oxp: fix default M1 and M2 key mappings > HID: hid-oxp: validate input report lengths before decoding > HID: hid-oxp: retain fractional brightness when reading RGB status > HID: hid-oxp: reject invalid Gen2 RGB status values > HID: hid-oxp: fix multicolor LED intensity scaling > HID: hid-oxp: serialize complete RGB updates > HID: hid-oxp: select brightness policy for the new RGB effect > HID: hid-oxp: stop configuration work during teardown > HID: hid-oxp: keep configuration state per HID interface > HID: hid-oxp: handle controller reinitialization across suspend > HID: hid-oxp: group declarations and protocol definitions > HID: hid-oxp: support three-page button maps on X2 controllers > HID: hid-oxp: represent RGB LEDs with a common array > HID: hid-oxp: add Gen3 joystick ring RGB support > HID: hid-oxp: add X2 auxiliary RGB zones > > .../ABI/testing/sysfs-driver-hid-oxp | 196 ++ > MAINTAINERS | 1 + > drivers/hid/hid-oxp.c | 1743 ++++++++++++++--- > 3 files changed, 1623 insertions(+), 317 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-oxp > > > base-commit: 50d05c7c76c96b90462f24debacca971d2e86713 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-10 20:24 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-10 3:21 [PATCH 00/15] HID: hid-oxp: fix and extend X2-family controller support Andrei Aldea 2026-09-10 3:21 ` [PATCH 15/15] HID: hid-oxp: add X2 auxiliary RGB zones Andrei Aldea 2026-09-10 20:20 ` Derek J. Clark 2026-09-10 20:24 ` [PATCH 00/15] HID: hid-oxp: fix and extend X2-family controller support Derek J. Clark
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox