Re: [PATCH 09/10] leds: rgb: ktd202x: Fail the probe when the chip does not answer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] leds: rgb: ktd202x: failing probe on reset write breaks KTD2026/KTD2027
--

commit cbade43891d8603172b94d1364ff11d464697a37
Author: Joseph Telaak <joseph@telaak.dev>

leds: rgb: ktd202x: Fail the probe when the chip does not answer

This commit checks the return value of the reset register write in
ktd202x_probe_fw() and fails probing if the I2C write fails.

> diff --git a/drivers/leds/rgb/leds-ktd202x.c b/drivers/leds/rgb/leds-ktd202x.c
> index 48521f1e5a704..5f51df9930cb2 100644
> --- a/drivers/leds/rgb/leds-ktd202x.c
> +++ b/drivers/leds/rgb/leds-ktd202x.c
> @@ -722,20 +722,23 @@ static int ktd202x_probe_fw(struct ktd202x *chip)
>  {
>  	struct device *dev = chip->dev;
>  	int count;
> +	int ret;
>  	int i = 0;
>  
>  	count = device_get_child_node_count(dev);
>  	if (!count || count > chip->num_leds)
>  		return -EINVAL;
>  
> -	regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
> +	/* Nothing is read back, so a failed reset is the only sign of a missing chip */
> +	ret = regmap_write(chip->regmap, KTD202X_REG_RESET_CONTROL, KTD202X_RSTR_RESET);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to reset the chip\n");

[Severity: High]
Will failing ktd202x_probe_fw() when the KTD202X_RSTR_RESET write returns an
error break probing on genuine Kinetic KTD2026 and KTD2027 hardware?

Per the Kinetic KTD2026/KTD2027 datasheet (Register 0 Reset/Control bits
[2:0] = 111, Reset Complete Chip), executing the reset command resets the
entire chip including its I2C state machine immediately upon latching the
data byte, so the chip does not return an I2C ACK pulse for the reset
command byte and requires a 200 us wait before the next I2C transfer.

Because the KTD2026/KTD2027 NACKs the KTD202X_RSTR_RESET write by design,
regmap_write() returns a NACK error (-ENXIO or -EREMOTEIO) during
ktd202x_probe() -> ktd202x_probe_fw(), which is why commit 0ebdb7210943
("leds: Add ktd202x driver") ignored the return value of the reset write.

To detect a missing chip at probe time without regressing KTD2026/KTD2027,
could the driver instead check the return value of a register write issued
after usleep_range(200, 300) completes (for example, writing
KTD202X_ENABLE_CTRL_SLEEP to KTD202X_REG_RESET_CONTROL after the reset
delay)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-ktd202x-et6326-v1-0-eb51eddad08c@telaak.dev?part=9