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

[Lee Jones <lee@kernel.org>]

Please do not attach subsequent versions to previous ones.

New versions should be sent on their own.

On Fri, 04 Sep 2026, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] LED class device name violates UAPI naming standards and breaks userspace parsing.
> - [Low] Compilation failure due to undefined macro `kzalloc_obj`.
> - [Low] Compilation failure due to missing argument in `blk_rq_map_kern()` call.

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

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

> --
> 
> commit c4b336ab5a8222a99fc342c60592e26cbc2f680e
> Author: Liang Haowen <nbg2974@gmail.com>
> 
> leds: asus-aura-scsi: Add ASUS Aura RGB LED driver for ROG NVMe enclosures
> 
> This commit introduces a driver for ASUS ROG external NVMe enclosures. It
> exposes the Aura LEDs connected to an ENE controller via vendor SCSI
> commands, implementing multicolor LED class devices for each LED.
> 
> > diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c
> > new file mode 100644
> > index 0000000000000..3ff33615a7934
> > --- /dev/null
> > +++ b/drivers/leds/leds-asus-aura-scsi.c
> [ ... ]
> > +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);
> 
> [Severity: Low]
> Will this compile successfully? Looking at blk_rq_map_kern() in
> block/blk-map.c, the function expects 5 arguments with the request queue as
> the first parameter, but this call only provides 4. Should this pass
> sdev->request_queue as the first argument?
> 
> [ ... ]
> > +static int asus_aura_register_led(struct asus_aura_zone *zone, int index)
> > +{
> [ ... ]
> > +	/*
> > +	 * 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);
> 
> [Severity: Medium]
> Does this format string comply with the standard LED class device naming
> conventions? Since the SCSI dev_name() format typically introduces multiple
> colons (like 0:0:0:0), and standard LED userspace tools expect the naming
> scheme devicename:color:function, will the extra colons break userspace
> parsing or udev rules?
> 
> [ ... ]
> > +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);
> 
> [Severity: Low]
> Will this cause a build failure? It appears kzalloc_obj is not a defined
> kernel allocation macro. Should this be using kzalloc() with sizeof(*zone)
> instead?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/202609042030.RFCv3-1.lhw@gmail.com?part=1

-- 
Lee Jones