From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB24637AA86 for ; Thu, 10 Sep 2026 20:19:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789071558; cv=none; b=EIe2pBQuoKiTTY+zyGLAnSCNuKo92AMifUNMWwqeuP0Qs4DhzVEbh6vXKR98CltdmalOTYBWG92JbiG9MGwXEAFl53u7ackHKl00FFKBxSBV+RKu+oN4av79Iwa0MB23J7nmgYS1I5vhQa+RzOTdrd80b0G7BYl4kRKFcfF7MWg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789071558; c=relaxed/simple; bh=CjS6wT0XL3nLO14AArKjHOf9FUHLkcJHk1QDiHfJG8c=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=TjeRydC6UqrKuJN8CQWQXKH4J+UAdsC0zt5e0y2ScA6/mFikIMUnvT6eKwbw7NeMUzkwM/klZYAHXvcRTd33+eRafVd62Q5L6CgkHshflr3L2XMM6zOSKq/l+E+hlrDvCPVeMh0xvsGdlvejw/RP9t24vu2yDEMFNVNNcx++60A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=S5WjP3AN; arc=none smtp.client-ip=209.85.216.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="S5WjP3AN" Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-39b24d114d4so183625a91.3 for ; Thu, 10 Sep 2026 13:19:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1789071555; x=1789676355; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=ybPeRZuwPoyzdVWR8FtPdVwF8AJkhS8QgRJ9tJz1l7o=; b=S5WjP3ANXWbwhu+NMZuOhaukiQhIjnHEbGdyK4J1UIQ885MjYjxfTqo/qYe1yA2duM KEJRFZivFhVhJhHHICm4TF1iU9bCW/yKqrXFkZrX/R9JI+8uojA4ja0oxnOU72PHPluQ CdzxyZNMGMnyNX1REpeQVmrKDpmH+dwoWajxBDBELuHFpzpFzqJL8kI5X2K4sxZK/wLc HpdJbPcjEhA6TeKFynsqNFu8qWzblPOkGNF+iwOcPZxqa1kTKnE7uo4QUJLwcp36mFzQ 1wTnrh9K4IOGW0CMz3GMdYcFK8fiNaXFbyvoXOSQvrHmH4sK1Tm51lS3Zsx0VbUpchDZ Rvaw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1789071555; x=1789676355; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=ybPeRZuwPoyzdVWR8FtPdVwF8AJkhS8QgRJ9tJz1l7o=; b=Ho/j8Sjks0bbbutWn+jel25LiO6dmRwhJPNaPaecDv+Jxb0V4xVNkc1cd5wMM2dBZG +jwkZ1V3gokZBYt/1WzlPjbkA2/D7oq5hpas/aX6EJw3Yuetp+NMPPx7ItITm19iVk+y Fz/cZ7uDy8gEloLZB70u6BJl7KM/AG23luRxUM6FhlzM3gGHjn2iVJc/5yWxlqfnO3t+ Jnj/auK+fQVNUPpjchpXv4LM8Bhcpf2QGvMreW+Lx6rp+aSICjSVwX3ZVuC4HnkHCB3d JGlC0wZUNExc6K1uQqtmd5DHs+33Jvijo1FBwRvm3Am+BpshZkpAJUtT8iswYOwbE8Wo dWwQ== X-Gm-Message-State: AFuF++mB/YbL1kBovviU4yVIsBToT7GPZSGx/g8HuZf8FP+0j1vsZ6IN ik/OeTovSfm8kab7jVn8xHqD5+ntAGExnpf9Wp5LOHrC+bpfE4njBVFa X-Gm-Gg: AYBFou04iQyItem/Z714Gqpxa1vluIkZVEwwhY6EcAwIqw1BiWZ2pUqi7Z0T46RF5bf EYAz9p0GP3iwLh7xPMABZbrWZXIPxm7z9fVPF5BzDYKfNvSHuRZfFJ4TMJwYYez8Z9ujFzhiYCa mBfQEWjKoJogjwxHhhhQFCtzKQ7TQC7JfC1J/YNWhcql/8quM2bq5J5FM3FTO208frKIRbJsy2X zIADAEUf6LFZ2B7QTiD+rJ9+cp3Bg6ZafBtmhGg4zlnQ7MqYciJ0hVzh5BjRA7dIHppgomvNkZZ RTC9nvJF77/1zg6LGWfYgUihHIjRqC/xx/LSuBQ9c9oykggkE8lnBAZAqBPFE3luFUmHuRjisLR DfCTbgopNaz4msza/urgQZcqeWuORqyQwT/ZU2JdRYxy38qk7R3BO0GilRl/FyUUQTBaCSNMX5I NOWdLVPittS920KBPyCvsYAQ3SxMKPKtlNDZlGhnRPO+s6uDK0HMtjqgBJsfdV89Nmc70gZfS2R 486FjWUsf9JsGSnOmy/sKffalfE+eC05bWlh32zjx906TwO+iMYKS2JXhLxn6EzlQ== X-Received: by 2002:a17:90b:280e:b0:398:c3a3:dbd0 with SMTP id 98e67ed59e1d1-39d9beba6ffmr701860a91.8.1789071554868; Thu, 10 Sep 2026 13:19:14 -0700 (PDT) Received: from [192.168.0.158] (108-228-232-20.lightspeed.sndgca.sbcglobal.net. [108.228.232.20]) by smtp.gmail.com with ESMTPSA id 5a478bee46e88-33ba4cf2e22sm538848eec.2.2026.09.10.13.19.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 10 Sep 2026 13:19:14 -0700 (PDT) Message-ID: <0c016e95-50cf-4c63-9bfa-35fa238e8f4f@gmail.com> Date: Thu, 10 Sep 2026 13:19:13 -0700 Precedence: bulk X-Mailing-List: linux-input@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 14/15] HID: hid-oxp: add Gen3 joystick ring RGB support To: Andrei Aldea , Jiri Kosina , Benjamin Tissoires Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, Lee Jones , Pavel Machek , linux-leds@vger.kernel.org References: <20260910032115.28669-1-andrei1998@gmail.com> <20260910032115.28669-15-andrei1998@gmail.com> Content-Language: en-US From: "Derek J. Clark" In-Reply-To: <20260910032115.28669-15-andrei1998@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 9/9/26 20:21, Andrei Aldea wrote: > Select the X2/Gen3 RGB protocol early for the ONEXPLAYER 3 and X2 Mini Pro. > These controllers share the Gen2 HID usage page but require a 59-byte > color payload and writes to ring zones 1, 2 and 7. Keep the Gen1 55-byte > and legacy Gen2 57-byte color builders separate and unchanged. > > Add dedicated Gen3 status, color, effect and brightness paths. Check its > known command/zone acknowledgments, retain cached color and effect when > status cannot report them, and retry one complete ring pass on failure. > Legacy transport success does not depend on receiving an acknowledgment. > > Restore cached X2 RGB once after the final controller-mode change. Generic > ACK discrimination and suspend work management are preceding fixes; this > patch does not add RGB replay to older controllers. > > Assisted-by: LLM > Reviewed-by: Derek J. Clark > Signed-off-by: Andrei Aldea > --- > drivers/hid/hid-oxp.c | 464 ++++++++++++++++++++++++++++++++++++------ > 1 file changed, 398 insertions(+), 66 deletions(-) > > diff --git a/drivers/hid/hid-oxp.c b/drivers/hid/hid-oxp.c > index 55f8b47..9a46602 100644 > --- a/drivers/hid/hid-oxp.c > +++ b/drivers/hid/hid-oxp.c > @@ -25,8 +25,8 @@ > #include "hid-ids.h" > > #define OXP_PACKET_SIZE 64 > -#define OXP_STATUS_HEADER_SIZE 6 > -#define OXP_STATUS_ACK 0x20 > +#define OXP_GEN2_EVENT_HEADER_SIZE 6 > +#define OXP_GEN2_RGB_ACK 0x20 > > #define GEN1_MESSAGE_ID 0xff > #define GEN2_MESSAGE_ID 0x3f > @@ -214,19 +214,20 @@ struct oxp_hid_cfg { > struct oxp_bmap_page_1 *bmap_1; > struct oxp_bmap_page_2 *bmap_2; > bool gen2_work_initialized; > - bool bmap_page_3; > u8 rumble_intensity; > + bool bmap_page_3; > u8 gamepad_mode; > u8 bmap_format; > > /* RGB state */ > struct oxp_rgb_led *rgb_leds; > - spinlock_t rgb_reply_lock; > + spinlock_t rgb_reply_lock; /* Protect pending RGB acknowledgment state. */ > struct mutex rgb_mutex; /*serialize complete RGB transactions*/ > bool rgb_reply_pending; > u8 rgb_reply_command; > u8 rgb_reply_zone; > u8 rgb_led_count; > + bool x2_rgb; > }; > > enum oxp_gamepad_mode_index { > @@ -315,6 +316,15 @@ struct oxp_gen_1_rgb_report { > u8 blue; > } __packed; > > +struct oxp_gen_2_event_header { > + u8 report_id; > + u8 message_id; > + u8 padding; > + u8 command; > + u8 zone; > + u8 status; > +} __packed; > + > struct oxp_gen_2_rgb_report { > u8 report_id; > u8 header_id; > @@ -331,6 +341,21 @@ struct oxp_gen_2_rgb_report { > u8 effect; > } __packed; > > +struct oxp_rgb_color { > + u8 red; > + u8 green; > + u8 blue; > +} __packed; > + > +struct oxp_gen_3_rgb_color_report { > + u8 effect; > + u8 zone; > + u8 mode; > + struct oxp_rgb_color colors[18]; > + u8 final_red; > + u8 final_green; > +} __packed; > + > enum oxp_rgb_type { > OXP_RGB_FULL, > }; > @@ -361,10 +386,11 @@ struct oxp_attr { > }; > > struct quirk_entry { > - bool hybrid_mcu; > - bool bmap_page_3; > u8 cfg_interface_num; > + bool bmap_page_3; > + bool hybrid_mcu; > u8 bmap_format; > + bool x2_rgb; > }; > > static u16 get_usage_page(struct hid_device *hdev) > @@ -443,6 +469,51 @@ static int oxp_gen_2_property_out(struct oxp_hid_cfg *cfg, > static int oxp_set_buttons(struct oxp_hid_cfg *cfg); > static int oxp_rumble_intensity_set(struct oxp_hid_cfg *cfg, u8 intensity); > > +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_restore(struct oxp_hid_cfg *cfg) > +{ > + struct oxp_rgb_full_state *state; > + struct oxp_rgb_led *led; > + int ret; > + int i; > + > + if (!cfg->x2_rgb) > + return; > + > + guard(mutex)(&cfg->rgb_mutex); > + for (i = 0; i < cfg->rgb_led_count; i++) { > + led = &cfg->rgb_leds[i]; > + switch (led->type) { > + case OXP_RGB_FULL: > + state = led->state; > + if (state->effect == OXP_UNKNOWN) > + break; > + > + ret = oxp_rgb_status_store(led, state->enabled, > + state->speed, > + state->brightness); > + if (ret) { > + dev_err(&cfg->hdev->dev, > + "Error: Failed to restore RGB status: %i\n", > + ret); > + break; > + } > + if (state->enabled == OXP_FEAT_DISABLED) > + break; > + > + ret = oxp_rgb_effect_set(led, state->effect); > + if (ret) > + dev_err(&cfg->hdev->dev, > + "Error: Failed to restore RGB effect: %i\n", > + ret); > + break; > + } > + } > +} > + > static void oxp_mcu_init_fn(struct work_struct *work) > { > struct oxp_hid_cfg *cfg = container_of(to_delayed_work(work), > @@ -459,92 +530,100 @@ static void oxp_mcu_init_fn(struct work_struct *work) > dev_err(&cfg->hdev->dev, > "Error: Failed to set button mapping: %i\n", ret); > > - /* Cycle the gamepad mode */ > - ret = oxp_gen_2_property_out(cfg, OXP_FID_GEN2_TOGGLE_MODE, gp_mode_data, 3); > + /* Cycle the gamepad mode before restoring lighting. */ > + ret = oxp_gen_2_property_out(cfg, OXP_FID_GEN2_TOGGLE_MODE, > + gp_mode_data, sizeof(gp_mode_data)); > if (ret) > dev_err(&cfg->hdev->dev, > "Error: Failed to set gamepad mode: %i\n", ret); > > - /* Remainder only applies for xinput mode */ > + if (cfg->gamepad_mode != OXP_GP_MODE_DEBUG) { > + gp_mode_data[0] = OXP_GP_MODE_XINPUT; > + ret = oxp_gen_2_property_out(cfg, OXP_FID_GEN2_TOGGLE_MODE, > + gp_mode_data, sizeof(gp_mode_data)); > + if (ret) > + dev_err(&cfg->hdev->dev, > + "Error: Failed to set gamepad mode: %i\n", ret); > + } > + > + /* The final mode change must precede the single RGB restore. */ > + oxp_rgb_restore(cfg); > if (cfg->gamepad_mode == OXP_GP_MODE_DEBUG) > return; > > - gp_mode_data[0] = OXP_GP_MODE_XINPUT; > - ret = oxp_gen_2_property_out(cfg, OXP_FID_GEN2_TOGGLE_MODE, gp_mode_data, 3); > - if (ret) > - dev_err(&cfg->hdev->dev, > - "Error: Failed to set gamepad mode: %i\n", ret); > - > - /* Set vibration level */ > ret = oxp_rumble_intensity_set(cfg, cfg->rumble_intensity); > if (ret) > dev_err(&cfg->hdev->dev, > "Error: Failed to set rumble intensity: %i\n", ret); > } > > -static int oxp_hid_raw_event_gen_2(struct hid_device *hdev, > - struct hid_report *report, u8 *data, > - int size) > +static bool oxp_gen_2_rgb_event(struct oxp_hid_cfg *cfg, > + const struct oxp_gen_2_event_header *header) > { > - struct oxp_hid_cfg *cfg = hid_get_drvdata(hdev); > - struct oxp_rgb_led *led = oxp_rgb_led_by_type(cfg, OXP_RGB_FULL); > - struct oxp_rgb_full_state *state = oxp_rgb_full_state(led); > - struct oxp_gen_2_rgb_report *rgb_rep; > - struct led_classdev_mc *led_mc; > bool solicited = false; > > - if (size < OXP_STATUS_HEADER_SIZE) > - return 0; > - > - if (data[0] != OXP_FID_GEN2_STATUS_EVENT) > - return 0; > - > - /* A monocolor acknowledgment is not an MCU reset notification. */ > - if (data[5] == OXP_STATUS_ACK) { > + if (header->status == OXP_GEN2_RGB_ACK) { > scoped_guard(spinlock_irqsave, &cfg->rgb_reply_lock) { > if (cfg->rgb_reply_pending && > - data[3] == cfg->rgb_reply_command && > - data[4] == cfg->rgb_reply_zone) { > + header->command == cfg->rgb_reply_command && > + header->zone == cfg->rgb_reply_zone) { > cfg->rgb_reply_pending = false; > solicited = true; > } > } > if (solicited) > - return 0; > + return true; > } > > - /* Sent ~6s after resume event, indicating the MCU has fully reset. > - * Re-apply our settings after this has been received. > - */ > - if (data[3] == OXP_EFFECT_MONO_TRUE) { > - if (READ_ONCE(cfg->gen2_work_initialized) && > - !READ_ONCE(cfg->suspended) && > - !READ_ONCE(cfg->removing)) > - mod_delayed_work(system_dfl_wq, &cfg->oxp_mcu_init, > - msecs_to_jiffies(50)); > - return 0; > - } > + /* A color-write acknowledgment must not be treated as an MCU reset. */ > + if (header->command != OXP_EFFECT_MONO_TRUE) > + return false; > + > + /* An unsolicited event is sent after the MCU resets on resume. */ > + if (READ_ONCE(cfg->gen2_work_initialized) && > + !READ_ONCE(cfg->suspended) && !READ_ONCE(cfg->removing)) > + mod_delayed_work(system_dfl_wq, &cfg->oxp_mcu_init, > + msecs_to_jiffies(50)); > + > + return true; > +} > + > +static int oxp_hid_raw_event_gen_2(struct hid_device *hdev, > + struct hid_report *report, u8 *data, > + int size) > +{ > + struct oxp_hid_cfg *cfg = hid_get_drvdata(hdev); > + struct oxp_rgb_led *led = oxp_rgb_led_by_type(cfg, OXP_RGB_FULL); > + struct oxp_rgb_full_state *state = oxp_rgb_full_state(led); > + struct oxp_gen_2_event_header *header = (void *)data; > + struct oxp_gen_2_rgb_report *rgb_rep; > > - if (data[3] != OXP_GET_PROPERTY) > + if (size < OXP_GEN2_EVENT_HEADER_SIZE) > + return 0; > + if (header->report_id != OXP_FID_GEN2_STATUS_EVENT) > + return 0; > + if (oxp_gen_2_rgb_event(cfg, header)) > + return 0; > + if (header->command != OXP_GET_PROPERTY) > return 0; > if (size < sizeof(*rgb_rep) || !state) > return 0; > > - led_mc = &led->mc_cdev; > rgb_rep = (struct oxp_gen_2_rgb_report *)data; > if (rgb_rep->enabled > OXP_FEAT_ENABLED || rgb_rep->speed > 9 || > rgb_rep->brightness > 4) > return 0; > > - /* Ensure we save monocolor as the list value */ > + /* Ensure we save monocolor as the list value. */ > state->effect = rgb_rep->effect == OXP_EFFECT_MONO_TRUE ? > OXP_EFFECT_MONO_LIST : rgb_rep->effect; > state->speed = rgb_rep->speed; > state->enabled = rgb_rep->enabled == 0 ? OXP_FEAT_DISABLED : > - OXP_FEAT_ENABLED; > + OXP_FEAT_ENABLED; > state->brightness = rgb_rep->brightness; > - led_mc->led_cdev.brightness = rgb_rep->brightness * > - led_mc->led_cdev.max_brightness / 4; > + led->mc_cdev.led_cdev.brightness = rgb_rep->brightness * > + led->mc_cdev.led_cdev.max_brightness; > + led->mc_cdev.led_cdev.brightness /= 4; > /* If monocolor had less than 100% brightness on the previous boot, > * there will be no reliable way to determine the real intensity. > * Since intensity scaling is used with a hardware brightness set at max, > @@ -552,9 +631,52 @@ static int oxp_hid_raw_event_gen_2(struct hid_device *hdev, > * prevent successive boots from lowering the brightness further. > * Brightness will be "wrong" but the effect will remain the same visually. > */ > - led_mc->subled_info[0].intensity = rgb_rep->red; > - led_mc->subled_info[1].intensity = rgb_rep->green; > - led_mc->subled_info[2].intensity = rgb_rep->blue; > + led->mc_cdev.subled_info[0].intensity = rgb_rep->red; > + led->mc_cdev.subled_info[1].intensity = rgb_rep->green; > + led->mc_cdev.subled_info[2].intensity = rgb_rep->blue; > + > + return 0; > +} > + > +static int oxp_hid_raw_event_gen_3(struct oxp_hid_cfg *cfg, > + struct hid_report *report, u8 *data, > + int size) > +{ > + struct oxp_rgb_led *led = oxp_rgb_led_by_type(cfg, OXP_RGB_FULL); > + struct oxp_rgb_full_state *state = oxp_rgb_full_state(led); > + struct oxp_gen_2_event_header *header = (void *)data; > + struct oxp_gen_2_rgb_report *rgb_rep; > + > + if (size < OXP_GEN2_EVENT_HEADER_SIZE) > + return 0; > + if (header->report_id != OXP_FID_GEN2_STATUS_EVENT) > + return 0; > + if (oxp_gen_2_rgb_event(cfg, header)) > + return 0; > + if (header->command != OXP_GET_PROPERTY) > + return 0; > + if (size < sizeof(*rgb_rep) || !state) > + return 0; > + > + rgb_rep = (struct oxp_gen_2_rgb_report *)data; > + if (rgb_rep->enabled > OXP_FEAT_ENABLED || rgb_rep->speed > 9 || > + rgb_rep->brightness > 4) > + return 0; > + > + /* Gen3 status replies do not report the current effect or RGB color. */ > + state->speed = rgb_rep->speed; > + state->enabled = rgb_rep->enabled == 0 ? OXP_FEAT_DISABLED : > + OXP_FEAT_ENABLED; > + /* > + * Monocolor uses scaled channels at maximum hardware brightness. > + * Retain the requested brightness rather than replacing it with 100%. > + */ > + if (state->effect != OXP_EFFECT_MONO_LIST) { > + state->brightness = rgb_rep->brightness; > + led->mc_cdev.led_cdev.brightness = rgb_rep->brightness * > + led->mc_cdev.led_cdev.max_brightness; > + led->mc_cdev.led_cdev.brightness /= 4; > + } > > return 0; > } > @@ -574,6 +696,9 @@ static int oxp_hid_raw_event(struct hid_device *hdev, struct hid_report *report, > case GEN1_USAGE_PAGE: > return oxp_hid_raw_event_gen_1(hdev, report, data, size); > case GEN2_USAGE_PAGE: > + /* X2/Gen3 controllers share the Gen2 HID usage page. */ > + if (cfg->x2_rgb) > + return oxp_hid_raw_event_gen_3(cfg, report, data, size); > return oxp_hid_raw_event_gen_2(hdev, report, data, size); > default: > break; > @@ -584,9 +709,11 @@ static int oxp_hid_raw_event(struct hid_device *hdev, struct hid_report *report, > > static int mcu_property_out(struct oxp_hid_cfg *cfg, u8 *header, > size_t header_size, u8 *data, size_t data_size, > - u8 *footer, size_t footer_size) > + u8 *footer, size_t footer_size, bool expect_rgb_ack) > { > unsigned char *dmabuf __free(kfree) = kzalloc(OXP_PACKET_SIZE, GFP_KERNEL); > + u8 rgb_command = 0; > + u8 rgb_zone = 0; > bool rgb_write; > int ret; > > @@ -602,13 +729,21 @@ static int mcu_property_out(struct oxp_hid_cfg *cfg, u8 *header, > if (READ_ONCE(cfg->suspended)) > return -EHOSTDOWN; > > + /* > + * Track legacy monocolor replies to distinguish them from MCU resets. > + * Only Gen3's known acknowledgment protocol makes a missing reply an > + * error; retain transport-only success semantics for older controllers. > + */ > rgb_write = header_size && data_size > 1 && > header[0] == OXP_FID_GEN2_STATUS_EVENT && > - data[0] == OXP_EFFECT_MONO_TRUE; > + data[0] != OXP_GET_PROPERTY && > + (expect_rgb_ack || data[0] == OXP_EFFECT_MONO_TRUE); > if (rgb_write) { > + rgb_command = data[0]; > + rgb_zone = data[1]; > scoped_guard(spinlock_irqsave, &cfg->rgb_reply_lock) { > - cfg->rgb_reply_command = data[0]; > - cfg->rgb_reply_zone = data[1]; > + cfg->rgb_reply_command = rgb_command; > + cfg->rgb_reply_zone = rgb_zone; > cfg->rgb_reply_pending = true; > } > } > @@ -628,7 +763,13 @@ static int mcu_property_out(struct oxp_hid_cfg *cfg, u8 *header, > > if (rgb_write) { > scoped_guard(spinlock_irqsave, &cfg->rgb_reply_lock) { > - cfg->rgb_reply_pending = false; > + if (cfg->rgb_reply_pending && > + cfg->rgb_reply_command == rgb_command && > + cfg->rgb_reply_zone == rgb_zone) { > + if (!ret && expect_rgb_ack) > + ret = -ETIMEDOUT; > + cfg->rgb_reply_pending = false; > + } > } > } > > @@ -642,7 +783,8 @@ static int oxp_gen_1_property_out(struct oxp_hid_cfg *cfg, > u8 header[] = { fid, GEN1_MESSAGE_ID }; > size_t header_size = ARRAY_SIZE(header); > > - return mcu_property_out(cfg, header, header_size, data, data_size, NULL, 0); > + return mcu_property_out(cfg, header, header_size, data, data_size, > + NULL, 0, false); > } > > static int oxp_gen_2_property_out(struct oxp_hid_cfg *cfg, > @@ -655,7 +797,65 @@ static int oxp_gen_2_property_out(struct oxp_hid_cfg *cfg, > size_t footer_size = ARRAY_SIZE(footer); > > return mcu_property_out(cfg, header, header_size, data, data_size, footer, > - footer_size); > + footer_size, false); > +} > + > +static const u8 oxp_x2_rgb_zones[] = { 0x01, 0x02, 0x07 }; > + > +static int oxp_gen_3_property_out(struct oxp_hid_cfg *cfg, u8 *data, > + u8 data_size) > +{ > + u8 header[] = { OXP_FID_GEN2_STATUS_EVENT, GEN2_MESSAGE_ID, 0x01 }; > + u8 footer[] = { GEN2_MESSAGE_ID, OXP_FID_GEN2_STATUS_EVENT }; > + size_t header_size = ARRAY_SIZE(header); > + size_t footer_size = ARRAY_SIZE(footer); > + > + return mcu_property_out(cfg, header, header_size, data, data_size, > + footer, footer_size, true); > +} > + > +static int oxp_gen_3_rgb_property_out(struct oxp_hid_cfg *cfg, u8 *data, > + u8 data_size) > +{ > + int first_err; > + int attempt; > + int ret; > + int i; > + > + /* > + * Each zone is addressed independently. Complete the pass even if one > + * write fails so the remaining ring zones still receive the setting. > + * Retry the full pass once to keep their requested settings identical. > + */ > + for (attempt = 0; attempt < 2; attempt++) { > + first_err = 0; > + for (i = 0; i < ARRAY_SIZE(oxp_x2_rgb_zones); i++) { > + data[1] = oxp_x2_rgb_zones[i]; > + ret = oxp_gen_3_property_out(cfg, data, data_size); > + if (ret && !first_err) > + first_err = ret; > + } > + if (!first_err) > + return 0; > + } > + > + return first_err; > +} > + > +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) > +{ > + int i; > + > + memset(report, 0, sizeof(*report)); > + report->effect = command; > + report->zone = zone; > + report->mode = 0x02; > + for (i = 0; i < ARRAY_SIZE(report->colors); i++) > + report->colors[i] = (struct oxp_rgb_color) { red, green, blue }; > + report->final_red = red; > + report->final_green = green; > } > > static ssize_t gamepad_mode_store(struct device *dev, > @@ -1206,6 +1406,20 @@ static const struct attribute_group oxp_cfg_attrs_group = { > .attrs = oxp_cfg_attrs, > }; > > +static int oxp_gen_3_rgb_status_store(struct oxp_rgb_led *led, u8 enabled, > + u8 speed, u8 brightness) > +{ > + struct oxp_rgb_full_state *state = oxp_rgb_full_state(led); > + u8 data[6] = { OXP_SET_PROPERTY, 0x00, 0x02, enabled, speed, brightness }; > + > + if (!state) > + return -ENODEV; > + if (state->effect == OXP_EFFECT_MONO_LIST) > + data[5] = 0x04; > + > + return oxp_gen_3_rgb_property_out(led->cfg, data, sizeof(data)); > +} > + > static int oxp_rgb_status_store(struct oxp_rgb_led *led, u8 enabled, u8 speed, > u8 brightness) > { > @@ -1214,6 +1428,11 @@ static int oxp_rgb_status_store(struct oxp_rgb_led *led, u8 enabled, u8 speed, > u16 up = get_usage_page(cfg->hdev); > u8 *data; > > + if (!state) > + return -ENODEV; > + if (cfg->x2_rgb) > + return oxp_gen_3_rgb_status_store(led, enabled, speed, brightness); > + > /* Always default to max brightness and use intensity scaling when in > * monocolor mode. > */ > @@ -1233,13 +1452,25 @@ static int oxp_rgb_status_store(struct oxp_rgb_led *led, u8 enabled, u8 speed, > } > } > > +static int oxp_gen_3_rgb_status_show(struct oxp_hid_cfg *cfg) > +{ > + u8 data[3] = { OXP_GET_PROPERTY, 0x07, 0x02 }; > + > + return oxp_gen_3_property_out(cfg, data, sizeof(data)); > +} > + > static ssize_t oxp_rgb_status_show(struct oxp_rgb_led *led) > { > struct oxp_hid_cfg *cfg = led->cfg; > u16 up = get_usage_page(cfg->hdev); > u8 *data; > > + if (!oxp_rgb_full_state(led)) > + return -ENODEV; > + > guard(mutex)(&cfg->rgb_mutex); > + if (cfg->x2_rgb) > + return oxp_gen_3_rgb_status_show(cfg); > > switch (up) { > case GEN1_USAGE_PAGE: > @@ -1253,6 +1484,22 @@ static ssize_t oxp_rgb_status_show(struct oxp_rgb_led *led) > } > } > > +static int oxp_gen_3_rgb_color_set(struct oxp_rgb_led *led) > +{ > + struct led_classdev_mc *mc_cdev = &led->mc_cdev; > + struct oxp_gen_3_rgb_color_report color_report; > + u8 brightness = mc_cdev->led_cdev.brightness; > + > + led_mc_calc_color_components(mc_cdev, brightness); > + oxp_gen_3_rgb_fill_color(&color_report, OXP_EFFECT_MONO_TRUE, 0x00, > + mc_cdev->subled_info[0].brightness, > + mc_cdev->subled_info[1].brightness, > + mc_cdev->subled_info[2].brightness); > + > + return oxp_gen_3_rgb_property_out(led->cfg, (u8 *)&color_report, > + sizeof(color_report)); > +} > + > static int oxp_rgb_color_set(struct oxp_rgb_led *led) > { > struct led_classdev_mc *led_mc = &led->mc_cdev; > @@ -1264,6 +1511,9 @@ static int oxp_rgb_color_set(struct oxp_rgb_led *led) > u8 *data; > int i; > > + if (cfg->x2_rgb) > + return oxp_gen_3_rgb_color_set(led); > + > led_mc_calc_color_components(led_mc, br); > red = led_mc->subled_info[0].brightness; > green = led_mc->subled_info[1].brightness; > @@ -1281,6 +1531,7 @@ static int oxp_rgb_color_set(struct oxp_rgb_led *led) > } > return oxp_gen_1_property_out(cfg, OXP_FID_GEN1_RGB_SET, data, size); > case GEN2_USAGE_PAGE: > + /* Preserve the legacy payload and its two zero bytes before the footer. */ > size = 57; > data = (u8[57]) { OXP_EFFECT_MONO_TRUE, 0x00, 0x02 }; > > @@ -1295,6 +1546,29 @@ static int oxp_rgb_color_set(struct oxp_rgb_led *led) > } > } > > +static int oxp_gen_3_rgb_effect_set(struct oxp_rgb_led *led, u8 effect) > +{ > + struct oxp_rgb_full_state *state = oxp_rgb_full_state(led); > + u8 data[3] = { effect, 0x00, 0x02 }; > + int ret; > + > + if (!state) > + return -ENODEV; > + > + if (effect > OXP_UNKNOWN && effect < OXP_EFFECT_MONO_LIST) > + ret = oxp_gen_3_rgb_property_out(led->cfg, data, sizeof(data)); > + else if (effect == OXP_EFFECT_MONO_LIST) > + ret = oxp_gen_3_rgb_color_set(led); > + else > + return -EINVAL; > + > + if (ret) > + return ret; > + > + state->effect = effect; > + return 0; > +} > + > static int oxp_rgb_effect_set(struct oxp_rgb_led *led, u8 effect) > { > struct oxp_rgb_full_state *state = oxp_rgb_full_state(led); > @@ -1303,6 +1577,11 @@ static int oxp_rgb_effect_set(struct oxp_rgb_led *led, u8 effect) > u8 *data; > int ret; > > + if (!state) > + return -ENODEV; > + if (cfg->x2_rgb) > + return oxp_gen_3_rgb_effect_set(led, effect); > + > switch (effect) { > case OXP_EFFECT_AURORA: > case OXP_EFFECT_BIRTHDAY: > @@ -1586,6 +1865,54 @@ static void oxp_rgb_full_queue(struct oxp_rgb_led *led, > "Error: Failed to write RGB color: %i\n", ret); > } > > +static void oxp_gen_3_rgb_queue(struct oxp_rgb_led *led, > + struct oxp_rgb_full_state *state) > +{ > + unsigned int max_brightness = led->mc_cdev.led_cdev.max_brightness; > + u8 old_effect; > + unsigned int brightness = led->mc_cdev.led_cdev.brightness; > + u8 enabled = brightness ? OXP_FEAT_ENABLED : OXP_FEAT_DISABLED; > + struct oxp_hid_cfg *cfg = led->cfg; > + u8 val = 4 * brightness / max_brightness; > + int ret; > + > + guard(mutex)(&cfg->rgb_mutex); > + old_effect = state->effect; > + > + if (!brightness) { > + ret = oxp_gen_3_rgb_status_store(led, OXP_FEAT_DISABLED, 5, 0); > + if (ret) { > + dev_err(led->mc_cdev.led_cdev.dev, > + "Error: Failed to disable RGB: %i\n", ret); > + return; > + } > + > + state->enabled = OXP_FEAT_DISABLED; > + state->brightness = 0; > + state->speed = 5; > + return; > + } > + > + /* Gen3 standard LED writes select solid color and follow brightness/off. */ > + state->effect = OXP_EFFECT_MONO_LIST; > + ret = oxp_gen_3_rgb_status_store(led, enabled, 5, val); > + if (ret) { > + state->effect = old_effect; > + dev_err(led->mc_cdev.led_cdev.dev, > + "Error: Failed to write RGB status: %i\n", ret); > + return; > + } > + > + state->enabled = enabled; > + state->brightness = val; > + state->speed = 5; > + > + ret = oxp_gen_3_rgb_effect_set(led, OXP_EFFECT_MONO_LIST); > + if (ret) > + dev_err(led->mc_cdev.led_cdev.dev, > + "Error: Failed to write RGB color: %i\n", ret); > +} > + > static void oxp_rgb_queue_fn(struct work_struct *work) > { > struct oxp_rgb_led *led = container_of(to_delayed_work(work), > @@ -1597,7 +1924,10 @@ static void oxp_rgb_queue_fn(struct work_struct *work) > > switch (led->type) { > case OXP_RGB_FULL: > - oxp_rgb_full_queue(led, oxp_rgb_full_state(led)); > + if (cfg->x2_rgb) > + oxp_gen_3_rgb_queue(led, led->state); > + else > + oxp_rgb_full_queue(led, led->state); > break; > } > } > @@ -1751,9 +2081,10 @@ static struct quirk_entry quirk_hybrid_mcu = { > .hybrid_mcu = true, > }; > > -static struct quirk_entry quirk_x2_bmap = { > +static struct quirk_entry quirk_x2 = { > .bmap_format = OXP_BMAP_FORMAT_X2, > .bmap_page_3 = true, > + .x2_rgb = true, > .cfg_interface_num = 2, > }; > > @@ -1788,7 +2119,7 @@ static const struct dmi_system_id oxp_quirk_list[] = { > DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ONE-NETBOOK"), > DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "ONEXPLAYER 3"), > }, > - .driver_data = &quirk_x2_bmap, > + .driver_data = &quirk_x2, > }, > { > .ident = "OneXPlayer X2 Mini Pro", > @@ -1796,7 +2127,7 @@ static const struct dmi_system_id oxp_quirk_list[] = { > DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ONE-NETBOOK"), > DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "ONEXPLAYER X2Mini PRO"), > }, > - .driver_data = &quirk_x2_bmap, > + .driver_data = &quirk_x2, > }, > {}, > }; > @@ -1868,6 +2199,7 @@ static int oxp_cfg_probe(struct hid_device *hdev, u16 up, > return -ENOMEM; > > cfg->hdev = hdev; > + cfg->x2_rgb = quirks && quirks->x2_rgb; > mutex_init(&cfg->cfg_mutex); > mutex_init(&cfg->rgb_mutex); > spin_lock_init(&cfg->rgb_reply_lock); I'm unable to test as these features are not available on my OneXPlayer F1 Pro. Reviewed-by: Derek J. Clark