[PATCH v2 2/2] pmic: pca9450: Add regulator driver
Stefano Babic
sbabic at denx.de
Fri May 20 09:00:39 CEST 2022
Hi Tim, Marek,
On 20.05.22 00:24, Tim Harvey wrote:
> On Sun, Apr 24, 2022 at 2:41 PM Marek Vasut <marex at denx.de> wrote:
>>
>> Add PCA9450 regulator driver. This is complementary driver for the BUCKn
>> and LDOn regulators provided by the PCA9450 PMIC driver. Currently the
>> driver permits reading the settngs and configuring the BUCKn and LDOn
>> regulators.
>>
>> Reviewed-by: Fabio Estevam <festevam at denx.de>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> Cc: Fabio Estevam <festevam at denx.de>
>> Cc: Peng Fan <peng.fan at nxp.com>
>> Cc: Stefano Babic <sbabic at denx.de>
>> ---
>> V2: Add RB by Fabio
>> ---
>> drivers/power/pmic/pca9450.c | 6 +-
>> drivers/power/regulator/Kconfig | 15 ++
>> drivers/power/regulator/Makefile | 1 +
>> drivers/power/regulator/pca9450.c | 333 ++++++++++++++++++++++++++++++
>> include/power/pca9450.h | 11 +
>> 5 files changed, 363 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/power/regulator/pca9450.c
>>
>> diff --git a/drivers/power/pmic/pca9450.c b/drivers/power/pmic/pca9450.c
>> index 26c876c9c45..116ac49a8db 100644
>> --- a/drivers/power/pmic/pca9450.c
>> +++ b/drivers/power/pmic/pca9450.c
>> @@ -83,9 +83,9 @@ static struct dm_pmic_ops pca9450_ops = {
>> };
>>
>> static const struct udevice_id pca9450_ids[] = {
>> - { .compatible = "nxp,pca9450a", .data = 0x25, },
>> - { .compatible = "nxp,pca9450b", .data = 0x25, },
>> - { .compatible = "nxp,pca9450c", .data = 0x25, },
>> + { .compatible = "nxp,pca9450a", .data = NXP_CHIP_TYPE_PCA9450A, },
>> + { .compatible = "nxp,pca9450b", .data = NXP_CHIP_TYPE_PCA9450BC, },
>> + { .compatible = "nxp,pca9450c", .data = NXP_CHIP_TYPE_PCA9450BC, },
>> { }
>> };
>>
>> diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig
>> index cd253b95f2f..d486bad6bdc 100644
>> --- a/drivers/power/regulator/Kconfig
>> +++ b/drivers/power/regulator/Kconfig
>> @@ -60,6 +60,21 @@ config SPL_DM_REGULATOR_BD71837
>> This config enables implementation of driver-model regulator uclass
>> features for regulators on ROHM BD71837 and BD71847 in SPL.
>>
>> +config DM_REGULATOR_PCA9450
>> + bool "Enable Driver Model for NXP PCA9450 regulators"
>> + depends on DM_REGULATOR && DM_PMIC_PCA9450
>> + help
>> + This config enables implementation of driver-model regulator uclass
>> + features for regulators on NXP PCA9450 PMICs. PCA9450 contains 6 bucks
>> + and 5 LDOS. The driver implements get/set api for value and enable.
>> +
>> +config SPL_DM_REGULATOR_PCA9450
>> + bool "Enable Driver Model for NXP PCA9450 regulators in SPL"
>> + depends on DM_REGULATOR_PCA9450
>> + help
>> + This config enables implementation of driver-model regulator uclass
>> + features for regulators on ROHM PCA9450 in SPL.
>> +
>> config DM_REGULATOR_DA9063
>> bool "Enable Driver Model for REGULATOR DA9063"
>> depends on DM_REGULATOR && DM_PMIC_DA9063
>> diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile
>> index 4efb32a3228..d2d17f7aed0 100644
>> --- a/drivers/power/regulator/Makefile
>> +++ b/drivers/power/regulator/Makefile
>> @@ -11,6 +11,7 @@ obj-$(CONFIG_$(SPL_)DM_REGULATOR_DA9063) += da9063.o
>> obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
>> obj-$(CONFIG_$(SPL_)DM_PMIC_PFUZE100) += pfuze100.o
>> obj-$(CONFIG_$(SPL_)DM_REGULATOR_BD71837) += bd71837.o
>> +obj-$(CONFIG_$(SPL_)DM_REGULATOR_PCA9450) += pca9450.o
>> obj-$(CONFIG_$(SPL_)REGULATOR_PWM) += pwm_regulator.o
>> obj-$(CONFIG_$(SPL_)DM_REGULATOR_FAN53555) += fan53555.o
>> obj-$(CONFIG_$(SPL_)DM_REGULATOR_COMMON) += regulator_common.o
>> diff --git a/drivers/power/regulator/pca9450.c b/drivers/power/regulator/pca9450.c
>> new file mode 100644
>> index 00000000000..4847c9f90f0
>> --- /dev/null
>> +++ b/drivers/power/regulator/pca9450.c
>> @@ -0,0 +1,333 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * NXP PCA9450 regulator driver
>> + * Copyright (C) 2022 Marek Vasut <marex at denx.de>
>> + *
>> + * Largely based on:
>> + * ROHM BD71837 regulator driver
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <log.h>
>> +#include <linux/bitops.h>
>> +#include <power/pca9450.h>
>> +#include <power/pmic.h>
>> +#include <power/regulator.h>
>> +
>> +#define HW_STATE_CONTROL 0
>> +#define DEBUG
>> +
>> +/**
>> + * struct pca9450_vrange - describe linear range of voltages
>> + *
>> + * @min_volt: smallest voltage in range
>> + * @step: how much voltage changes at each selector step
>> + * @min_sel: smallest selector in the range
>> + * @max_sel: maximum selector in the range
>> + */
>> +struct pca9450_vrange {
>> + unsigned int min_volt;
>> + unsigned int step;
>> + u8 min_sel;
>> + u8 max_sel;
>> +};
>> +
>> +/**
>> + * struct pca9450_plat - describe regulator control registers
>> + *
>> + * @name: name of the regulator. Used for matching the dt-entry
>> + * @enable_reg: register address used to enable/disable regulator
>> + * @enablemask: register mask used to enable/disable regulator
>> + * @volt_reg: register address used to configure regulator voltage
>> + * @volt_mask: register mask used to configure regulator voltage
>> + * @ranges: pointer to ranges of regulator voltages and matching register
>> + * values
>> + * @numranges: number of voltage ranges pointed by ranges
>> + * @dvs: whether the voltage can be changed when regulator is enabled
>> + */
>> +struct pca9450_plat {
>> + const char *name;
>> + u8 enable_reg;
>> + u8 enablemask;
>> + u8 volt_reg;
>> + u8 volt_mask;
>> + struct pca9450_vrange *ranges;
>> + unsigned int numranges;
>> + bool dvs;
>> +};
>> +
>> +#define PCA_RANGE(_min, _vstep, _sel_low, _sel_hi) \
>> +{ \
>> + .min_volt = (_min), .step = (_vstep), \
>> + .min_sel = (_sel_low), .max_sel = (_sel_hi), \
>> +}
>> +
>> +#define PCA_DATA(_name, enreg, enmask, vreg, vmask, _range, _dvs) \
>> +{ \
>> + .name = (_name), .enable_reg = (enreg), .enablemask = (enmask), \
>> + .volt_reg = (vreg), .volt_mask = (vmask), .ranges = (_range), \
>> + .numranges = ARRAY_SIZE(_range), .dvs = (_dvs), \
>> +}
>> +
>> +static struct pca9450_vrange pca9450_buck123_vranges[] = {
>> + PCA_RANGE(600000, 12500, 0, 0x7f),
>> +};
>> +
>> +static struct pca9450_vrange pca9450_buck456_vranges[] = {
>> + PCA_RANGE(600000, 25000, 0, 0x70),
>> + PCA_RANGE(3400000, 0, 0x71, 0x7f),
>> +};
>> +
>> +static struct pca9450_vrange pca9450_ldo1_vranges[] = {
>> + PCA_RANGE(1600000, 100000, 0x0, 0x3),
>> + PCA_RANGE(3000000, 100000, 0x4, 0x7),
>> +};
>> +
>> +static struct pca9450_vrange pca9450_ldo2_vranges[] = {
>> + PCA_RANGE(800000, 50000, 0x0, 0x7),
>> +};
>> +
>> +static struct pca9450_vrange pca9450_ldo34_vranges[] = {
>> + PCA_RANGE(800000, 100000, 0x0, 0x19),
>> + PCA_RANGE(3300000, 0, 0x1a, 0x1f),
>> +};
>> +
>> +static struct pca9450_vrange pca9450_ldo5_vranges[] = {
>> + PCA_RANGE(1800000, 100000, 0x0, 0xf),
>> +};
>> +
>> +/*
>> + * We use enable mask 'HW_STATE_CONTROL' to indicate that this regulator
>> + * must not be enabled or disabled by SW. The typical use-case for PCA9450
>> + * is powering NXP i.MX8. In this use-case we (for now) only allow control
>> + * for BUCK4, BUCK5, BUCK6 which are not boot critical.
>> + */
>> +static struct pca9450_plat pca9450_reg_data[] = {
>> + /* Bucks 1-3 which support dynamic voltage scaling */
>> + PCA_DATA("BUCK1", PCA9450_BUCK1CTRL, HW_STATE_CONTROL,
>> + PCA9450_BUCK1OUT_DVS0, DVS_BUCK_RUN_MASK,
>> + pca9450_buck123_vranges, true),
>> + PCA_DATA("BUCK2", PCA9450_BUCK2CTRL, HW_STATE_CONTROL,
>> + PCA9450_BUCK2OUT_DVS0, DVS_BUCK_RUN_MASK,
>> + pca9450_buck123_vranges, true),
>> + PCA_DATA("BUCK3", PCA9450_BUCK3CTRL, HW_STATE_CONTROL,
>> + PCA9450_BUCK3OUT_DVS0, DVS_BUCK_RUN_MASK,
>> + pca9450_buck123_vranges, true),
>> + /* Bucks 4-6 which do not support dynamic voltage scaling */
>> + PCA_DATA("BUCK4", PCA9450_BUCK4CTRL, HW_STATE_CONTROL,
>> + PCA9450_BUCK4OUT, DVS_BUCK_RUN_MASK,
>> + pca9450_buck456_vranges, false),
>> + PCA_DATA("BUCK5", PCA9450_BUCK5CTRL, HW_STATE_CONTROL,
>> + PCA9450_BUCK5OUT, DVS_BUCK_RUN_MASK,
>> + pca9450_buck456_vranges, false),
>> + PCA_DATA("BUCK6", PCA9450_BUCK6CTRL, HW_STATE_CONTROL,
>> + PCA9450_BUCK6OUT, DVS_BUCK_RUN_MASK,
>> + pca9450_buck456_vranges, false),
>> + /* LDOs */
>> + PCA_DATA("LDO1", PCA9450_LDO1CTRL, HW_STATE_CONTROL,
>> + PCA9450_LDO1CTRL, PCA9450_LDO12_MASK,
>> + pca9450_ldo1_vranges, false),
>> + PCA_DATA("LDO2", PCA9450_LDO2CTRL, HW_STATE_CONTROL,
>> + PCA9450_LDO2CTRL, PCA9450_LDO12_MASK,
>> + pca9450_ldo2_vranges, false),
>> + PCA_DATA("LDO3", PCA9450_LDO3CTRL, HW_STATE_CONTROL,
>> + PCA9450_LDO3CTRL, PCA9450_LDO34_MASK,
>> + pca9450_ldo34_vranges, false),
>> + PCA_DATA("LDO4", PCA9450_LDO4CTRL, HW_STATE_CONTROL,
>> + PCA9450_LDO4CTRL, PCA9450_LDO34_MASK,
>> + pca9450_ldo34_vranges, false),
>> + PCA_DATA("LDO5", PCA9450_LDO5CTRL_H, HW_STATE_CONTROL,
>> + PCA9450_LDO5CTRL_H, PCA9450_LDO5_MASK,
>> + pca9450_ldo5_vranges, false),
>> +};
>> +
>> +static int vrange_find_value(struct pca9450_vrange *r, unsigned int sel,
>> + unsigned int *val)
>> +{
>> + if (!val || sel < r->min_sel || sel > r->max_sel)
>> + return -EINVAL;
>> +
>> + *val = r->min_volt + r->step * (sel - r->min_sel);
>> + return 0;
>> +}
>> +
>> +static int vrange_find_selector(struct pca9450_vrange *r, int val,
>> + unsigned int *sel)
>> +{
>> + int ret = -EINVAL;
>> + int num_vals = r->max_sel - r->min_sel + 1;
>> +
>> + if (val >= r->min_volt &&
>> + val <= r->min_volt + r->step * (num_vals - 1)) {
>> + if (r->step) {
>> + *sel = r->min_sel + ((val - r->min_volt) / r->step);
>> + ret = 0;
>> + } else {
>> + *sel = r->min_sel;
>> + ret = 0;
>> + }
>> + }
>> + return ret;
>> +}
>> +
>> +static int pca9450_get_enable(struct udevice *dev)
>> +{
>> + struct pca9450_plat *plat = dev_get_plat(dev);
>> + int val;
>> +
>> + /*
>> + * boot critical regulators on pca9450 must not be controlled by sw
>> + * due to the 'feature' which leaves power rails down if pca9450 is
>> + * reseted to snvs state. hence we can't get the state here.
>> + *
>> + * if we are alive it means we probably are on run state and
>> + * if the regulator can't be controlled we can assume it is
>> + * enabled.
>> + */
>> + if (plat->enablemask == HW_STATE_CONTROL)
>> + return 1;
>> +
>> + val = pmic_reg_read(dev->parent, plat->enable_reg);
>> + if (val < 0)
>> + return val;
>> +
>> + return (val & plat->enablemask);
>> +}
>> +
>> +static int pca9450_set_enable(struct udevice *dev, bool enable)
>> +{
>> + int val = 0;
>> + struct pca9450_plat *plat = dev_get_plat(dev);
>> +
>> + /*
>> + * boot critical regulators on pca9450 must not be controlled by sw
>> + * due to the 'feature' which leaves power rails down if pca9450 is
>> + * reseted to snvs state. Hence we can't set the state here.
>> + */
>> + if (plat->enablemask == HW_STATE_CONTROL)
>> + return enable ? 0 : -EINVAL;
>> +
>> + if (enable)
>> + val = plat->enablemask;
>> +
>> + return pmic_clrsetbits(dev->parent, plat->enable_reg, plat->enablemask,
>> + val);
>> +}
>> +
>> +static int pca9450_get_value(struct udevice *dev)
>> +{
>> + struct pca9450_plat *plat = dev_get_plat(dev);
>> + unsigned int reg, tmp;
>> + int i, ret;
>> +
>> + ret = pmic_reg_read(dev->parent, plat->volt_reg);
>> + if (ret < 0)
>> + return ret;
>> +
>> + reg = ret;
>> + reg &= plat->volt_mask;
>> +
>> + for (i = 0; i < plat->numranges; i++) {
>> + struct pca9450_vrange *r = &plat->ranges[i];
>> +
>> + if (!vrange_find_value(r, reg, &tmp))
>> + return tmp;
>> + }
>> +
>> + pr_err("Unknown voltage value read from pmic\n");
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static int pca9450_set_value(struct udevice *dev, int uvolt)
>> +{
>> + struct pca9450_plat *plat = dev_get_plat(dev);
>> + unsigned int sel;
>> + int i, found = 0;
>> +
>> + /*
>> + * An under/overshooting may occur if voltage is changed for other
>> + * regulators but buck 1,2,3 or 4 when regulator is enabled. Prevent
>> + * change to protect the HW
>> + */
>> + if (!plat->dvs)
>> + if (pca9450_get_enable(dev)) {
>> + /* If the value is already set, skip the warning. */
>> + if (pca9450_get_value(dev) == uvolt)
>> + return 0;
>> + pr_err("Only DVS bucks can be changed when enabled\n");
>> + return -EINVAL;
>> + }
>> +
>> + for (i = 0; i < plat->numranges; i++) {
>> + struct pca9450_vrange *r = &plat->ranges[i];
>> +
>> + found = !vrange_find_selector(r, uvolt, &sel);
>> + if (found) {
>> + unsigned int tmp;
>> +
>> + /*
>> + * We require exactly the requested value to be
>> + * supported - this can be changed later if needed
>> + */
>> + found = !vrange_find_value(r, sel, &tmp);
>> + if (found && tmp == uvolt)
>> + break;
>> + found = 0;
>> + }
>> + }
>> +
>> + if (!found)
>> + return -EINVAL;
>> +
>> + return pmic_clrsetbits(dev->parent, plat->volt_reg,
>> + plat->volt_mask, sel);
>> +}
>> +
>> +static int pca9450_regulator_probe(struct udevice *dev)
>> +{
>> + struct pca9450_plat *plat = dev_get_plat(dev);
>> + int i, type;
>> +
>> + type = dev_get_driver_data(dev_get_parent(dev));
>> +
>> + if (type != NXP_CHIP_TYPE_PCA9450A && type != NXP_CHIP_TYPE_PCA9450BC) {
>> + debug("Unknown PMIC type\n");
>> + return -EINVAL;
>> + }
>> +
>> + for (i = 0; i < ARRAY_SIZE(pca9450_reg_data); i++) {
>> + if (strcmp(dev->name, pca9450_reg_data[i].name))
>> + continue;
>> +
>> + /* PCA9450B/PCA9450C uses BUCK1 and BUCK3 in dual-phase */
>> + if (type == NXP_CHIP_TYPE_PCA9450BC &&
>> + !strcmp(pca9450_reg_data[i].name, "BUCK3")) {
>> + continue;
>> + }
>> +
>> + *plat = pca9450_reg_data[i];
>> +
>> + return 0;
>> + }
>> +
>> + pr_err("Unknown regulator '%s'\n", dev->name);
>> +
>> + return -ENOENT;
>> +}
>> +
>> +static const struct dm_regulator_ops pca9450_regulator_ops = {
>> + .get_value = pca9450_get_value,
>> + .set_value = pca9450_set_value,
>> + .get_enable = pca9450_get_enable,
>> + .set_enable = pca9450_set_enable,
>> +};
>> +
>> +U_BOOT_DRIVER(pca9450_regulator) = {
>> + .name = PCA9450_REGULATOR_DRIVER,
>> + .id = UCLASS_REGULATOR,
>> + .ops = &pca9450_regulator_ops,
>> + .probe = pca9450_regulator_probe,
>> + .plat_auto = sizeof(struct pca9450_plat),
>> +};
>> diff --git a/include/power/pca9450.h b/include/power/pca9450.h
>> index 27703bb1f91..b714fc3477d 100644
>> --- a/include/power/pca9450.h
>> +++ b/include/power/pca9450.h
>> @@ -56,4 +56,15 @@ enum {
>>
>> int power_pca9450_init(unsigned char bus, unsigned char addr);
>>
>> +enum {
>> + NXP_CHIP_TYPE_PCA9450A = 0,
>> + NXP_CHIP_TYPE_PCA9450BC,
>> + NXP_CHIP_TYPE_AMOUNT
>> +};
>> +
>> +#define DVS_BUCK_RUN_MASK 0x7f
>
> Marek,
>
> Looks like DVS_BUCK_RUN_MASK is a bad choice of names and should be
> something like PCA9450_DVS_BUCK_RUN_MASK.
>
You have anticipated me - DVS_BUCK_RUN_MASK is redefined and breaks CI.
https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/436881
> It's defined differently in include/power/bd71837.h (where it should
> also likely be changed to something that doesn't collide)
> $ git grep DVS_BUCK_RUN_MASK include/
> include/power/bd71837.h:#define DVS_BUCK_RUN_MASK 0x3f
> include/power/pca9450.h:#define DVS_BUCK_RUN_MASK 0x7f
>
> Granted, board/gateworks/venice/spl.c is the only file I see that
> includes both of those.
>
> Is this merged yet?
No, I set in patchworks that a new version is needed.
Regards,
Stefano
>
> Best Regards,
>
> Tim
--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
More information about the U-Boot
mailing list