Re: [PATCH v13 5/7] leds: Add driver for ASUS Transformer LEDs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] leds: non-standard two-part LED name format in asus_ec_led_probe()
- [Low] leds: missing driver prefix in commit subject
--

commit 8625f763ce281e3f67385ab547a18ea8401f88e9
Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>

leds: Add driver for ASUS Transformer LEDs

This commit adds an LED class driver for the green and amber charging
indicator LEDs on the ASUS Transformer Pad and Dock embedded controllers.

>     leds: Add driver for ASUS Transformer LEDs
>     
>     ASUS Transformer tablets have a green and an amber LED on both the Pad
>     and the Dock.
>     
>     Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
>     Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
>     Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

[Severity: Low]
This isn't a bug, but LED subsystem commit subjects normally include the
driver sub-prefix in the form "leds: <driver>: <Capitalized description>".
Should the subject line be updated to something like:

  leds: asus-transformer-ec: Add driver for ASUS Transformer LEDs

[ ... ]

> diff --git a/drivers/leds/leds-asus-transformer-ec.c b/drivers/leds/leds-asus-transformer-ec.c
> new file mode 100644
> index 0000000000000..f797dcefd16e6
> --- /dev/null
> +++ b/drivers/leds/leds-asus-transformer-ec.c

[ ... ]

> +static int asus_ec_led_probe(struct platform_device *pdev)
> +{
> +	const struct asusec_core *ec = dev_get_drvdata(pdev->dev.parent);
> +	struct device *dev = &pdev->dev;
> +	struct asus_ec_led *leds;
> +	int ret;
> +
> +	leds = devm_kcalloc(dev, ASUSEC_LED_MAX, sizeof(*leds), GFP_KERNEL);
> +	if (!leds)
> +		return -ENOMEM;
> +
> +	for (int i = 0; i < ASUSEC_LED_MAX; i++) {
> +		const struct asus_ec_led_config *cfg = &asus_ec_leds[i];
> +		struct asus_ec_led *led = &leds[i];
> +
> +		led->cdev.name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s", ec->name, cfg->name);

[Severity: Medium]
Does formatting led->cdev.name as "%s:%s" with ec->name ("pad" or "dock")
and cfg->name ("amber" or "green") omit the function section expected by
the LED class naming convention?

Documentation/leds/leds-class.rst specifies the three-section
devicename:color:function format (or color:function when devicename is
omitted). When a two-section name containing a single colon such as
"pad:amber" or "dock:green" is exposed in /sys/class/leds/, userspace
tooling such as tools/leds/get_led_device_info.sh parses the first field
(ec->name) as the color and the second field (cfg->name) as the function.

Should led->cdev.name include the function segment as well (for example,
using LED_FUNCTION_CHARGING to form "%s:%s:" LED_FUNCTION_CHARGING)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104122606.215629-1-clamor95@gmail.com?part=5