From: Lee Jones <lee@kernel.org>
To: Liang Haowen <nbg2974@gmail.com>
Cc: linux-leds@vger.kernel.org, Pavel Machek <pavel@kernel.org>,
"Martin K . Petersen" <mkp@kernel.org>,
linux-scsi@vger.kernel.org, platform-driver-x86@vger.kernel.org,
linux-kernel@vger.kernel.org,
Denis Benato <denis.benato@linux.dev>,
Armin Wolf <W_Armin@gmx.de>, Hans de Goede <hansg@kernel.org>,
Ilpo Jarvinen <ilpo.jarvinen@linux.intel.com>,
Marco Scardovi <scardracs@disroot.org>
Subject: Re: [RFC v9 1/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures
Date: Thu, 15 Oct 2026 12:50:43 +0100 [thread overview]
Message-ID: <20261015115043.GA1863739@google.com> (raw)
In-Reply-To: <179198503855.839288.7797731903424311774@kernel.org>
On Fri, 25 Sep 2026, Lee Jones wrote:
> --- checkpatch.pl: clean (0 issues) ---
>
> On Fri, 25 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,
> > BE16 register index, argument count in byte 13), modelled by
> > struct ene_cdb. 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, wire order R, B, G; both tables are
> > written because firmware revisions pull from one or the other);
> > APPLY 0x80a0 takes 0x01 to apply; the controller also accepts 0xaa
> > to save the state to its flash, which this driver never sends: the
> > LED class interface is runtime state and does not promise persistence
> > across power cycles, and a save on every update would wear the
> > controller flash. The colour reverts to the stored default on power
> > cycle, like any other runtime-only LED.
> >
> > 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 byte 13 is dropped and the device silently ignores the
> > write (GOOD status, no error). scsi_execute_cmd() also offers no way
> > to override the derived length. The request is therefore built with
> > scsi_alloc_request(), which initializes the scsi_cmnd parts a
> > passthrough needs, with cmd_len set to the CDB size, mirroring what
> > SG_IO does from userspace. A 5 s timeout and a single retry-less
> > quiet submission cover the register writes.
> >
> > The write payload is copied into a per-device buffer before it is
> > mapped with blk_rq_map_kern(), so no stack memory is ever mapped for
> > DMA (VMAP_STACK).
> >
> > brightness_set only caches the colour and marks the LED pending
> > 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 enclosure then
> > snapshots the pending LEDs and runs one ENE sequence for all of
> > them. 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. 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 pending flag 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 | 404 +++++++++++++++++++++++++
> > 1 file changed, 404 insertions(+)
> > create mode 100644 drivers/leds/rgb/leds-asus-aura-scsi.c
>
> A driver source file without 'Kconfig' or 'Makefile' integration is dead code in
> the tree. Either wire it up now or hold off on submitting until the rest of the
> pieces are ready.
Also:
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] leds: asus-aura-scsi: undefined function kzalloc_obj() causes build failure
- [Low] leds: asus-aura-scsi: blk_rq_map_kern() missing required request_queue argument
- [Low] leds: asus-aura-scsi: strscpy() called with missing size argument
Was this even tested?
--
Lee Jones
prev parent reply other threads:[~2026-10-15 11:50 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-25 13:40 [RFC v9 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures Liang Haowen
2026-09-25 13:40 ` [RFC v9 1/1] " Liang Haowen
2026-09-25 13:52 ` sashiko-bot
[not found] ` <179198503855.839288.7797731903424311774@kernel.org>
2026-10-15 11:50 ` Lee Jones [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261015115043.GA1863739@google.com \
--to=lee@kernel.org \
--cc=W_Armin@gmx.de \
--cc=denis.benato@linux.dev \
--cc=hansg@kernel.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=mkp@kernel.org \
--cc=nbg2974@gmail.com \
--cc=pavel@kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=scardracs@disroot.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox