| 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
| ||
|
Message-ID: <aYYilSSZKcx0LauR@lizhi-Precision-Tower-5810>
Date: Fri, 6 Feb 2026 12:19:17 -0500
From: Frank Li <Frank.li@....com>
To: Aman Kumar Pandey <aman.kumarpandey@....com>
Cc: linux-kernel@...r.kernel.org, linux-i3c@...ts.infradead.org,
alexandre.belloni@...tlin.com, krzk+dt@...nel.org, robh@...nel.org,
conor+dt@...nel.org, devicetree@...r.kernel.org, broonie@...nel.org,
lee@...nel.org, lgirdwood@...il.com, vikash.bansal@....com,
priyanka.jain@....com, shashank.rebbapragada@....com
Subject: Re: [PATCH v5 4/5] regulator: p3h2x4x: Add driver for on-die
regulators in NXP P3H2x4x i3c hub
On Fri, Feb 06, 2026 at 02:01:20PM +0200, Aman Kumar Pandey wrote:
> The NXP P3H2x4x family integrates on-die regulators alongside I3C hub
> functionality. This driver registers the regulators using the MFD
Register the regulaltor ...
> framework and exposes them via the regulator subsystem.
>
> Signed-off-by: Aman Kumar Pandey <aman.kumarpandey@....com>
> Signed-off-by: Vikash Bansal <vikash.bansal@....com>
>
> ---
> Changes in v5:
> - Updated dev_err_probe() for regmap_init failure.
> - Updated module description
>
> Changes in v4:
> - Split the driver into three separate patches (mfd, regulator and I3C hub)
> - Introduced driver for on-die regulators in NXP P3H2x4x I3C hub
> ---
> ---
> MAINTAINERS | 1 +
> drivers/regulator/Kconfig | 10 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/p3h2840_i3c_hub_regulator.c | 247 ++++++++++++++++++
> 4 files changed, 259 insertions(+)
> create mode 100644 drivers/regulator/p3h2840_i3c_hub_regulator.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6c247e4a0c12..6601830b6e86 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18914,6 +18914,7 @@ L: linux-i3c-owner@...ts.infradead.org
> S: Maintained
> F: Documentation/devicetree/bindings/i3c/nxp,p3h2840.yaml
> F: drivers/mfd/p3h2840.c
> +F: drivers/regulator/p3h2840_i3c_hub_regulator.c
> F: include/linux/mfd/p3h2840.h
>
> NXP PF5300/PF5301/PF5302 PMIC REGULATOR DEVICE DRIVER
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index d2335276cce5..84684701dad7 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -1010,6 +1010,16 @@ config REGULATOR_MTK_DVFSRC
> of Mediatek. It allows for voting on regulator state
> between multiple users.
>
> +config REGULATOR_P3H2X4X
> + tristate "P3H2X4X regulator support"
> + depends on MFD_P3H2X4X
> + help
> + This driver provides support for the voltage regulators of the
> + P3H244x/P3H284x multi-function I3C Hub device.
> +
> + Say M here if you want to include support for this regulator as
> + a module. The module will be named "p3h2840_i3c_hub_regulator".
> +
> config REGULATOR_PALMAS
> tristate "TI Palmas PMIC Regulators"
> depends on MFD_PALMAS
> diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
> index 1beba1493241..145138f437d8 100644
> --- a/drivers/regulator/Makefile
> +++ b/drivers/regulator/Makefile
> @@ -127,6 +127,7 @@ obj-$(CONFIG_REGULATOR_QCOM_RPMH) += qcom-rpmh-regulator.o
> obj-$(CONFIG_REGULATOR_QCOM_SMD_RPM) += qcom_smd-regulator.o
> obj-$(CONFIG_REGULATOR_QCOM_SPMI) += qcom_spmi-regulator.o
> obj-$(CONFIG_REGULATOR_QCOM_USB_VBUS) += qcom_usb_vbus-regulator.o
> +obj-$(CONFIG_REGULATOR_P3H2X4X) += p3h2840_i3c_hub_regulator.o
> obj-$(CONFIG_REGULATOR_PALMAS) += palmas-regulator.o
> obj-$(CONFIG_REGULATOR_PCA9450) += pca9450-regulator.o
> obj-$(CONFIG_REGULATOR_PF0900) += pf0900-regulator.o
> diff --git a/drivers/regulator/p3h2840_i3c_hub_regulator.c b/drivers/regulator/p3h2840_i3c_hub_regulator.c
> new file mode 100644
> index 000000000000..1e9abc398f5a
> --- /dev/null
> +++ b/drivers/regulator/p3h2840_i3c_hub_regulator.c
> @@ -0,0 +1,247 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2025 NXP
> + * This P3H2x4x driver file contain functions for enable/disable regulator and voltage set/get.
> + */
> +#include <linux/bitfield.h>
> +#include <linux/mfd/p3h2840.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +
> +#define P3H2x4x_LDO_AND_PULLUP_CONF 0x19
> +#define P3H2x4x_LDO_ENABLE_DISABLE_MASK GENMASK(3, 0)
> +#define P3H2x4x_CP0_EN_LDO BIT(0)
> +#define P3H2x4x_CP1_EN_LDO BIT(1)
> +#define P3H2x4x_TP0145_EN_LDO BIT(2)
> +#define P3H2x4x_TP2367_EN_LDO BIT(3)
> +
> +#define P3H2x4x_NET_OPER_MODE_CONF 0x15
> +#define P3H2x4x_VCCIO_LDO_CONF 0x16
> +#define P3H2x4x_CP0_VCCIO_LDO_VOLTAGE_MASK GENMASK(1, 0)
> +#define P3H2x4x_CP0_VCCIO_LDO_VOLTAGE(x) \
> + FIELD_PREP(P3H2x4x_CP0_VCCIO_LDO_VOLTAGE_MASK, x)
> +#define P3H2x4x_CP1_VCCIO_LDO_VOLTAGE_MASK GENMASK(3, 2)
> +#define P3H2x4x_CP1_VCCIO_LDO_VOLTAGE(x) \
> + FIELD_PREP(P3H2x4x_CP1_VCCIO_LDO_VOLTAGE_MASK, x)
> +#define P3H2x4x_TP0145_VCCIO_LDO_VOLTAGE_MASK GENMASK(5, 4)
> +#define P3H2x4x_TP0145_VCCIO_LDO_VOLTAGE(x) \
> + FIELD_PREP(P3H2x4x_TP0145_VCCIO_LDO_VOLTAGE_MASK, x)
> +#define P3H2x4x_TP2367_VCCIO_LDO_VOLTAGE_MASK GENMASK(7, 6)
> +#define P3H2x4x_TP2367_VCCIO_LDO_VOLTAGE(x) \
> + FIELD_PREP(P3H2x4x_TP2367_VCCIO_LDO_VOLTAGE_MASK, x)
> +#define P3H2x4x_LDO_COUNT 4
> +
> +struct p3h2x4x_regulator_dev {
> + struct regulator_dev *rp3h2x4x_dev[P3H2x4x_LDO_COUNT];
> + struct regmap *regmap;
> +};
> +
> +static int p3h2x4x_regulator_enable(struct regulator_dev *rdev)
> +{
> + int ret;
> + unsigned int orig;
> +
> + ret = regmap_read(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE, &orig);
> + if (ret)
> + return ret;
> +
> + if (orig != P3H2x4x_REGISTERS_UNLOCK_CODE) {
> + ret = regmap_write(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE,
> + P3H2x4x_REGISTERS_UNLOCK_CODE);
> + if (ret)
> + return ret;
> + }
> +
> + ret = regulator_enable_regmap(rdev);
> + if (ret)
> + return ret;
AI:
- If enable fails, protection register is left unlocked
- Should restore original state in error path
> +
> + if (orig != P3H2x4x_REGISTERS_UNLOCK_CODE) {
> + ret = regmap_write(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE, orig);
> + if (ret)
> + return ret;
> + }
AI:
- `p3h2x4x_regulator_enable()`, `_disable()`, and `_set_voltage_sel()` have identical unlock/lock logic
- Should extract to helper function
you can use cleanup to define macro, like mutex, use guard(...) to simple
code.
Frank
> + return 0;
> +}
> +
> +static int p3h2x4x_regulator_disable(struct regulator_dev *rdev)
> +{
> + int ret;
> + unsigned int orig;
> +
> + ret = regmap_read(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE, &orig);
> + if (ret)
> + return ret;
> +
> + if (orig != P3H2x4x_REGISTERS_UNLOCK_CODE) {
> + ret = regmap_write(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE,
> + P3H2x4x_REGISTERS_UNLOCK_CODE);
> + if (ret)
> + return ret;
> + }
> +
> + ret = regulator_disable_regmap(rdev);
> + if (ret)
> + return ret;
> +
> + if (orig != P3H2x4x_REGISTERS_UNLOCK_CODE) {
> + ret = regmap_write(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE, orig);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +
> +static int p3h2x4x_regulator_set_voltage_sel(struct regulator_dev *rdev, unsigned int sel)
> +{
> + int ret;
> + unsigned int orig;
> +
> + ret = regmap_read(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE, &orig);
> + if (ret)
> + return ret;
> +
> + if (orig != P3H2x4x_REGISTERS_UNLOCK_CODE) {
> + ret = regmap_write(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE,
> + P3H2x4x_REGISTERS_UNLOCK_CODE);
> + if (ret)
> + return ret;
> + }
> +
> + ret = regulator_set_voltage_sel_regmap(rdev, sel);
> + if (ret)
> + return ret;
> +
> + if (orig != P3H2x4x_REGISTERS_UNLOCK_CODE) {
> + ret = regmap_write(rdev->regmap, P3H2x4x_DEV_REG_PROTECTION_CODE, orig);
> + if (ret)
> + return ret;
> + }
> + return 0;
> +}
> +
> +static const struct regulator_ops p3h2x4x_ldo_ops = {
> + .list_voltage = regulator_list_voltage_table,
> + .map_voltage = regulator_map_voltage_iterate,
> + .set_voltage_sel = p3h2x4x_regulator_set_voltage_sel,
> + .get_voltage_sel = regulator_get_voltage_sel_regmap,
> + .enable = p3h2x4x_regulator_enable,
> + .disable = p3h2x4x_regulator_disable,
> + .is_enabled = regulator_is_enabled_regmap,
> +};
> +
> +static const unsigned int p3h2x4x_voltage_table[] = {
> + 1000000,
> + 1100000,
> + 1200000,
> + 1800000,
> +};
> +
> +static struct regulator_desc p3h2x4x_regulators[] = {
> + {
> + .name = "LDO_CP0",
> + .of_match = of_match_ptr("LDO_CP0"),
> + .regulators_node = of_match_ptr("regulators"),
> + .volt_table = p3h2x4x_voltage_table,
> + .n_voltages = ARRAY_SIZE(p3h2x4x_voltage_table),
> + .ops = &p3h2x4x_ldo_ops,
> + .type = REGULATOR_VOLTAGE,
> + .owner = THIS_MODULE,
> + .enable_reg = P3H2x4x_LDO_AND_PULLUP_CONF,
> + .enable_mask = P3H2x4x_CP0_EN_LDO,
> + .vsel_reg = P3H2x4x_VCCIO_LDO_CONF,
> + .vsel_mask = P3H2x4x_CP0_VCCIO_LDO_VOLTAGE_MASK,
> + },
> + {
> + .name = "LDO_CP1",
> + .of_match = of_match_ptr("LDO_CP1"),
> + .regulators_node = of_match_ptr("regulators"),
> + .volt_table = p3h2x4x_voltage_table,
> + .n_voltages = ARRAY_SIZE(p3h2x4x_voltage_table),
> + .ops = &p3h2x4x_ldo_ops,
> + .type = REGULATOR_VOLTAGE,
> + .owner = THIS_MODULE,
> + .enable_reg = P3H2x4x_LDO_AND_PULLUP_CONF,
> + .enable_mask = P3H2x4x_CP1_EN_LDO,
> + .vsel_reg = P3H2x4x_VCCIO_LDO_CONF,
> + .vsel_mask = P3H2x4x_CP1_VCCIO_LDO_VOLTAGE_MASK,
> + },
> + {
> + .name = "LDO_TPG0",
> + .of_match = of_match_ptr("LDO_TPG0"),
> + .regulators_node = of_match_ptr("regulators"),
> + .volt_table = p3h2x4x_voltage_table,
> + .n_voltages = ARRAY_SIZE(p3h2x4x_voltage_table),
> + .ops = &p3h2x4x_ldo_ops,
> + .type = REGULATOR_VOLTAGE,
> + .owner = THIS_MODULE,
> + .enable_reg = P3H2x4x_LDO_AND_PULLUP_CONF,
> + .enable_mask = P3H2x4x_TP0145_EN_LDO,
> + .vsel_reg = P3H2x4x_VCCIO_LDO_CONF,
> + .vsel_mask = P3H2x4x_TP0145_VCCIO_LDO_VOLTAGE_MASK,
> + },
> + {
> + .name = "LDO_TPG1",
> + .of_match = of_match_ptr("LDO_TPG1"),
> + .regulators_node = of_match_ptr("regulators"),
> + .volt_table = p3h2x4x_voltage_table,
> + .n_voltages = ARRAY_SIZE(p3h2x4x_voltage_table),
> + .ops = &p3h2x4x_ldo_ops,
> + .type = REGULATOR_VOLTAGE,
> + .owner = THIS_MODULE,
> + .enable_reg = P3H2x4x_LDO_AND_PULLUP_CONF,
> + .enable_mask = P3H2x4x_TP2367_EN_LDO,
> + .vsel_reg = P3H2x4x_VCCIO_LDO_CONF,
> + .vsel_mask = P3H2x4x_TP2367_VCCIO_LDO_VOLTAGE_MASK,
> + },
> +};
> +
> +static int p3h2x4x_regulator_probe(struct platform_device *pdev)
> +{
> + struct p3h2x4x_dev *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
> + struct p3h2x4x_regulator_dev *p3h2x4x_regulator;
> + struct regulator_config rcfg = { };
> + struct device *dev = &pdev->dev;
> + struct regulator_dev *rdev;
> + int ret, i;
> +
> + p3h2x4x_regulator = devm_kzalloc(dev, sizeof(*p3h2x4x_regulator), GFP_KERNEL);
> + if (!p3h2x4x_regulator)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, p3h2x4x_regulator);
> +
> + p3h2x4x_regulator->regmap = p3h2x4x->regmap;
> + device_set_of_node_from_dev(dev, dev->parent);
> +
> + rcfg.dev = dev;
> + rcfg.dev->of_node = dev->of_node;
> + rcfg.regmap = p3h2x4x_regulator->regmap;
> + rcfg.driver_data = p3h2x4x_regulator;
> +
> + for (i = 0; i < ARRAY_SIZE(p3h2x4x_regulators); i++) {
> + rdev = devm_regulator_register(&pdev->dev, &p3h2x4x_regulators[i], &rcfg);
> + if (IS_ERR(rdev)) {
> + return dev_err_probe(dev, PTR_ERR(rdev), "Failed to register %s\n",
> + p3h2x4x_regulators[i].name);
> + }
> + p3h2x4x_regulator->rp3h2x4x_dev[i] = rdev;
> + }
> + return 0;
> +}
> +
> +static struct platform_driver p3h2x4x_regulator_driver = {
> + .driver = {
> + .name = "p3h2x4x-regulator",
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> + .probe = p3h2x4x_regulator_probe,
> +};
> +module_platform_driver(p3h2x4x_regulator_driver);
> +
> +MODULE_AUTHOR("Aman Kumar Pandey <aman.kumarpandey@....com>");
> +MODULE_AUTHOR("vikash Bansal <vikash.bansal@....com>");
> +MODULE_DESCRIPTION("P3H2x4x I3C HUB Regulator driver");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>
Powered by blists - more mailing lists