Re: [PATCH v3 2/4] mfd: axp20x: Add support for AXP318W PMIC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The newly added I2C device ID `axp318w` fails to use named initializers and omits the `.driver_data` field, resulting in it defaulting to `0` (`AXP152_ID`).
- [Medium] The MFD cell and resources for the power key (`axp20x-pek`) are completely omitted from `axp318_cells`, despite the commit message claiming to support it and IRQs being defined.
- [Low] The `axp318_cells` array uses a hard-coded ID of `1` for the regulator cell, which violates MFD subsystem guidelines and risks sysfs naming collisions.
--

--- Patch [2]: [PATCH v3 2/4] mfd: axp20x: Add support for AXP318W PMIC ---
commit c0a61d967fff87b52d26e4cb329e69e10684fb89
Author: Andre Przywara <andre.przywara@arm.com>

mfd: axp20x: Add support for AXP318W PMIC

This patch adds support for the AXP318W PMIC connected via I2C. It sets up
the regmap configurations, volatile ranges, interrupts, and registers the
MFD cells for the regulator devices.

> Describe the regmap and the MFD bits, along with the registers exposed
> via I2C only. This covers the regulator, interrupts and power key
> devices for now.

[Severity: Medium]
Is the power key device missing from the MFD cells array in this patch? The
commit message mentions it is covered, but there are no power key resources
or cells defined for the AXP318W.

> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index 5c93136f977e7..4e4ebfc78525c 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -83,6 +84,7 @@ static const struct i2c_device_id axp20x_i2c_id[] = {
>  	{ "axp221" },
>  	{ "axp223" },
>  	{ "axp313a" },
> +	{ "axp318w" },

[Severity: Medium]
Does this need named initializers to provide the .driver_data payload?

Without setting .driver_data like the of_device_id table does, won't this
default to 0, which is AXP152_ID? If this device is instantiated via legacy
I2C methods, device_get_match_data() might return AXP152_ID instead,
causing the driver to mistakenly configure it as an AXP152 variant.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 679364189ea58..c8aeebd01bbcd 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
[ ... ]
> @@ -1061,6 +1129,10 @@ static 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: Low]
Should this use PLATFORM_DEVID_AUTO instead of a hard-coded ID of 1?

Assigning a hardcoded ID can lead to device naming collisions in sysfs if
multiple instances of the device are present.

> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-axp318-regulator-v3-0-779dc1b97dd4@baylibre.com?part=2