Re: [PATCH v2 8/8] leds: pca995x: Add support for group brightness control

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] sysfs: race condition between LED registration and parent attributes creation
--

commit 519bef49574c49d4cda0dcf149ce7b8d6e09657e
Author: Nora Schiffer <nora.schiffer@ew.tq-group.com>

leds: pca995x: Add support for group brightness control

When LEDs are set to PWM mode with group control enabled, their
brightness can control using a global "group PWM" setting, modulating
the individual LEDs' brightness with a second PWM running at a different
frequency. This patch introduces sysfs attributes to expose group
brightness control and clear error flags.

> diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c
> index 963e453ce117d..33da3cd9cbbbf 100644
> --- a/drivers/leds/leds-pca995x.c
> +++ b/drivers/leds/leds-pca995x.c
[ ... ]
> @@ -190,10 +194,41 @@ static ssize_t has_errors_store(struct device *dev, struct device_attribute *att
>  	return ret ?: count;
>  }
>  
> +static ssize_t group_brightness_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
[ ... ]
> +static ssize_t group_brightness_store(struct device *dev, struct device_attribute *attr,
> +				       const char *buf, size_t count)
> +{
[ ... ]
>  static DEVICE_ATTR_RW(has_errors);
> +static DEVICE_ATTR_RW(group_brightness);
>  
>  static struct attribute *pca995x_attrs[] = {
>  	&dev_attr_has_errors.attr,
> +	&dev_attr_group_brightness.attr,
>  	NULL,
>  };

[Severity: Medium]
Does this attribute registration order create a race condition with userspace?

Looking at pca995x_probe(), devm_led_classdev_register_ext() is called to
register the LEDs, which emits KOBJ_ADD uevents. Later in the function,
sysfs_create_groups() is called to create these parent attributes.

Because the attributes are created after the uevent is emitted, udev rules
triggered by the LED addition might fail to read the new group_brightness
attribute because it is not yet created.

Similarly on device removal, sysfs_remove_groups() is called in
pca995x_remove() before the managed devres cleanup unregisters the child
LED devices. This causes the attributes to disappear while the LEDs are
still registered and potentially active.

Could we avoid this race during teardown and initialization by using
devm_device_add_groups() before registering the LED class devices?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/bd687915af26ccdbfc8cc911eee0787fc8145ae1.1789477502.git.nora.schiffer@ew.tq-group.com?part=8