Re: [PATCH v5 2/2] leds: is31f132xx: Add support for is31fl3206

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] leds: is31fl32xx: mixed declarations and code in
  is31fl32xx_init_regs()
--

commit 17585c728364ffbaee88a6c09e1a1085eccc5391
Author: Flavio Amieiro <flavio@amieiro.net>

leds: is31f132xx: Add support for is31fl3206

This commit adds support for the ISSI IS31FL3206 LED controller by allowing
the chip definition to specify a bit offset for the enable bits in the LED
control register.

> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index 581c05bab9697..1c76b0fb1f159 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c

[ ... ]

> @@ -350,8 +358,10 @@ static int is31fl32xx_init_regs(struct is31fl32xx_priv *priv)
>  	 * We will control state with PWM registers alone.
>  	 */
>  	if (cdef->led_control_register_base != IS31FL32XX_REG_NONE) {
> -		u8 value =
> -		    GENMASK(cdef->enable_bits_per_led_control_register-1, 0);
> +		u8 value = GENMASK(cdef->enable_bits_per_led_control_register-1, 0);
> +
> +		value <<= cdef->enable_bits_offset_in_led_control_reg;
> +
>  		u8 num_regs = cdef->channels /
>  				cdef->enable_bits_per_led_control_register;
>  		int i;

[Severity: Low]
As reported by Lee Jones on v4 [1], placing the shift of value before the
declarations of num_regs and i mixes declarations and statements in
is31fl32xx_init_regs(). Could the shift be moved below the variable
declarations or folded into the initializer?

[1] https://lore.kernel.org/r/179370064610.793788.8239015594326628135@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-add-is31fl3206-support-v5-0-332db80233df@amieiro.net?part=2