Re: [PATCH v5 1/3] hwmon: (pmbus/tps25990): Rework driver for multi-device support

On 7/27/26 19:07, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] The static array `tps25990_base_info` is missing the `const` qualifier, placing it in mutable `.data` memory instead of read-only `.rodata`.
> - [Low] The `tps25990_i2c_id` array uses positional initializers, which violates the I2C subsystem's coding guidelines.

Both reported issues are valid. Please fix.

Thasnks,
Guenter

> --
> 
> --- Patch [1]: [PATCH v5 1/3] hwmon: (pmbus/tps25990): Rework driver for multi-device support ---
> commit 7c6d03feb4ad36395a77df5021ef6a8c6f2df2a8
> Author: Stoyan Bogdanov <[email protected]>
> 
> hwmon: (pmbus/tps25990): Rework driver for multi-device support
> 
> This commit reworks the tps25990 driver to allow adding support for new
> devices. It introduces a chip enumeration and a data structure to hold
> per-device measurement coefficients.
> 
>> diff --git a/drivers/hwmon/pmbus/tps25990.c b/drivers/hwmon/pmbus/tps25990.c
>> index 9d318e6509abf..490dcd2a5fa51 100644
>> --- a/drivers/hwmon/pmbus/tps25990.c
>> +++ b/drivers/hwmon/pmbus/tps25990.c
> [ ... ]
>> -static const struct pmbus_driver_info tps25990_base_info = {
>> -	.pages = 1,
> [ ... ]
>> -#if IS_ENABLED(CONFIG_SENSORS_TPS25990_REGULATOR)
>> -	.reg_desc = tps25990_reg_desc,
>> -	.num_regulators = ARRAY_SIZE(tps25990_reg_desc),
>> -#endif
>> -};
>> -
>> +static struct pmbus_driver_info tps25990_base_info[] = {
>> +	[tps25990] = {
>> +		.pages = 1,
> 
> [Severity: Medium]
> Should this array be declared const?
> 
> The previous single structure was const, but the new array drops the const
> qualifier. Leaving tps25990_base_info mutable places the struct's function
> pointers in mutable .data memory instead of read-only .rodata.
> 
> [ ... ]
>>   static const struct i2c_device_id tps25990_i2c_id[] = {
>> -	{ .name = "tps25990" },
>> -	{ }
>> +	{ "tps25990", tps25990 },
>> +	{}
>>   };
>>   MODULE_DEVICE_TABLE(i2c, tps25990_i2c_id);
> 
> [Severity: Low]
> Does this code run counter to the I2C subsystem coding guidelines?
> 
> The guidelines specify that initialized arrays of type struct i2c_device_id
> must use named initializers, but this patch changes the initialization to
> use positional initializers instead.
>