Re: [PATCH v5 4/7] ABI: Add sysfs documentation for leds-group-virtualcolor

[Andriy Shevencho <andriy.shevchenko@linux.intel.com>]

On Tue, Dec 30, 2025 at 09:23:17PM +1300, Jonathan Brophy wrote:
> 
> Document the sysfs ABI for the virtual LED group driver, including:
> 
> - mc/multi_intensity: Per-channel intensity control (0-255)
> - mc/multi_index: Channel-to-color-ID mapping (read-only)
> - mc/multi_multipliers: Per-channel scale factors (read-only)
> - brightness: Master brightness control with arbitration trigger
> - max_brightness: Maximum brightness value (mode-dependent)
> 
> Channel ordering is deterministic, sorted by ascending LED_COLOR_ID
> value. For RGBW LEDs, white (ID=0) appears first, followed by RGB.
> 
> The multi_intensity attribute is rate-limited to 100 updates/second
> per virtual LED by default, with counters visible in debugfs when
> CONFIG_DEBUG_FS is enabled.

...

> +Date:		December 2024
> +KernelVersion:	6.x

Both fields better to be filled with help of prediction crystal ball:
https://hansen.beer/~dave/phb/

Take the -rc1 or .0 date of the expected release (as of now 6.20.

Ditto for all new documented attributes.


...

> +		Example (RGB LED with 3 channels):
> +		  $ cat /sys/class/leds/myled/mc/multi_index
> +		  1 2 3
> +		  $ cat /sys/class/leds/myled/mc/multi_intensity
> +		  255 128 0
> +		  $ echo "128 64 200" > /sys/class/leds/myled/mc/multi_intensity

I believe for the arrays we use ',' (comma) as a separator.

> +		Note: In standard mode (led-mode = "standard"), intensity
> +		changes are rejected with -EPERM and the color is fixed by the
> +		channel multipliers defined in the device tree. In multicolor
> +		mode (led-mode = "multicolor", default), intensity can be
> +		freely modified.
> +
> +		This attribute is rate-limited to prevent system overload
> +		(default: 100 updates/second per virtual LED). Excessive
> +		updates will be silently dropped with incremented rate limit
> +		counters (visible in debugfs when CONFIG_DEBUG_FS enabled).

...

> +		Common color ID values:
> +		  - 0: LED_COLOR_ID_WHITE
> +		  - 1: LED_COLOR_ID_RED
> +		  - 2: LED_COLOR_ID_GREEN
> +		  - 3: LED_COLOR_ID_BLUE
> +		  - 4: LED_COLOR_ID_AMBER
> +		  - 5: LED_COLOR_ID_VIOLET
> +		  - 6: LED_COLOR_ID_YELLOW
> +		  - 7: LED_COLOR_ID_IR
> +		  - 8: LED_COLOR_ID_MULTI
> +		  - 9: LED_COLOR_ID_RGB
> +		  - 10: LED_COLOR_ID_UV

This doesn't seem to be scalable. Better to use plain color names I think.
In that case the indexing schema is kept internal to the kernel.

...

> +		Example (RGB LED):
> +		  $ cat /sys/class/leds/myled/mc/multi_index
> +		  1 2 3
> +		  (Shows: red=1, green=2, blue=3)
> +
> +		Example (RGBW LED):
> +		  $ cat /sys/class/leds/myled/mc/multi_index
> +		  0 1 2 3
> +		  (Shows: white=0, red=1, green=2, blue=3)

With my proposal it will be

		  (Shows: white,red,green,blue)

and note that if kernel wants to change index, it's flexible to do so and user
space will work the same way as it will keep their own index mapping if they
need so.

The

		  (Shows: red,green,blue,white)

is more aligned with the RGBW given name in your example.

-- 
With Best Regards,
Andy Shevchenko