Re: [PATCH 5/7] leds: pca995x: add sysfs files for error reporting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `has_errors_store` function uses `strcmp()` to perform an exact match on "clear\n", which incorrectly rejects valid sysfs writes that omit a trailing newline.
- [Low] The commit message description is not capitalized, violating subsystem guidelines.
--

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

leds: pca995x: add sysfs files for error reporting

The PCA995x has builtin failure detection. Provide sysfs files for
individual LED status (reporting "okay", "short-circuit" or
"open-circuit") as well as a global "has_errors" flag. has_errors is
sticky and must be cleared by writing "clear" to the sysfs file.

> leds: pca995x: add sysfs files for error reporting

[Severity: Low]
This isn't a bug, but the commit message description starts with a lowercase
letter. The LED subsystem guidelines request capitalizing the description
after the subsystem prefix.

> diff --git a/drivers/leds/leds-pca995x.c b/drivers/leds/leds-pca995x.c
> index b069b93d39a54..2232b74528a53 100644
> --- a/drivers/leds/leds-pca995x.c
> +++ b/drivers/leds/leds-pca995x.c
[ ... ]
> +static ssize_t has_errors_store(struct device *dev, struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	struct pca995x_chip *chip = i2c_get_clientdata(to_i2c_client(dev));
> +	int ret;
> +
> +	if (strcmp(buf, "clear\n"))
> +		return -EINVAL;

[Severity: Medium]
Does this exact string match in has_errors_store() reject valid sysfs writes 
that omit a trailing newline? 

If userspace writes exactly the word clear without a newline 
(for example, using echo -n clear > has_errors), strcmp will fail and return 
-EINVAL. Using sysfs_streq() instead of strcmp() would transparently handle 
the presence or absence of a trailing newline from sysfs inputs.

> +
> +	ret = regmap_update_bits(chip->regmap, PCA995X_MODE2,
> +				 PCA995X_MODE2_CLRERR, PCA995X_MODE2_CLRERR);
> +
> +	return ret ?: count;
> +}
[ ... ]

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