Re: [PATCH v5 2/3] leds: aw2013: Add AW2027 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] leds: aw2013: build error and wrong imax mask in
  aw2013_probe_dt()
--

commit 07caacf25b9b9285cf8d8cd5387ef8435b68fb1e
Author: Paul Sajna <sajattack@postmarketos.org>

leds: aw2013: Add AW2027 support

This commit adds support for the Awinic AW2027 3-channel LED controller
to the aw2013 driver, introducing chip-specific definitions and current
configuration handling.

> diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
> index cf20e94920fd6..d3858b06e594d 100644
> --- a/drivers/leds/leds-aw2013.c
> +++ b/drivers/leds/leds-aw2013.c
> @@ -20,13 +20,15 @@
>  #define AW2013_GCR 0x01
>  #define AW2013_GCR_ENABLE BIT(0)
>  
> +#define AW20XX_GCR2 0x04
> +
>  /* LED channel enable register */
>  #define AW2013_LCTR 0x30
>  #define AW2013_LCTR_LE(x) BIT((x))
>  
>  /* LED channel control registers */
>  #define AW2013_LCFG(x) (0x31 + (x))
> -#define AW2013_LCFG_IMAX_MASK (BIT(0) | BIT(1)) // Should be 0-3
> +#define AW2013_IMAX_MASK (BIT(0) | BIT(1)) // Should be 0-3

[ ... ]

> @@ -293,9 +403,9 @@ static int aw2013_probe_dt(struct aw2013 *chip)
>  		init_data.fwnode = of_fwnode_handle(child);
>  
>  		if (!of_property_read_u32(child, "led-max-microamp", &imax)) {
> -			led->imax = min_t(u32, imax / 5000, 3);
> +			led->imax = aw2013_microamp_to_imax(imax, chip->cdef);
>  		} else {
> -			led->imax = 1; // 5mA
> +			led->imax = chip->cdef->default_imax;
>  			dev_info(&chip->client->dev,
>  				 "DT property led-max-microamp is missing\n");
>  		}

[Severity: High]
Does renaming AW2013_LCFG_IMAX_MASK to AW2013_IMAX_MASK break the build in
aw2013_probe_dt()?

Just below this hunk, aw2013_probe_dt() still references the removed
AW2013_LCFG_IMAX_MASK macro when configuring subsequent LEDs after an
earlier LED enables the chip (for example via a default trigger during
devm_led_classdev_register_ext()):

drivers/leds/leds-aw2013.c:aw2013_probe_dt() {
	...
		if (chip->enabled) {
			ret = regmap_update_bits(chip->regmap,
						 AW2013_LCFG(led->num),
						 AW2013_LCFG_IMAX_MASK,
						 led->imax);
	...
}

In addition to failing to compile, on AW2027 the per-LED current field in
AW2013_LCFG(led->num) uses the 4-bit AW2027_LOCAL_IMAX_MASK rather than the
2-bit AW2013_IMAX_MASK, so using the 2-bit mask here would truncate
led->imax for those LEDs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-aw2013-aw20xx-rename-v5-0-b2c5e323a90a@postmarketos.org?part=2