Re: [PATCH v6 4/5] mfd: axp20x: Add support for AXP318W PMIC

Jerome Brunet <[email protected]> Fri, 31 Jul 2026 17:45:02 +0200
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
On ven. 31 juil. 2026 at 14:53, [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The `axp318_cells` array defines the `axp20x-regulator` cell
> with a hard-coded ID of 1 instead of using `PLATFORM_DEVID_AUTO`, violating
> MFD subsystem guidelines.
> --
>
> --- Patch [4]: [PATCH v6 4/5] mfd: axp20x: Add support for AXP318W PMIC ---
> commit a5dbdb5469c59acc76c36e5d6f37cd6f060e0f47
> Author: Andre Przywara <[email protected]>
>
> mfd: axp20x: Add support for AXP318W PMIC
>
> This commit adds support for the AXP318W PMIC to the axp20x MFD core and
> I2C driver. It exposes the device's regulators and interrupts.
>
>> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
>> --- a/drivers/mfd/axp20x.c
>> +++ b/drivers/mfd/axp20x.c
> [ ... ]
>> @@ -1061,6 +1129,10 @@ static const struct mfd_cell axp313a_cells[] = {
>>  	MFD_CELL_RES("axp313a-pek", axp313a_pek_resources),
>>  };
>>  
>> +static const struct mfd_cell axp318_cells[] = {
>> +	MFD_CELL_BASIC("axp20x-regulator", NULL, NULL, 0, 1),
>
> [Severity: Medium]
> Does this hard-coded ID of 1 risk sysfs naming collisions?
>
> The MFD subsystem guidelines recommend using PLATFORM_DEVID_AUTO for
> automatic cell indexing. If multiple devices share this explicit ID, it could
> lead to duplicate device names in sysfs (e.g., axp20x-regulator.1) and
> subsequent driver load failures.

Commented multiple time on this. This is not where PLATFORM_DEVID_AUTO
would go. The driver uses PLATFORM_DEVID_NONE for now (well before this
change). Changing this is out of the scope of this patchset.

>
>> +};
>> +
>>  static const struct mfd_cell axp717_cells[] = {
>>  	MFD_CELL_NAME("axp20x-regulator"),
>>  	MFD_CELL_RES("axp20x-pek", axp717_pek_resources),

-- 
Jerome