[PATCH RFC 0/1] leds: add ASUS Aura SCSI driver for ROG NVMe enclosures

[PATCH RFC 0/1] leds: add ASUS Aura SCSI driver for ROG NVMe enclosures

[Liang Haowen]

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

[PATCH RFC 1/1] leds: add ASUS Aura SCSI driver for ROG NVMe enclosures

[Liang Haowen]

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");

[PATCH RFC v2 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

[PATCH RFC v2 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

[PATCH RFC v3 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

[PATCH RFC v3 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

[PATCH RFC v4 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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>

[PATCH RFC v4 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

Re: [PATCH RFC v3 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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.

[PATCH RFC v5 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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>

[PATCH RFC v5 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

Re: [PATCH RFC v4 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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?

[PATCH RFC v6 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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>

[PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

Re: [PATCH RFC v5 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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?

Re: [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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.

Re: [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Lee Jones]

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

Re: [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Ilpo Järvinen]

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.

Re: [PATCH RFC v6 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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.

[RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

[RFC v7 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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

Re: [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Ilpo Järvinen]

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
> 
> 

Re: [RFC v7 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Lee Jones]

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

Re: [RFC v7 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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.

Re: [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Denis Benato]

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

Re: [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Liang Haowen]

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.

Re: [RFC v7 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures

[Marco Scardovi]

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