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

[Lee Jones <lee@kernel.org>]

On Wed, 16 Sep 2026, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Low] block: build failure due to missing request_queue argument in blk_rq_map_kern()
> - [Low] leds: build failure due to undefined macro kzalloc_obj()
> - [Low] leds: UAPI naming convention violation for LED class device
> --

Please review, explain or fix the Sashiko review(s).

If this is a set, this request includes the other patches too.

> commit fe7900b55dc69157a3ef071f13e8783fac33da59
> Author: Liang Haowen <nbg2974@gmail.com>
> 
> leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures
> 
> This commit adds a new SCSI driver for ASUS Aura RGB LEDs found on ROG
> NVMe enclosures. It exposes each LED as a multicolor LED class device by
> routing vendor SCSI commands. Updates are batched and scheduled via a
> workqueue to prevent concurrent writes.
> 
> > diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c
> > new file mode 100644
> > index 0000000000000..a1e31dc3f0101
> > --- /dev/null
> > +++ b/drivers/leds/leds-asus-aura-scsi.c
> [ ... ]
> > +	if (arg_count) {
> > +		ret = blk_rq_map_kern(rq, (void *)data, arg_count, GFP_NOIO);
> 
> [Severity: Low]
> Does this build against the current block layer?
> 
> The blk_rq_map_kern() function requires five arguments, including a
> struct request_queue pointer. Only four arguments are provided here.
> 
> > +		if (ret)
> > +			goto out;
> > +	}
> [ ... ]
> > +	/*
> > +	 * 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);
> 
> [Severity: Low]
> Does this naming scheme violate the UAPI LED naming convention?
> 
> The standard format requires two colons (devicename:color:function), as
> documented in Documentation/leds/leds-class.rst. The comment above states
> this follows the devicename:color:function convention, but the generated
> name (e.g., asus-arion-xxxx:led0) only contains a single colon.
> 
> > +	if (!cdev->name)
> > +		return -ENOMEM;
> [ ... ]
> > +	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);
> 
> [Severity: Low]
> Is kzalloc_obj() a valid macro in the kernel API?
> 
> It appears to be undefined, which will cause a build failure.
> Should this just be kzalloc(sizeof(*zone), GFP_KERNEL)?
> 
> > +	if (!zone)
> > +		return SCSI_DH_NOMEM;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/202609162015.RFCv5-1.lhw@gmail.com?part=1

-- 
Lee Jones