Re: [PATCH v3 2/4] leds: flash: Add support for the TI LM3643 dual LED flash driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] leds: missing hardware startup delay after asserting enable GPIO
- [Low] leds: non-standard commit message prefix used
--

commit e9482e46025f29c98cd459b067571fee928be4b6
Author: Rillian Grant <rillian.grant@gmail.com>

leds: flash: Add support for the TI LM3643 dual LED flash driver

This commit introduces support for the Texas Instruments LM3643 dual LED
flash driver over I2C. The chip provides torch and flash modes with
programmable current and timeouts via a shared synchronous boost converter.

> leds: flash: Add support for the TI LM3643 dual LED flash driver

[Severity: Low]
This isn't a bug, but does this commit subject violate the subsystem naming
conventions?

According to the LED subsystem guidelines, embedding subsystem subdirectory
names like flash into the prefix hinders Git history categorization. Embedding
implementation details can also create rigid and non-reusable drivers.

Could the driver name lm3643 be used in the prefix instead, such as
"leds: lm3643: "?

> diff --git a/drivers/leds/flash/leds-lm3643.c b/drivers/leds/flash/leds-lm3643.c
> --- /dev/null
> +++ b/drivers/leds/flash/leds-lm3643.c
[ ... ]
> +static int lm3643_probe(struct i2c_client *client)
> +{
[ ... ]
> +	hwen = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> +	if (IS_ERR(hwen))
> +		return dev_err_probe(dev, PTR_ERR(hwen), "failed to get enable GPIO\n");
> +
> +	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +	i2c_set_clientdata(client, chip);
> +
> +	chip->regmap = devm_regmap_init_i2c(client, &lm3643_regmap_config);
> +	if (IS_ERR(chip->regmap))
> +		return dev_err_probe(dev,
> +				     PTR_ERR(chip->regmap),
> +				     "failed to allocate register map\n");
> +
> +	ret = regmap_read(chip->regmap, LM3643_REG_DEV_ID, &dev_id);

[Severity: Medium]
Does this sequence require a hardware startup delay after asserting the enable
GPIO?

The lm3643_probe() function drives the HWEN GPIO high and then shortly after
initiates an I2C regmap_read() to fetch the device ID. Failing to provide a
programmatic delay (e.g., via usleep_range) might cause the I2C transfer
to fail on platforms where the CPU executes the bus transaction before the
hardware state machine and internal oscillator are fully initialized.

Could a small delay be added here to ensure the chip is ready for I2C traffic
before the register read is attempted?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011191026.149338-1-rillian.grant@gmail.com?part=2