This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
leds: is32fl3207: Add controller driver
This adds an I2C driver for the Lumissil IS32FL3207 18-channel LED
controller, exposing single and multicolor LEDs via the LED class.
Per-output current limits are programmed into the scaling registers based
on the RISET value and each LED's led-max-microamp property.
> diff --git a/drivers/leds/rgb/leds-is32fl3207.c
> b/drivers/leds/rgb/leds-is32fl3207.c
> new file mode 100644
> index 0000000000000..6a46f97ba50a5
> --- /dev/null
> +++ b/drivers/leds/rgb/leds-is32fl3207.c
> @@ -0,0 +1,736 @@
[ ... ]
> +static int is32fl3207_configure_channel(struct is32fl3207 *chip,
> + struct fwnode_handle *fwnode,
> + unsigned int *channel)
> +{
[ ... ]
> + ret = fwnode_property_read_u32(fwnode, "led-max-microamp",
> + &max_microamp);
> + if (ret)
> + return dev_err_probe(chip->dev, ret,
> + "failed to read current limit for channel
> %u\n",
> + reg);
> +
> + if (!max_microamp || max_microamp > chip->output_max_microamp)
> + return dev_err_probe(chip->dev, -EINVAL,
> + "invalid current limit %u uA for channel
> %u\n",
> + max_microamp, reg);
> +
> + /* GCC is fixed at 0xff, so use each output's scaling register. */
> + scaling = div_u64((u64)max_microamp * 256 * 256,
> + (u64)chip->output_max_microamp *
> + IS32FL3207_GLOBAL_CURRENT_MAX);
> + if (!scaling)
> + return dev_err_probe(chip->dev, -EINVAL,
> + "current limit %u uA is below channel %u
> resolution\n",
> + max_microamp, reg);
[Severity: Medium]
Can this reject device trees that are valid according to the binding?
Documentation/devicetree/bindings/leds/issi,is32fl3207.yaml declares
led-max-microamp with minimum: 1, both for the top-level led@ nodes and for
the multi-led component nodes:
led-max-microamp:
minimum: 1
maximum: 38250
description: |
Board-safe peak current for this output. The value must not exceed
the full-scale current derived from issi,riset-ohms.
In is32fl3207_configure_channel() the scaling quotient truncates to zero for
small currents, and the !scaling case aborts probe with -EINVAL via
is32fl3207_probe() -> is32fl3207_register_led() ->
is32fl3207_register_single()/is32fl3207_register_multicolor() ->
is32fl3207_configure_channel().
With the smallest RISET the binding permits, 2000 ohms,
output_max_microamp is 76500000 / 2000 = 38250, so scaling stays zero for
any led-max-microamp below 149 uA (38250 * 255 / 65536 = 148.8). With the
RISET used in the binding example, 4700 ohms, output_max_microamp is 16276
and the cut-off is 64 uA.
So a device tree that passes dt_binding_check with, say,
led-max-microamp = <50> fails to bind, and the actual lower bound is
board-dependent and not documented anywhere.
Should the binding describe the RISET-dependent lower bound, or should the
driver clamp sub-resolution values to a scaling of 1 instead of failing
probe?
Cross-instance finding from sashiko-gemini
(abacfa25c43c61e8a044d405172e276c795bb984d2cf45e93b49076c16419dfc):
[Severity: Medium]
`is32fl3207_clear_retained_scaling` fails to latch the zeroed scaling
registers, leading to a brief LED flash during hardware initialization.
