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

[Lee Jones <lee@kernel.org>]

On Tue, 15 Sep 2026, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] leds: data race on subled_info array causes torn color updates
> - [Low] leds: build failure due to undefined macro kzalloc_obj
> - [Low] leds: build failure due to missing argument in blk_rq_map_kern()

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

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

> --
> 
> commit e6ce9b3b67cc2aa4897215acc2c62600810790a8
> 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 that
> exposes 4 independently addressable LEDs via the LED multicolor class. It
> uses a SCSI device handler matched by vendor INQUIRY to send custom ENE
> sequences over passthrough SCSI commands.
> 
> > diff --git a/drivers/leds/leds-asus-aura-scsi.c b/drivers/leds/leds-asus-aura-scsi.c
> > new file mode 100644
> > index 0000000000000..089cc3dcee105
> > --- /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]
> Does this cause a build failure? The kernel API blk_rq_map_kern() requires 5
> arguments, where the first parameter should be the struct request_queue *q,
> but it is called here with only 4 arguments.
> 
> > +		if (ret)
> > +			goto out;
> > +	}
> [ ... ]
> > +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);
> 
> [Severity: Medium]
> Could this lead to a data race on the subled_info array? The LED core allows
> concurrent brightness_set callbacks for the same LED (for instance, from
> sysfs and a hardware trigger). The helper led_mc_calc_color_components()
> modifies the shared mc->subled_info array locklessly before the driver
> acquires its spinlock. If multiple threads update the brightness
> concurrently, they might interleave their writes to the array, causing the
> winning thread to read a mix of old and new color components.
> 
> > +	/* 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_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]
> Does this code fail to compile? It looks like it invokes an undefined macro
> kzalloc_obj() instead of the standard kzalloc() function to allocate memory.
> 
> > +	if (!zone)
> > +		return SCSI_DH_NOMEM;
> > +	zone->sdev = sdev;
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/202609152112.RFCv4-1.lhw@gmail.com?part=1

-- 
Lee Jones