* [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures
@ 2026-09-23 10:30 Liang Haowen
2026-09-23 10:30 ` [RFC v7 1/1] " Liang Haowen
2026-09-23 10:41 ` [RFC v7 0/1] " Ilpo Järvinen
0 siblings, 2 replies; 27+ messages in thread
From: Liang Haowen @ 2026-09-23 10:30 UTC (permalink / raw)
To: linux-leds
Cc: Lee Jones, Pavel Machek, Martin K . Petersen, linux-scsi,
platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf,
Hans de Goede, Ilpo Jarvinen
Hello,
v7, as its own thread, addressing Lee's review of v6.
Changes since v6:
- The driver moved to drivers/leds/rgb/, where the other multicolor
LED drivers live.
- The series is submitted with git send-email this time, so the
patch format is the standard one.
The SCSI device handler attachment is unchanged; why it is a device
handler at all, and what the in-tree split should look like, is the
open discussion in the v6 thread.
Everything else is unchanged from v6: the hardware description, the
scsi_device_handler that does not claim the sdev, the multicolor LED
interface, the protocol handling and the known caveats (manual
attach until the split lands; SAVE on every update writes the
enclosure flash, wear uncharacterized; NULL-parent LED registration
to avoid the sdev reference cycle).
Liang Haowen (1):
leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe
enclosures
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [RFC v7 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-23 10:30 [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures Liang Haowen @ 2026-09-23 10:30 ` Liang Haowen 2026-09-23 10:45 ` Lee Jones 2026-09-23 10:41 ` [RFC v7 0/1] " Ilpo Järvinen 1 sibling, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-23 10:30 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K . Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are plain USB mass-storage devices with no HID interface: the Aura LEDs hang off an ENE controller driven by vendor SCSI commands on the same LUN as the disk. The enclosure has 4 independently addressable LEDs, verified on hardware. Register a scsi_device_handler matched by INQUIRY (vendor "ROG", model "ESD-S1C"); it does not claim the sdev (sd keeps owning the disk) and exposes each LED as a multicolor LED class device, /sys/class/leds/asus-arion-0-0-0-0::led-0 through led-3. The H:C:T:L part of the sdev name keeps the names unique when more than one enclosure is connected, with its colons flattened to dashes. The color section stays empty, since multicolor LEDs enumerate their palette via multi_intensity, and the four identical zones use the function name with a "-N" ordinal, as the naming section in Documentation/leds/leds-class.rst asks for. Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, register index, argument count in cdb[13]). MODE 0x8021 (Static) must be written first in every sequence or the device ignores it; colours go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; both tables are written because firmware revisions pull from one or the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. The CDB cannot go through scsi_execute_cmd(): it sizes the command via scsi_command_size(opcode), which maps vendor opcode 0xec to 10 bytes, so cdb[13] is dropped and the device silently ignores the write (GOOD status, no error). The request is built with scsi_alloc_request() instead, which initializes the scsi_cmnd parts a passthrough needs (command buffer, lengths, rcu head), with cmd_len forced to 16, mirroring what SG_IO does from userspace. brightness_set only caches the colour and marks the LED in a per-zone dirty mask under a spinlock; led_mc_calc_color_components() runs under that lock too, because it writes the shared subled_info array and trigger events call led_set_brightness() without the led_access lock that serializes sysfs stores. A single work item per zone then snapshots the mask and colours and runs one ENE sequence for all pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every update through one work item keeps the sequences from interleaving between concurrent LED updates and batches multi-LED updates into a single APPLY/SAVE. A re-queued run with nothing pending returns before touching the device, so it cannot wear the flash with a pointless SAVE, and the lock keeps a colour write from being reordered after its dirty bit on weakly ordered architectures. This is the monolithic out-of-tree version as verified on hardware; the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split into a SCSI transport helper and a shared ASUS Aura LED interface. Signed-off-by: Liang Haowen <nbg2974@gmail.com> --- drivers/leds/rgb/leds-asus-aura-scsi.c | 392 +++++++++++++++++++++++++ 1 file changed, 392 insertions(+) create mode 100644 drivers/leds/rgb/leds-asus-aura-scsi.c diff --git a/drivers/leds/rgb/leds-asus-aura-scsi.c b/drivers/leds/rgb/leds-asus-aura-scsi.c new file mode 100644 index 0000000..4e039bd --- /dev/null +++ b/drivers/leds/rgb/leds-asus-aura-scsi.c @@ -0,0 +1,392 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures + * (e.g. ROG STRIX Arion, USB 0b05:1932). + * + * USB mass-storage device, no HID; the ENE LED controller is driven via + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), + * does NOT claim the sdev (sd keeps owning the disk). + * + * The Arion exposes 4 independently addressable LEDs (verified on hardware): + * each is a multicolor LED class device (asus-arion-<H-C-T-L>::led-0..led-3, + * unique per enclosure). A colour change writes that LED's slot only: + * EFFECT 0x8160 + 3*led, DIRECT 0x8100 + 3*led (3 bytes, byte order R, B, G), + * then APPLY (0x01) and SAVE (0xaa). MODE (0x8021 = Static) is written first + * in every sequence; skipping it makes the device ignore the whole sequence. + * + * Scheduling: brightness_set (LED core fast path) caches the colour and + * marks the LED in a per-zone dirty mask under a spinlock; a single work + * item per zone snapshots the mask and colours, then runs one ENE + * sequence for all pending LEDs (MODE once, colour slots, APPLY, SAVE). + * Funneling every update through that one work item also serializes the + * sequences: the MODE/colour/APPLY/SAVE chain must never interleave + * between concurrent LED updates. The snapshot makes re-queued runs with + * nothing left to do return before touching the device, so a re-queue + * cannot wear the flash with a pointless SAVE, and the lock keeps a + * colour write from being reordered after its dirty bit on weakly + * ordered architectures. + * + * CDB length: scsi_execute_cmd() sizes the CDB via COMMAND_SIZE(opcode), + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] + * and the device silently ignores the write. ene_write() therefore mirrors + * scsi_execute_cmd() on top of scsi_alloc_request() and forces cmd_len = 16 + * (what SG_IO does from userspace). + * + * Attach manually until a notifier lands: + * echo asus_aura > /sys/block/sdX/device/dh_state + */ + +#include <linux/module.h> +#include <linux/bits.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/string.h> +#include <linux/leds.h> +#include <linux/led-class-multicolor.h> +#include <linux/blk_types.h> +#include <linux/blkdev.h> +#include <linux/blk-mq.h> +#include <linux/workqueue.h> +#include <scsi/scsi.h> +#include <scsi/scsi_cmnd.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi_dh.h> + +#define ARION_INQ_VENDOR "ROG" +#define ARION_INQ_MODEL "ESD-S1C" + +#define ENE_OPCODE 0xec +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ +#define ENE_REG_APPLY 0x80a0 +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ +#define ENE_APPLY 0x01 +#define ENE_SAVE 0xaa +#define ENE_MODE_STATIC 1 +#define ENE_CDB_LEN 16 +#define ENE_RGB_LEN 3 +#define ENE_TIMEOUT (10 * HZ) + +/* + * Verified on hardware: the enclosure has 4 independently settable LEDs. + * (The colour table reserves 16 slots; only the first 4 drive anything.) + */ +#define ARION_NUM_LEDS 4 + +struct asus_aura_led { + struct asus_aura_zone *zone; + int index; + struct led_classdev_mc mc_cdev; + struct mc_subled subled[3]; + u8 rgb[ENE_RGB_LEN]; +}; + +struct asus_aura_zone { + struct scsi_device *sdev; + struct asus_aura_led leds[ARION_NUM_LEDS]; + spinlock_t lock; /* protects dirty and cached colours */ + u8 dirty; /* bit i: led i needs a colour write */ + struct work_struct work; +}; + +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) +{ + memset(cdb, 0, ENE_CDB_LEN); + cdb[0] = ENE_OPCODE; + cdb[1] = 'A'; + cdb[2] = 'S'; + cdb[3] = (reg >> 8) & 0xff; + cdb[4] = reg & 0xff; + cdb[13] = arg_count; +} + +/* + * scsi_execute_cmd() with cmd_len forced to 16. scsi_alloc_request() + * initializes the parts of the scsi_cmnd a passthrough needs (zeroed + * cmnd, cmd_len = MAX_COMMAND_SIZE, sense_len, rcu head, retries); + * a raw blk_mq_alloc_request() does none of that. + */ +static int ene_write(struct scsi_device *sdev, u16 reg, + const void *data, u8 arg_count) +{ + struct request *rq; + struct scsi_cmnd *scmd; + u8 cdb[ENE_CDB_LEN]; + int ret; + + ene_build_cdb(cdb, reg, arg_count); + + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); + if (IS_ERR(rq)) + return PTR_ERR(rq); + + if (arg_count) { + ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO); + if (ret) + goto out; + } + + scmd = blk_mq_rq_to_pdu(rq); + scmd->cmd_len = ENE_CDB_LEN; + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); + scmd->allowed = 1; + rq->timeout = ENE_TIMEOUT; + rq->rq_flags |= RQF_QUIET; + + blk_execute_rq(rq, true); + ret = scmd->result; +out: + blk_mq_free_request(rq); + return ret; +} + +/* + * Sleepable: runs on the system workqueue. One ENE sequence for every LED + * marked in the dirty mask. The mask and colours are snapshotted under the + * zone lock: asus_aura_set() may run concurrently on another CPU, and the + * lock keeps a colour write from being reordered after its dirty bit on + * weakly ordered architectures. A colour cached while this runs requeues + * the work and is picked up by the next sequence. + */ +static void asus_aura_zone_work(struct work_struct *work) +{ + struct asus_aura_zone *zone = + container_of(work, struct asus_aura_zone, work); + struct scsi_device *sdev = zone->sdev; + u8 rgb[ARION_NUM_LEDS][ENE_RGB_LEN]; + u8 apply = ENE_APPLY; + u8 save = ENE_SAVE; + u8 mode = ENE_MODE_STATIC; + unsigned long flags; + u8 pending; + int i, ret; + + spin_lock_irqsave(&zone->lock, flags); + pending = zone->dirty; + zone->dirty = 0; + for (i = 0; i < ARION_NUM_LEDS; i++) + memcpy(rgb[i], zone->leds[i].rgb, ENE_RGB_LEN); + spin_unlock_irqrestore(&zone->lock, flags); + + /* + * schedule_work() while this function runs requeues it, and the + * pending colour may already have been consumed above; the requeued + * run then has nothing to do. Return before touching the device: + * SAVE writes its flash. + */ + if (!pending) + return; + + if (!scsi_device_online(sdev)) + return; + + /* Mode first: without it the device ignores the whole sequence. */ + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); + if (ret) + goto err; + + for (i = 0; i < ARION_NUM_LEDS; i++) { + if (!(pending & BIT(i))) + continue; + + ret = ene_write(sdev, ENE_REG_COLORS + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + + /* + * Cover the DIRECT colour set too; some firmware revisions + * pull from 0x8100 instead of 0x8160. + */ + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + } + + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); + if (ret) + goto err; + + /* + * The change only takes effect after SAVE (0xaa). NOTE: saving on + * every brightness change writes flash each time; revisit for wear + * once confirmed. + */ + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); + if (ret) + goto err; + + return; +err: + dev_err(&sdev->sdev_gendev, + "asus_aura: colour update failed: %d\n", ret); +} + +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ +static void asus_aura_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); + struct asus_aura_led *led = + container_of(mc, struct asus_aura_led, mc_cdev); + struct asus_aura_zone *zone = led->zone; + unsigned long flags; + + /* + * led_mc_calc_color_components() writes the shared subled_info + * array. The LED core serializes sysfs stores with led_access, + * but trigger events call led_set_brightness() without it, so + * computing and copying the components under the same lock keeps + * a trigger-driven update and a sysfs store from reading a mix of + * each other's colours. + */ + spin_lock_irqsave(&zone->lock, flags); + led_mc_calc_color_components(mc, brightness); + /* ENE colour register byte order is R, B, G. */ + led->rgb[0] = led->subled[0].brightness; + led->rgb[1] = led->subled[2].brightness; + led->rgb[2] = led->subled[1].brightness; + zone->dirty |= BIT(led->index); + spin_unlock_irqrestore(&zone->lock, flags); + + schedule_work(&zone->work); +} + +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) +{ + struct asus_aura_led *led = &zone->leds[index]; + struct led_classdev *cdev = &led->mc_cdev.led_cdev; + char hctl[32]; + int ret; + + led->zone = zone; + led->index = index; + + led->subled[0].color_index = LED_COLOR_ID_RED; + led->subled[1].color_index = LED_COLOR_ID_GREEN; + led->subled[2].color_index = LED_COLOR_ID_BLUE; + led->mc_cdev.num_colors = 3; + led->mc_cdev.subled_info = led->subled; + + /* + * Include the sdev's H:C:T:L: every enclosure gets its own SCSI + * host, so the names stay unique when more than one is connected. + * With a static name the LED core would register the second + * enclosure's LEDs under renamed nodes (asus-arion::led-0_1), + * which is the wrong device identity. The names are per-attachment, + * like sd X letters, and userspace is expected to enumerate. + * + * dev_name() renders the sdev as H:C:T:L; the extra colons would + * break the devicename:color:function scheme userspace parses LED + * class names with, so they are flattened to dashes. The color + * section stays empty (these are multicolor LEDs, the palette is + * enumerated via multi_intensity), and the four identical zones + * get the function name with a "-N" ordinal, like the + * Documentation/leds/leds-class.rst naming section asks for. + */ + strscpy(hctl, dev_name(&zone->sdev->sdev_gendev), sizeof(hctl)); + strreplace(hctl, ':', '-'); + cdev->name = kasprintf(GFP_KERNEL, "asus-arion-%s::led-%d", hctl, index); + if (!cdev->name) + return -ENOMEM; + cdev->max_brightness = 255; + cdev->brightness_set = asus_aura_set; + + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); + + /* + * Register with NULL parent: parenting the LED to the sdev takes a + * device reference, which blocks the sdev's final release on unplug, + * which is what calls scsi_dh_release_device() -> our .detach() that + * unregisters the LEDs. That reference cycle leaked the LED nodes and + * the module refcount on every hot-unplug. + */ + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); + if (ret) + kfree(cdev->name); + return ret; +} + +/* + * Unregister the LED devices before cancelling the work: unregistering + * removes the sysfs attributes, so no new brightness_set can schedule the + * zone work afterwards, and it waits for in-flight sysfs callbacks. + * Cancelling first would leave a window where a brightness write requeues + * the work after cancel_work_sync() returned, and the work would then run + * on freed memory. + */ +static void asus_aura_release(struct asus_aura_zone *zone, int num_leds) +{ + int i; + + for (i = 0; i < num_leds; i++) + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); + cancel_work_sync(&zone->work); + for (i = 0; i < num_leds; i++) + kfree(zone->leds[i].mc_cdev.led_cdev.name); + kfree(zone); +} + +static int asus_aura_attach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone; + int i, ret; + + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) + return SCSI_DH_DEV_UNSUPP; + + zone = kzalloc_obj(*zone, GFP_KERNEL); + if (!zone) + return SCSI_DH_NOMEM; + zone->sdev = sdev; + spin_lock_init(&zone->lock); + INIT_WORK(&zone->work, asus_aura_zone_work); + + for (i = 0; i < ARION_NUM_LEDS; i++) { + ret = asus_aura_register_led(zone, i); + if (ret) { + asus_aura_release(zone, i); + return SCSI_DH_NOMEM; + } + } + + sdev->handler_data = zone; + return SCSI_DH_OK; +} + +static void asus_aura_detach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone = sdev->handler_data; + + if (!zone) + return; + asus_aura_release(zone, ARION_NUM_LEDS); + sdev->handler_data = NULL; +} + +static struct scsi_device_handler asus_aura_dh = { + .name = "asus_aura", + .module = THIS_MODULE, + .attach = asus_aura_attach, + .detach = asus_aura_detach, +}; + +static int __init asus_aura_init(void) +{ + return scsi_register_device_handler(&asus_aura_dh); +} + +static void __exit asus_aura_exit(void) +{ + scsi_unregister_device_handler(&asus_aura_dh); +} + +module_init(asus_aura_init); +module_exit(asus_aura_exit); + +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); +MODULE_AUTHOR("Liang Haowen"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC v7 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-23 10:30 ` [RFC v7 1/1] " Liang Haowen @ 2026-09-23 10:45 ` Lee Jones 2026-09-23 11:19 ` Liang Haowen 0 siblings, 1 reply; 27+ messages in thread From: Lee Jones @ 2026-09-23 10:45 UTC (permalink / raw) To: Liang Haowen Cc: linux-leds, Pavel Machek, Martin K . Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen On Wed, 23 Sep 2026, Liang Haowen wrote: > ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are > plain USB mass-storage devices with no HID interface: the Aura LEDs > hang off an ENE controller driven by vendor SCSI commands on the same > LUN as the disk. The enclosure has 4 independently addressable LEDs, > verified on hardware. > > Register a scsi_device_handler matched by INQUIRY (vendor "ROG", > model "ESD-S1C"); it does not claim the sdev (sd keeps owning the > disk) and exposes each LED as a multicolor LED class device, > /sys/class/leds/asus-arion-0-0-0-0::led-0 through led-3. The > H:C:T:L part of the sdev name keeps the names unique when more than > one enclosure is connected, with its colons flattened to dashes. The > color section stays empty, since multicolor LEDs enumerate their > palette via multi_intensity, and the four identical zones use the > function name with a "-N" ordinal, as the naming section in > Documentation/leds/leds-class.rst asks for. > > Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, > register index, argument count in cdb[13]). MODE 0x8021 (Static) must > be written first in every sequence or the device ignores it; colours > go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; > both tables are written because firmware revisions pull from one or > the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. > > The CDB cannot go through scsi_execute_cmd(): it sizes the command > via scsi_command_size(opcode), which maps vendor opcode 0xec to > 10 bytes, so cdb[13] is dropped and the device silently ignores the > write (GOOD status, no error). The request is built with > scsi_alloc_request() instead, which initializes the scsi_cmnd parts a > passthrough needs (command buffer, lengths, rcu head), with cmd_len > forced to 16, mirroring what SG_IO does from userspace. > > brightness_set only caches the colour and marks the LED in a per-zone > dirty mask under a spinlock; led_mc_calc_color_components() runs under > that lock too, because it writes the shared subled_info array and > trigger events call led_set_brightness() without the led_access lock > that serializes sysfs stores. A single work item per zone then > snapshots the mask and colours and runs one ENE sequence for all > pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every > update through one work item keeps the sequences from interleaving > between concurrent LED updates and batches multi-LED updates into a > single APPLY/SAVE. A re-queued run with nothing pending returns > before touching the device, so it cannot wear the flash with a > pointless SAVE, and the lock keeps a colour write from being > reordered after its dirty bit on weakly ordered architectures. > > This is the monolithic out-of-tree version as verified on hardware; > the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split > into a SCSI transport helper and a shared ASUS Aura LED interface. > > Signed-off-by: Liang Haowen <nbg2974@gmail.com> > --- > drivers/leds/rgb/leds-asus-aura-scsi.c | 392 +++++++++++++++++++++++++ > 1 file changed, 392 insertions(+) > create mode 100644 drivers/leds/rgb/leds-asus-aura-scsi.c Many (if not all?) of my review comments from v6 still stand. I will not be reviewing this submission. -- Lee Jones ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC v7 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-23 10:45 ` Lee Jones @ 2026-09-23 11:19 ` Liang Haowen 0 siblings, 0 replies; 27+ messages in thread From: Liang Haowen @ 2026-09-23 11:19 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds, linux-scsi, linux-kernel, sashiko-reviews You are right, and I owe you an apology. I submitted v7 within the hour of your review, having read only the three comments above the diff and none of the ones you interleaved in it. There is no excuse for that. I am very sorry for wasting your time. I have now read the whole mail, all 33 comments, and I am reworking the driver against the full list. Where I think a comment rests on a misunderstanding, or where there is a real constraint on my side, I will answer it in the next submission instead of silently keeping the code as it is. The next version does not go out until every comment of yours and Ilpo's is either addressed or discussed. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-23 10:30 [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures Liang Haowen 2026-09-23 10:30 ` [RFC v7 1/1] " Liang Haowen @ 2026-09-23 10:41 ` Ilpo Järvinen 2026-09-23 12:35 ` Denis Benato 1 sibling, 1 reply; 27+ messages in thread From: Ilpo Järvinen @ 2026-09-23 10:41 UTC (permalink / raw) To: Liang Haowen Cc: linux-leds, Lee Jones, Pavel Machek, Martin K . Petersen, linux-scsi, platform-driver-x86, LKML, Denis Benato, Armin Wolf, Hans de Goede On Wed, 23 Sep 2026, Liang Haowen wrote: > Hello, > > v7, as its own thread, addressing Lee's review of v6. No, you didn't address Lee's comments but only a small part of them. :-( Please slow down so you've time to address all feedback properly and double check before the next submission you've addressed all feedback you've received, not just part of it. In case you think there's a comment where the reviewer is wrong, do not just silently ignore reviewer comments but engage by explaining why you think the patch is fine as is. -- i. > Changes since v6: > > - The driver moved to drivers/leds/rgb/, where the other multicolor > LED drivers live. > > - The series is submitted with git send-email this time, so the > patch format is the standard one. > > The SCSI device handler attachment is unchanged; why it is a device > handler at all, and what the in-tree split should look like, is the > open discussion in the v6 thread. > > Everything else is unchanged from v6: the hardware description, the > scsi_device_handler that does not claim the sdev, the multicolor LED > interface, the protocol handling and the known caveats (manual > attach until the split lands; SAVE on every update writes the > enclosure flash, wear uncharacterized; NULL-parent LED registration > to avoid the sdev reference cycle). > > Liang Haowen (1): > leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe > enclosures > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-23 10:41 ` [RFC v7 0/1] " Ilpo Järvinen @ 2026-09-23 12:35 ` Denis Benato 2026-09-23 12:59 ` Liang Haowen 0 siblings, 1 reply; 27+ messages in thread From: Denis Benato @ 2026-09-23 12:35 UTC (permalink / raw) To: Ilpo Järvinen, Liang Haowen Cc: linux-leds, Lee Jones, Pavel Machek, Martin K . Petersen, linux-scsi, platform-driver-x86, LKML, Armin Wolf, Hans de Goede, Derek J. Clark, Marco Scardovi, Ahmed Yaseen On 9/23/26 12:41, Ilpo Järvinen wrote: > On Wed, 23 Sep 2026, Liang Haowen wrote: > >> Hello, >> >> v7, as its own thread, addressing Lee's review of v6. > No, you didn't address Lee's comments but only a small part of them. :-( > > Please slow down so you've time to address all feedback properly and > double check before the next submission you've addressed all feedback > you've received, not just part of it. > > In case you think there's a comment where the reviewer is wrong, do not > just silently ignore reviewer comments but engage by explaining why you > think the patch is fine as is. > Hi all, This person is currently in asus-linux discor and he's doing what I asked him to do: move LEDs commands from asusd (userspace) to the kernel and for me the important part (and what I suggest review focus on) is having a verified hardware handling code, while the led interface won't be final. The weirdness of the driver comes from the fact that we agree on touching the least amount possible of SCSI code since the storage part works very well already and we simply want to bolt LEDs on top of it without risking regressions on essential functionality. Marco has drafted and is working on the new interface and published a first version that I haven't got the time to review yet (university exams period). Anyway this interface should also be able to support what lenovo legion go drivers currently do and when accepted it should be used by at the very least asus, msi, lenovo but work will be long. Liang please coordinate with Marco to use that interface (or draft a version that can do that today). This will make like of userspace tools developers (including other members of asus-linux) much easier. Thanks. Link: https://github.com/OpenGamingCollective/linux-unstable/pull/17 Best regards, Denis Benato ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-23 12:35 ` Denis Benato @ 2026-09-23 12:59 ` Liang Haowen 2026-09-23 15:11 ` Marco Scardovi 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-23 12:59 UTC (permalink / raw) To: Denis Benato, Ilpo Järvinen Cc: linux-leds, Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Armin Wolf, Hans de Goede, Derek J. Clark, Marco Scardovi, Ahmed Yaseen Hi Denis, Thanks for the context. I have read Marco's Dynamic Lighting class series (PR #17). The direct frame and palette interfaces map cleanly onto what the Arion needs: its colour tables are just small RGB frames, and the enclosure firmware effects map onto the class effect controls. I will coordinate with him on using it for the LED side, on top of the verified SCSI handling this series carries. On the open review items: the next revision will isolate the DMA buffer into its own cacheline (a real issue on non-coherent architectures, as the bot notes), and I will take the time to get the full pass right instead of rushing again. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-23 12:59 ` Liang Haowen @ 2026-09-23 15:11 ` Marco Scardovi 0 siblings, 0 replies; 27+ messages in thread From: Marco Scardovi @ 2026-09-23 15:11 UTC (permalink / raw) To: Denis Benato, Ilpo Järvinen, Liang Haowen Cc: linux-leds, Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Armin Wolf, Hans de Goede, Derek J. Clark, Ahmed Yaseen In data mercoledì 23 settembre 2026 14:59:36 Ora legale dell’Europa centrale, Liang Haowen ha scritto: > Hi Denis, > > Thanks for the context. > > I have read Marco's Dynamic Lighting class series (PR #17). The > direct frame and palette interfaces map cleanly onto what the Arion > needs: its colour tables are just small RGB frames, and the > enclosure firmware effects map onto the class effect controls. I > will coordinate with him on using it for the LED side, on top of > the verified SCSI handling this series carries. > > On the open review items: the next revision will isolate the DMA > buffer into its own cacheline (a real issue on non-coherent > architectures, as the bot notes), and I will take the time to get > the full pass right instead of rushing again. Hi everyone, as for now please consider the new interface as a far from done one: it has basic functions and works good on my laptop but, that means it is tested only on my device using kernel 7.2.y. If you have time and want to test it out/give feedbacks they are more than welcomed (tbh I've yet to address these given by @Dereck due to personal reasons but I promise I'll work on them too asap). @Liang if you look into it there is a basic version of SCSI for your device using the new interface: if you want to look at it feel free to do so/suggest changes: I'll probably drop it in a future rebase to make the patchset smaller (then again I'll have to do countless threads here in lore for leds, hid, wmi, etc etc etc so it will take months). Best regards, Marco ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC 0/1] leds: add ASUS Aura SCSI driver for ROG NVMe enclosures @ 2026-09-01 14:26 Liang Haowen 2026-09-01 14:34 ` [PATCH RFC 1/1] " Liang Haowen 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-01 14:26 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen Hello, this is the v1 I promised in the "Placement of ASUS Aura and platform/x86 ASUS files relocation" thread: the LED side of ASUS Aura RGB on ROG external NVMe enclosures, posted for review in drivers/leds as Armin suggested, and in line with the shared ASUS Aura interface Denis has been coordinating. The hardware: ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are plain USB mass-storage devices. They expose two mass storage interfaces (BOT and UAS) and no HID interface; the Aura LEDs hang off an ENE controller reached through vendor SCSI commands on the same LUN as the disk. The Arion has 4 independently addressable LEDs, verified on hardware. The interface: the driver registers a scsi_device_handler, matches by INQUIRY strings (vendor "ROG", model "ESD-S1C"), does not claim the sdev (sd keeps owning the disk), and exposes each LED as a multicolor LED class device, /sys/class/leds/asus-arion:led0 through led3. Protocol summary: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, register index, argument count in cdb[13]). MODE 0x8021 (Static) must be written first in every sequence or the device ignores the whole sequence; colours are written to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes each, order R, B, G; both tables are written because firmware revisions pull from one or the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save, and only the save makes a change stick. A transport gotcha that seems worth wider visibility: the CDB cannot go through scsi_execute_cmd(), because it derives the command length from scsi_command_size(opcode), which maps vendor opcode 0xec to 10 bytes. The ENE protocol is a 16-byte CDB with the data length in cdb[13], so scsi_execute_cmd() drops that byte and the device silently ignores the write (GOOD status, no error). The driver builds the block request by hand and forces cmd_len = 16, which is what SG_IO does from userspace. Any kernel driver sending a vendor CDB whose real length does not match scsi_command_size(opcode) is going to hit the same thing. Placement: per Armin's suggestion in the thread, the LED side belongs in drivers/leds, since the enclosure is not a platform device and the user-facing interface is the multicolor LED sysfs. What is posted here is the driver as verified on hardware, still monolithic. The agreed shape going forward, with Denis, is a SCSI transport helper in drivers/scsi feeding an Aura LED driver in drivers/leds behind a shared ASUS Aura interface; the Kconfig, Makefile and MAINTAINERS wiring lands with that split. So the main questions for this round are the LED interface, the protocol handling and the placement. Known caveats, stated up front: - the handler attaches manually until a notifier lands (echo asus_aura > /sys/block/sdX/device/dh_state); - SAVE (0xaa) is issued on every colour change, which writes the enclosure flash each time; wear has not been characterized yet; - the LEDs are registered with a NULL parent device, because parenting them to the sdev creates a reference cycle that blocks the sdev's final release on unplug and leaks the LED nodes and the module refcount. Comments on the interface shape and on folding this into the shared Aura work are very welcome. Signed-off-by: Liang Haowen <nbg2974@gmail.com> Liang Haowen (1): leds: add ASUS Aura SCSI driver for ROG NVMe enclosures drivers/leds/leds-asus-aura-scsi.c | 302 +++++++++++++++++++++++++++++++++++++ 1 file changed, 302 insertions(+) -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC 1/1] leds: add ASUS Aura SCSI driver for ROG NVMe enclosures 2026-09-01 14:26 [PATCH RFC 0/1] leds: add ASUS Aura SCSI " Liang Haowen @ 2026-09-01 14:34 ` Liang Haowen 2026-09-03 12:00 ` [PATCH RFC v2 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED " Liang Haowen 2026-09-03 12:00 ` [PATCH RFC v2 0/1] " Liang Haowen 0 siblings, 2 replies; 27+ messages in thread From: Liang Haowen @ 2026-09-01 14:34 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are plain USB mass-storage devices with no HID interface: the Aura LEDs hang off an ENE controller driven by vendor SCSI commands on the same LUN as the disk. The enclosure has 4 independently addressable LEDs, verified on hardware. Register a scsi_device_handler matched by INQUIRY (vendor "ROG", model "ESD-S1C"); it does not claim the sdev (sd keeps owning the disk) and exposes each LED as a multicolor LED class device, /sys/class/leds/asus-arion:led0 through led3. Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, register index, argument count in cdb[13]). MODE 0x8021 (Static) must be written first in every sequence or the device ignores it; colours go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; both tables are written because firmware revisions pull from one or the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. The CDB cannot go through scsi_execute_cmd(): it sizes the command via scsi_command_size(opcode), which maps vendor opcode 0xec to 10 bytes, so cdb[13] is dropped and the device silently ignores the write (GOOD status, no error). Build the block request by hand and force cmd_len = 16, mirroring what SG_IO does from userspace. This is the monolithic out-of-tree version as verified on hardware; the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split into a SCSI transport helper and a shared ASUS Aura LED interface. Signed-off-by: Liang Haowen <nbg2974@gmail.com> --- ------------------------------------------------------------------------ drivers/leds/leds-asus-aura-scsi.c | 302 ++++++++++ 1 file changed, 302 insertions(+) ------------------------------------------------------------------------ diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c new file mode 100644 index 000000000000..111111111111 --- /dev/null +++ b/drivers/leds/leds-asus-aura-scsi.c @@ -0,0 +1,302 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures + * (e.g. ROG STRIX Arion, USB 0b05:1932). + * + * USB mass-storage device, no HID; the ENE LED controller is driven via + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), + * does NOT claim the sdev (sd keeps owning the disk). + * + * The Arion exposes 4 independently addressable LEDs (verified on hardware): + * each is a multicolor LED class device (asus-arion:led0..led3). A colour + * change writes that LED's slot only: EFFECT 0x8160 + 3*led, DIRECT + * 0x8100 + 3*led (3 bytes, byte order R, B, G), then APPLY (0x01) and + * SAVE (0xaa). MODE (0x8021 = Static) is written first in every sequence; + * skipping it makes the device ignore the whole sequence. + * + * Uses brightness_set (non-blocking LED core fast path) + a work_struct + * for the sleeping block-request vendor CDB send. + * + * CDB length: scsi_execute_cmd() sizes the CDB via scsi_command_size(opcode), + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] + * and the device silently ignores the write. We build the request by hand + * and force cmd_len = 16 (what SG_IO does from userspace). + * + * Attach manually until a notifier lands: + * echo asus_aura > /sys/block/sdX/device/dh_state + */ + +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/leds.h> +#include <linux/led-class-multicolor.h> +#include <linux/blk_types.h> +#include <linux/blkdev.h> +#include <linux/blk-mq.h> +#include <linux/workqueue.h> +#include <scsi/scsi.h> +#include <scsi/scsi_cmnd.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi_dh.h> + +#define ARION_INQ_VENDOR "ROG" +#define ARION_INQ_MODEL "ESD-S1C" + +#define ENE_OPCODE 0xec +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ +#define ENE_REG_APPLY 0x80a0 +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ +#define ENE_APPLY 0x01 +#define ENE_SAVE 0xaa +#define ENE_MODE_STATIC 1 +#define ENE_CDB_LEN 16 +#define ENE_RGB_LEN 3 + +/* + * Verified on hardware: the enclosure has 4 independently settable LEDs. + * (The colour table reserves 16 slots; only the first 4 drive anything.) + */ +#define ARION_NUM_LEDS 4 + +struct asus_aura_led { + struct asus_aura_zone *zone; + int index; + struct led_classdev_mc mc_cdev; + struct mc_subled subled[3]; + u8 rgb[ENE_RGB_LEN]; + struct work_struct work; +}; + +struct asus_aura_zone { + struct scsi_device *sdev; + struct asus_aura_led leds[ARION_NUM_LEDS]; +}; + +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) +{ + memset(cdb, 0, ENE_CDB_LEN); + cdb[0] = ENE_OPCODE; + cdb[1] = 'A'; + cdb[2] = 'S'; + cdb[3] = (reg >> 8) & 0xff; + cdb[4] = reg & 0xff; + cdb[13] = arg_count; +} + +/* Raw block request so we can force cmd_len=16 (see file header). */ +static int ene_write(struct scsi_device *sdev, u16 reg, + const void *data, u8 arg_count) +{ + struct request *rq; + struct scsi_cmnd *scmd; + u8 cdb[ENE_CDB_LEN]; + int ret; + + ene_build_cdb(cdb, reg, arg_count); + + rq = blk_mq_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); + if (IS_ERR(rq)) + return PTR_ERR(rq); + + scmd = blk_mq_rq_to_pdu(rq); + scmd->cmd_len = ENE_CDB_LEN; + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); + + if (arg_count) { + ret = blk_rq_map_kern(rq, + (void *)data, arg_count, GFP_KERNEL); + if (ret) + goto out; + } + + blk_execute_rq(rq, true); + ret = scmd->result; +out: + blk_mq_free_request(rq); + return ret; +} + +/* Sleepable: runs on the system workqueue. Writes one LED's slot. */ +static void asus_aura_led_work(struct work_struct *work) +{ + struct asus_aura_led *led = + container_of(work, struct asus_aura_led, work); + struct scsi_device *sdev = led->zone->sdev; + u8 apply = ENE_APPLY; + u8 save = ENE_SAVE; + u8 mode = ENE_MODE_STATIC; + int ret; + + if (!scsi_device_online(sdev)) + return; + + /* Mode first: without it the device ignores the whole sequence. */ + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); + if (ret) + goto err; + + ret = ene_write(sdev, ENE_REG_COLORS + led->index * ENE_RGB_LEN, + led->rgb, ENE_RGB_LEN); + if (ret) + goto err; + + /* + * Cover the DIRECT colour set too; some firmware revisions pull + * from 0x8100 instead of 0x8160. + */ + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + led->index * ENE_RGB_LEN, + led->rgb, ENE_RGB_LEN); + if (ret) + goto err; + + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); + if (ret) + goto err; + + /* + * The change only takes effect after SAVE (0xaa). NOTE: saving on + * every brightness change writes flash each time; revisit for wear + * once confirmed. + */ + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); + if (ret) + goto err; + + return; +err: + dev_err(&sdev->sdev_gendev, + "asus_aura: led%d write failed: %d\n", led->index, ret); +} + +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ +static void asus_aura_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); + struct asus_aura_led *led = + container_of(mc, struct asus_aura_led, mc_cdev); + + led_mc_calc_color_components(mc, brightness); + /* ENE colour register byte order is R, B, G. */ + led->rgb[0] = led->subled[0].brightness; + led->rgb[1] = led->subled[2].brightness; + led->rgb[2] = led->subled[1].brightness; + + schedule_work(&led->work); +} + +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) +{ + struct asus_aura_led *led = &zone->leds[index]; + struct led_classdev *cdev = &led->mc_cdev.led_cdev; + int ret; + + led->zone = zone; + led->index = index; + + led->subled[0].color_index = LED_COLOR_ID_RED; + led->subled[1].color_index = LED_COLOR_ID_GREEN; + led->subled[2].color_index = LED_COLOR_ID_BLUE; + led->mc_cdev.num_colors = 3; + led->mc_cdev.subled_info = led->subled; + + cdev->name = kasprintf(GFP_KERNEL, "asus-arion:led%d", index); + if (!cdev->name) + return -ENOMEM; + cdev->max_brightness = 255; + cdev->brightness_set = asus_aura_set; + + INIT_WORK(&led->work, asus_aura_led_work); + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); + + /* + * Register with NULL parent: parenting the LED to the sdev takes a + * device reference, which blocks the sdev's final release on unplug, + * which is what calls scsi_dh_release_device() -> our .detach() that + * unregisters the LEDs. That reference cycle leaked the LED nodes and + * the module refcount on every hot-unplug. + */ + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); + if (ret) + kfree(cdev->name); + return ret; +} + +static int asus_aura_attach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone; + int i, ret; + + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) + return SCSI_DH_DEV_UNSUPP; + + zone = kzalloc_obj(*zone, GFP_KERNEL); + if (!zone) + return SCSI_DH_NOMEM; + zone->sdev = sdev; + + for (i = 0; i < ARION_NUM_LEDS; i++) { + ret = asus_aura_register_led(zone, i); + if (ret) + goto err_free; + } + + sdev->handler_data = zone; + sdev_printk(KERN_INFO, sdev, + "asus_aura: %d per-LED multicolor LEDs registered\n", + ARION_NUM_LEDS); + return SCSI_DH_OK; + +err_free: + while (i--) { + cancel_work_sync(&zone->leds[i].work); + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); + kfree(zone->leds[i].mc_cdev.led_cdev.name); + } + kfree(zone); + return SCSI_DH_NOMEM; +} + +static void asus_aura_detach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone = sdev->handler_data; + int i; + + if (!zone) + return; + for (i = 0; i < ARION_NUM_LEDS; i++) { + cancel_work_sync(&zone->leds[i].work); + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); + kfree(zone->leds[i].mc_cdev.led_cdev.name); + } + kfree(zone); + sdev->handler_data = NULL; +} + +static struct scsi_device_handler asus_aura_dh = { + .name = "asus_aura", + .module = THIS_MODULE, + .attach = asus_aura_attach, + .detach = asus_aura_detach, +}; + +static int __init asus_aura_init(void) +{ + return scsi_register_device_handler(&asus_aura_dh); +} + +static void __exit asus_aura_exit(void) +{ + scsi_unregister_device_handler(&asus_aura_dh); +} + +module_init(asus_aura_init); +module_exit(asus_aura_exit); + +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); +MODULE_AUTHOR("Liang Haowen"); +MODULE_LICENSE("GPL"); ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC v2 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-01 14:34 ` [PATCH RFC 1/1] " Liang Haowen @ 2026-09-03 12:00 ` Liang Haowen [not found] ` <20260903161357.GX2133376@google.com> 2026-09-03 12:00 ` [PATCH RFC v2 0/1] " Liang Haowen 1 sibling, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-03 12:00 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are plain USB mass-storage devices with no HID interface: the Aura LEDs hang off an ENE controller driven by vendor SCSI commands on the same LUN as the disk. The enclosure has 4 independently addressable LEDs, verified on hardware. Register a scsi_device_handler matched by INQUIRY (vendor "ROG", model "ESD-S1C"); it does not claim the sdev (sd keeps owning the disk) and exposes each LED as a multicolor LED class device, /sys/class/leds/asus-arion:led0 through led3. Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, register index, argument count in cdb[13]). MODE 0x8021 (Static) must be written first in every sequence or the device ignores it; colours go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; both tables are written because firmware revisions pull from one or the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. The CDB cannot go through scsi_execute_cmd(): it sizes the command via scsi_command_size(opcode), which maps vendor opcode 0xec to 10 bytes, so cdb[13] is dropped and the device silently ignores the write (GOOD status, no error). The request is built with scsi_alloc_request() instead, which fully initializes the scsi_cmnd, with cmd_len forced to 16, mirroring what SG_IO does from userspace. brightness_set only caches the colour and marks the LED in a per-zone dirty bitmap; a single work item per zone then runs one ENE sequence for all pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every update through one work item keeps the sequences from interleaving between concurrent LED updates and batches multi-LED updates into a single APPLY/SAVE. This is the monolithic out-of-tree version as verified on hardware; the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split into a SCSI transport helper and a shared ASUS Aura LED interface. Signed-off-by: Liang Haowen <nbg2974@gmail.com> --- drivers/leds/leds-asus-aura-scsi.c | 332 +++++++++++++++++++++++++++++ 1 file changed, 332 insertions(+) create mode 100644 drivers/leds/leds-asus-aura-scsi.c diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c new file mode 100644 index 0000000..4dd559a --- /dev/null +++ b/drivers/leds/leds-asus-aura-scsi.c @@ -0,0 +1,332 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures + * (e.g. ROG STRIX Arion, USB 0b05:1932). + * + * USB mass-storage device, no HID; the ENE LED controller is driven via + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), + * does NOT claim the sdev (sd keeps owning the disk). + * + * The Arion exposes 4 independently addressable LEDs (verified on hardware): + * each is a multicolor LED class device (asus-arion:led0..led3). A colour + * change writes that LED's slot only: EFFECT 0x8160 + 3*led, DIRECT + * 0x8100 + 3*led (3 bytes, byte order R, B, G), then APPLY (0x01) and + * SAVE (0xaa). MODE (0x8021 = Static) is written first in every sequence; + * skipping it makes the device ignore the whole sequence. + * + * Scheduling: brightness_set (LED core fast path) caches the colour and + * marks the LED in a per-zone dirty bitmap; a single work item per zone + * runs one ENE sequence for all pending LEDs (MODE once, colour slots, + * APPLY, SAVE). Funneling every update through that one work item also + * serializes the sequences: the MODE/colour/APPLY/SAVE chain must never + * interleave between concurrent LED updates. + * + * CDB length: scsi_execute_cmd() sizes the CDB via COMMAND_SIZE(opcode), + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] + * and the device silently ignores the write. ene_write() therefore mirrors + * scsi_execute_cmd() on top of scsi_alloc_request() and forces cmd_len = 16 + * (what SG_IO does from userspace). + * + * Attach manually until a notifier lands: + * echo asus_aura > /sys/block/sdX/device/dh_state + */ + +#include <linux/module.h> +#include <linux/slab.h> +#include <linux/string.h> +#include <linux/leds.h> +#include <linux/led-class-multicolor.h> +#include <linux/blk_types.h> +#include <linux/blkdev.h> +#include <linux/blk-mq.h> +#include <linux/workqueue.h> +#include <scsi/scsi.h> +#include <scsi/scsi_cmnd.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi_dh.h> + +#define ARION_INQ_VENDOR "ROG" +#define ARION_INQ_MODEL "ESD-S1C" + +#define ENE_OPCODE 0xec +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ +#define ENE_REG_APPLY 0x80a0 +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ +#define ENE_APPLY 0x01 +#define ENE_SAVE 0xaa +#define ENE_MODE_STATIC 1 +#define ENE_CDB_LEN 16 +#define ENE_RGB_LEN 3 +#define ENE_TIMEOUT (10 * HZ) + +/* + * Verified on hardware: the enclosure has 4 independently settable LEDs. + * (The colour table reserves 16 slots; only the first 4 drive anything.) + */ +#define ARION_NUM_LEDS 4 + +struct asus_aura_led { + struct asus_aura_zone *zone; + int index; + struct led_classdev_mc mc_cdev; + struct mc_subled subled[3]; + u8 rgb[ENE_RGB_LEN]; +}; + +struct asus_aura_zone { + struct scsi_device *sdev; + struct asus_aura_led leds[ARION_NUM_LEDS]; + unsigned long dirty; /* bit i: led i needs a colour write */ + struct work_struct work; +}; + +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) +{ + memset(cdb, 0, ENE_CDB_LEN); + cdb[0] = ENE_OPCODE; + cdb[1] = 'A'; + cdb[2] = 'S'; + cdb[3] = (reg >> 8) & 0xff; + cdb[4] = reg & 0xff; + cdb[13] = arg_count; +} + +/* + * scsi_execute_cmd() with cmd_len forced to 16. scsi_alloc_request() + * initializes the scsi_cmnd fully (scsi_initialize_rq(): zeroed cmnd, sense + * and rcu head, retries), which a raw blk_mq_alloc_request() does not. + */ +static int ene_write(struct scsi_device *sdev, u16 reg, + const void *data, u8 arg_count) +{ + struct request *rq; + struct scsi_cmnd *scmd; + u8 cdb[ENE_CDB_LEN]; + int ret; + + ene_build_cdb(cdb, reg, arg_count); + + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); + if (IS_ERR(rq)) + return PTR_ERR(rq); + + if (arg_count) { + ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO); + if (ret) + goto out; + } + + scmd = blk_mq_rq_to_pdu(rq); + scmd->cmd_len = ENE_CDB_LEN; + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); + scmd->allowed = 1; + rq->timeout = ENE_TIMEOUT; + rq->rq_flags |= RQF_QUIET; + + blk_execute_rq(rq, true); + ret = scmd->result; +out: + blk_mq_free_request(rq); + return ret; +} + +/* + * Sleepable: runs on the system workqueue. One ENE sequence for every LED + * marked in the dirty bitmap. test_and_clear_bit() pairs with the set_bit() + * in asus_aura_set(): a colour cached while this runs requeues the work and + * is picked up by the next sequence. + */ +static void asus_aura_zone_work(struct work_struct *work) +{ + struct asus_aura_zone *zone = + container_of(work, struct asus_aura_zone, work); + struct scsi_device *sdev = zone->sdev; + u8 apply = ENE_APPLY; + u8 save = ENE_SAVE; + u8 mode = ENE_MODE_STATIC; + int i, ret; + + if (!scsi_device_online(sdev)) + return; + + /* Mode first: without it the device ignores the whole sequence. */ + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); + if (ret) + goto err; + + for (i = 0; i < ARION_NUM_LEDS; i++) { + struct asus_aura_led *led = &zone->leds[i]; + + if (!test_and_clear_bit(i, &zone->dirty)) + continue; + + ret = ene_write(sdev, ENE_REG_COLORS + i * ENE_RGB_LEN, + led->rgb, ENE_RGB_LEN); + if (ret) + goto err; + + /* + * Cover the DIRECT colour set too; some firmware revisions + * pull from 0x8100 instead of 0x8160. + */ + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + i * ENE_RGB_LEN, + led->rgb, ENE_RGB_LEN); + if (ret) + goto err; + } + + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); + if (ret) + goto err; + + /* + * The change only takes effect after SAVE (0xaa). NOTE: saving on + * every brightness change writes flash each time; revisit for wear + * once confirmed. + */ + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); + if (ret) + goto err; + + return; +err: + dev_err(&sdev->sdev_gendev, + "asus_aura: colour update failed: %d\n", ret); +} + +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ +static void asus_aura_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); + struct asus_aura_led *led = + container_of(mc, struct asus_aura_led, mc_cdev); + + led_mc_calc_color_components(mc, brightness); + /* ENE colour register byte order is R, B, G. */ + led->rgb[0] = led->subled[0].brightness; + led->rgb[1] = led->subled[2].brightness; + led->rgb[2] = led->subled[1].brightness; + + set_bit(led->index, &led->zone->dirty); + schedule_work(&led->zone->work); +} + +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) +{ + struct asus_aura_led *led = &zone->leds[index]; + struct led_classdev *cdev = &led->mc_cdev.led_cdev; + int ret; + + led->zone = zone; + led->index = index; + + led->subled[0].color_index = LED_COLOR_ID_RED; + led->subled[1].color_index = LED_COLOR_ID_GREEN; + led->subled[2].color_index = LED_COLOR_ID_BLUE; + led->mc_cdev.num_colors = 3; + led->mc_cdev.subled_info = led->subled; + + cdev->name = kasprintf(GFP_KERNEL, "asus-arion:led%d", index); + if (!cdev->name) + return -ENOMEM; + cdev->max_brightness = 255; + cdev->brightness_set = asus_aura_set; + + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); + + /* + * Register with NULL parent: parenting the LED to the sdev takes a + * device reference, which blocks the sdev's final release on unplug, + * which is what calls scsi_dh_release_device() -> our .detach() that + * unregisters the LEDs. That reference cycle leaked the LED nodes and + * the module refcount on every hot-unplug. + */ + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); + if (ret) + kfree(cdev->name); + return ret; +} + +/* + * Unregister the LED devices before cancelling the work: unregistering + * removes the sysfs attributes, so no new brightness_set can schedule the + * zone work afterwards, and it waits for in-flight sysfs callbacks. + * Cancelling first would leave a window where a brightness write requeues + * the work after cancel_work_sync() returned, and the work would then run + * on freed memory. + */ +static void asus_aura_release(struct asus_aura_zone *zone, int num_leds) +{ + int i; + + for (i = 0; i < num_leds; i++) + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); + cancel_work_sync(&zone->work); + for (i = 0; i < num_leds; i++) + kfree(zone->leds[i].mc_cdev.led_cdev.name); + kfree(zone); +} + +static int asus_aura_attach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone; + int i, ret; + + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) + return SCSI_DH_DEV_UNSUPP; + + zone = kzalloc_obj(*zone, GFP_KERNEL); + if (!zone) + return SCSI_DH_NOMEM; + zone->sdev = sdev; + INIT_WORK(&zone->work, asus_aura_zone_work); + + for (i = 0; i < ARION_NUM_LEDS; i++) { + ret = asus_aura_register_led(zone, i); + if (ret) { + asus_aura_release(zone, i); + return SCSI_DH_NOMEM; + } + } + + sdev->handler_data = zone; + return SCSI_DH_OK; +} + +static void asus_aura_detach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone = sdev->handler_data; + + if (!zone) + return; + asus_aura_release(zone, ARION_NUM_LEDS); + sdev->handler_data = NULL; +} + +static struct scsi_device_handler asus_aura_dh = { + .name = "asus_aura", + .module = THIS_MODULE, + .attach = asus_aura_attach, + .detach = asus_aura_detach, +}; + +static int __init asus_aura_init(void) +{ + return scsi_register_device_handler(&asus_aura_dh); +} + +static void __exit asus_aura_exit(void) +{ + scsi_unregister_device_handler(&asus_aura_dh); +} + +module_init(asus_aura_init); +module_exit(asus_aura_exit); + +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); +MODULE_AUTHOR("Liang Haowen"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20260903161357.GX2133376@google.com>]
* [PATCH RFC v3 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures [not found] ` <20260903161357.GX2133376@google.com> @ 2026-09-04 12:30 ` Liang Haowen 2026-09-04 12:30 ` [PATCH RFC v3 1/1] " Liang Haowen 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-04 12:30 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen Hello, v3, addressing the five points of the second sashiko review round, the ones Lee asked to review, explain or fix. Changes since v2: - Empty work runs no longer touch the device. schedule_work() while the zone work is executing re-queues it, and a colour cached in the meantime may already have been consumed; the re-queued run then had an empty dirty set but still issued MODE, APPLY and SAVE, and SAVE writes the enclosure flash. The work now snapshots the dirty mask first and returns before any SCSI command when nothing is pending. - The dirty mask and the cached colours are now protected by a per-zone spinlock, and the work writes from a snapshot taken under that lock. Previously the colour write and the (unlocked) bit set could be reordered on weakly ordered architectures, letting the work consume the dirty bit with a stale colour and lose the update. - LED class device names now include the sdev's H:C:T:L (asus-arion-<H:C:T:L>:ledN). Every enclosure gets its own SCSI host, so the names stay unique when more than one is connected. With the static names the LED core would register a second enclosure's LEDs under renamed nodes (asus-arion:led0_1), which is the wrong device identity. Like sd letters, the names are per-attachment. The two low-severity items are false positives: - blk_rq_map_kern() takes four arguments on current kernels (rq, buf, len, gfp); drivers/scsi/scsi_lib.c calls it exactly this way from scsi_execute_cmd(). - kzalloc_obj() exists in include/linux/slab.h since v7.0. v3 was re-verified on hardware: per-LED colours, repeated identical writes, concurrent updates from four writers and unplug under load are clean. Everything else is unchanged: the hardware description, the scsi_device_handler that does not claim the sdev, the multicolor LED interface, the protocol handling and the known caveats (manual attach until a notifier lands; SAVE on every update writes the enclosure flash, wear uncharacterized; NULL-parent LED registration to avoid the sdev reference cycle). One open question for the RFC stage: the driver is deliberately not wired into Kconfig/Makefile/MAINTAINERS yet, because the agreed direction with the SCSI side is a split into a SCSI transport helper and a shared ASUS Aura LED interface, and the wiring would follow that shape. Is deferring the wiring to that split acceptable for an RFC, or would you rather have the driver buildable in-tree from this series already? Comments on the interface shape and on folding this into the shared Aura work with Denis remain very welcome. Signed-off-by: Liang Haowen <nbg2974@gmail.com> Liang Haowen (1): leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures drivers/leds/leds-asus-aura-scsi.c | 375 +++++++++++++++++++++++++++++ 1 file changed, 375 insertions(+) -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC v3 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-04 12:30 ` [PATCH RFC v3 0/1] " Liang Haowen @ 2026-09-04 12:30 ` Liang Haowen [not found] ` <20260904131503.355C41F00A3E@smtp.kernel.org> 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-04 12:30 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are plain USB mass-storage devices with no HID interface: the Aura LEDs hang off an ENE controller driven by vendor SCSI commands on the same LUN as the disk. The enclosure has 4 independently addressable LEDs, verified on hardware. Register a scsi_device_handler matched by INQUIRY (vendor "ROG", model "ESD-S1C"); it does not claim the sdev (sd keeps owning the disk) and exposes each LED as a multicolor LED class device, /sys/class/leds/asus-arion-<H:C:T:L>:led0 through led3. The H:C:T:L part keeps the names unique when more than one enclosure is connected. Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, register index, argument count in cdb[13]). MODE 0x8021 (Static) must be written first in every sequence or the device ignores it; colours go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; both tables are written because firmware revisions pull from one or the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. The CDB cannot go through scsi_execute_cmd(): it sizes the command via scsi_command_size(opcode), which maps vendor opcode 0xec to 10 bytes, so cdb[13] is dropped and the device silently ignores the write (GOOD status, no error). The request is built with scsi_alloc_request() instead, which initializes the scsi_cmnd parts a passthrough needs (command buffer, lengths, rcu head), with cmd_len forced to 16, mirroring what SG_IO does from userspace. brightness_set only caches the colour and marks the LED in a per-zone dirty mask under a spinlock; a single work item per zone then snapshots the mask and colours and runs one ENE sequence for all pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every update through one work item keeps the sequences from interleaving between concurrent LED updates and batches multi-LED updates into a single APPLY/SAVE. A re-queued run with nothing pending returns before touching the device, so it cannot wear the flash with a pointless SAVE, and the lock keeps a colour write from being reordered after its dirty bit on weakly ordered architectures. This is the monolithic out-of-tree version as verified on hardware; the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split into a SCSI transport helper and a shared ASUS Aura LED interface. Signed-off-by: Liang Haowen <nbg2974@gmail.com> --- drivers/leds/leds-asus-aura-scsi.c | 375 +++++++++++++++++++++++++++++ 1 file changed, 375 insertions(+) create mode 100644 drivers/leds/leds-asus-aura-scsi.c diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c new file mode 100644 index 0000000..3ff3361 --- /dev/null +++ b/drivers/leds/leds-asus-aura-scsi.c @@ -0,0 +1,375 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures + * (e.g. ROG STRIX Arion, USB 0b05:1932). + * + * USB mass-storage device, no HID; the ENE LED controller is driven via + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), + * does NOT claim the sdev (sd keeps owning the disk). + * + * The Arion exposes 4 independently addressable LEDs (verified on hardware): + * each is a multicolor LED class device (asus-arion-<H:C:T:L>:led0..led3, + * unique per enclosure). A colour change writes that LED's slot only: + * EFFECT 0x8160 + 3*led, DIRECT 0x8100 + 3*led (3 bytes, byte order R, B, G), + * then APPLY (0x01) and SAVE (0xaa). MODE (0x8021 = Static) is written first + * in every sequence; skipping it makes the device ignore the whole sequence. + * + * Scheduling: brightness_set (LED core fast path) caches the colour and + * marks the LED in a per-zone dirty mask under a spinlock; a single work + * item per zone snapshots the mask and colours, then runs one ENE + * sequence for all pending LEDs (MODE once, colour slots, APPLY, SAVE). + * Funneling every update through that one work item also serializes the + * sequences: the MODE/colour/APPLY/SAVE chain must never interleave + * between concurrent LED updates. The snapshot makes re-queued runs with + * nothing left to do return before touching the device, so a re-queue + * cannot wear the flash with a pointless SAVE, and the lock keeps a + * colour write from being reordered after its dirty bit on weakly + * ordered architectures. + * + * CDB length: scsi_execute_cmd() sizes the CDB via COMMAND_SIZE(opcode), + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] + * and the device silently ignores the write. ene_write() therefore mirrors + * scsi_execute_cmd() on top of scsi_alloc_request() and forces cmd_len = 16 + * (what SG_IO does from userspace). + * + * Attach manually until a notifier lands: + * echo asus_aura > /sys/block/sdX/device/dh_state + */ + +#include <linux/module.h> +#include <linux/bits.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/string.h> +#include <linux/leds.h> +#include <linux/led-class-multicolor.h> +#include <linux/blk_types.h> +#include <linux/blkdev.h> +#include <linux/blk-mq.h> +#include <linux/workqueue.h> +#include <scsi/scsi.h> +#include <scsi/scsi_cmnd.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi_dh.h> + +#define ARION_INQ_VENDOR "ROG" +#define ARION_INQ_MODEL "ESD-S1C" + +#define ENE_OPCODE 0xec +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ +#define ENE_REG_APPLY 0x80a0 +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ +#define ENE_APPLY 0x01 +#define ENE_SAVE 0xaa +#define ENE_MODE_STATIC 1 +#define ENE_CDB_LEN 16 +#define ENE_RGB_LEN 3 +#define ENE_TIMEOUT (10 * HZ) + +/* + * Verified on hardware: the enclosure has 4 independently settable LEDs. + * (The colour table reserves 16 slots; only the first 4 drive anything.) + */ +#define ARION_NUM_LEDS 4 + +struct asus_aura_led { + struct asus_aura_zone *zone; + int index; + struct led_classdev_mc mc_cdev; + struct mc_subled subled[3]; + u8 rgb[ENE_RGB_LEN]; +}; + +struct asus_aura_zone { + struct scsi_device *sdev; + struct asus_aura_led leds[ARION_NUM_LEDS]; + spinlock_t lock; /* protects dirty and cached colours */ + u8 dirty; /* bit i: led i needs a colour write */ + struct work_struct work; +}; + +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) +{ + memset(cdb, 0, ENE_CDB_LEN); + cdb[0] = ENE_OPCODE; + cdb[1] = 'A'; + cdb[2] = 'S'; + cdb[3] = (reg >> 8) & 0xff; + cdb[4] = reg & 0xff; + cdb[13] = arg_count; +} + +/* + * scsi_execute_cmd() with cmd_len forced to 16. scsi_alloc_request() + * initializes the parts of the scsi_cmnd a passthrough needs (zeroed + * cmnd, cmd_len = MAX_COMMAND_SIZE, sense_len, rcu head, retries); + * a raw blk_mq_alloc_request() does none of that. + */ +static int ene_write(struct scsi_device *sdev, u16 reg, + const void *data, u8 arg_count) +{ + struct request *rq; + struct scsi_cmnd *scmd; + u8 cdb[ENE_CDB_LEN]; + int ret; + + ene_build_cdb(cdb, reg, arg_count); + + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); + if (IS_ERR(rq)) + return PTR_ERR(rq); + + if (arg_count) { + ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO); + if (ret) + goto out; + } + + scmd = blk_mq_rq_to_pdu(rq); + scmd->cmd_len = ENE_CDB_LEN; + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); + scmd->allowed = 1; + rq->timeout = ENE_TIMEOUT; + rq->rq_flags |= RQF_QUIET; + + blk_execute_rq(rq, true); + ret = scmd->result; +out: + blk_mq_free_request(rq); + return ret; +} + +/* + * Sleepable: runs on the system workqueue. One ENE sequence for every LED + * marked in the dirty mask. The mask and colours are snapshotted under the + * zone lock: asus_aura_set() may run concurrently on another CPU, and the + * lock keeps a colour write from being reordered after its dirty bit on + * weakly ordered architectures. A colour cached while this runs requeues + * the work and is picked up by the next sequence. + */ +static void asus_aura_zone_work(struct work_struct *work) +{ + struct asus_aura_zone *zone = + container_of(work, struct asus_aura_zone, work); + struct scsi_device *sdev = zone->sdev; + u8 rgb[ARION_NUM_LEDS][ENE_RGB_LEN]; + u8 apply = ENE_APPLY; + u8 save = ENE_SAVE; + u8 mode = ENE_MODE_STATIC; + unsigned long flags; + u8 pending; + int i, ret; + + spin_lock_irqsave(&zone->lock, flags); + pending = zone->dirty; + zone->dirty = 0; + for (i = 0; i < ARION_NUM_LEDS; i++) + memcpy(rgb[i], zone->leds[i].rgb, ENE_RGB_LEN); + spin_unlock_irqrestore(&zone->lock, flags); + + /* + * schedule_work() while this function runs requeues it, and the + * pending colour may already have been consumed above; the requeued + * run then has nothing to do. Return before touching the device: + * SAVE writes its flash. + */ + if (!pending) + return; + + if (!scsi_device_online(sdev)) + return; + + /* Mode first: without it the device ignores the whole sequence. */ + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); + if (ret) + goto err; + + for (i = 0; i < ARION_NUM_LEDS; i++) { + if (!(pending & BIT(i))) + continue; + + ret = ene_write(sdev, ENE_REG_COLORS + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + + /* + * Cover the DIRECT colour set too; some firmware revisions + * pull from 0x8100 instead of 0x8160. + */ + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + } + + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); + if (ret) + goto err; + + /* + * The change only takes effect after SAVE (0xaa). NOTE: saving on + * every brightness change writes flash each time; revisit for wear + * once confirmed. + */ + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); + if (ret) + goto err; + + return; +err: + dev_err(&sdev->sdev_gendev, + "asus_aura: colour update failed: %d\n", ret); +} + +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ +static void asus_aura_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); + struct asus_aura_led *led = + container_of(mc, struct asus_aura_led, mc_cdev); + struct asus_aura_zone *zone = led->zone; + unsigned long flags; + + led_mc_calc_color_components(mc, brightness); + + spin_lock_irqsave(&zone->lock, flags); + /* ENE colour register byte order is R, B, G. */ + led->rgb[0] = led->subled[0].brightness; + led->rgb[1] = led->subled[2].brightness; + led->rgb[2] = led->subled[1].brightness; + zone->dirty |= BIT(led->index); + spin_unlock_irqrestore(&zone->lock, flags); + + schedule_work(&zone->work); +} + +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) +{ + struct asus_aura_led *led = &zone->leds[index]; + struct led_classdev *cdev = &led->mc_cdev.led_cdev; + int ret; + + led->zone = zone; + led->index = index; + + led->subled[0].color_index = LED_COLOR_ID_RED; + led->subled[1].color_index = LED_COLOR_ID_GREEN; + led->subled[2].color_index = LED_COLOR_ID_BLUE; + led->mc_cdev.num_colors = 3; + led->mc_cdev.subled_info = led->subled; + + /* + * Include the sdev's H:C:T:L: every enclosure gets its own SCSI + * host, so the names stay unique when more than one is connected. + * With a static name the LED core would register the second + * enclosure's LEDs under renamed nodes (asus-arion:led0_1), which + * is the wrong device identity. The names are per-attachment, like + * sd X letters, and userspace is expected to enumerate. + */ + cdev->name = kasprintf(GFP_KERNEL, "asus-arion-%s:led%d", + dev_name(&zone->sdev->sdev_gendev), index); + if (!cdev->name) + return -ENOMEM; + cdev->max_brightness = 255; + cdev->brightness_set = asus_aura_set; + + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); + + /* + * Register with NULL parent: parenting the LED to the sdev takes a + * device reference, which blocks the sdev's final release on unplug, + * which is what calls scsi_dh_release_device() -> our .detach() that + * unregisters the LEDs. That reference cycle leaked the LED nodes and + * the module refcount on every hot-unplug. + */ + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); + if (ret) + kfree(cdev->name); + return ret; +} + +/* + * Unregister the LED devices before cancelling the work: unregistering + * removes the sysfs attributes, so no new brightness_set can schedule the + * zone work afterwards, and it waits for in-flight sysfs callbacks. + * Cancelling first would leave a window where a brightness write requeues + * the work after cancel_work_sync() returned, and the work would then run + * on freed memory. + */ +static void asus_aura_release(struct asus_aura_zone *zone, int num_leds) +{ + int i; + + for (i = 0; i < num_leds; i++) + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); + cancel_work_sync(&zone->work); + for (i = 0; i < num_leds; i++) + kfree(zone->leds[i].mc_cdev.led_cdev.name); + kfree(zone); +} + +static int asus_aura_attach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone; + int i, ret; + + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) + return SCSI_DH_DEV_UNSUPP; + + zone = kzalloc_obj(*zone, GFP_KERNEL); + if (!zone) + return SCSI_DH_NOMEM; + zone->sdev = sdev; + spin_lock_init(&zone->lock); + INIT_WORK(&zone->work, asus_aura_zone_work); + + for (i = 0; i < ARION_NUM_LEDS; i++) { + ret = asus_aura_register_led(zone, i); + if (ret) { + asus_aura_release(zone, i); + return SCSI_DH_NOMEM; + } + } + + sdev->handler_data = zone; + return SCSI_DH_OK; +} + +static void asus_aura_detach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone = sdev->handler_data; + + if (!zone) + return; + asus_aura_release(zone, ARION_NUM_LEDS); + sdev->handler_data = NULL; +} + +static struct scsi_device_handler asus_aura_dh = { + .name = "asus_aura", + .module = THIS_MODULE, + .attach = asus_aura_attach, + .detach = asus_aura_detach, +}; + +static int __init asus_aura_init(void) +{ + return scsi_register_device_handler(&asus_aura_dh); +} + +static void __exit asus_aura_exit(void) +{ + scsi_unregister_device_handler(&asus_aura_dh); +} + +module_init(asus_aura_init); +module_exit(asus_aura_exit); + +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); +MODULE_AUTHOR("Liang Haowen"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20260904131503.355C41F00A3E@smtp.kernel.org>]
[parent not found: <20260910093232.GO2133376@google.com>]
* [PATCH RFC v4 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures @ 2026-09-15 12:48 ` Liang Haowen 2026-09-15 12:49 ` [PATCH RFC v4 1/1] " Liang Haowen 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-15 12:48 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen Hello, v4, sent as its own thread this time, addressing the third sashiko review round (the Medium naming item is a real bug, the two Low ones are false positives again). Changes since v3: - LED class device names follow the devicename:color:function scheme again. v3 embedded the sdev's H:C:T:L verbatim (asus-arion-0:0:0:0:led0); the extra colons break the single separator userspace parses LED names with. The colons are now flattened to dashes (asus-arion-0-0-0-0:led0), keeping the per-attachment uniqueness while leaving exactly one separator. The two low-severity items are the same false positives as before: - kzalloc_obj() exists in include/linux/slab.h since v7.0 (Kees Cook's overflow-refactor series); this driver builds against 7.2. - blk_rq_map_kern() takes four arguments on current kernels (rq, buf, len, gfp); drivers/scsi/scsi_lib.c calls it exactly this way from scsi_execute_cmd(). v4 was re-verified on hardware: per-LED colours, 100 sequential updates, concurrent updates from four writers, and unplug under load (zero splats, zero leaked LED nodes, clean rmmod). Everything else is unchanged: the hardware description, the scsi_device_handler that does not claim the sdev, the multicolor LED interface, the protocol handling and the known caveats (manual attach until a notifier lands; SAVE on every update writes the enclosure flash, wear uncharacterized; NULL-parent LED registration to avoid the sdev reference cycle). The open question from v3 stands: the driver is deliberately not wired into Kconfig/Makefile/MAINTAINERS yet, because the agreed direction with the SCSI side is a split into a SCSI transport helper and a shared ASUS Aura LED interface, and the wiring would follow that shape. Is deferring the wiring to that split acceptable for an RFC, or would you rather have the driver buildable in-tree from this series already? Comments on the interface shape and on folding this into the shared Aura work with Denis remain very welcome. Signed-off-by: Liang Haowen <nbg2974@gmail.com> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC v4 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-15 12:48 ` [PATCH RFC v4 0/1] " Liang Haowen @ 2026-09-15 12:49 ` Liang Haowen [not found] ` <20260915130050.E8CDB1F000FF@smtp.kernel.org> 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-15 12:49 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen From: Liang Haowen <nbg2974@gmail.com> Date: Tue, 15 Sep 2026 20:18:15 +0800 Subject: [PATCH RFC v4] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are plain USB mass-storage devices with no HID interface: the Aura LEDs hang off an ENE controller driven by vendor SCSI commands on the same LUN as the disk. The enclosure has 4 independently addressable LEDs, verified on hardware. Register a scsi_device_handler matched by INQUIRY (vendor "ROG", model "ESD-S1C"); it does not claim the sdev (sd keeps owning the disk) and exposes each LED as a multicolor LED class device, /sys/class/leds/asus-arion-0-0-0-0:led0 through led3. The H:C:T:L part of the sdev name keeps the names unique when more than one enclosure is connected, with its colons flattened to dashes so the name keeps a single separator and follows the devicename:color:function convention userspace parses LED class names with. Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, register index, argument count in cdb[13]). MODE 0x8021 (Static) must be written first in every sequence or the device ignores it; colours go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; both tables are written because firmware revisions pull from one or the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. The CDB cannot go through scsi_execute_cmd(): it sizes the command via scsi_command_size(opcode), which maps vendor opcode 0xec to 10 bytes, so cdb[13] is dropped and the device silently ignores the write (GOOD status, no error). The request is built with scsi_alloc_request() instead, which initializes the scsi_cmnd parts a passthrough needs (command buffer, lengths, rcu head), with cmd_len forced to 16, mirroring what SG_IO does from userspace. brightness_set only caches the colour and marks the LED in a per-zone dirty mask under a spinlock; a single work item per zone then snapshots the mask and colours and runs one ENE sequence for all pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every update through one work item keeps the sequences from interleaving between concurrent LED updates and batches multi-LED updates into a single APPLY/SAVE. A re-queued run with nothing pending returns before touching the device, so it cannot wear the flash with a pointless SAVE, and the lock keeps a colour write from being reordered after its dirty bit on weakly ordered architectures. This is the monolithic out-of-tree version as verified on hardware; the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split into a SCSI transport helper and a shared ASUS Aura LED interface. Signed-off-by: Liang Haowen <nbg2974@gmail.com> --- drivers/leds/leds-asus-aura-scsi.c | 382 +++++++++++++++++++++++++++++ 1 file changed, 382 insertions(+) create mode 100644 drivers/leds/leds-asus-aura-scsi.c diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c new file mode 100644 index 0000000..089cc3d --- /dev/null +++ b/drivers/leds/leds-asus-aura-scsi.c @@ -0,0 +1,382 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures + * (e.g. ROG STRIX Arion, USB 0b05:1932). + * + * USB mass-storage device, no HID; the ENE LED controller is driven via + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), + * does NOT claim the sdev (sd keeps owning the disk). + * + * The Arion exposes 4 independently addressable LEDs (verified on hardware): + * each is a multicolor LED class device (asus-arion-<H-C-T-L>:led0..led3, + * unique per enclosure). A colour change writes that LED's slot only: + * EFFECT 0x8160 + 3*led, DIRECT 0x8100 + 3*led (3 bytes, byte order R, B, G), + * then APPLY (0x01) and SAVE (0xaa). MODE (0x8021 = Static) is written first + * in every sequence; skipping it makes the device ignore the whole sequence. + * + * Scheduling: brightness_set (LED core fast path) caches the colour and + * marks the LED in a per-zone dirty mask under a spinlock; a single work + * item per zone snapshots the mask and colours, then runs one ENE + * sequence for all pending LEDs (MODE once, colour slots, APPLY, SAVE). + * Funneling every update through that one work item also serializes the + * sequences: the MODE/colour/APPLY/SAVE chain must never interleave + * between concurrent LED updates. The snapshot makes re-queued runs with + * nothing left to do return before touching the device, so a re-queue + * cannot wear the flash with a pointless SAVE, and the lock keeps a + * colour write from being reordered after its dirty bit on weakly + * ordered architectures. + * + * CDB length: scsi_execute_cmd() sizes the CDB via COMMAND_SIZE(opcode), + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] + * and the device silently ignores the write. ene_write() therefore mirrors + * scsi_execute_cmd() on top of scsi_alloc_request() and forces cmd_len = 16 + * (what SG_IO does from userspace). + * + * Attach manually until a notifier lands: + * echo asus_aura > /sys/block/sdX/device/dh_state + */ + +#include <linux/module.h> +#include <linux/bits.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/string.h> +#include <linux/leds.h> +#include <linux/led-class-multicolor.h> +#include <linux/blk_types.h> +#include <linux/blkdev.h> +#include <linux/blk-mq.h> +#include <linux/workqueue.h> +#include <scsi/scsi.h> +#include <scsi/scsi_cmnd.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi_dh.h> + +#define ARION_INQ_VENDOR "ROG" +#define ARION_INQ_MODEL "ESD-S1C" + +#define ENE_OPCODE 0xec +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ +#define ENE_REG_APPLY 0x80a0 +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ +#define ENE_APPLY 0x01 +#define ENE_SAVE 0xaa +#define ENE_MODE_STATIC 1 +#define ENE_CDB_LEN 16 +#define ENE_RGB_LEN 3 +#define ENE_TIMEOUT (10 * HZ) + +/* + * Verified on hardware: the enclosure has 4 independently settable LEDs. + * (The colour table reserves 16 slots; only the first 4 drive anything.) + */ +#define ARION_NUM_LEDS 4 + +struct asus_aura_led { + struct asus_aura_zone *zone; + int index; + struct led_classdev_mc mc_cdev; + struct mc_subled subled[3]; + u8 rgb[ENE_RGB_LEN]; +}; + +struct asus_aura_zone { + struct scsi_device *sdev; + struct asus_aura_led leds[ARION_NUM_LEDS]; + spinlock_t lock; /* protects dirty and cached colours */ + u8 dirty; /* bit i: led i needs a colour write */ + struct work_struct work; +}; + +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) +{ + memset(cdb, 0, ENE_CDB_LEN); + cdb[0] = ENE_OPCODE; + cdb[1] = 'A'; + cdb[2] = 'S'; + cdb[3] = (reg >> 8) & 0xff; + cdb[4] = reg & 0xff; + cdb[13] = arg_count; +} + +/* + * scsi_execute_cmd() with cmd_len forced to 16. scsi_alloc_request() + * initializes the parts of the scsi_cmnd a passthrough needs (zeroed + * cmnd, cmd_len = MAX_COMMAND_SIZE, sense_len, rcu head, retries); + * a raw blk_mq_alloc_request() does none of that. + */ +static int ene_write(struct scsi_device *sdev, u16 reg, + const void *data, u8 arg_count) +{ + struct request *rq; + struct scsi_cmnd *scmd; + u8 cdb[ENE_CDB_LEN]; + int ret; + + ene_build_cdb(cdb, reg, arg_count); + + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); + if (IS_ERR(rq)) + return PTR_ERR(rq); + + if (arg_count) { + ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO); + if (ret) + goto out; + } + + scmd = blk_mq_rq_to_pdu(rq); + scmd->cmd_len = ENE_CDB_LEN; + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); + scmd->allowed = 1; + rq->timeout = ENE_TIMEOUT; + rq->rq_flags |= RQF_QUIET; + + blk_execute_rq(rq, true); + ret = scmd->result; +out: + blk_mq_free_request(rq); + return ret; +} + +/* + * Sleepable: runs on the system workqueue. One ENE sequence for every LED + * marked in the dirty mask. The mask and colours are snapshotted under the + * zone lock: asus_aura_set() may run concurrently on another CPU, and the + * lock keeps a colour write from being reordered after its dirty bit on + * weakly ordered architectures. A colour cached while this runs requeues + * the work and is picked up by the next sequence. + */ +static void asus_aura_zone_work(struct work_struct *work) +{ + struct asus_aura_zone *zone = + container_of(work, struct asus_aura_zone, work); + struct scsi_device *sdev = zone->sdev; + u8 rgb[ARION_NUM_LEDS][ENE_RGB_LEN]; + u8 apply = ENE_APPLY; + u8 save = ENE_SAVE; + u8 mode = ENE_MODE_STATIC; + unsigned long flags; + u8 pending; + int i, ret; + + spin_lock_irqsave(&zone->lock, flags); + pending = zone->dirty; + zone->dirty = 0; + for (i = 0; i < ARION_NUM_LEDS; i++) + memcpy(rgb[i], zone->leds[i].rgb, ENE_RGB_LEN); + spin_unlock_irqrestore(&zone->lock, flags); + + /* + * schedule_work() while this function runs requeues it, and the + * pending colour may already have been consumed above; the requeued + * run then has nothing to do. Return before touching the device: + * SAVE writes its flash. + */ + if (!pending) + return; + + if (!scsi_device_online(sdev)) + return; + + /* Mode first: without it the device ignores the whole sequence. */ + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); + if (ret) + goto err; + + for (i = 0; i < ARION_NUM_LEDS; i++) { + if (!(pending & BIT(i))) + continue; + + ret = ene_write(sdev, ENE_REG_COLORS + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + + /* + * Cover the DIRECT colour set too; some firmware revisions + * pull from 0x8100 instead of 0x8160. + */ + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + } + + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); + if (ret) + goto err; + + /* + * The change only takes effect after SAVE (0xaa). NOTE: saving on + * every brightness change writes flash each time; revisit for wear + * once confirmed. + */ + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); + if (ret) + goto err; + + return; +err: + dev_err(&sdev->sdev_gendev, + "asus_aura: colour update failed: %d\n", ret); +} + +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ +static void asus_aura_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); + struct asus_aura_led *led = + container_of(mc, struct asus_aura_led, mc_cdev); + struct asus_aura_zone *zone = led->zone; + unsigned long flags; + + led_mc_calc_color_components(mc, brightness); + + spin_lock_irqsave(&zone->lock, flags); + /* ENE colour register byte order is R, B, G. */ + led->rgb[0] = led->subled[0].brightness; + led->rgb[1] = led->subled[2].brightness; + led->rgb[2] = led->subled[1].brightness; + zone->dirty |= BIT(led->index); + spin_unlock_irqrestore(&zone->lock, flags); + + schedule_work(&zone->work); +} + +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) +{ + struct asus_aura_led *led = &zone->leds[index]; + struct led_classdev *cdev = &led->mc_cdev.led_cdev; + char hctl[32]; + int ret; + + led->zone = zone; + led->index = index; + + led->subled[0].color_index = LED_COLOR_ID_RED; + led->subled[1].color_index = LED_COLOR_ID_GREEN; + led->subled[2].color_index = LED_COLOR_ID_BLUE; + led->mc_cdev.num_colors = 3; + led->mc_cdev.subled_info = led->subled; + + /* + * Include the sdev's H:C:T:L: every enclosure gets its own SCSI + * host, so the names stay unique when more than one is connected. + * With a static name the LED core would register the second + * enclosure's LEDs under renamed nodes (asus-arion:led0_1), which + * is the wrong device identity. The names are per-attachment, like + * sd X letters, and userspace is expected to enumerate. + * + * dev_name() renders the sdev as H:C:T:L; the extra colons would + * break the devicename:color:function scheme userspace parses LED + * class names with, so they are flattened to dashes and the name + * keeps exactly one separator. + */ + strscpy(hctl, dev_name(&zone->sdev->sdev_gendev), sizeof(hctl)); + strreplace(hctl, ':', '-'); + cdev->name = kasprintf(GFP_KERNEL, "asus-arion-%s:led%d", hctl, index); + if (!cdev->name) + return -ENOMEM; + cdev->max_brightness = 255; + cdev->brightness_set = asus_aura_set; + + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); + + /* + * Register with NULL parent: parenting the LED to the sdev takes a + * device reference, which blocks the sdev's final release on unplug, + * which is what calls scsi_dh_release_device() -> our .detach() that + * unregisters the LEDs. That reference cycle leaked the LED nodes and + * the module refcount on every hot-unplug. + */ + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); + if (ret) + kfree(cdev->name); + return ret; +} + +/* + * Unregister the LED devices before cancelling the work: unregistering + * removes the sysfs attributes, so no new brightness_set can schedule the + * zone work afterwards, and it waits for in-flight sysfs callbacks. + * Cancelling first would leave a window where a brightness write requeues + * the work after cancel_work_sync() returned, and the work would then run + * on freed memory. + */ +static void asus_aura_release(struct asus_aura_zone *zone, int num_leds) +{ + int i; + + for (i = 0; i < num_leds; i++) + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); + cancel_work_sync(&zone->work); + for (i = 0; i < num_leds; i++) + kfree(zone->leds[i].mc_cdev.led_cdev.name); + kfree(zone); +} + +static int asus_aura_attach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone; + int i, ret; + + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) + return SCSI_DH_DEV_UNSUPP; + + zone = kzalloc_obj(*zone, GFP_KERNEL); + if (!zone) + return SCSI_DH_NOMEM; + zone->sdev = sdev; + spin_lock_init(&zone->lock); + INIT_WORK(&zone->work, asus_aura_zone_work); + + for (i = 0; i < ARION_NUM_LEDS; i++) { + ret = asus_aura_register_led(zone, i); + if (ret) { + asus_aura_release(zone, i); + return SCSI_DH_NOMEM; + } + } + + sdev->handler_data = zone; + return SCSI_DH_OK; +} + +static void asus_aura_detach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone = sdev->handler_data; + + if (!zone) + return; + asus_aura_release(zone, ARION_NUM_LEDS); + sdev->handler_data = NULL; +} + +static struct scsi_device_handler asus_aura_dh = { + .name = "asus_aura", + .module = THIS_MODULE, + .attach = asus_aura_attach, + .detach = asus_aura_detach, +}; + +static int __init asus_aura_init(void) +{ + return scsi_register_device_handler(&asus_aura_dh); +} + +static void __exit asus_aura_exit(void) +{ + scsi_unregister_device_handler(&asus_aura_dh); +} + +module_init(asus_aura_init); +module_exit(asus_aura_exit); + +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); +MODULE_AUTHOR("Liang Haowen"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20260915130050.E8CDB1F000FF@smtp.kernel.org>]
[parent not found: <20260916104305.GM11487@google.com>]
* [PATCH RFC v5 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures @ 2026-09-16 12:15 ` Liang Haowen 2026-09-16 12:15 ` [PATCH RFC v5 1/1] " Liang Haowen 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-16 12:15 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen Hello, v5, as its own thread, addressing the fourth sashiko round (one real Medium, the same two Low false positives). Changes since v4: - led_mc_calc_color_components() now runs under the zone spinlock. It writes the shared subled_info array, and the LED core allows concurrent brightness_set callbacks for the same LED: sysfs stores are serialized with led_access, but trigger events call led_set_brightness() without it, so two concurrent setters could interleave their component writes and the thread winning the lock would cache a mix of their colours. The two low-severity items are the same false positives as in the previous three rounds: - kzalloc_obj() exists in include/linux/slab.h since v7.0 (Kees Cook's overflow-refactor series); this driver builds against 7.2. - blk_rq_map_kern() takes four arguments on current kernels (rq, buf, len, gfp); drivers/scsi/scsi_lib.c calls it exactly this way from scsi_execute_cmd(). The five-argument form with the request_queue first parameter is from older trees. v5 was re-verified on hardware: per-LED colours, 100 sequential updates, concurrent same-LED updates, concurrent four-writer updates, and unplug under load (zero splats, zero leaked LED nodes, clean rmmod). Everything else is unchanged: the hardware description, the scsi_device_handler that does not claim the sdev, the multicolor LED interface, the protocol handling and the known caveats (manual attach until a notifier lands; SAVE on every update writes the enclosure flash, wear uncharacterized; NULL-parent LED registration to avoid the sdev reference cycle). The open question from v3 and v4 stands: the driver is deliberately not wired into Kconfig/Makefile/MAINTAINERS yet, because the agreed direction with the SCSI side is a split into a SCSI transport helper and a shared ASUS Aura LED interface, and the wiring would follow that shape. Is deferring the wiring to that split acceptable for an RFC, or would you rather have the driver buildable in-tree from this series already? Comments on the interface shape and on folding this into the shared Aura work with Denis remain very welcome. Signed-off-by: Liang Haowen <nbg2974@gmail.com> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC v5 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-16 12:15 ` [PATCH RFC v5 0/1] " Liang Haowen @ 2026-09-16 12:15 ` Liang Haowen [not found] ` <20260916122740.921E51F000FF@smtp.kernel.org> 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-16 12:15 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen From: Liang Haowen <nbg2974@gmail.com> Date: Wed, 16 Sep 2026 19:05:00 +0800 Subject: [PATCH RFC v5] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are plain USB mass-storage devices with no HID interface: the Aura LEDs hang off an ENE controller driven by vendor SCSI commands on the same LUN as the disk. The enclosure has 4 independently addressable LEDs, verified on hardware. Register a scsi_device_handler matched by INQUIRY (vendor "ROG", model "ESD-S1C"); it does not claim the sdev (sd keeps owning the disk) and exposes each LED as a multicolor LED class device, /sys/class/leds/asus-arion-0-0-0-0:led0 through led3. The H:C:T:L part of the sdev name keeps the names unique when more than one enclosure is connected, with its colons flattened to dashes so the name keeps a single separator and follows the devicename:color:function convention userspace parses LED class names with. Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, register index, argument count in cdb[13]). MODE 0x8021 (Static) must be written first in every sequence or the device ignores it; colours go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; both tables are written because firmware revisions pull from one or the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. The CDB cannot go through scsi_execute_cmd(): it sizes the command via scsi_command_size(opcode), which maps vendor opcode 0xec to 10 bytes, so cdb[13] is dropped and the device silently ignores the write (GOOD status, no error). The request is built with scsi_alloc_request() instead, which initializes the scsi_cmnd parts a passthrough needs (command buffer, lengths, rcu head), with cmd_len forced to 16, mirroring what SG_IO does from userspace. brightness_set only caches the colour and marks the LED in a per-zone dirty mask under a spinlock; led_mc_calc_color_components() runs under that lock too, because it writes the shared subled_info array and trigger events call led_set_brightness() without the led_access lock that serializes sysfs stores. A single work item per zone then snapshots the mask and colours and runs one ENE sequence for all pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every update through one work item keeps the sequences from interleaving between concurrent LED updates and batches multi-LED updates into a single APPLY/SAVE. A re-queued run with nothing pending returns before touching the device, so it cannot wear the flash with a pointless SAVE, and the lock keeps a colour write from being reordered after its dirty bit on weakly ordered architectures. This is the monolithic out-of-tree version as verified on hardware; the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split into a SCSI transport helper and a shared ASUS Aura LED interface. Signed-off-by: Liang Haowen <nbg2974@gmail.com> --- drivers/leds/leds-asus-aura-scsi.c | 389 +++++++++++++++++++++++++++++ 1 file changed, 389 insertions(+) create mode 100644 drivers/leds/leds-asus-aura-scsi.c diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c new file mode 100644 index 0000000..a1e31dc --- /dev/null +++ b/drivers/leds/leds-asus-aura-scsi.c @@ -0,0 +1,389 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures + * (e.g. ROG STRIX Arion, USB 0b05:1932). + * + * USB mass-storage device, no HID; the ENE LED controller is driven via + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), + * does NOT claim the sdev (sd keeps owning the disk). + * + * The Arion exposes 4 independently addressable LEDs (verified on hardware): + * each is a multicolor LED class device (asus-arion-<H-C-T-L>:led0..led3, + * unique per enclosure). A colour change writes that LED's slot only: + * EFFECT 0x8160 + 3*led, DIRECT 0x8100 + 3*led (3 bytes, byte order R, B, G), + * then APPLY (0x01) and SAVE (0xaa). MODE (0x8021 = Static) is written first + * in every sequence; skipping it makes the device ignore the whole sequence. + * + * Scheduling: brightness_set (LED core fast path) caches the colour and + * marks the LED in a per-zone dirty mask under a spinlock; a single work + * item per zone snapshots the mask and colours, then runs one ENE + * sequence for all pending LEDs (MODE once, colour slots, APPLY, SAVE). + * Funneling every update through that one work item also serializes the + * sequences: the MODE/colour/APPLY/SAVE chain must never interleave + * between concurrent LED updates. The snapshot makes re-queued runs with + * nothing left to do return before touching the device, so a re-queue + * cannot wear the flash with a pointless SAVE, and the lock keeps a + * colour write from being reordered after its dirty bit on weakly + * ordered architectures. + * + * CDB length: scsi_execute_cmd() sizes the CDB via COMMAND_SIZE(opcode), + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] + * and the device silently ignores the write. ene_write() therefore mirrors + * scsi_execute_cmd() on top of scsi_alloc_request() and forces cmd_len = 16 + * (what SG_IO does from userspace). + * + * Attach manually until a notifier lands: + * echo asus_aura > /sys/block/sdX/device/dh_state + */ + +#include <linux/module.h> +#include <linux/bits.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/string.h> +#include <linux/leds.h> +#include <linux/led-class-multicolor.h> +#include <linux/blk_types.h> +#include <linux/blkdev.h> +#include <linux/blk-mq.h> +#include <linux/workqueue.h> +#include <scsi/scsi.h> +#include <scsi/scsi_cmnd.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi_dh.h> + +#define ARION_INQ_VENDOR "ROG" +#define ARION_INQ_MODEL "ESD-S1C" + +#define ENE_OPCODE 0xec +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ +#define ENE_REG_APPLY 0x80a0 +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ +#define ENE_APPLY 0x01 +#define ENE_SAVE 0xaa +#define ENE_MODE_STATIC 1 +#define ENE_CDB_LEN 16 +#define ENE_RGB_LEN 3 +#define ENE_TIMEOUT (10 * HZ) + +/* + * Verified on hardware: the enclosure has 4 independently settable LEDs. + * (The colour table reserves 16 slots; only the first 4 drive anything.) + */ +#define ARION_NUM_LEDS 4 + +struct asus_aura_led { + struct asus_aura_zone *zone; + int index; + struct led_classdev_mc mc_cdev; + struct mc_subled subled[3]; + u8 rgb[ENE_RGB_LEN]; +}; + +struct asus_aura_zone { + struct scsi_device *sdev; + struct asus_aura_led leds[ARION_NUM_LEDS]; + spinlock_t lock; /* protects dirty and cached colours */ + u8 dirty; /* bit i: led i needs a colour write */ + struct work_struct work; +}; + +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) +{ + memset(cdb, 0, ENE_CDB_LEN); + cdb[0] = ENE_OPCODE; + cdb[1] = 'A'; + cdb[2] = 'S'; + cdb[3] = (reg >> 8) & 0xff; + cdb[4] = reg & 0xff; + cdb[13] = arg_count; +} + +/* + * scsi_execute_cmd() with cmd_len forced to 16. scsi_alloc_request() + * initializes the parts of the scsi_cmnd a passthrough needs (zeroed + * cmnd, cmd_len = MAX_COMMAND_SIZE, sense_len, rcu head, retries); + * a raw blk_mq_alloc_request() does none of that. + */ +static int ene_write(struct scsi_device *sdev, u16 reg, + const void *data, u8 arg_count) +{ + struct request *rq; + struct scsi_cmnd *scmd; + u8 cdb[ENE_CDB_LEN]; + int ret; + + ene_build_cdb(cdb, reg, arg_count); + + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); + if (IS_ERR(rq)) + return PTR_ERR(rq); + + if (arg_count) { + ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO); + if (ret) + goto out; + } + + scmd = blk_mq_rq_to_pdu(rq); + scmd->cmd_len = ENE_CDB_LEN; + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); + scmd->allowed = 1; + rq->timeout = ENE_TIMEOUT; + rq->rq_flags |= RQF_QUIET; + + blk_execute_rq(rq, true); + ret = scmd->result; +out: + blk_mq_free_request(rq); + return ret; +} + +/* + * Sleepable: runs on the system workqueue. One ENE sequence for every LED + * marked in the dirty mask. The mask and colours are snapshotted under the + * zone lock: asus_aura_set() may run concurrently on another CPU, and the + * lock keeps a colour write from being reordered after its dirty bit on + * weakly ordered architectures. A colour cached while this runs requeues + * the work and is picked up by the next sequence. + */ +static void asus_aura_zone_work(struct work_struct *work) +{ + struct asus_aura_zone *zone = + container_of(work, struct asus_aura_zone, work); + struct scsi_device *sdev = zone->sdev; + u8 rgb[ARION_NUM_LEDS][ENE_RGB_LEN]; + u8 apply = ENE_APPLY; + u8 save = ENE_SAVE; + u8 mode = ENE_MODE_STATIC; + unsigned long flags; + u8 pending; + int i, ret; + + spin_lock_irqsave(&zone->lock, flags); + pending = zone->dirty; + zone->dirty = 0; + for (i = 0; i < ARION_NUM_LEDS; i++) + memcpy(rgb[i], zone->leds[i].rgb, ENE_RGB_LEN); + spin_unlock_irqrestore(&zone->lock, flags); + + /* + * schedule_work() while this function runs requeues it, and the + * pending colour may already have been consumed above; the requeued + * run then has nothing to do. Return before touching the device: + * SAVE writes its flash. + */ + if (!pending) + return; + + if (!scsi_device_online(sdev)) + return; + + /* Mode first: without it the device ignores the whole sequence. */ + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); + if (ret) + goto err; + + for (i = 0; i < ARION_NUM_LEDS; i++) { + if (!(pending & BIT(i))) + continue; + + ret = ene_write(sdev, ENE_REG_COLORS + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + + /* + * Cover the DIRECT colour set too; some firmware revisions + * pull from 0x8100 instead of 0x8160. + */ + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + } + + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); + if (ret) + goto err; + + /* + * The change only takes effect after SAVE (0xaa). NOTE: saving on + * every brightness change writes flash each time; revisit for wear + * once confirmed. + */ + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); + if (ret) + goto err; + + return; +err: + dev_err(&sdev->sdev_gendev, + "asus_aura: colour update failed: %d\n", ret); +} + +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ +static void asus_aura_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); + struct asus_aura_led *led = + container_of(mc, struct asus_aura_led, mc_cdev); + struct asus_aura_zone *zone = led->zone; + unsigned long flags; + + /* + * led_mc_calc_color_components() writes the shared subled_info + * array. The LED core serializes sysfs stores with led_access, + * but trigger events call led_set_brightness() without it, so + * computing and copying the components under the same lock keeps + * a trigger-driven update and a sysfs store from reading a mix of + * each other's colours. + */ + spin_lock_irqsave(&zone->lock, flags); + led_mc_calc_color_components(mc, brightness); + /* ENE colour register byte order is R, B, G. */ + led->rgb[0] = led->subled[0].brightness; + led->rgb[1] = led->subled[2].brightness; + led->rgb[2] = led->subled[1].brightness; + zone->dirty |= BIT(led->index); + spin_unlock_irqrestore(&zone->lock, flags); + + schedule_work(&zone->work); +} + +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) +{ + struct asus_aura_led *led = &zone->leds[index]; + struct led_classdev *cdev = &led->mc_cdev.led_cdev; + char hctl[32]; + int ret; + + led->zone = zone; + led->index = index; + + led->subled[0].color_index = LED_COLOR_ID_RED; + led->subled[1].color_index = LED_COLOR_ID_GREEN; + led->subled[2].color_index = LED_COLOR_ID_BLUE; + led->mc_cdev.num_colors = 3; + led->mc_cdev.subled_info = led->subled; + + /* + * Include the sdev's H:C:T:L: every enclosure gets its own SCSI + * host, so the names stay unique when more than one is connected. + * With a static name the LED core would register the second + * enclosure's LEDs under renamed nodes (asus-arion:led0_1), which + * is the wrong device identity. The names are per-attachment, like + * sd X letters, and userspace is expected to enumerate. + * + * dev_name() renders the sdev as H:C:T:L; the extra colons would + * break the devicename:color:function scheme userspace parses LED + * class names with, so they are flattened to dashes and the name + * keeps exactly one separator. + */ + strscpy(hctl, dev_name(&zone->sdev->sdev_gendev), sizeof(hctl)); + strreplace(hctl, ':', '-'); + cdev->name = kasprintf(GFP_KERNEL, "asus-arion-%s:led%d", hctl, index); + if (!cdev->name) + return -ENOMEM; + cdev->max_brightness = 255; + cdev->brightness_set = asus_aura_set; + + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); + + /* + * Register with NULL parent: parenting the LED to the sdev takes a + * device reference, which blocks the sdev's final release on unplug, + * which is what calls scsi_dh_release_device() -> our .detach() that + * unregisters the LEDs. That reference cycle leaked the LED nodes and + * the module refcount on every hot-unplug. + */ + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); + if (ret) + kfree(cdev->name); + return ret; +} + +/* + * Unregister the LED devices before cancelling the work: unregistering + * removes the sysfs attributes, so no new brightness_set can schedule the + * zone work afterwards, and it waits for in-flight sysfs callbacks. + * Cancelling first would leave a window where a brightness write requeues + * the work after cancel_work_sync() returned, and the work would then run + * on freed memory. + */ +static void asus_aura_release(struct asus_aura_zone *zone, int num_leds) +{ + int i; + + for (i = 0; i < num_leds; i++) + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); + cancel_work_sync(&zone->work); + for (i = 0; i < num_leds; i++) + kfree(zone->leds[i].mc_cdev.led_cdev.name); + kfree(zone); +} + +static int asus_aura_attach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone; + int i, ret; + + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) + return SCSI_DH_DEV_UNSUPP; + + zone = kzalloc_obj(*zone, GFP_KERNEL); + if (!zone) + return SCSI_DH_NOMEM; + zone->sdev = sdev; + spin_lock_init(&zone->lock); + INIT_WORK(&zone->work, asus_aura_zone_work); + + for (i = 0; i < ARION_NUM_LEDS; i++) { + ret = asus_aura_register_led(zone, i); + if (ret) { + asus_aura_release(zone, i); + return SCSI_DH_NOMEM; + } + } + + sdev->handler_data = zone; + return SCSI_DH_OK; +} + +static void asus_aura_detach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone = sdev->handler_data; + + if (!zone) + return; + asus_aura_release(zone, ARION_NUM_LEDS); + sdev->handler_data = NULL; +} + +static struct scsi_device_handler asus_aura_dh = { + .name = "asus_aura", + .module = THIS_MODULE, + .attach = asus_aura_attach, + .detach = asus_aura_detach, +}; + +static int __init asus_aura_init(void) +{ + return scsi_register_device_handler(&asus_aura_dh); +} + +static void __exit asus_aura_exit(void) +{ + scsi_unregister_device_handler(&asus_aura_dh); +} + +module_init(asus_aura_init); +module_exit(asus_aura_exit); + +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); +MODULE_AUTHOR("Liang Haowen"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20260916122740.921E51F000FF@smtp.kernel.org>]
[parent not found: <20260916130602.GS11487@google.com>]
* [PATCH RFC v6 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures @ 2026-09-16 14:22 ` Liang Haowen 2026-09-16 14:22 ` [PATCH RFC v6 1/1] " Liang Haowen 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-16 14:22 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen Hello, v6, as its own thread, addressing the fifth sashiko round (one naming Low taken as a fix, the other two Low are the same false positives). Changes since v5: - LED names use the empty color section from the naming scheme now: asus-arion-0-0-0-0::led-0 through led-3, instead of the single separator v5 used. The color section is empty because these are multicolor LEDs whose palette is enumerated via multi_intensity, and the four identical zones take the function name with a "-N" ordinal, matching the examples in Documentation/leds/leds-class.rst ("phy3::wlan", ":kbd_backlight"). The sdev's H:C:T:L is still flattened to dashes to keep the name unambiguous. The two remaining Low items are the same false positives as in all previous rounds: - kzalloc_obj() exists in include/linux/slab.h since v7.0 (Kees Cook's overflow-refactor series); this driver builds against 7.2. - blk_rq_map_kern() takes four arguments on current kernels (rq, buf, len, gfp); drivers/scsi/scsi_lib.c calls it exactly this way from scsi_execute_cmd(). The five-argument form with the request_queue first parameter is from older trees. v6 was re-verified on hardware: the four LEDs appear under the new names, per-LED colours, sequential and concurrent updates, and unplug under load are clean. Everything else is unchanged: the hardware description, the scsi_device_handler that does not claim the sdev, the multicolor LED interface, the protocol handling and the known caveats (manual attach until a notifier lands; SAVE on every update writes the enclosure flash, wear uncharacterized; NULL-parent LED registration to avoid the sdev reference cycle). The open question from v3 through v5 stands: the driver is deliberately not wired into Kconfig/Makefile/MAINTAINERS yet, because the agreed direction with the SCSI side is a split into a SCSI transport helper and a shared ASUS Aura LED interface, and the wiring would follow that shape. Is deferring the wiring to that split acceptable for an RFC, or would you rather have the driver buildable in-tree from this series already? One more question on the naming: "led" is not a LED_FUNCTION_* value. The four zones are identical decorative RGB segments with no distinct function each, so I did not find a fitting predefined function. The closest in-tree pattern is LED_FUNCTION_PLAYER1..5, separate defines for identical things that only differ in number. Would you prefer a new LED_FUNCTION_* entry for this, or is a plain ordinal function fine for an RFC? Comments on the interface shape and on folding this into the shared Aura work with Denis remain very welcome. Signed-off-by: Liang Haowen <nbg2974@gmail.com> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-16 14:22 ` [PATCH RFC v6 0/1] " Liang Haowen @ 2026-09-16 14:22 ` Liang Haowen [not found] ` <20260916143832.520721F000FF@smtp.kernel.org> ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Liang Haowen @ 2026-09-16 14:22 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen From: Liang Haowen <nbg2974@gmail.com> Date: Wed, 16 Sep 2026 21:20:00 +0800 Subject: [PATCH RFC v6] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are plain USB mass-storage devices with no HID interface: the Aura LEDs hang off an ENE controller driven by vendor SCSI commands on the same LUN as the disk. The enclosure has 4 independently addressable LEDs, verified on hardware. Register a scsi_device_handler matched by INQUIRY (vendor "ROG", model "ESD-S1C"); it does not claim the sdev (sd keeps owning the disk) and exposes each LED as a multicolor LED class device, /sys/class/leds/asus-arion-0-0-0-0::led-0 through led-3. The H:C:T:L part of the sdev name keeps the names unique when more than one enclosure is connected, with its colons flattened to dashes. The color section stays empty, since multicolor LEDs enumerate their palette via multi_intensity, and the four identical zones use the function name with a "-N" ordinal, as the naming section in Documentation/leds/leds-class.rst asks for. Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, register index, argument count in cdb[13]). MODE 0x8021 (Static) must be written first in every sequence or the device ignores it; colours go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; both tables are written because firmware revisions pull from one or the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. The CDB cannot go through scsi_execute_cmd(): it sizes the command via scsi_command_size(opcode), which maps vendor opcode 0xec to 10 bytes, so cdb[13] is dropped and the device silently ignores the write (GOOD status, no error). The request is built with scsi_alloc_request() instead, which initializes the scsi_cmnd parts a passthrough needs (command buffer, lengths, rcu head), with cmd_len forced to 16, mirroring what SG_IO does from userspace. brightness_set only caches the colour and marks the LED in a per-zone dirty mask under a spinlock; led_mc_calc_color_components() runs under that lock too, because it writes the shared subled_info array and trigger events call led_set_brightness() without the led_access lock that serializes sysfs stores. A single work item per zone then snapshots the mask and colours and runs one ENE sequence for all pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every update through one work item keeps the sequences from interleaving between concurrent LED updates and batches multi-LED updates into a single APPLY/SAVE. A re-queued run with nothing pending returns before touching the device, so it cannot wear the flash with a pointless SAVE, and the lock keeps a colour write from being reordered after its dirty bit on weakly ordered architectures. This is the monolithic out-of-tree version as verified on hardware; the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split into a SCSI transport helper and a shared ASUS Aura LED interface. Signed-off-by: Liang Haowen <nbg2974@gmail.com> --- drivers/leds/leds-asus-aura-scsi.c | 392 +++++++++++++++++++++++++++++ 1 file changed, 392 insertions(+) create mode 100644 drivers/leds/leds-asus-aura-scsi.c diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c new file mode 100644 index 0000000..4e039bd --- /dev/null +++ b/drivers/leds/leds-asus-aura-scsi.c @@ -0,0 +1,392 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures + * (e.g. ROG STRIX Arion, USB 0b05:1932). + * + * USB mass-storage device, no HID; the ENE LED controller is driven via + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), + * does NOT claim the sdev (sd keeps owning the disk). + * + * The Arion exposes 4 independently addressable LEDs (verified on hardware): + * each is a multicolor LED class device (asus-arion-<H-C-T-L>::led-0..led-3, + * unique per enclosure). A colour change writes that LED's slot only: + * EFFECT 0x8160 + 3*led, DIRECT 0x8100 + 3*led (3 bytes, byte order R, B, G), + * then APPLY (0x01) and SAVE (0xaa). MODE (0x8021 = Static) is written first + * in every sequence; skipping it makes the device ignore the whole sequence. + * + * Scheduling: brightness_set (LED core fast path) caches the colour and + * marks the LED in a per-zone dirty mask under a spinlock; a single work + * item per zone snapshots the mask and colours, then runs one ENE + * sequence for all pending LEDs (MODE once, colour slots, APPLY, SAVE). + * Funneling every update through that one work item also serializes the + * sequences: the MODE/colour/APPLY/SAVE chain must never interleave + * between concurrent LED updates. The snapshot makes re-queued runs with + * nothing left to do return before touching the device, so a re-queue + * cannot wear the flash with a pointless SAVE, and the lock keeps a + * colour write from being reordered after its dirty bit on weakly + * ordered architectures. + * + * CDB length: scsi_execute_cmd() sizes the CDB via COMMAND_SIZE(opcode), + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] + * and the device silently ignores the write. ene_write() therefore mirrors + * scsi_execute_cmd() on top of scsi_alloc_request() and forces cmd_len = 16 + * (what SG_IO does from userspace). + * + * Attach manually until a notifier lands: + * echo asus_aura > /sys/block/sdX/device/dh_state + */ + +#include <linux/module.h> +#include <linux/bits.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/string.h> +#include <linux/leds.h> +#include <linux/led-class-multicolor.h> +#include <linux/blk_types.h> +#include <linux/blkdev.h> +#include <linux/blk-mq.h> +#include <linux/workqueue.h> +#include <scsi/scsi.h> +#include <scsi/scsi_cmnd.h> +#include <scsi/scsi_device.h> +#include <scsi/scsi_dh.h> + +#define ARION_INQ_VENDOR "ROG" +#define ARION_INQ_MODEL "ESD-S1C" + +#define ENE_OPCODE 0xec +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ +#define ENE_REG_APPLY 0x80a0 +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ +#define ENE_APPLY 0x01 +#define ENE_SAVE 0xaa +#define ENE_MODE_STATIC 1 +#define ENE_CDB_LEN 16 +#define ENE_RGB_LEN 3 +#define ENE_TIMEOUT (10 * HZ) + +/* + * Verified on hardware: the enclosure has 4 independently settable LEDs. + * (The colour table reserves 16 slots; only the first 4 drive anything.) + */ +#define ARION_NUM_LEDS 4 + +struct asus_aura_led { + struct asus_aura_zone *zone; + int index; + struct led_classdev_mc mc_cdev; + struct mc_subled subled[3]; + u8 rgb[ENE_RGB_LEN]; +}; + +struct asus_aura_zone { + struct scsi_device *sdev; + struct asus_aura_led leds[ARION_NUM_LEDS]; + spinlock_t lock; /* protects dirty and cached colours */ + u8 dirty; /* bit i: led i needs a colour write */ + struct work_struct work; +}; + +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) +{ + memset(cdb, 0, ENE_CDB_LEN); + cdb[0] = ENE_OPCODE; + cdb[1] = 'A'; + cdb[2] = 'S'; + cdb[3] = (reg >> 8) & 0xff; + cdb[4] = reg & 0xff; + cdb[13] = arg_count; +} + +/* + * scsi_execute_cmd() with cmd_len forced to 16. scsi_alloc_request() + * initializes the parts of the scsi_cmnd a passthrough needs (zeroed + * cmnd, cmd_len = MAX_COMMAND_SIZE, sense_len, rcu head, retries); + * a raw blk_mq_alloc_request() does none of that. + */ +static int ene_write(struct scsi_device *sdev, u16 reg, + const void *data, u8 arg_count) +{ + struct request *rq; + struct scsi_cmnd *scmd; + u8 cdb[ENE_CDB_LEN]; + int ret; + + ene_build_cdb(cdb, reg, arg_count); + + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); + if (IS_ERR(rq)) + return PTR_ERR(rq); + + if (arg_count) { + ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO); + if (ret) + goto out; + } + + scmd = blk_mq_rq_to_pdu(rq); + scmd->cmd_len = ENE_CDB_LEN; + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); + scmd->allowed = 1; + rq->timeout = ENE_TIMEOUT; + rq->rq_flags |= RQF_QUIET; + + blk_execute_rq(rq, true); + ret = scmd->result; +out: + blk_mq_free_request(rq); + return ret; +} + +/* + * Sleepable: runs on the system workqueue. One ENE sequence for every LED + * marked in the dirty mask. The mask and colours are snapshotted under the + * zone lock: asus_aura_set() may run concurrently on another CPU, and the + * lock keeps a colour write from being reordered after its dirty bit on + * weakly ordered architectures. A colour cached while this runs requeues + * the work and is picked up by the next sequence. + */ +static void asus_aura_zone_work(struct work_struct *work) +{ + struct asus_aura_zone *zone = + container_of(work, struct asus_aura_zone, work); + struct scsi_device *sdev = zone->sdev; + u8 rgb[ARION_NUM_LEDS][ENE_RGB_LEN]; + u8 apply = ENE_APPLY; + u8 save = ENE_SAVE; + u8 mode = ENE_MODE_STATIC; + unsigned long flags; + u8 pending; + int i, ret; + + spin_lock_irqsave(&zone->lock, flags); + pending = zone->dirty; + zone->dirty = 0; + for (i = 0; i < ARION_NUM_LEDS; i++) + memcpy(rgb[i], zone->leds[i].rgb, ENE_RGB_LEN); + spin_unlock_irqrestore(&zone->lock, flags); + + /* + * schedule_work() while this function runs requeues it, and the + * pending colour may already have been consumed above; the requeued + * run then has nothing to do. Return before touching the device: + * SAVE writes its flash. + */ + if (!pending) + return; + + if (!scsi_device_online(sdev)) + return; + + /* Mode first: without it the device ignores the whole sequence. */ + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); + if (ret) + goto err; + + for (i = 0; i < ARION_NUM_LEDS; i++) { + if (!(pending & BIT(i))) + continue; + + ret = ene_write(sdev, ENE_REG_COLORS + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + + /* + * Cover the DIRECT colour set too; some firmware revisions + * pull from 0x8100 instead of 0x8160. + */ + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + i * ENE_RGB_LEN, + rgb[i], ENE_RGB_LEN); + if (ret) + goto err; + } + + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); + if (ret) + goto err; + + /* + * The change only takes effect after SAVE (0xaa). NOTE: saving on + * every brightness change writes flash each time; revisit for wear + * once confirmed. + */ + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); + if (ret) + goto err; + + return; +err: + dev_err(&sdev->sdev_gendev, + "asus_aura: colour update failed: %d\n", ret); +} + +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ +static void asus_aura_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); + struct asus_aura_led *led = + container_of(mc, struct asus_aura_led, mc_cdev); + struct asus_aura_zone *zone = led->zone; + unsigned long flags; + + /* + * led_mc_calc_color_components() writes the shared subled_info + * array. The LED core serializes sysfs stores with led_access, + * but trigger events call led_set_brightness() without it, so + * computing and copying the components under the same lock keeps + * a trigger-driven update and a sysfs store from reading a mix of + * each other's colours. + */ + spin_lock_irqsave(&zone->lock, flags); + led_mc_calc_color_components(mc, brightness); + /* ENE colour register byte order is R, B, G. */ + led->rgb[0] = led->subled[0].brightness; + led->rgb[1] = led->subled[2].brightness; + led->rgb[2] = led->subled[1].brightness; + zone->dirty |= BIT(led->index); + spin_unlock_irqrestore(&zone->lock, flags); + + schedule_work(&zone->work); +} + +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) +{ + struct asus_aura_led *led = &zone->leds[index]; + struct led_classdev *cdev = &led->mc_cdev.led_cdev; + char hctl[32]; + int ret; + + led->zone = zone; + led->index = index; + + led->subled[0].color_index = LED_COLOR_ID_RED; + led->subled[1].color_index = LED_COLOR_ID_GREEN; + led->subled[2].color_index = LED_COLOR_ID_BLUE; + led->mc_cdev.num_colors = 3; + led->mc_cdev.subled_info = led->subled; + + /* + * Include the sdev's H:C:T:L: every enclosure gets its own SCSI + * host, so the names stay unique when more than one is connected. + * With a static name the LED core would register the second + * enclosure's LEDs under renamed nodes (asus-arion::led-0_1), + * which is the wrong device identity. The names are per-attachment, + * like sd X letters, and userspace is expected to enumerate. + * + * dev_name() renders the sdev as H:C:T:L; the extra colons would + * break the devicename:color:function scheme userspace parses LED + * class names with, so they are flattened to dashes. The color + * section stays empty (these are multicolor LEDs, the palette is + * enumerated via multi_intensity), and the four identical zones + * get the function name with a "-N" ordinal, like the + * Documentation/leds/leds-class.rst naming section asks for. + */ + strscpy(hctl, dev_name(&zone->sdev->sdev_gendev), sizeof(hctl)); + strreplace(hctl, ':', '-'); + cdev->name = kasprintf(GFP_KERNEL, "asus-arion-%s::led-%d", hctl, index); + if (!cdev->name) + return -ENOMEM; + cdev->max_brightness = 255; + cdev->brightness_set = asus_aura_set; + + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); + + /* + * Register with NULL parent: parenting the LED to the sdev takes a + * device reference, which blocks the sdev's final release on unplug, + * which is what calls scsi_dh_release_device() -> our .detach() that + * unregisters the LEDs. That reference cycle leaked the LED nodes and + * the module refcount on every hot-unplug. + */ + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); + if (ret) + kfree(cdev->name); + return ret; +} + +/* + * Unregister the LED devices before cancelling the work: unregistering + * removes the sysfs attributes, so no new brightness_set can schedule the + * zone work afterwards, and it waits for in-flight sysfs callbacks. + * Cancelling first would leave a window where a brightness write requeues + * the work after cancel_work_sync() returned, and the work would then run + * on freed memory. + */ +static void asus_aura_release(struct asus_aura_zone *zone, int num_leds) +{ + int i; + + for (i = 0; i < num_leds; i++) + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); + cancel_work_sync(&zone->work); + for (i = 0; i < num_leds; i++) + kfree(zone->leds[i].mc_cdev.led_cdev.name); + kfree(zone); +} + +static int asus_aura_attach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone; + int i, ret; + + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) + return SCSI_DH_DEV_UNSUPP; + + zone = kzalloc_obj(*zone, GFP_KERNEL); + if (!zone) + return SCSI_DH_NOMEM; + zone->sdev = sdev; + spin_lock_init(&zone->lock); + INIT_WORK(&zone->work, asus_aura_zone_work); + + for (i = 0; i < ARION_NUM_LEDS; i++) { + ret = asus_aura_register_led(zone, i); + if (ret) { + asus_aura_release(zone, i); + return SCSI_DH_NOMEM; + } + } + + sdev->handler_data = zone; + return SCSI_DH_OK; +} + +static void asus_aura_detach(struct scsi_device *sdev) +{ + struct asus_aura_zone *zone = sdev->handler_data; + + if (!zone) + return; + asus_aura_release(zone, ARION_NUM_LEDS); + sdev->handler_data = NULL; +} + +static struct scsi_device_handler asus_aura_dh = { + .name = "asus_aura", + .module = THIS_MODULE, + .attach = asus_aura_attach, + .detach = asus_aura_detach, +}; + +static int __init asus_aura_init(void) +{ + return scsi_register_device_handler(&asus_aura_dh); +} + +static void __exit asus_aura_exit(void) +{ + scsi_unregister_device_handler(&asus_aura_dh); +} + +module_init(asus_aura_init); +module_exit(asus_aura_exit); + +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); +MODULE_AUTHOR("Liang Haowen"); +MODULE_LICENSE("GPL"); -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20260916143832.520721F000FF@smtp.kernel.org>]
[parent not found: <20260917112840.GJ1605367@google.com>]
* Re: [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures [not found] ` <20260917112840.GJ1605367@google.com> @ 2026-09-17 11:49 ` Liang Haowen 0 siblings, 0 replies; 27+ messages in thread From: Liang Haowen @ 2026-09-17 11:49 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds, linux-scsi, linux-kernel, sashiko-reviews This one is the same false positive as in every round so far: kzalloc_obj() has been in include/linux/slab.h since v7.0 (line 1152 in the 7.2.4 tree this driver builds against), so the bot's reference tree simply predates it. Nothing to fix. I considered switching to plain kzalloc() to end the loop, but checkpatch on 7.2.4 flags that form with "Prefer kzalloc_obj over kzalloc with sizeof", so the two tools disagree and I would rather keep the checkpatch-clean one. Happy to switch if you prefer kzalloc(). Both questions from the v6 cover letter are still open, by the way: whether deferring the Kconfig/Makefile/MAINTAINERS wiring to the agreed SCSI transport split is acceptable for an RFC, and whether "led" should become a LED_FUNCTION_* value or the plain ordinal function is fine. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-16 14:22 ` [PATCH RFC v6 1/1] " Liang Haowen [not found] ` <20260916143832.520721F000FF@smtp.kernel.org> @ 2026-09-23 9:45 ` Lee Jones 2026-09-23 10:18 ` Liang Haowen 2026-09-23 10:12 ` Ilpo Järvinen 2 siblings, 1 reply; 27+ messages in thread From: Lee Jones @ 2026-09-23 9:45 UTC (permalink / raw) To: Liang Haowen Cc: linux-leds, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen On Wed, 16 Sep 2026, Liang Haowen wrote: > From: Liang Haowen <nbg2974@gmail.com> > Date: Wed, 16 Sep 2026 21:20:00 +0800 > Subject: [PATCH RFC v6] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for > ROG NVMe enclosures What patch format is this? How did you submit it? > ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are > plain USB mass-storage devices with no HID interface: the Aura LEDs > hang off an ENE controller driven by vendor SCSI commands on the same > LUN as the disk. The enclosure has 4 independently addressable LEDs, > verified on hardware. > > Register a scsi_device_handler matched by INQUIRY (vendor "ROG", If the SCSI DH the right approach here? I only see references to it in the SCSI subsystem and only in its own dedicated driver. This is all very unusual. > model "ESD-S1C"); it does not claim the sdev (sd keeps owning the > disk) and exposes each LED as a multicolor LED class device, > /sys/class/leds/asus-arion-0-0-0-0::led-0 through led-3. The > H:C:T:L part of the sdev name keeps the names unique when more than > one enclosure is connected, with its colons flattened to dashes. The > color section stays empty, since multicolor LEDs enumerate their > palette via multi_intensity, and the four identical zones use the > function name with a "-N" ordinal, as the naming section in > Documentation/leds/leds-class.rst asks for. > > Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, > register index, argument count in cdb[13]). MODE 0x8021 (Static) must > be written first in every sequence or the device ignores it; colours > go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; > both tables are written because firmware revisions pull from one or > the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. > > The CDB cannot go through scsi_execute_cmd(): it sizes the command > via scsi_command_size(opcode), which maps vendor opcode 0xec to > 10 bytes, so cdb[13] is dropped and the device silently ignores the > write (GOOD status, no error). The request is built with > scsi_alloc_request() instead, which initializes the scsi_cmnd parts a > passthrough needs (command buffer, lengths, rcu head), with cmd_len > forced to 16, mirroring what SG_IO does from userspace. > > brightness_set only caches the colour and marks the LED in a per-zone > dirty mask under a spinlock; led_mc_calc_color_components() runs under > that lock too, because it writes the shared subled_info array and > trigger events call led_set_brightness() without the led_access lock > that serializes sysfs stores. A single work item per zone then > snapshots the mask and colours and runs one ENE sequence for all > pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every > update through one work item keeps the sequences from interleaving > between concurrent LED updates and batches multi-LED updates into a > single APPLY/SAVE. A re-queued run with nothing pending returns > before touching the device, so it cannot wear the flash with a > pointless SAVE, and the lock keeps a colour write from being > reordered after its dirty bit on weakly ordered architectures. > > This is the monolithic out-of-tree version as verified on hardware; > the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split > into a SCSI transport helper and a shared ASUS Aura LED interface. > > Signed-off-by: Liang Haowen <nbg2974@gmail.com> > --- > drivers/leds/leds-asus-aura-scsi.c | 392 +++++++++++++++++++++++++++++ Shouldn't this live in drivers/leds/rgb ? > 1 file changed, 392 insertions(+) > create mode 100644 drivers/leds/leds-asus-aura-scsi.c > > diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c > new file mode 100644 > index 0000000..4e039bd > --- /dev/null > +++ b/drivers/leds/leds-asus-aura-scsi.c > @@ -0,0 +1,392 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures > + * (e.g. ROG STRIX Arion, USB 0b05:1932). > + * > + * USB mass-storage device, no HID; the ENE LED controller is driven via > + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), > + * does NOT claim the sdev (sd keeps owning the disk). > + * > + * The Arion exposes 4 independently addressable LEDs (verified on hardware): > + * each is a multicolor LED class device (asus-arion-<H-C-T-L>::led-0..led-3, > + * unique per enclosure). A colour change writes that LED's slot only: > + * EFFECT 0x8160 + 3*led, DIRECT 0x8100 + 3*led (3 bytes, byte order R, B, G), > + * then APPLY (0x01) and SAVE (0xaa). MODE (0x8021 = Static) is written first > + * in every sequence; skipping it makes the device ignore the whole sequence. > + * > + * Scheduling: brightness_set (LED core fast path) caches the colour and > + * marks the LED in a per-zone dirty mask under a spinlock; a single work > + * item per zone snapshots the mask and colours, then runs one ENE > + * sequence for all pending LEDs (MODE once, colour slots, APPLY, SAVE). > + * Funneling every update through that one work item also serializes the > + * sequences: the MODE/colour/APPLY/SAVE chain must never interleave > + * between concurrent LED updates. The snapshot makes re-queued runs with > + * nothing left to do return before touching the device, so a re-queue > + * cannot wear the flash with a pointless SAVE, and the lock keeps a > + * colour write from being reordered after its dirty bit on weakly > + * ordered architectures. > + * > + * CDB length: scsi_execute_cmd() sizes the CDB via COMMAND_SIZE(opcode), > + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte > + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] > + * and the device silently ignores the write. ene_write() therefore mirrors > + * scsi_execute_cmd() on top of scsi_alloc_request() and forces cmd_len = 16 > + * (what SG_IO does from userspace). > + * > + * Attach manually until a notifier lands: > + * echo asus_aura > /sys/block/sdX/device/dh_state > + */ > + > +#include <linux/module.h> > +#include <linux/bits.h> > +#include <linux/slab.h> > +#include <linux/spinlock.h> > +#include <linux/string.h> > +#include <linux/leds.h> > +#include <linux/led-class-multicolor.h> > +#include <linux/blk_types.h> > +#include <linux/blkdev.h> > +#include <linux/blk-mq.h> > +#include <linux/workqueue.h> > +#include <scsi/scsi.h> > +#include <scsi/scsi_cmnd.h> > +#include <scsi/scsi_device.h> > +#include <scsi/scsi_dh.h> Alphabetical. > +#define ARION_INQ_VENDOR "ROG" > +#define ARION_INQ_MODEL "ESD-S1C" > + > +#define ENE_OPCODE 0xec > +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ > +#define ENE_REG_APPLY 0x80a0 > +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ > +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ Not sure these comments make things any clearer? > +#define ENE_APPLY 0x01 > +#define ENE_SAVE 0xaa > +#define ENE_MODE_STATIC 1 > +#define ENE_CDB_LEN 16 > +#define ENE_RGB_LEN 3 > +#define ENE_TIMEOUT (10 * HZ) Isn't 10s a lifetime? > + > +/* > + * Verified on hardware: the enclosure has 4 independently settable LEDs. > + * (The colour table reserves 16 slots; only the first 4 drive anything.) > + */ Do we really need to know about the trial and error during development? > +#define ARION_NUM_LEDS 4 > + > +struct asus_aura_led { > + struct asus_aura_zone *zone; Is 'zone' SCSI terminology? > + int index; > + struct mc_subled subled[3]; > + u8 rgb[ENE_RGB_LEN]; > +}; > + > +struct asus_aura_zone { > + struct scsi_device *sdev; > + struct asus_aura_led leds[ARION_NUM_LEDS]; > + spinlock_t lock; /* protects dirty and cached colours */ > + u8 dirty; /* bit i: led i needs a colour write */ What does dirty even mean in this context? I suggest the nomenclature needs improvement. If you make this an LED-level attribute, you can remove the faux LED indexing. > + struct work_struct work; > +}; > + > +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) > +{ > + memset(cdb, 0, ENE_CDB_LEN); > + cdb[0] = ENE_OPCODE; Would defining these offsets make sense? Nicer to read if they have a name. > + cdb[1] = 'A'; > + cdb[2] = 'S'; > + cdb[3] = (reg >> 8) & 0xff; > + cdb[4] = reg & 0xff; > + cdb[13] = arg_count; > +} > + > +/* > + * scsi_execute_cmd() with cmd_len forced to 16. scsi_alloc_request() > + * initializes the parts of the scsi_cmnd a passthrough needs (zeroed > + * cmnd, cmd_len = MAX_COMMAND_SIZE, sense_len, rcu head, retries); > + * a raw blk_mq_alloc_request() does none of that. And you're telling us this because? Was there a decision based off of this? Please elaborate. > + */ > +static int ene_write(struct scsi_device *sdev, u16 reg, > + const void *data, u8 arg_count) Data is a terrible variable name. It's also odd that we're passing back read-data in a write() function! > +{ > + struct request *rq; > + struct scsi_cmnd *scmd; > + u8 cdb[ENE_CDB_LEN]; > + int ret; > + > + ene_build_cdb(cdb, reg, arg_count); > + > + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); > + if (IS_ERR(rq)) > + return PTR_ERR(rq); > + > + if (arg_count) { When is this not true? > + ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO); > + if (ret) > + goto out; > + } > + > + scmd = blk_mq_rq_to_pdu(rq); > + scmd->cmd_len = ENE_CDB_LEN; > + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); > + scmd->allowed = 1; > + rq->timeout = ENE_TIMEOUT; > + rq->rq_flags |= RQF_QUIET; > + > + blk_execute_rq(rq, true); > + ret = scmd->result; I have no idea what any of this means. I'd need a SCSI person to help here. > +out: > + blk_mq_free_request(rq); > + return ret; > +} > + > +/* > + * Sleepable: runs on the system workqueue. One ENE sequence for every LED > + * marked in the dirty mask. The mask and colours are snapshotted under the > + * zone lock: asus_aura_set() may run concurrently on another CPU, and the > + * lock keeps a colour write from being reordered after its dirty bit on > + * weakly ordered architectures. A colour cached while this runs requeues > + * the work and is picked up by the next sequence. > + */ > +static void asus_aura_zone_work(struct work_struct *work) > +{ > + struct asus_aura_zone *zone = > + container_of(work, struct asus_aura_zone, work); > + struct scsi_device *sdev = zone->sdev; > + u8 rgb[ARION_NUM_LEDS][ENE_RGB_LEN]; > + u8 apply = ENE_APPLY; > + u8 save = ENE_SAVE; > + u8 mode = ENE_MODE_STATIC; > + unsigned long flags; > + u8 pending; > + int i, ret; > + > + spin_lock_irqsave(&zone->lock, flags); > + pending = zone->dirty; > + zone->dirty = 0; > + for (i = 0; i < ARION_NUM_LEDS; i++) > + memcpy(rgb[i], zone->leds[i].rgb, ENE_RGB_LEN); > + spin_unlock_irqrestore(&zone->lock, flags); > + > + /* > + * schedule_work() while this function runs requeues it, and the > + * pending colour may already have been consumed above; the requeued > + * run then has nothing to do. Return before touching the device: > + * SAVE writes its flash. > + */ > + if (!pending) > + return; All of the above is pointless if (zone->dirty == 0), so why not move this check to the top and skip all of it? > + > + if (!scsi_device_online(sdev)) > + return; > + > + /* Mode first: without it the device ignores the whole sequence. */ > + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); Are we sure it is safe to pass stack variables (like 'mode', 'apply', 'save', and 'rgb') to 'blk_rq_map_kern()'? Stack memory is not DMA-safe and will cause issues with VMAP_STACK. Should we allocate a DMA-safe bounce buffer in 'struct asus_aura_zone' instead? > + if (ret) > + goto err; > + > + for (i = 0; i < ARION_NUM_LEDS; i++) { for (int leds = 0; ... > + if (!(pending & BIT(i))) zone->leds[led]->pending ? > + continue; > + > + ret = ene_write(sdev, ENE_REG_COLORS + i * ENE_RGB_LEN, > + rgb[i], ENE_RGB_LEN); Use 100-chars to unwrap some of these. > + if (ret) > + goto err; > + > + /* > + * Cover the DIRECT colour set too; some firmware revisions > + * pull from 0x8100 instead of 0x8160. > + */ > + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + i * ENE_RGB_LEN, > + rgb[i], ENE_RGB_LEN); > + if (ret) > + goto err; > + } > + > + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); Are these variables even used? > + if (ret) > + goto err; > + > + /* > + * The change only takes effect after SAVE (0xaa). NOTE: saving on > + * every brightness change writes flash each time; revisit for wear > + * once confirmed. > + */ > + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); > + if (ret) > + goto err; > + > + return; > +err: > + dev_err(&sdev->sdev_gendev, > + "asus_aura: colour update failed: %d\n", ret); Shouldn't we be telling the caller than there was an error? > +} > + > +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ > +static void asus_aura_set(struct led_classdev *cdev, > + enum led_brightness brightness) Un-wrap. > +{ > + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); > + struct asus_aura_led *led = > + container_of(mc, struct asus_aura_led, mc_cdev); Un-wrap. Etc. > + struct asus_aura_zone *zone = led->zone; > + unsigned long flags; > + > + /* > + * led_mc_calc_color_components() writes the shared subled_info > + * array. The LED core serializes sysfs stores with led_access, > + * but trigger events call led_set_brightness() without it, so > + * computing and copying the components under the same lock keeps > + * a trigger-driven update and a sysfs store from reading a mix of > + * each other's colours. > + */ > + spin_lock_irqsave(&zone->lock, flags); > + led_mc_calc_color_components(mc, brightness); > + /* ENE colour register byte order is R, B, G. */ > + led->rgb[0] = led->subled[0].brightness; > + led->rgb[1] = led->subled[2].brightness; > + led->rgb[2] = led->subled[1].brightness; > + zone->dirty |= BIT(led->index); > + spin_unlock_irqrestore(&zone->lock, flags); This is all very claustrophobic. Can you space some of these out in groups? Other places too. Nicely spaced out code is easier to parse. > + > + schedule_work(&zone->work); > +} > + > +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) > +{ > + struct asus_aura_led *led = &zone->leds[index]; > + struct led_classdev *cdev = &led->mc_cdev.led_cdev; > + char hctl[32]; > + int ret; > + > + led->zone = zone; > + led->index = index; > + > + led->subled[0].color_index = LED_COLOR_ID_RED; > + led->subled[1].color_index = LED_COLOR_ID_GREEN; > + led->subled[2].color_index = LED_COLOR_ID_BLUE; > + led->mc_cdev.num_colors = 3; > + led->mc_cdev.subled_info = led->subled; > + > + /* > + * Include the sdev's H:C:T:L: every enclosure gets its own SCSI > + * host, so the names stay unique when more than one is connected. > + * With a static name the LED core would register the second > + * enclosure's LEDs under renamed nodes (asus-arion::led-0_1), > + * which is the wrong device identity. The names are per-attachment, > + * like sd X letters, and userspace is expected to enumerate. > + * > + * dev_name() renders the sdev as H:C:T:L; the extra colons would > + * break the devicename:color:function scheme userspace parses LED > + * class names with, so they are flattened to dashes. The color > + * section stays empty (these are multicolor LEDs, the palette is > + * enumerated via multi_intensity), and the four identical zones > + * get the function name with a "-N" ordinal, like the > + * Documentation/leds/leds-class.rst naming section asks for. > + */ > + strscpy(hctl, dev_name(&zone->sdev->sdev_gendev), sizeof(hctl)); > + strreplace(hctl, ':', '-'); > + cdev->name = kasprintf(GFP_KERNEL, "asus-arion-%s::led-%d", hctl, index); > + if (!cdev->name) > + return -ENOMEM; > + cdev->max_brightness = 255; > + cdev->brightness_set = asus_aura_set; > + > + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); > + > + /* > + * Register with NULL parent: parenting the LED to the sdev takes a > + * device reference, which blocks the sdev's final release on unplug, > + * which is what calls scsi_dh_release_device() -> our .detach() that > + * unregisters the LEDs. That reference cycle leaked the LED nodes and > + * the module refcount on every hot-unplug. > + */ > + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); > + if (ret) > + kfree(cdev->name); > + return ret; > +} > + > +/* > + * Unregister the LED devices before cancelling the work: unregistering > + * removes the sysfs attributes, so no new brightness_set can schedule the > + * zone work afterwards, and it waits for in-flight sysfs callbacks. > + * Cancelling first would leave a window where a brightness write requeues > + * the work after cancel_work_sync() returned, and the work would then run > + * on freed memory. > + */ > +static void asus_aura_release(struct asus_aura_zone *zone, int num_leds) > +{ > + int i; > + > + for (i = 0; i < num_leds; i++) > + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); > + cancel_work_sync(&zone->work); > + for (i = 0; i < num_leds; i++) > + kfree(zone->leds[i].mc_cdev.led_cdev.name); > + kfree(zone); > +} > + > +static int asus_aura_attach(struct scsi_device *sdev) > +{ > + struct asus_aura_zone *zone; > + int i, ret; > + > + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || > + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) > + return SCSI_DH_DEV_UNSUPP; > + > + zone = kzalloc_obj(*zone, GFP_KERNEL); > + if (!zone) > + return SCSI_DH_NOMEM; > + zone->sdev = sdev; > + spin_lock_init(&zone->lock); > + INIT_WORK(&zone->work, asus_aura_zone_work); > + > + for (i = 0; i < ARION_NUM_LEDS; i++) { > + ret = asus_aura_register_led(zone, i); > + if (ret) { > + asus_aura_release(zone, i); Shouldn't asus_aura_register_led unwind itself on failure? > + return SCSI_DH_NOMEM; > + } > + } > + > + sdev->handler_data = zone; > + return SCSI_DH_OK; > +} > + > +static void asus_aura_detach(struct scsi_device *sdev) > +{ > + struct asus_aura_zone *zone = sdev->handler_data; > + > + if (!zone) > + return; > + asus_aura_release(zone, ARION_NUM_LEDS); > + sdev->handler_data = NULL; > +} > + > +static struct scsi_device_handler asus_aura_dh = { > + .name = "asus_aura", > + .module = THIS_MODULE, Are you sure this isn't handled for you by the subsystem? > + .attach = asus_aura_attach, > + .detach = asus_aura_detach, > +}; > + > +static int __init asus_aura_init(void) > +{ > + return scsi_register_device_handler(&asus_aura_dh); > +} > + > +static void __exit asus_aura_exit(void) > +{ > + scsi_unregister_device_handler(&asus_aura_dh); > +} > + > +module_init(asus_aura_init); > +module_exit(asus_aura_exit); > + > +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); > +MODULE_AUTHOR("Liang Haowen"); > +MODULE_LICENSE("GPL"); > -- > 2.55.0 > > -- Lee Jones ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-23 9:45 ` Lee Jones @ 2026-09-23 10:18 ` Liang Haowen [not found] ` <20260923102812.2353292-1-nbg2974@gmail.com> 0 siblings, 1 reply; 27+ messages in thread From: Liang Haowen @ 2026-09-23 10:18 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds, linux-scsi, linux-kernel, sashiko-reviews Answers inline; v7 follows as its own thread. > What patch format is this? How did you submit it? A hand-rolled Python SMTP script, which is as ill-advised as it sounds. v7 goes out with git send-email, so the format is the standard one from here on. > If the SCSI DH the right approach here? It is unusual, and it was a pragmatic out-of-tree choice rather than a design statement. The enclosure exposes a single mass-storage interface that usb-storage owns, so a USB interface driver cannot claim it without taking the disk down with it, and hooking the SCSI bus from a module is not possible without the unexported scsi_bus_type. The device handler was the one hook that observes the sdev without claiming it, which keeps the enclosure a normal block device. For in-tree the plan is the split mentioned in the v6 cover letter: the SCSI attachment moves under drivers/scsi (notifier or an exported attach helper, whichever the SCSI side prefers) and the LED part stays in drivers/leds. Happy to rework the attachment in whichever form you and the SCSI folks prefer. > Shouldn't this live in drivers/leds/rgb ? Yes. v7 moves it to drivers/leds/rgb/leds-asus-aura-scsi.c. It still has no Kconfig wiring; it is the verified out-of-tree module posted for the API discussion, as the commit message notes. ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <20260923102812.2353292-1-nbg2974@gmail.com>]
* Re: [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-16 14:22 ` [PATCH RFC v6 1/1] " Liang Haowen [not found] ` <20260916143832.520721F000FF@smtp.kernel.org> 2026-09-23 9:45 ` Lee Jones @ 2026-09-23 10:12 ` Ilpo Järvinen 2 siblings, 0 replies; 27+ messages in thread From: Ilpo Järvinen @ 2026-09-23 10:12 UTC (permalink / raw) To: Liang Haowen Cc: linux-leds, Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, LKML, Denis Benato, Armin Wolf, Hans de Goede On Wed, 16 Sep 2026, Liang Haowen wrote: > From: Liang Haowen <nbg2974@gmail.com> > Date: Wed, 16 Sep 2026 21:20:00 +0800 > Subject: [PATCH RFC v6] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for > ROG NVMe enclosures > > ASUS ROG external NVMe enclosures (ROG STRIX Arion, USB 0b05:1932) are > plain USB mass-storage devices with no HID interface: the Aura LEDs > hang off an ENE controller driven by vendor SCSI commands on the same > LUN as the disk. The enclosure has 4 independently addressable LEDs, > verified on hardware. > > Register a scsi_device_handler matched by INQUIRY (vendor "ROG", > model "ESD-S1C"); it does not claim the sdev (sd keeps owning the > disk) and exposes each LED as a multicolor LED class device, > /sys/class/leds/asus-arion-0-0-0-0::led-0 through led-3. The > H:C:T:L part of the sdev name keeps the names unique when more than > one enclosure is connected, with its colons flattened to dashes. The > color section stays empty, since multicolor LEDs enumerate their > palette via multi_intensity, and the four identical zones use the > function name with a "-N" ordinal, as the naming section in > Documentation/leds/leds-class.rst asks for. > > Protocol: a 16-byte vendor CDB (opcode 0xec, 'A' 'S' signature, > register index, argument count in cdb[13]). MODE 0x8021 (Static) must > be written first in every sequence or the device ignores it; colours > go to 0x8160 + 3 * led and 0x8100 + 3 * led (3 bytes, order R, B, G; > both tables are written because firmware revisions pull from one or > the other); APPLY 0x80a0 takes 0x01 to apply and 0xaa to save. > > The CDB cannot go through scsi_execute_cmd(): it sizes the command > via scsi_command_size(opcode), which maps vendor opcode 0xec to > 10 bytes, so cdb[13] is dropped and the device silently ignores the > write (GOOD status, no error). The request is built with > scsi_alloc_request() instead, which initializes the scsi_cmnd parts a > passthrough needs (command buffer, lengths, rcu head), with cmd_len > forced to 16, mirroring what SG_IO does from userspace. > > brightness_set only caches the colour and marks the LED in a per-zone > dirty mask under a spinlock; led_mc_calc_color_components() runs under > that lock too, because it writes the shared subled_info array and > trigger events call led_set_brightness() without the led_access lock > that serializes sysfs stores. A single work item per zone then > snapshots the mask and colours and runs one ENE sequence for all > pending LEDs (MODE, colour slots, APPLY, SAVE). Funneling every > update through one work item keeps the sequences from interleaving > between concurrent LED updates and batches multi-LED updates into a > single APPLY/SAVE. A re-queued run with nothing pending returns > before touching the device, so it cannot wear the flash with a > pointless SAVE, and the lock keeps a colour write from being > reordered after its dirty bit on weakly ordered architectures. > > This is the monolithic out-of-tree version as verified on hardware; > the Kconfig/Makefile/MAINTAINERS wiring lands with the agreed split > into a SCSI transport helper and a shared ASUS Aura LED interface. > > Signed-off-by: Liang Haowen <nbg2974@gmail.com> > --- > drivers/leds/leds-asus-aura-scsi.c | 392 +++++++++++++++++++++++++++++ > 1 file changed, 392 insertions(+) > create mode 100644 drivers/leds/leds-asus-aura-scsi.c > > diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c > new file mode 100644 > index 0000000..4e039bd > --- /dev/null > +++ b/drivers/leds/leds-asus-aura-scsi.c > @@ -0,0 +1,392 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * ASUS Aura RGB over SCSI for ROG external NVMe enclosures > + * (e.g. ROG STRIX Arion, USB 0b05:1932). > + * > + * USB mass-storage device, no HID; the ENE LED controller is driven via > + * vendor SCSI commands. Matched by INQUIRY (vendor "ROG", model "ESD-S1C"), > + * does NOT claim the sdev (sd keeps owning the disk). > + * > + * The Arion exposes 4 independently addressable LEDs (verified on hardware): > + * each is a multicolor LED class device (asus-arion-<H-C-T-L>::led-0..led-3, > + * unique per enclosure). A colour change writes that LED's slot only: > + * EFFECT 0x8160 + 3*led, DIRECT 0x8100 + 3*led (3 bytes, byte order R, B, G), > + * then APPLY (0x01) and SAVE (0xaa). MODE (0x8021 = Static) is written first > + * in every sequence; skipping it makes the device ignore the whole sequence. > + * > + * Scheduling: brightness_set (LED core fast path) caches the colour and > + * marks the LED in a per-zone dirty mask under a spinlock; a single work > + * item per zone snapshots the mask and colours, then runs one ENE > + * sequence for all pending LEDs (MODE once, colour slots, APPLY, SAVE). > + * Funneling every update through that one work item also serializes the > + * sequences: the MODE/colour/APPLY/SAVE chain must never interleave > + * between concurrent LED updates. The snapshot makes re-queued runs with > + * nothing left to do return before touching the device, so a re-queue > + * cannot wear the flash with a pointless SAVE, and the lock keeps a > + * colour write from being reordered after its dirty bit on weakly > + * ordered architectures. > + * > + * CDB length: scsi_execute_cmd() sizes the CDB via COMMAND_SIZE(opcode), > + * which maps vendor opcode 0xec to 10 bytes. The ENE protocol uses a 16-byte > + * CDB with the data length in cdb[13], so scsi_execute_cmd() drops cdb[13] > + * and the device silently ignores the write. ene_write() therefore mirrors > + * scsi_execute_cmd() on top of scsi_alloc_request() and forces cmd_len = 16 > + * (what SG_IO does from userspace). > + * > + * Attach manually until a notifier lands: > + * echo asus_aura > /sys/block/sdX/device/dh_state > + */ > + > +#include <linux/module.h> > +#include <linux/bits.h> > +#include <linux/slab.h> > +#include <linux/spinlock.h> > +#include <linux/string.h> > +#include <linux/leds.h> > +#include <linux/led-class-multicolor.h> > +#include <linux/blk_types.h> > +#include <linux/blkdev.h> > +#include <linux/blk-mq.h> > +#include <linux/workqueue.h> > +#include <scsi/scsi.h> > +#include <scsi/scsi_cmnd.h> > +#include <scsi/scsi_device.h> > +#include <scsi/scsi_dh.h> > + > +#define ARION_INQ_VENDOR "ROG" > +#define ARION_INQ_MODEL "ESD-S1C" > + > +#define ENE_OPCODE 0xec > +#define ENE_REG_MODE 0x8021 /* AuraMode value: Static=1, Breathe=2, ... */ > +#define ENE_REG_APPLY 0x80a0 > +#define ENE_REG_COLORS 0x8160 /* + 3*led, 3 bytes per LED, order R,B,G */ > +#define ENE_REG_COLORS_DIRECT 0x8100 /* + 3*led, same layout */ > +#define ENE_APPLY 0x01 > +#define ENE_SAVE 0xaa > +#define ENE_MODE_STATIC 1 > +#define ENE_CDB_LEN 16 > +#define ENE_RGB_LEN 3 > +#define ENE_TIMEOUT (10 * HZ) > + > +/* > + * Verified on hardware: the enclosure has 4 independently settable LEDs. > + * (The colour table reserves 16 slots; only the first 4 drive anything.) > + */ > +#define ARION_NUM_LEDS 4 > + > +struct asus_aura_led { > + struct asus_aura_zone *zone; > + int index; > + struct led_classdev_mc mc_cdev; > + struct mc_subled subled[3]; > + u8 rgb[ENE_RGB_LEN]; > +}; > + > +struct asus_aura_zone { > + struct scsi_device *sdev; > + struct asus_aura_led leds[ARION_NUM_LEDS]; > + spinlock_t lock; /* protects dirty and cached colours */ > + u8 dirty; /* bit i: led i needs a colour write */ > + struct work_struct work; > +}; > + > +static void ene_build_cdb(u8 *cdb, u16 reg, u8 arg_count) > +{ > + memset(cdb, 0, ENE_CDB_LEN); > + cdb[0] = ENE_OPCODE; > + cdb[1] = 'A'; > + cdb[2] = 'S'; > + cdb[3] = (reg >> 8) & 0xff; > + cdb[4] = reg & 0xff; This should use endianness typing and conversion functions. Consider if using a __packed struct would make this code easier to understand. Or is this some scsi related structure for which a struct already exists? > + cdb[13] = arg_count; > +} > + > +/* > + * scsi_execute_cmd() with cmd_len forced to 16. scsi_alloc_request() > + * initializes the parts of the scsi_cmnd a passthrough needs (zeroed > + * cmnd, cmd_len = MAX_COMMAND_SIZE, sense_len, rcu head, retries); > + * a raw blk_mq_alloc_request() does none of that. > + */ > +static int ene_write(struct scsi_device *sdev, u16 reg, > + const void *data, u8 arg_count) > +{ > + struct request *rq; > + struct scsi_cmnd *scmd; > + u8 cdb[ENE_CDB_LEN]; > + int ret; > + > + ene_build_cdb(cdb, reg, arg_count); > + > + rq = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_OUT, 0); > + if (IS_ERR(rq)) > + return PTR_ERR(rq); Add include for IS_ERR/PTR_ERR(). > + > + if (arg_count) { > + ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO); > + if (ret) > + goto out; > + } > + > + scmd = blk_mq_rq_to_pdu(rq); > + scmd->cmd_len = ENE_CDB_LEN; > + memcpy(scmd->cmnd, cdb, ENE_CDB_LEN); > + scmd->allowed = 1; > + rq->timeout = ENE_TIMEOUT; > + rq->rq_flags |= RQF_QUIET; > + > + blk_execute_rq(rq, true); > + ret = scmd->result; > +out: > + blk_mq_free_request(rq); > + return ret; > +} > + > +/* > + * Sleepable: runs on the system workqueue. One ENE sequence for every LED > + * marked in the dirty mask. The mask and colours are snapshotted under the > + * zone lock: asus_aura_set() may run concurrently on another CPU, and the > + * lock keeps a colour write from being reordered after its dirty bit on > + * weakly ordered architectures. A colour cached while this runs requeues > + * the work and is picked up by the next sequence. > + */ > +static void asus_aura_zone_work(struct work_struct *work) > +{ > + struct asus_aura_zone *zone = > + container_of(work, struct asus_aura_zone, work); > + struct scsi_device *sdev = zone->sdev; > + u8 rgb[ARION_NUM_LEDS][ENE_RGB_LEN]; > + u8 apply = ENE_APPLY; > + u8 save = ENE_SAVE; > + u8 mode = ENE_MODE_STATIC; > + unsigned long flags; > + u8 pending; > + int i, ret; > + > + spin_lock_irqsave(&zone->lock, flags); > + pending = zone->dirty; > + zone->dirty = 0; > + for (i = 0; i < ARION_NUM_LEDS; i++) > + memcpy(rgb[i], zone->leds[i].rgb, ENE_RGB_LEN); > + spin_unlock_irqrestore(&zone->lock, flags); > + > + /* > + * schedule_work() while this function runs requeues it, and the > + * pending colour may already have been consumed above; the requeued > + * run then has nothing to do. Return before touching the device: > + * SAVE writes its flash. > + */ > + if (!pending) > + return; > + > + if (!scsi_device_online(sdev)) > + return; > + > + /* Mode first: without it the device ignores the whole sequence. */ > + ret = ene_write(sdev, ENE_REG_MODE, &mode, 1); > + if (ret) > + goto err; > + > + for (i = 0; i < ARION_NUM_LEDS; i++) { > + if (!(pending & BIT(i))) > + continue; > + > + ret = ene_write(sdev, ENE_REG_COLORS + i * ENE_RGB_LEN, > + rgb[i], ENE_RGB_LEN); > + if (ret) > + goto err; > + > + /* > + * Cover the DIRECT colour set too; some firmware revisions > + * pull from 0x8100 instead of 0x8160. > + */ > + ret = ene_write(sdev, ENE_REG_COLORS_DIRECT + i * ENE_RGB_LEN, > + rgb[i], ENE_RGB_LEN); > + if (ret) > + goto err; > + } > + > + ret = ene_write(sdev, ENE_REG_APPLY, &apply, 1); > + if (ret) > + goto err; > + > + /* > + * The change only takes effect after SAVE (0xaa). NOTE: saving on > + * every brightness change writes flash each time; revisit for wear > + * once confirmed. > + */ > + ret = ene_write(sdev, ENE_REG_APPLY, &save, 1); > + if (ret) > + goto err; > + > + return; > +err: > + dev_err(&sdev->sdev_gendev, > + "asus_aura: colour update failed: %d\n", ret); Can there be a problem that results in this repeating and filling the logs? Consider making it dev_err_once(). Add include. > +} > + > +/* Non-blocking LED callback (LED core fast path). Cache colour, defer SCSI. */ > +static void asus_aura_set(struct led_classdev *cdev, > + enum led_brightness brightness) > +{ > + struct led_classdev_mc *mc = lcdev_to_mccdev(cdev); > + struct asus_aura_led *led = > + container_of(mc, struct asus_aura_led, mc_cdev); > + struct asus_aura_zone *zone = led->zone; > + unsigned long flags; > + > + /* > + * led_mc_calc_color_components() writes the shared subled_info > + * array. The LED core serializes sysfs stores with led_access, > + * but trigger events call led_set_brightness() without it, so > + * computing and copying the components under the same lock keeps > + * a trigger-driven update and a sysfs store from reading a mix of > + * each other's colours. > + */ > + spin_lock_irqsave(&zone->lock, flags); > + led_mc_calc_color_components(mc, brightness); > + /* ENE colour register byte order is R, B, G. */ Yet its size is defined by ENE_RGB_LEN ? > + led->rgb[0] = led->subled[0].brightness; > + led->rgb[1] = led->subled[2].brightness; > + led->rgb[2] = led->subled[1].brightness; The mismatched indexing will surely add confusion if you don't properly name things with defines. > + zone->dirty |= BIT(led->index); > + spin_unlock_irqrestore(&zone->lock, flags); > + > + schedule_work(&zone->work); > +} > + > +static int asus_aura_register_led(struct asus_aura_zone *zone, int index) > +{ > + struct asus_aura_led *led = &zone->leds[index]; > + struct led_classdev *cdev = &led->mc_cdev.led_cdev; > + char hctl[32]; > + int ret; > + > + led->zone = zone; > + led->index = index; > + > + led->subled[0].color_index = LED_COLOR_ID_RED; > + led->subled[1].color_index = LED_COLOR_ID_GREEN; > + led->subled[2].color_index = LED_COLOR_ID_BLUE; > + led->mc_cdev.num_colors = 3; > + led->mc_cdev.subled_info = led->subled; > + > + /* > + * Include the sdev's H:C:T:L: every enclosure gets its own SCSI > + * host, so the names stay unique when more than one is connected. > + * With a static name the LED core would register the second > + * enclosure's LEDs under renamed nodes (asus-arion::led-0_1), > + * which is the wrong device identity. The names are per-attachment, > + * like sd X letters, and userspace is expected to enumerate. > + * > + * dev_name() renders the sdev as H:C:T:L; the extra colons would > + * break the devicename:color:function scheme userspace parses LED > + * class names with, so they are flattened to dashes. The color > + * section stays empty (these are multicolor LEDs, the palette is > + * enumerated via multi_intensity), and the four identical zones > + * get the function name with a "-N" ordinal, like the > + * Documentation/leds/leds-class.rst naming section asks for. > + */ > + strscpy(hctl, dev_name(&zone->sdev->sdev_gendev), sizeof(hctl)); Use 2-arg strscpy() variant. > + strreplace(hctl, ':', '-'); > + cdev->name = kasprintf(GFP_KERNEL, "asus-arion-%s::led-%d", hctl, index); > + if (!cdev->name) > + return -ENOMEM; > + cdev->max_brightness = 255; > + cdev->brightness_set = asus_aura_set; > + > + led_mc_calc_color_components(&led->mc_cdev, cdev->brightness); > + > + /* > + * Register with NULL parent: parenting the LED to the sdev takes a > + * device reference, which blocks the sdev's final release on unplug, > + * which is what calls scsi_dh_release_device() -> our .detach() that > + * unregisters the LEDs. That reference cycle leaked the LED nodes and > + * the module refcount on every hot-unplug. > + */ > + ret = led_classdev_multicolor_register(NULL, &led->mc_cdev); > + if (ret) > + kfree(cdev->name); > + return ret; > +} > + > +/* > + * Unregister the LED devices before cancelling the work: unregistering > + * removes the sysfs attributes, so no new brightness_set can schedule the > + * zone work afterwards, and it waits for in-flight sysfs callbacks. > + * Cancelling first would leave a window where a brightness write requeues > + * the work after cancel_work_sync() returned, and the work would then run > + * on freed memory. > + */ > +static void asus_aura_release(struct asus_aura_zone *zone, int num_leds) > +{ > + int i; > + > + for (i = 0; i < num_leds; i++) > + led_classdev_multicolor_unregister(&zone->leds[i].mc_cdev); > + cancel_work_sync(&zone->work); > + for (i = 0; i < num_leds; i++) > + kfree(zone->leds[i].mc_cdev.led_cdev.name); > + kfree(zone); > +} > + > +static int asus_aura_attach(struct scsi_device *sdev) > +{ > + struct asus_aura_zone *zone; > + int i, ret; > + > + if (strncmp(sdev->vendor, ARION_INQ_VENDOR, strlen(ARION_INQ_VENDOR)) || > + strncmp(sdev->model, ARION_INQ_MODEL, strlen(ARION_INQ_MODEL))) > + return SCSI_DH_DEV_UNSUPP; > + > + zone = kzalloc_obj(*zone, GFP_KERNEL); > + if (!zone) > + return SCSI_DH_NOMEM; > + zone->sdev = sdev; > + spin_lock_init(&zone->lock); > + INIT_WORK(&zone->work, asus_aura_zone_work); > + > + for (i = 0; i < ARION_NUM_LEDS; i++) { > + ret = asus_aura_register_led(zone, i); > + if (ret) { > + asus_aura_release(zone, i); > + return SCSI_DH_NOMEM; > + } > + } > + > + sdev->handler_data = zone; > + return SCSI_DH_OK; > +} > + > +static void asus_aura_detach(struct scsi_device *sdev) > +{ > + struct asus_aura_zone *zone = sdev->handler_data; > + > + if (!zone) > + return; > + asus_aura_release(zone, ARION_NUM_LEDS); > + sdev->handler_data = NULL; > +} > + > +static struct scsi_device_handler asus_aura_dh = { > + .name = "asus_aura", > + .module = THIS_MODULE, > + .attach = asus_aura_attach, > + .detach = asus_aura_detach, > +}; > + > +static int __init asus_aura_init(void) > +{ > + return scsi_register_device_handler(&asus_aura_dh); > +} > + > +static void __exit asus_aura_exit(void) > +{ > + scsi_unregister_device_handler(&asus_aura_dh); > +} > + > +module_init(asus_aura_init); > +module_exit(asus_aura_exit); > + > +MODULE_DESCRIPTION("ASUS Aura RGB over SCSI for ROG NVMe enclosures (per-LED)"); > +MODULE_AUTHOR("Liang Haowen"); > +MODULE_LICENSE("GPL"); > -- i. ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC v5 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures [not found] ` <20260916130602.GS11487@google.com> 2026-09-16 14:22 ` [PATCH RFC v6 0/1] " Liang Haowen @ 2026-09-16 14:23 ` Liang Haowen 1 sibling, 0 replies; 27+ messages in thread From: Liang Haowen @ 2026-09-16 14:23 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds, linux-scsi, linux-kernel, sashiko-reviews On the three sashiko items: the naming one I took as a fix. The doc says sections that don't apply are left blank and shows "phy3::wlan" for that, so v6 uses the empty color section explicitly, asus-arion-0-0-0-0::led-0, with the "-N" ordinal the naming section asks for when several LEDs share a function. v6 goes out as its own thread. The other two are the same false positives as in every previous round: kzalloc_obj() has been in include/linux/slab.h since v7.0 (this driver builds against 7.2), and blk_rq_map_kern() takes four arguments on current kernels, called exactly this way from scsi_execute_cmd() in drivers/scsi/scsi_lib.c. The bot's reference tree looks outdated; is there a way to retire these two from further rounds? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC v4 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures [not found] ` <20260916104305.GM11487@google.com> 2026-09-16 12:15 ` [PATCH RFC v5 0/1] " Liang Haowen @ 2026-09-16 12:15 ` Liang Haowen 1 sibling, 0 replies; 27+ messages in thread From: Liang Haowen @ 2026-09-16 12:15 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds, linux-scsi, linux-kernel, sashiko-reviews On the three sashiko items: the Medium one was a real race. led_mc_calc_color_components() writes the shared subled_info array outside the zone lock, and concurrent brightness_set callbacks for the same LED could cache a mix of each other's colours. v5 moves it under the spinlock; v5 goes out as its own thread. The two Low items are false positives, and the same two the bot has now reported in all four rounds: kzalloc_obj() has been in include/linux/slab.h since v7.0 (this driver builds against 7.2), and blk_rq_map_kern() takes four arguments on current kernels, called exactly this way from scsi_execute_cmd() in drivers/scsi/scsi_lib.c. The five-argument form it keeps describing is from older trees. The bot's reference tree looks outdated; can these two be retired from further rounds? ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH RFC v3 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures [not found] ` <20260910093232.GO2133376@google.com> 2026-09-15 12:48 ` [PATCH RFC v4 0/1] " Liang Haowen @ 2026-09-15 12:49 ` Liang Haowen 1 sibling, 0 replies; 27+ messages in thread From: Liang Haowen @ 2026-09-15 12:49 UTC (permalink / raw) To: Lee Jones; +Cc: linux-leds, linux-scsi, linux-kernel, sashiko-reviews Understood, sorry about the threading. v4 goes out as its own thread. On the three sashiko items: the naming one was a real bug, the LED names now flatten the sdev colons to dashes (asus-arion-0-0-0-0:led0), so there is exactly one separator again. The other two are false positives: kzalloc_obj() has been in include/linux/slab.h since v7.0, and blk_rq_map_kern() takes four arguments on current kernels, called exactly this way from scsi_execute_cmd() in drivers/scsi/scsi_lib.c. Details in the v4 cover letter. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH RFC v2 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures 2026-09-01 14:34 ` [PATCH RFC 1/1] " Liang Haowen 2026-09-03 12:00 ` [PATCH RFC v2 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED " Liang Haowen @ 2026-09-03 12:00 ` Liang Haowen 1 sibling, 0 replies; 27+ messages in thread From: Liang Haowen @ 2026-09-03 12:00 UTC (permalink / raw) To: linux-leds Cc: Lee Jones, Pavel Machek, Martin K. Petersen, linux-scsi, platform-driver-x86, linux-kernel, Denis Benato, Armin Wolf, Hans de Goede, Ilpo Jarvinen Hello, v2 of the LED driver for ASUS Aura RGB on ROG external NVMe enclosures. It addresses all seven points from the review v1 received (from the sashiko AI bot; there has been no human review on v1 yet). Changes since v1: - Teardown order: .detach() and the attach error path now unregister the LED class devices first, then cancel_work_sync(), then free. The v1 order (cancel first) left a window where a brightness write could requeue the work after cancel_work_sync() returned, so the work would run on freed memory. - Request allocation: ene_write() now builds the request with scsi_alloc_request() instead of a raw blk_mq_alloc_request(). scsi_initialize_rq() zeroes cmnd, initializes the rcu head, sense length and retries; skipping that left those fields uninitialized. An explicit timeout and RQF_QUIET are set, matching what scsi_execute_cmd() does. - Serialization of the ENE sequence: brightness_set() now only caches the colour and marks the LED in a per-zone dirty bitmap; a single work item per zone runs one sequence (MODE, colour slots, APPLY, SAVE) for all pending LEDs. The v1 per-LED works could interleave their sequences between concurrent updates. As a side effect, multi-LED updates now batch into one APPLY/SAVE. - Subject line: switched to the leds subsystem prefix and capitalization. - The attach success log message is gone. Two of the reported items did not hold up against the kernel this driver is built against (7.2.2): - kzalloc_obj() is not an undefined macro; it lives in include/linux/slab.h (since v6.17). - blk_rq_map_kern() with four arguments is the current signature (rq, buf, len, gfp); drivers/scsi/scsi_lib.c calls it that way from scsi_execute_cmd(). Everything else is unchanged from v1: the hardware (ROG external NVMe enclosures, e.g. ROG STRIX Arion, USB 0b05:1932, no HID, ENE LED controller behind vendor SCSI commands on the disk's LUN, 4 independently addressable LEDs), the scsi_device_handler that does not claim the sdev, the multicolor LED interface, and the protocol handling. v2 was re-verified on hardware. Known caveats, unchanged: - the handler attaches manually until a notifier lands (echo asus_aura > /sys/block/sdX/device/dh_state); - SAVE (0xaa) is issued with every colour update, which writes the enclosure flash each time; wear has not been characterized yet; - the LEDs are registered with a NULL parent device, because parenting them to the sdev creates a reference cycle that blocks the sdev's final release on unplug. Comments on the interface shape and on folding this into the shared Aura work with Denis remain very welcome. Signed-off-by: Liang Haowen <nbg2974@gmail.com> Liang Haowen (1): leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures drivers/leds/leds-asus-aura-scsi.c | 332 +++++++++++++++++++++++++++++ 1 file changed, 332 insertions(+) -- 2.55.0 ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2026-09-23 15:11 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23 10:30 [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures Liang Haowen
2026-09-23 10:30 ` [RFC v7 1/1] " Liang Haowen
2026-09-23 10:45 ` Lee Jones
2026-09-23 11:19 ` Liang Haowen
2026-09-23 10:41 ` [RFC v7 0/1] " Ilpo Järvinen
2026-09-23 12:35 ` Denis Benato
2026-09-23 12:59 ` Liang Haowen
2026-09-23 15:11 ` Marco Scardovi
-- strict thread matches above, loose matches on Subject: below --
2026-09-01 14:26 [PATCH RFC 0/1] leds: add ASUS Aura SCSI " Liang Haowen
2026-09-01 14:34 ` [PATCH RFC 1/1] " Liang Haowen
2026-09-03 12:00 ` [PATCH RFC v2 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED " Liang Haowen
[not found] ` <20260903161357.GX2133376@google.com>
2026-09-04 12:30 ` [PATCH RFC v3 0/1] " Liang Haowen
2026-09-04 12:30 ` [PATCH RFC v3 1/1] " Liang Haowen
[not found] ` <20260904131503.355C41F00A3E@smtp.kernel.org>
[not found] ` <20260910093232.GO2133376@google.com>
2026-09-15 12:48 ` [PATCH RFC v4 0/1] " Liang Haowen
2026-09-15 12:49 ` [PATCH RFC v4 1/1] " Liang Haowen
[not found] ` <20260915130050.E8CDB1F000FF@smtp.kernel.org>
[not found] ` <20260916104305.GM11487@google.com>
2026-09-16 12:15 ` [PATCH RFC v5 0/1] " Liang Haowen
2026-09-16 12:15 ` [PATCH RFC v5 1/1] " Liang Haowen
[not found] ` <20260916122740.921E51F000FF@smtp.kernel.org>
[not found] ` <20260916130602.GS11487@google.com>
2026-09-16 14:22 ` [PATCH RFC v6 0/1] " Liang Haowen
2026-09-16 14:22 ` [PATCH RFC v6 1/1] " Liang Haowen
[not found] ` <20260916143832.520721F000FF@smtp.kernel.org>
[not found] ` <20260917112840.GJ1605367@google.com>
2026-09-17 11:49 ` Liang Haowen
2026-09-23 9:45 ` Lee Jones
2026-09-23 10:18 ` Liang Haowen
[not found] ` <20260923102812.2353292-1-nbg2974@gmail.com>
2026-09-23 10:12 ` Ilpo Järvinen
2026-09-16 14:23 ` [PATCH RFC v5 " Liang Haowen
2026-09-16 12:15 ` [PATCH RFC v4 " Liang Haowen
2026-09-15 12:49 ` [PATCH RFC v3 " Liang Haowen
2026-09-03 12:00 ` [PATCH RFC v2 0/1] " Liang Haowen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®