Re: [PATCH 4/7] leds: pca995x: make output gain configurable

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The patch introduces an un-prefixed, hardware-specific Device Tree property `output-gain` that exposes a raw register value instead of using a standard physical property or a vendor prefix.
- [Medium] Mismatch between the driver fallback logic and the commit message regarding the default `output-gain` value.
- [Medium] The commit message description is not capitalized, violating the LED subsystem commit message formatting rules.
--

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

leds: pca995x: make output gain configurable

This commit makes the output gain of the pca995x LED driver configurable
via the Device Tree. It allows reading a value to adjust the LED current
for different hardware designs instead of using a hardcoded value.

> leds: pca995x: make output gain configurable

[Severity: Medium]
This isn't a bug, but the commit message subject does not follow the LED
subsystem's capitalization rules. Should the description after the prefix
be capitalized, for example "leds: pca995x: Make output gain configurable"?

> diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c
> index 8eb9847b9709f..b069b93d39a54 100644
> --- a/drivers/leds/leds-pca995x.c
> +++ b/drivers/leds/leds-pca995x.c
[ ... ]
> @@ -153,6 +154,11 @@ static int pca995x_probe(struct i2c_client *client)
>  
>  	i2c_set_clientdata(client, chip);
>  
> +	if (device_property_read_u32(dev, "output-gain", &iref))

[Severity: Medium]
Does introducing the un-prefixed "output-gain" property pollute the global
Device Tree namespace? Should this use a standard physical property or a
vendor prefix instead, to avoid long-term ABI maintenance debt?

> +		iref = PCA995X_IREFALL_HALF_CFG;

[Severity: Medium]
The commit message explicitly claims the default output gain value was
hardcoded to 128. However, PCA995X_IREFALL_HALF_CFG is used here as the
fallback, which evaluates to 127. Is this mismatch between the commit
message and the implemented logic intended?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6d3600f99c77c31166f0b6a5ccfb7f518b53c4c9.1788350618.git.nora.schiffer@ew.tq-group.com?part=4