Re: [PATCH v2 2/3] mfd: axp20x: Add support for AXP318W PMIC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The Power Key (PEK) MFD cell is incorrectly named `axp318w-pek`, which has no corresponding platform driver, resulting in a dead device.
- [High] The `axp20x-regulator` driver does not handle `AXP318_ID`, causing the regulator probe to unconditionally fail.
- [Low] The newly added MFD cell array `axp318_cells` is not declared as `static const`, violating MFD subsystem guidelines.
- [Low] The `axp20x-regulator` cell uses a hard-coded ID instead of `PLATFORM_DEVID_AUTO`, violating MFD subsystem guidelines.
--

--- Patch [2]: [PATCH v2 2/3] mfd: axp20x: Add support for AXP318W PMIC ---
commit 1a5eb1e56ed118dbcf34674ba079b5c5e2ba9f27
Author: Andre Przywara <andre.przywara@arm.com>

mfd: axp20x: Add support for AXP318W PMIC

The AXP318W is a PMIC chip produced by X-Powers, it can be connected to
an I2C bus. It adds support for regulators, interrupts and power key devices.

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 679364189ea58..956a0370e6a5e 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c

[ ... ]

> @@ -1061,6 +1134,11 @@ static struct mfd_cell axp313a_cells[] = {
>  	MFD_CELL_RES("axp313a-pek", axp313a_pek_resources),
>  };
>  
> +static struct mfd_cell axp318_cells[] = {

[Severity: Low]
Does this array need to be declared as static const?

The MFD subsystem guidelines recommend using static const for mfd_cell
arrays to prevent dynamic amendments and keep the array in immutable memory.

> +	MFD_CELL_BASIC("axp20x-regulator", NULL, NULL, 0, 1),

[Severity: Low]
Should this cell use PLATFORM_DEVID_AUTO instead of a hard-coded ID?

Using a hard-coded ID of 1 instead of PLATFORM_DEVID_AUTO might lead to
device naming collisions in sysfs if multiple PMICs are present.

[Severity: High]
Will the axp20x-regulator driver probe successfully for this device?

It appears the axp20x-regulator driver's axp20x_regulator_probe() in
drivers/regulator/axp20x-regulator.c is missing a switch case for AXP318_ID.
Would this cause the probe to fall through to the default case, log an
unsupported variant error, and return -EINVAL unconditionally?

> +	MFD_CELL_RES("axp318w-pek", axp318_pek_resources),

[Severity: High]
Is there a matching platform driver for axp318w-pek?

Looking at the child driver's ID match table axp_pek_id_match in
drivers/input/misc/axp20x-pek.c, it seems to only contain axp20x-pek and
axp221-pek. Will assigning this specific name cause the power key device
to silently fail to bind to a driver?

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-axp318-regulator-v2-0-ee5f1c56b49f@baylibre.com?part=2