Re: [PATCH v9 8/9] regulator: Add MediaTek MT6392 regulator

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The driver mutates the statically allocated global array `mt6392_regulators` during probe, causing data races and breaking support for multiple PMIC instances.
--

--- Patch [8]: [PATCH v9 8/9] regulator: Add MediaTek MT6392 regulator ---
commit 942bc64d63d9355df7d6368a340ca5b124e3e738
Author: Fabien Parent <[email protected]>

regulator: Add MediaTek MT6392 regulator

This patch introduces a new regulator driver for the MediaTek MT6392 PMIC.
It provides support for various power rails, including buck and LDO
regulators, controlled via regmap.

> diff --git a/drivers/regulator/mt6392-regulator.c b/drivers/regulator/mt6392-regulator.c
> new file mode 100644
> index 0000000000000..93fe887709c3b
> --- /dev/null
> +++ b/drivers/regulator/mt6392-regulator.c

[ ... ]

> +static int mt6392_set_buck_vsel_reg(struct platform_device *pdev)
> +{
> +	struct mt6397_chip *mt6392 = dev_get_drvdata(pdev->dev.parent);
> +	int i;
> +	u32 regval;
> +
> +	for (i = 0; i < MT6392_MAX_REGULATOR; i++) {
> +		if (mt6392_regulators[i].vselctrl_reg) {
> +			// Read the vselctrl_reg register
> +			if (regmap_read(mt6392->regmap,
> +					mt6392_regulators[i].vselctrl_reg,
> +					&regval) < 0) {
> +				dev_err(&pdev->dev,
> +					"Failed to read buck ctrl\n");
> +				return -EIO;
> +			}
> +
> +			// vselctrl_reg[vselctrl_mask] defines the mode
> +			if (regval & mt6392_regulators[i].vselctrl_mask) {
> +				// Regulator in Normal mode
> +				mt6392_regulators[i].desc.vsel_reg =
> +					mt6392_regulators[i].vsel_normal_mode_reg;
> +				mt6392_regulators[i].desc.vsel_mask =
> +					mt6392_regulators[i].vsel_normal_mode_mask;

[Severity: Medium]
Will this corrupt the configuration for previously registered devices if
multiple MT6392 PMIC instances are present?

This modifies the statically allocated global array mt6392_regulators based on
per-instance hardware state. Because the regulator core keeps a pointer to the
provided desc, mutating it here affects all instances of the PMIC in the
system.

> +			} else {
> +				// Regulator in Register mode
> +				mt6392_regulators[i].desc.vsel_reg =
> +					mt6392_regulators[i].vsel_reg_mode_reg;
> +				mt6392_regulators[i].desc.vsel_mask =
> +					mt6392_regulators[i].vsel_reg_mode_mask;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}

[ ... ]

> +static struct platform_driver mt6392_regulator_driver = {
> +	.driver = {
> +		.name = "mt6392-regulator",
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,

[Severity: Medium]
Does enabling asynchronous probing introduce a data race with the global array
modifications above?

If multiple PMIC instances are probed concurrently, they will race to overwrite
the vsel_reg and vsel_mask fields of the shared global mt6392_regulators array
in mt6392_set_buck_vsel_reg().

> +	},
> +	.probe = mt6392_regulator_probe,
> +	.id_table = mt6392_platform_ids,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8