Re: [PATCH v2] leds: is31fl32xx: Fix NULL pointer dereference when binding via I2C ID table

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] leds: is31fl32xx: positional initializers used in changed i2c_device_id table
--

commit 7fdebced20c3197505f19872c5b5fdcf8d9db517
Author: Rosen Penev <rosenp@gmail.com>

leds: is31fl32xx: Fix NULL pointer dereference when binding via I2C ID table

This commit populates driver_data in is31fl32xx_id and switches
is31fl32xx_probe() to use i2c_get_match_data() so that devices binding via
the I2C ID table receive a valid chip definition instead of NULL.

> diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
> index 285085d274538..5ef4fd649d608 100644
> --- a/drivers/leds/leds-is31fl32xx.c
> +++ b/drivers/leds/leds-is31fl32xx.c

[ ... ]

> @@ -611,19 +611,15 @@ static void is31fl32xx_remove(struct i2c_client *client)
>  			ERR_PTR(ret));
>  }
>  
> -/*
> - * i2c-core (and modalias) requires that id_table be properly filled,
> - * even though it is not used for DeviceTree based instantiation.
> - */
>  static const struct i2c_device_id is31fl32xx_id[] = {
> -	{ .name = "is31fl3293" },
> -	{ .name = "is31fl3236" },
> -	{ .name = "is31fl3236a" },
> -	{ .name = "is31fl3235" },
> -	{ .name = "is31fl3218" },
> -	{ .name = "sn3218" },
> -	{ .name = "is31fl3216" },
> -	{ .name = "sn3216" },
> +	{ "is31fl3293",  (kernel_ulong_t)&is31fl3293_cdef },
> +	{ "is31fl3236",  (kernel_ulong_t)&is31fl3236_cdef },
> +	{ "is31fl3236a", (kernel_ulong_t)&is31fl3236a_cdef },
> +	{ "is31fl3235",  (kernel_ulong_t)&is31fl3235_cdef },
> +	{ "is31fl3218",  (kernel_ulong_t)&is31fl3218_cdef },
> +	{ "sn3218",      (kernel_ulong_t)&is31fl3218_cdef },
> +	{ "is31fl3216",  (kernel_ulong_t)&is31fl3216_cdef },
> +	{ "sn3216",      (kernel_ulong_t)&is31fl3216_cdef },
>  	{ }
>  };
>  

[Severity: Low]
This isn't a bug, but commit a031b5fce526 ("leds: Use named initializers for
arrays of i2c_device_data") previously converted is31fl32xx_id[] in
drivers/leds/leds-is31fl32xx.c to use named initializers.

Should the updated entries in is31fl32xx_id[] keep using named initializers
for .name and .driver_data instead of switching back to positional
initialization?

For example:

	{ .name = "is31fl3293", .driver_data = (kernel_ulong_t)&is31fl3293_cdef },

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017004351.1249547-1-rosenp@gmail.com?part=1