HI Matti, On Wed, 24 Apr 2019 at 06:37, Matti Vaittinen <[email protected]> wrote: > > BD71837 and BD71847 is PMIC intended for powering single-core, > dual-core, and quad-core SoC’s such as NXP-i.MX 8M. BD71847 > is used for example on NXP imx8mm EVK. > > Add regulator driver for ROHM BD71837 and BD71847 PMICs. > BD71837 contains 8 bucks and 7 LDOS. BD71847 is reduced > version containing 6 bucks and 6 LDOs. Voltages for DVS
This is great info and I think it should be in your Kconfig help - i.e.a bit more detail in your description of the chip. > bucks (1-4 on BD71837, 1 and 2 on BD71847) can be adjusted > when regulators are enabled. For other bucks and LDOs we may > have over- or undershooting if voltage is adjusted when > regulator is enabled. Thus this is prevented by default. > > BD718x7 has a quirk which may leave power output disabled > after reset if enable/disable state was controlled by SW. > Thus the SW control is only allowed for BD71837 bucks > 3 and 4 by default. The impact of this limitation must be > evaluated board-by board and restrictions may need to be > modified. (Linux driver get's these limitations from DT and we > may want to implement same on u-Boot driver). > > Signed-off-by: Matti Vaittinen <[email protected]> > --- > > Changelog v1 => v2: > - document structs containing the platdata > - use pmic_clrsetbits() instead of using separate reads and writes > - fix styling issues > > drivers/power/pmic/bd71837.c | 42 ++- > drivers/power/regulator/Kconfig | 15 + > drivers/power/regulator/Makefile | 1 + > drivers/power/regulator/bd71837.c | 469 ++++++++++++++++++++++++++++++ > include/power/bd71837.h | 147 ++++++---- > 5 files changed, 616 insertions(+), 58 deletions(-) > create mode 100644 drivers/power/regulator/bd71837.c > > diff --git a/drivers/power/pmic/bd71837.c b/drivers/power/pmic/bd71837.c > index b749f9430a..12c2e15a19 100644 > --- a/drivers/power/pmic/bd71837.c > +++ b/drivers/power/pmic/bd71837.c > @@ -3,6 +3,8 @@ > * Copyright 2018 NXP > */ > > +#define DEBUG > + > #include <common.h> > #include <fdtdec.h> > #include <errno.h> > @@ -16,15 +18,15 @@ DECLARE_GLOBAL_DATA_PTR; > > static const struct pmic_child_info pmic_children_info[] = { > /* buck */ > - { .prefix = "b", .driver = BD71837_REGULATOR_DRIVER}, > + { .prefix = "b", .driver = BD718XX_REGULATOR_DRIVER}, > /* ldo */ > - { .prefix = "l", .driver = BD71837_REGULATOR_DRIVER}, > + { .prefix = "l", .driver = BD718XX_REGULATOR_DRIVER}, > { }, > }; > > static int bd71837_reg_count(struct udevice *dev) > { > - return BD71837_REG_NUM; > + return BD718XX_MAX_REGISTER - 1; > } > > static int bd71837_write(struct udevice *dev, uint reg, const uint8_t *buff, > @@ -55,7 +57,7 @@ static int bd71837_bind(struct udevice *dev) > > regulators_node = dev_read_subnode(dev, "regulators"); > if (!ofnode_valid(regulators_node)) { > - debug("%s: %s regulators subnode not found!", __func__, > + debug("%s: %s regulators subnode not found!\n", __func__, > dev->name); > return -ENXIO; > } > @@ -70,6 +72,34 @@ static int bd71837_bind(struct udevice *dev) > return 0; > } > > +static int bd718x7_probe(struct udevice *dev) > +{ > + int ret; > + u8 unlock; > + > + /* Unlock the PMIC regulator control before probing the children */ > + ret = pmic_reg_read(dev, BD718XX_REGLOCK); > + if (ret < 0) { > + debug("%s: %s Failed to read lock register, error %d\n", > + __func__, dev->name, ret); > + return ret; > + } > + > + unlock = ret; > + unlock &= ~(BD718XX_REGLOCK_PWRSEQ | BD718XX_REGLOCK_VREG); > + > + ret = pmic_reg_write(dev, BD718XX_REGLOCK, unlock); Can you use pmic_clrsetbits() ? > + if (ret) { > + debug("%s: %s Failed to unlock regulator control\n", __func__, > + dev->name); > + return ret; > + } > + debug("%s: '%s' - BD718x7 PMIC register unlocked\n", __func__, > + dev->name); > + > + return 0; > +} > + > static struct dm_pmic_ops bd71837_ops = { > .reg_count = bd71837_reg_count, > .read = bd71837_read, > @@ -77,7 +107,8 @@ static struct dm_pmic_ops bd71837_ops = { > }; > > static const struct udevice_id bd71837_ids[] = { > - { .compatible = "rohm,bd71837", .data = 0x4b, }, > + { .compatible = "rohm,bd71837", .data = ROHM_CHIP_TYPE_BD71837, }, > + { .compatible = "rohm,bd71847", .data = ROHM_CHIP_TYPE_BD71847, }, > { } > }; > > @@ -86,5 +117,6 @@ U_BOOT_DRIVER(pmic_bd71837) = { > .id = UCLASS_PMIC, > .of_match = bd71837_ids, > .bind = bd71837_bind, > + .probe = bd718x7_probe, > .ops = &bd71837_ops, > }; > diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig > index 3ed0dd2264..323516587c 100644 > --- a/drivers/power/regulator/Kconfig > +++ b/drivers/power/regulator/Kconfig > @@ -43,6 +43,21 @@ config REGULATOR_AS3722 > but does not yet support change voltages. Currently this must be > done using direct register writes to the PMIC. > > +config DM_REGULATOR_BD71837 > + bool "Enable Driver Model for REGULATOR BD71837" > + depends on DM_REGULATOR && DM_PMIC_BD71837 > + help > + This config enables implementation of driver-model regulator uclass > + features for REGULATOR BD71837. The driver implements get/set api for: > + value and enable. > + > +config SPL_DM_REGULATOR_BD71837 > + bool "Enable Driver Model for REGULATOR BD71837 in SPL" > + depends on DM_REGULATOR_BD71837 > + help > + This config enables implementation of driver-model regulator uclass > + features for REGULATOR BD71837 in SPL. > + > config DM_REGULATOR_PFUZE100 > bool "Enable Driver Model for REGULATOR PFUZE100" > depends on DM_REGULATOR && DM_PMIC_PFUZE100 > diff --git a/drivers/power/regulator/Makefile > b/drivers/power/regulator/Makefile > index f617ce723a..898ed5f084 100644 > --- a/drivers/power/regulator/Makefile > +++ b/drivers/power/regulator/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_REGULATOR_ACT8846) += act8846.o > obj-$(CONFIG_REGULATOR_AS3722) += as3722_regulator.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_)REGULATOR_PWM) += pwm_regulator.o > obj-$(CONFIG_$(SPL_)DM_REGULATOR_FAN53555) += fan53555.o > obj-$(CONFIG_$(SPL_)DM_REGULATOR_FIXED) += fixed.o > diff --git a/drivers/power/regulator/bd71837.c > b/drivers/power/regulator/bd71837.c > new file mode 100644 > index 0000000000..060e6efe74 > --- /dev/null > +++ b/drivers/power/regulator/bd71837.c > @@ -0,0 +1,469 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Copyright (C) 2019 ROHM Semiconductors > + * > + * ROHM BD71837 regulator driver > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <errno.h> Drop this? > +#include <i2c.h> > +#include <power/pmic.h> > +#include <power/regulator.h> > +#include <power/bd71837.h> Put above power/pmic to keep ordering > + > +#define HW_STATE_CONTROL 0 > +#define DEBUG > + > +/** > + * struct bd71837_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 > + * @rangeval: register value used to select this range if selectible > + * ranges are supported > + */ > +struct bd71837_vrange { > + unsigned int min_volt; > + unsigned int step; > + u8 min_sel; > + u8 max_sel; > + u8 rangeval; > +}; > + > +/** > + * struct bd71837_platdata - 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 > + * @rangemask: mask for selecting used ranges if multiple ranges are > supported > + * @sel_mask: bit to toggle in order to transfer the register control to SW > + * @dvs: whether the voltage can be changed when regulator is enabled > + */ > +struct bd71837_platdata { > + const char *name; > + u8 enable_reg; > + u8 enablemask; > + u8 volt_reg; > + u8 volt_mask; > + struct bd71837_vrange *ranges; > + unsigned int numranges; > + u8 rangemask; > + u8 sel_mask; > + bool dvs; > +}; > + > +#define BD_RANGE(_min, _vstep, _sel_low, _sel_hi, _range_sel) \ > +{ \ > + .min_volt = (_min), .step = (_vstep), .min_sel = (_sel_low), \ > + .max_sel = (_sel_hi), .rangeval = (_range_sel) \ > +} > + > +#define BD_DATA(_name, enreg, enmask, vreg, vmask, _range, rmask, _dvs, sel) > \ > +{ \ > + .name = (_name), .enable_reg = (enreg), .enablemask = (enmask), \ > + .volt_reg = (vreg), .volt_mask = (vmask), .ranges = (_range), \ > + .numranges = ARRAY_SIZE(_range), .rangemask = (rmask), .dvs = (_dvs), > \ > + .sel_mask = (sel) \ > +} > + > +static struct bd71837_vrange dvs_buck_vranges[] = { > + BD_RANGE(700000, 10000, 0, 0x3c, 0), > + BD_RANGE(1300000, 0, 0x3d, 0x3f, 0), > +}; > + > +static struct bd71837_vrange bd71847_buck3_vranges[] = { > + BD_RANGE(700000, 100000, 0x00, 0x03, 0), > + BD_RANGE(1050000, 50000, 0x04, 0x05, 0), > + BD_RANGE(1200000, 150000, 0x06, 0x07, 0), > + BD_RANGE(550000, 50000, 0x0, 0x7, 0x40), > + BD_RANGE(675000, 100000, 0x0, 0x3, 0x80), > + BD_RANGE(1025000, 50000, 0x4, 0x5, 0x80), > + BD_RANGE(1175000, 150000, 0x6, 0x7, 0x80), > +}; > + > +static struct bd71837_vrange bd71847_buck4_vranges[] = { > + BD_RANGE(3000000, 100000, 0x00, 0x03, 0), > + BD_RANGE(2600000, 100000, 0x00, 0x03, 40), > +}; > + > +static struct bd71837_vrange bd71837_buck5_vranges[] = { > + BD_RANGE(700000, 100000, 0, 0x3, 0), > + BD_RANGE(1050000, 50000, 0x04, 0x05, 0), > + BD_RANGE(1200000, 150000, 0x06, 0x07, 0), > + BD_RANGE(675000, 100000, 0x0, 0x3, 0x80), > + BD_RANGE(1025000, 50000, 0x04, 0x05, 0x80), > + BD_RANGE(1175000, 150000, 0x06, 0x07, 0x80), > +}; > + > +static struct bd71837_vrange bd71837_buck6_vranges[] = { > + BD_RANGE(3000000, 100000, 0x00, 0x03, 0), > +}; > + > +static struct bd71837_vrange nodvs_buck3_vranges[] = { > + BD_RANGE(1605000, 90000, 0, 1, 0), > + BD_RANGE(1755000, 45000, 2, 4, 0), > + BD_RANGE(1905000, 45000, 5, 7, 0), > +}; > + > +static struct bd71837_vrange nodvs_buck4_vranges[] = { > + BD_RANGE(800000, 10000, 0x00, 0x3C, 0), > +}; > + > +static struct bd71837_vrange ldo1_vranges[] = { > + BD_RANGE(3000000, 100000, 0x00, 0x03, 0), > + BD_RANGE(1600000, 100000, 0x00, 0x03, 0x20), > +}; > + > +static struct bd71837_vrange ldo2_vranges[] = { > + BD_RANGE(900000, 0, 0, 0, 0), > + BD_RANGE(800000, 0, 1, 1, 0), > +}; > + > +static struct bd71837_vrange ldo3_vranges[] = { > + BD_RANGE(1800000, 100000, 0x00, 0x0f, 0), > +}; > + > +static struct bd71837_vrange ldo4_vranges[] = { > + BD_RANGE(900000, 100000, 0x00, 0x09, 0), > +}; > + > +static struct bd71837_vrange bd71837_ldo5_vranges[] = { > + BD_RANGE(1800000, 100000, 0x00, 0x0f, 0), > +}; > + > +static struct bd71837_vrange bd71847_ldo5_vranges[] = { > + BD_RANGE(1800000, 100000, 0x00, 0x0f, 0), > + BD_RANGE(800000, 100000, 0x00, 0x0f, 0x20), > +}; > + > +static struct bd71837_vrange ldo6_vranges[] = { > + BD_RANGE(900000, 100000, 0x00, 0x09, 0), > +}; > + > +static struct bd71837_vrange ldo7_vranges[] = { > + BD_RANGE(1800000, 100000, 0x00, 0x0f, 0), > +}; > + > +/* > + * 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 BD71837 > + * is powering NXP i.MX8. In this use-case we (for now) only allow control > + * for BUCK3 and BUCK4 which are not boot critical. > + */ > +static struct bd71837_platdata bd71837_reg_data[] = { > +/* Bucks 1-4 which support dynamic voltage scaling */ > + BD_DATA("BUCK1", BD718XX_BUCK1_CTRL, HW_STATE_CONTROL, > + BD718XX_BUCK1_VOLT_RUN, DVS_BUCK_RUN_MASK, dvs_buck_vranges, > 0, > + true, BD718XX_BUCK_SEL), > + BD_DATA("BUCK2", BD718XX_BUCK2_CTRL, HW_STATE_CONTROL, > + BD718XX_BUCK2_VOLT_RUN, DVS_BUCK_RUN_MASK, dvs_buck_vranges, > 0, > + true, BD718XX_BUCK_SEL), > + BD_DATA("BUCK3", BD71837_BUCK3_CTRL, BD718XX_BUCK_EN, > + BD71837_BUCK3_VOLT_RUN, DVS_BUCK_RUN_MASK, dvs_buck_vranges, > 0, > + true, BD718XX_BUCK_SEL), > + BD_DATA("BUCK4", BD71837_BUCK4_CTRL, BD718XX_BUCK_EN, > + BD71837_BUCK4_VOLT_RUN, DVS_BUCK_RUN_MASK, dvs_buck_vranges, > 0, > + true, BD718XX_BUCK_SEL), > +/* Bucks 5-8 which do not support dynamic voltage scaling */ > + BD_DATA("BUCK5", BD718XX_1ST_NODVS_BUCK_CTRL, HW_STATE_CONTROL, > + BD718XX_1ST_NODVS_BUCK_VOLT, BD718XX_1ST_NODVS_BUCK_MASK, > + bd71837_buck5_vranges, 0x80, false, BD718XX_BUCK_SEL), > + BD_DATA("BUCK6", BD718XX_2ND_NODVS_BUCK_CTRL, HW_STATE_CONTROL, > + BD718XX_2ND_NODVS_BUCK_VOLT, BD71837_BUCK6_MASK, > + bd71837_buck6_vranges, 0, false, BD718XX_BUCK_SEL), > + BD_DATA("BUCK7", BD718XX_3RD_NODVS_BUCK_CTRL, HW_STATE_CONTROL, > + BD718XX_3RD_NODVS_BUCK_VOLT, BD718XX_3RD_NODVS_BUCK_MASK, > + nodvs_buck3_vranges, 0, false, BD718XX_BUCK_SEL), > + BD_DATA("BUCK8", BD718XX_4TH_NODVS_BUCK_CTRL, HW_STATE_CONTROL, > + BD718XX_4TH_NODVS_BUCK_VOLT, BD718XX_4TH_NODVS_BUCK_MASK, > + nodvs_buck4_vranges, 0, false, BD718XX_BUCK_SEL), > +/* LDOs */ > + BD_DATA("LDO1", BD718XX_LDO1_VOLT, HW_STATE_CONTROL, > BD718XX_LDO1_VOLT, > + BD718XX_LDO1_MASK, ldo1_vranges, 0x20, false, > BD718XX_LDO_SEL), > + BD_DATA("LDO2", BD718XX_LDO2_VOLT, HW_STATE_CONTROL, > BD718XX_LDO2_VOLT, > + BD718XX_LDO2_MASK, ldo2_vranges, 0, false, BD718XX_LDO_SEL), > + BD_DATA("LDO3", BD718XX_LDO3_VOLT, HW_STATE_CONTROL, > BD718XX_LDO3_VOLT, > + BD718XX_LDO3_MASK, ldo3_vranges, 0, false, BD718XX_LDO_SEL), > + BD_DATA("LDO4", BD718XX_LDO4_VOLT, HW_STATE_CONTROL, > BD718XX_LDO4_VOLT, > + BD718XX_LDO4_MASK, ldo4_vranges, 0, false, BD718XX_LDO_SEL), > + BD_DATA("LDO5", BD718XX_LDO5_VOLT, HW_STATE_CONTROL, > BD718XX_LDO5_VOLT, > + BD71837_LDO5_MASK, bd71837_ldo5_vranges, 0, false, > + BD718XX_LDO_SEL), > + BD_DATA("LDO6", BD718XX_LDO6_VOLT, HW_STATE_CONTROL, > BD718XX_LDO6_VOLT, > + BD718XX_LDO6_MASK, ldo6_vranges, 0, false, BD718XX_LDO_SEL), > + BD_DATA("LDO7", BD71837_LDO7_VOLT, HW_STATE_CONTROL, > BD71837_LDO7_VOLT, > + BD71837_LDO7_MASK, ldo7_vranges, 0, false, BD718XX_LDO_SEL), > +}; > + > +static struct bd71837_platdata bd71847_reg_data[] = { > +/* Bucks 1 and 2 which support dynamic voltage scaling */ > + BD_DATA("BUCK1", BD718XX_BUCK1_CTRL, HW_STATE_CONTROL, > + BD718XX_BUCK1_VOLT_RUN, DVS_BUCK_RUN_MASK, dvs_buck_vranges, > 0, > + true, BD718XX_BUCK_SEL), > + BD_DATA("BUCK2", BD718XX_BUCK2_CTRL, HW_STATE_CONTROL, > + BD718XX_BUCK2_VOLT_RUN, DVS_BUCK_RUN_MASK, dvs_buck_vranges, > 0, > + true, BD718XX_BUCK_SEL), > +/* Bucks 3-6 which do not support dynamic voltage scaling */ > + BD_DATA("BUCK3", BD718XX_1ST_NODVS_BUCK_CTRL, HW_STATE_CONTROL, > + BD718XX_1ST_NODVS_BUCK_VOLT, BD718XX_1ST_NODVS_BUCK_MASK, > + bd71847_buck3_vranges, 0xc0, false, BD718XX_BUCK_SEL), > + BD_DATA("BUCK4", BD718XX_2ND_NODVS_BUCK_CTRL, HW_STATE_CONTROL, > + BD718XX_2ND_NODVS_BUCK_VOLT, BD71837_BUCK6_MASK, > + bd71847_buck4_vranges, 0x40, false, BD718XX_BUCK_SEL), > + BD_DATA("BUCK5", BD718XX_3RD_NODVS_BUCK_CTRL, HW_STATE_CONTROL, > + BD718XX_3RD_NODVS_BUCK_VOLT, BD718XX_3RD_NODVS_BUCK_MASK, > + nodvs_buck3_vranges, 0, false, BD718XX_BUCK_SEL), > + BD_DATA("BUCK6", BD718XX_4TH_NODVS_BUCK_CTRL, HW_STATE_CONTROL, > + BD718XX_4TH_NODVS_BUCK_VOLT, BD718XX_4TH_NODVS_BUCK_MASK, > + nodvs_buck4_vranges, 0, false, BD718XX_BUCK_SEL), > +/* LDOs */ > + BD_DATA("LDO1", BD718XX_LDO1_VOLT, HW_STATE_CONTROL, > BD718XX_LDO1_VOLT, > + BD718XX_LDO1_MASK, ldo1_vranges, 0x20, false, > BD718XX_LDO_SEL), > + BD_DATA("LDO2", BD718XX_LDO2_VOLT, HW_STATE_CONTROL, > BD718XX_LDO2_VOLT, > + BD718XX_LDO2_MASK, ldo2_vranges, 0, false, BD718XX_LDO_SEL), > + BD_DATA("LDO3", BD718XX_LDO3_VOLT, HW_STATE_CONTROL, > BD718XX_LDO3_VOLT, > + BD718XX_LDO3_MASK, ldo3_vranges, 0, false, BD718XX_LDO_SEL), > + BD_DATA("LDO4", BD718XX_LDO4_VOLT, HW_STATE_CONTROL, > BD718XX_LDO4_VOLT, > + BD718XX_LDO4_MASK, ldo4_vranges, 0, false, BD718XX_LDO_SEL), > + BD_DATA("LDO5", BD718XX_LDO5_VOLT, HW_STATE_CONTROL, > BD718XX_LDO5_VOLT, > + BD71847_LDO5_MASK, bd71847_ldo5_vranges, 0x20, false, > + BD718XX_LDO_SEL), > + BD_DATA("LDO6", BD718XX_LDO6_VOLT, HW_STATE_CONTROL, > BD718XX_LDO6_VOLT, > + BD718XX_LDO6_MASK, ldo6_vranges, 0, false, BD718XX_LDO_SEL), > +}; > + > +static int vrange_find_value(struct bd71837_vrange *r, u8 sel, Can you use uint instea of u8? > + 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 bd71837_vrange *r, int val, u8 *sel) Same here > +{ > + 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 bd71837_get_enable(struct udevice *dev) > +{ > + int val; > + struct bd71837_platdata *plat = dev_get_platdata(dev); > + > + /* > + * boot critical regulators on bd71837 must not be controlled by sw > + * due to the 'feature' which leaves power rails down if bd71837 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 bd71837_set_enable(struct udevice *dev, bool enable) > +{ > + int val = 0; > + struct bd71837_platdata *plat = dev_get_platdata(dev); > + > + /* > + * boot critical regulators on bd71837 must not be controlled by sw > + * due to the 'feature' which leaves power rails down if bd71837 is > + * reseted to snvs state. Hence we can't set the state here. > + */ > + if (plat->enablemask == HW_STATE_CONTROL) > + return -EINVAL; > + > + if (enable) > + val = plat->enablemask; > + > + return pmic_clrsetbits(dev->parent, plat->enable_reg, > plat->enablemask, > + val); > +} > + > +static int bd71837_set_value(struct udevice *dev, int uvolt) > +{ > + u8 sel; > + u8 range; and here > + int i; > + int not_found = 1; I think the logic would be easier if you used 'found' > + struct bd71837_platdata *plat = dev_get_platdata(dev); > + > + /* > + * 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 (bd71837_get_enable(dev)) { > + pr_err("Only DVS bucks can be changed when > enabled\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < plat->numranges; i++) { > + struct bd71837_vrange *r = &plat->ranges[i]; > + > + not_found = vrange_find_selector(r, uvolt, &sel); > + if (!not_found) { > + unsigned int tmp; > + > + /* > + * We require exactly the requested value to be > + * supported - this can be changed later if needed > + */ > + range = r->rangeval; > + not_found = vrange_find_value(r, sel, &tmp); > + if (!not_found && tmp == uvolt) > + break; > + not_found = 1; > + } > + } > + > + if (not_found) > + return -EINVAL; > + > + sel <<= ffs(plat->volt_mask) - 1; > + > + if (plat->rangemask) > + sel |= range; > + > + return pmic_clrsetbits(dev->parent, plat->volt_reg, plat->volt_mask | > + plat->rangemask, sel); > +} > + > +static int bd71837_get_value(struct udevice *dev) > +{ > + u8 reg, range; and here > + unsigned int tmp; > + struct bd71837_platdata *plat = dev_get_platdata(dev); > + int i; > + [..] Regards, Simon _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot

