Re: [PATCH v4 4/5] mfd: axp20x-i2c: Fix i2c instantiation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing Error Handling / API Ambiguity causing unmatched devices to probe as AXP152.
- [High] MFD subsystem guideline violation: Hard-coded IDs used in mfd_cell array instead of PLATFORM_DEVID_AUTO.
--

--- Patch [4]: [PATCH v4 4/5] mfd: axp20x-i2c: Fix i2c instantiation ---
commit df6eff33eb956b610bd8100dff8a5beb1887390f
Author: Jerome Brunet <jbrunet@baylibre.com>

mfd: axp20x-i2c: Fix i2c instantiation

This patch fixes I2C instantiation for the AXP20X driver by moving the
variant matching directly into the bus-specific probes. It also adds support
for the AXP323 device.

Link: https://lore.kernel.org/r/20260717090209.2A09B1F000E9@smtp.kernel.org

> diff --git a/drivers/mfd/axp20x-i2c.c b/drivers/mfd/axp20x-i2c.c
> index 4e4ebfc78525c..64c212b76e5e6 100644
> --- a/drivers/mfd/axp20x-i2c.c
> +++ b/drivers/mfd/axp20x-i2c.c
> @@ -35,6 +35,7 @@ static int axp20x_i2c_probe(struct i2c_client *i2c)
>  	axp20x->irq = i2c->irq;
>  	dev_set_drvdata(axp20x->dev, axp20x);
>  
> +	axp20x->variant = (kernel_ulong_t)i2c_get_match_data(i2c);

[Severity: High]
This isn't a bug introduced by this patch, but because AXP152_ID is defined
as 0, a match failure returning NULL is indistinguishable from a successful
AXP152 match.

Since the driver blindly defaults to AXP152_ID on NULL, it could configure
an unrelated device as an AXP152 PMIC if forced via a sysfs bind. Writing
PMIC configurations to an unrelated I2C device can result in hardware
damage or system crashes.

Should the variant IDs be updated to start from 1 to allow unambiguous
error checking?

>  	ret = axp20x_match_device(axp20x);
>  	if (ret)
>  		return ret;

[ ... ]

> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 2bbe73afff435..25509e47bedc8 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -1339,7 +1339,6 @@ int axp20x_match_device(struct axp20x_dev *axp20x)
>  	const struct mfd_cell *cells_no_irq = NULL;
>  	int nr_cells_no_irq = 0;
>  
> -	axp20x->variant = (long)device_get_match_data(dev);
>  	switch (axp20x->variant) {
>  	case AXP152_ID:
>  		axp20x->nr_cells = ARRAY_SIZE(axp152_cells);

[Severity: High]
This is a pre-existing issue, but there are hard-coded IDs used in the
mfd_cell arrays defined in this file:

drivers/mfd/axp20x.c:axp806_cells[] = {
	{
		.id		= 2,
		.name		= "axp20x-regulator",
	},
};

Using hard-coded IDs in the mfd_cell array bypasses standard subsystem
indexing. This can lead to device naming collisions in sysfs and driver
load failures. A regulator driver load failure here could cause critical
power domains to remain uninitialized, leading to an unusable system.

Would it be possible to use PLATFORM_DEVID_AUTO in mfd_add_devices and
avoid hard-coded id fields in the mfd_cell arrays?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-axp318-regulator-v4-0-50aff97084dd@baylibre.com?part=4