linux-kernel - Re: [PATCH v8 3/6] regulator: pf1550: add support for regulator

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ni3bmj4ye3dp3opolk466r2ayx7iuk6hhyx4pdikydizqykfx7@nc5qdok32hsm>
Date: Thu, 10 Jul 2025 14:49:21 +0000
From: Sean Nyekjaer <sean@...nix.com>
To: Samuel Kayode <samuel.kayode@...oirfairelinux.com>
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Dmitry Torokhov <dmitry.torokhov@...il.com>, Sebastian Reichel <sre@...nel.org>, Frank Li <Frank.li@....com>, imx@...ts.linux.dev, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, linux-input@...r.kernel.org, linux-pm@...r.kernel.org, Abel Vesa <abelvesa@...nel.org>, Abel Vesa <abelvesa@...ux.com>, Robin Gong <b38343@...escale.com>, Robin Gong <yibin.gong@....com>, Enric Balletbo i Serra <eballetbo@...il.com>
Subject: Re: [PATCH v8 3/6] regulator: pf1550: add support for regulator

Hi Samuel,

On Mon, Jul 07, 2025 at 05:37:22PM +0100, Samuel Kayode wrote:
> Add regulator support for the pf1550 PMIC.
> 
> Reviewed-by: Frank Li <Frank.Li@....com>
> Reviewed-by: Mark Brown <broonie@...nel.org>
> Signed-off-by: Samuel Kayode <samuel.kayode@...oirfairelinux.com>

[...]

> diff --git a/drivers/regulator/pf1550-regulator.c b/drivers/regulator/pf1550-regulator.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..fa8c4b17270e750e64fef91074727951511d14f3
> --- /dev/null
> +++ b/drivers/regulator/pf1550-regulator.c

[...]

> +
> +#define PF_SW1(_chip, match, _name, mask, voltages)	{	\
> +	.desc = {	\
> +		.name = #_name,	\
> +		.of_match = of_match_ptr(match),	\
> +		.regulators_node = of_match_ptr("regulators"),	\
> +		.n_voltages = ARRAY_SIZE(voltages),	\
> +		.ops = &pf1550_sw1_ops,	\
> +		.type = REGULATOR_VOLTAGE,	\
> +		.id = _chip ## _ ## _name,	\
> +		.owner = THIS_MODULE,	\
> +		.volt_table = voltages,	\
> +		.vsel_reg = _chip ## _PMIC_REG_ ## _name ## _VOLT, \
> +		.vsel_mask = (mask),	\
> +	},	\
> +	.stby_reg = _chip ## _PMIC_REG_ ## _name ## _STBY_VOLT,	\
> +	.stby_mask = (mask),	\
> +}

This is unused.

> +
> +#define PF_SW3(_chip, match, _name, min, max, mask, step)	{	\

[...]

> +
> +static struct pf1550_desc pf1550_regulators[] = {
> +	PF_SW3(PF1550, "sw1", SW1, 600000, 1387500, 0x3f, 12500),
> +	PF_SW3(PF1550, "sw2", SW2, 600000, 1387500, 0x3f, 12500),
> +	PF_SW3(PF1550, "sw3", SW3, 1800000, 3300000, 0xf, 100000),

Seems weird they all use the PF_SW3 macro.

> +	PF_VREF(PF1550, "vrefddr", VREFDDR, 1200000),
> +	PF_LDO1(PF1550, "ldo1", LDO1, 0x1f, pf1550_ldo13_volts),
> +	PF_LDO2(PF1550, "ldo2", LDO2, 0xf, 1800000, 3300000, 100000),
> +	PF_LDO1(PF1550, "ldo3", LDO3, 0x1f, pf1550_ldo13_volts),
> +};
> +

[...]

> +
> +static int pf1550_regulator_probe(struct platform_device *pdev)
> +{
> +	const struct pf1550_ddata *pf1550 = dev_get_drvdata(pdev->dev.parent);
> +	struct regulator_config config = { };
> +	struct pf1550_regulator_info *info;
> +	int i, irq = -1, ret = 0;
> +
> +	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	config.regmap = dev_get_regmap(pf1550->dev, NULL);
> +	if (!config.regmap)
> +		return dev_err_probe(&pdev->dev, -ENODEV,
> +				     "failed to get parent regmap\n");
> +
> +	config.dev = pf1550->dev;
> +	config.regmap = pf1550->regmap;
> +	info->dev = &pdev->dev;
> +	info->pf1550 = pf1550;
> +
> +	memcpy(info->regulator_descs, pf1550_regulators,
> +	       sizeof(info->regulator_descs));
> +
> +	for (i = 0; i < ARRAY_SIZE(pf1550_regulators); i++) {
> +		struct regulator_desc *desc;
> +
> +		desc = &info->regulator_descs[i].desc;
> +
> +		if (desc->id == PF1550_SW2 && pf1550->dvs_enb) {

We should enter here if dvs_enb == false.
My A6 variant reported 0.625V instead of the correct 1.35V

> +			/* OTP_SW2_DVS_ENB == 1? */
> +			desc->volt_table = pf1550_sw12_volts;
> +			desc->n_voltages = ARRAY_SIZE(pf1550_sw12_volts);
> +			desc->ops = &pf1550_sw1_ops;
> +		}
>

/Sean