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

Liang Haowen posted 1 patch 23 hours ago
[RFC v8 0/1] leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures
Posted by Liang Haowen 23 hours ago
Hello,

v8, as its own thread. It addresses every comment from Lee's v6
review and Ilpo's line review; the notes below say what was done per
comment and where I think a comment rests on a misunderstanding, I
say so instead of silently keeping the code.

The submission format is git send-email from v7 on, the driver lives
in drivers/leds/rgb/, and it still has no Kconfig wiring: it is the
verified out-of-tree module, the wiring lands with the agreed SCSI
split. On the SCSI device handler itself: it is an out-of-tree
expedient, not 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, and hooking the
SCSI bus from a module needs the unexported scsi_bus_type. The
device handler is the one hook that observes the sdev without
claiming it. For in-tree, the attachment should move under
drivers/scsi in whichever form the SCSI side prefers, notifier or an
exported attach helper, and I will rework it accordingly.

Changes against the review comments:

- Includes sorted alphabetically, linux/err.h added (Ilpo).

- The vendor CDB is now struct ene_cdb, __packed, with named fields
  and a __be16 register set with cpu_to_be16() (Lee, Ilpo), plus a
  static_assert on the 16-byte size.

- The ENE register defines carry plain comments; the R,B,G wire
  order has named indices (ENE_WIRE_R/B/G) so the reordering against
  the R,G,B subleds no longer relies on magic numbers (Lee, Ilpo).

- struct asus_aura_zone became struct asus_aura, the per-enclosure
  context; the dirty bitmask is gone, each LED carries its own
  pending flag (Lee: LED-level attribute, faux indexing removed).

- The work item returns before touching the device when nothing is
  pending: an unlocked scan first, since a colour arriving after the
  scan re-queues the work (Lee). The locked snapshot follows.

- The write payload is copied into a DMA-safe per-device buffer
  before blk_rq_map_kern(); no stack memory is ever mapped for DMA
  (Lee, VMAP_STACK).

- ene_write() became ene_write_reg() with a tx buffer argument, the
  arg_count branch is gone (never false), and it returns 0/-EIO
  instead of the raw SAM status; the rationale for not using
  scsi_execute_cmd() moved to the commit message (Lee).

- The 10 s timeout dropped to 5 s, the development-history comments
  are gone, declarations are unwrapped, loop variables are declared
  in the loop, lines are reflowed to 100 columns, the code is grouped
  with blank lines, and
  asus_aura_register_led() frees its own name on failure (Lee).

- dev_err_ratelimited() instead of dev_err() on the work error path
  (Ilpo); _once() would hide a persistently failing device, and
  there is no caller to report to, the work runs detached from the
  LED core (Lee).

- strscpy() uses the 2-arg form (Ilpo).

Two comments I did not act on:

- .module = THIS_MODULE in the device handler: the in-tree handlers
  (scsi_dh_alua, scsi_dh_emc) set it explicitly as well, so I kept
  it, but I am happy to drop it if the subsystem prefers.

- kzalloc_obj() stays: checkpatch on 7.2 prefers it over kzalloc()
  with sizeof, and the sashiko report against it is based on an
  older tree (it has been in include/linux/slab.h since v7.0).

v8 was verified on hardware again: the four LEDs, per-LED colours,
60 sequential updates, unplug under load (zero splats, zero leaked
nodes, clean rmmod).

Comments on the interface shape and on folding this into the shared
Aura work with Denis remain very welcome.

Liang Haowen (1):
  leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe
    enclosures

-- 
2.55.0