| Message ID | 3ec638180b97ce4acb8da2710c3a42c1ed816441.1553690211.git.matti.vaittinen@fi.rohmeurope.com |
|---|---|
| State | RFC |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot,RFC,v1,1/2] copy the bd71837 pmic driver from NXP imx u-boot | expand |
Hi de Ho Peeps, On Wed, Mar 27, 2019 at 02:40:47PM +0200, Matti Vaittinen wrote: > Add regulator driver for ROHM BD71837 PMIC. BD71837 contains > 8 bucks and 7 LDOS. Voltages for bucks 1-4 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. > > BD71837 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 bucks3 and 4 by > default. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > drivers/power/regulator/Kconfig | 15 ++ > drivers/power/regulator/Makefile | 1 + > drivers/power/regulator/bd71837.c | 373 ++++++++++++++++++++++++++++++ > include/power/bd71837.h | 20 ++ > 4 files changed, 409 insertions(+) > This Patch: This was my first patch to U-boot so I wonder if there is something to improve. Is this Ok like this or should I have formatted it somehow differently? Also, is this type of submissions welcome? Any place to check if submissions are rejected, being reviewed or forgotten? Next Steps: Finally, the BD71837 is mainly targeted for powering the i.MX8M. There's another ROHM PMIC BD71847 - which is mainly used for powering the i.MX8MM. The Linux driver I wrote does support both of these PMICs and I was thinking that maybe I should add support for BD71847 in this u-Boot driver too. But before investing on that work I would like to get some feedback regarding the BD71837 u-boot driver. At least a sign that this kind of submissions are welcome or information if I am doing something completely wrong =) About RFC tag: I used RFC tag here mainly because I am unsure if the driver design fits what the u-boot is heading on or if this is kind of driver u-Boot should include. Is this correct use of RFC, and what are the consequences of using RFC-tag? My assumption was that the patch with RFC is reviewed as ither patches, but it is also a sign that the patch might have something that makes it unsuitable for applying to u-Boot. Is this correct? Finally, I guess I need to (re)submit the driver(s) without the RFC tag at some point, any suggestions when? Best Regards Matti Vaittinen
Hi Matti, On Wed, 27 Mar 2019 at 06:40, Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> wrote: > > Add regulator driver for ROHM BD71837 PMIC. BD71837 contains > 8 bucks and 7 LDOS. Voltages for bucks 1-4 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. > > BD71837 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 bucks3 and 4 by > default. > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > drivers/power/regulator/Kconfig | 15 ++ > drivers/power/regulator/Makefile | 1 + > drivers/power/regulator/bd71837.c | 373 ++++++++++++++++++++++++++++++ > include/power/bd71837.h | 20 ++ > 4 files changed, 409 insertions(+) > Reviewed-by: Simon Glass <sjg@chromium.org> But please see nits below. > 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..5b32425ba9 > --- /dev/null > +++ b/drivers/power/regulator/bd71837.c > @@ -0,0 +1,373 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +// > +// Copyright (C) 2019 ROHM Semiconductors > +// > +// ROHM BD71837 regulator driver The SPDX line has a // comment but everything else should use C comments: /* * Copyright ... * * ROHM ... */ > + > +#include <common.h> > +#include <dm.h> > +#include <errno.h> > +#include <i2c.h> > +#include <power/pmic.h> > +#include <power/regulator.h> > +#include <power/bd71837.h> > + > +#define HW_STATE_CONTROL 0 > + Need struct comment. /** * struct bd71837_vrange - description here * * @min_volt: Description here * ... */ > +struct bd71837_vrange { > + unsigned int min_volt; > + unsigned int step; > + u8 min_sel; > + u8 max_sel; > + u8 rangeval; > +}; > + Need struct comment > +struct bd71837_data { How about 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 buck1to4_vranges[] = { > + BD_RANGE(700000, 10000, 0, 0x3C, 0), Please use lower-case hex throughout. > + BD_RANGE(1300000, 0, 0x3D, 0x3F, 0), > +}; > + > +static struct bd71837_vrange 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 buck6_vranges[] = { > + BD_RANGE(3000000, 100000, 0x00, 0x03, 0), > +}; > + > +static struct bd71837_vrange buck7_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 buck8_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 ldo5_vranges[] = { > + BD_RANGE(1800000, 100000, 0x00, 0x0F, 0), > +}; > + > +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 The first line of multi-line comments should be /* by itself. So: /* * We use ... * ... * last line. */ Please fix throughout. > + * 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_data bd71837_reg_data[] = { > + BD_DATA("BUCK1", BD71837_BUCK1_CTRL, HW_STATE_CONTROL, > + BD71837_BUCK1_VOLT_RUN, DVS_BUCK_RUN_MASK, buck1to4_vranges, 0, > + true, BD71837_BUCK_SEL), > + BD_DATA("BUCK2", BD71837_BUCK2_CTRL, HW_STATE_CONTROL, > + BD71837_BUCK2_VOLT_RUN, DVS_BUCK_RUN_MASK, buck1to4_vranges, 0, > + true, BD71837_BUCK_SEL), > + BD_DATA("BUCK3", BD71837_BUCK3_CTRL, BD71837_BUCK_EN, > + BD71837_BUCK3_VOLT_RUN, DVS_BUCK_RUN_MASK, buck1to4_vranges, 0, > + true, BD71837_BUCK_SEL), > + BD_DATA("BUCK4", BD71837_BUCK4_CTRL, BD71837_BUCK_EN, > + BD71837_BUCK4_VOLT_RUN, DVS_BUCK_RUN_MASK, buck1to4_vranges, 0, > + true, BD71837_BUCK_SEL), > + BD_DATA("BUCK5", BD71837_BUCK5_CTRL, HW_STATE_CONTROL, > + BD71837_BUCK5_VOLT, BD71837_BUCK5_MASK, buck5_vranges, 0x80, > + false, BD71837_BUCK_SEL), > + BD_DATA("BUCK6", BD71837_BUCK6_CTRL, HW_STATE_CONTROL, > + BD71837_BUCK6_VOLT, BD71837_BUCK6_MASK, buck6_vranges, 0, > + false, BD71837_BUCK_SEL), > + BD_DATA("BUCK7", BD71837_BUCK7_CTRL, HW_STATE_CONTROL, > + BD71837_BUCK7_VOLT, BD71837_BUCK7_MASK, buck7_vranges, 0, > + false, BD71837_BUCK_SEL), > + BD_DATA("BUCK8", BD71837_BUCK8_CTRL, HW_STATE_CONTROL, > + BD71837_BUCK8_VOLT, BD71837_BUCK8_MASK, buck8_vranges, 0, > + false, BD71837_BUCK_SEL), > + BD_DATA("LDO1", BD71837_LDO1_VOLT, HW_STATE_CONTROL, BD71837_LDO1_VOLT, > + BD71837_LDO1_MASK, ldo1_vranges, 0x20, false, BD71837_LDO_SEL), > + BD_DATA("LDO2", BD71837_LDO2_VOLT, HW_STATE_CONTROL, BD71837_LDO2_VOLT, > + BD71837_LDO2_MASK, ldo2_vranges, 0, false, BD71837_LDO_SEL), > + BD_DATA("LDO3", BD71837_LDO3_VOLT, HW_STATE_CONTROL, BD71837_LDO3_VOLT, > + BD71837_LDO3_MASK, ldo3_vranges, 0, false, BD71837_LDO_SEL), > + BD_DATA("LDO4", BD71837_LDO4_VOLT, HW_STATE_CONTROL, BD71837_LDO4_VOLT, > + BD71837_LDO4_MASK, ldo4_vranges, 0, false, BD71837_LDO_SEL), > + BD_DATA("LDO5", BD71837_LDO5_VOLT, HW_STATE_CONTROL, BD71837_LDO5_VOLT, > + BD71837_LDO5_MASK, ldo5_vranges, 0, false, BD71837_LDO_SEL), > + BD_DATA("LDO6", BD71837_LDO6_VOLT, HW_STATE_CONTROL, BD71837_LDO6_VOLT, > + BD71837_LDO6_MASK, ldo6_vranges, 0, false, BD71837_LDO_SEL), > + BD_DATA("LDO7", BD71837_LDO7_VOLT, HW_STATE_CONTROL, BD71837_LDO7_VOLT, > + BD71837_LDO7_MASK, ldo7_vranges, 0, false, BD71837_LDO_SEL), > +}; > + > +static int vrange_find_value(struct bd71837_vrange *r, u8 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 bd71837_vrange *r, int val, u8 *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 bd71837_get_enable(struct udevice *dev) > +{ > + int val; > + struct bd71837_data *d = dev_get_platdata(dev); Can you use plat instead of d? d is too short! > + > + /* 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 (d->enablemask == HW_STATE_CONTROL) > + return 1; > + > + val = pmic_reg_read(dev->parent, d->enable_reg); > + Drop blank line, since the next line relates to it. Same below. > + if (val < 0) > + return val; > + > + return (val & d->enablemask); > +} > + > +static int bd71837_set_enable(struct udevice *dev, bool enable) > +{ > + int val; > + struct bd71837_data *d = 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 (d->enablemask == HW_STATE_CONTROL) > + return -EINVAL; > + > + val = pmic_reg_read(dev->parent, d->enable_reg); > + > + if (val < 0) > + return val; > + > + if (enable) > + val |= d->enablemask; > + else > + val &= ~d->enablemask; > + > + return pmic_reg_write(dev->parent, d->enable_reg, val); > +} > + > +static int bd71837_set_value(struct udevice *dev, int uvolt) > +{ > + u8 sel, reg; > + u8 range; > + int i; > + int not_found = 1; > + struct bd71837_data *d = 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 (!d->dvs) > + if (bd71837_get_enable(dev)) { > + pr_err("Only buck1-4 can be changed when enabled\n"); > + return -EINVAL; > + } > + > + for (i = 0; i < d->numranges; i++) { > + struct bd71837_vrange *r = &d->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(d->volt_mask) - 1; > + > + reg = pmic_reg_read(dev->parent, d->volt_reg); > + if (reg < 0) > + return reg; > + > + reg &= ~d->volt_mask; > + reg |= sel; > + if (d->rangemask) { > + reg &= ~d->rangemask; > + reg |= range; > + } > + > + return pmic_reg_write(dev->parent, d->volt_reg, reg); Can you use pmic_clrsetbits() ? > +} > + > +static int bd71837_get_value(struct udevice *dev) > +{ > + u8 reg, range; > + unsigned int tmp; > + struct bd71837_data *d = dev_get_platdata(dev); > + int i; > + > + reg = pmic_reg_read(dev->parent, d->volt_reg); > + if (reg < 0) > + return reg; > + > + range = reg & d->rangemask; > + > + reg &= d->volt_mask; > + reg >>= ffs(d->volt_mask) - 1; > + > + for (i = 0; i < d->numranges; i++) { > + struct bd71837_vrange *r = &d->ranges[i]; > + > + if (d->rangemask && ((d->rangemask & range) != r->rangeval)) > + continue; > + > + if (!vrange_find_value(r, reg, &tmp)) > + return tmp; > + } > + > + pr_err("Unknown voltage value read from pmic\n"); > + > + return -EINVAL; > +} > + > +static int bd71837_regulator_probe(struct udevice *dev) > +{ > + struct bd71837_data *d = dev_get_platdata(dev); > + int i, ret; > + struct dm_regulator_uclass_platdata *uc_pdata; > + > + for (i = 0; i < ARRAY_SIZE(bd71837_reg_data); i++) { > + if (!strcmp(dev->name, bd71837_reg_data[i].name)) { > + *d = bd71837_reg_data[i]; > + if (d->enablemask != HW_STATE_CONTROL) { > + u8 ctrl; > + > + /* Take the regulator under SW control. Ensure > + * the initial state matches dt flags and then > + * write the SEL bit > + */ > + uc_pdata = dev_get_uclass_platdata(dev); > + ret = bd71837_set_enable(dev, > + !!(uc_pdata->boot_on || > + uc_pdata->always_on)); > + if (ret) > + return ret; > + > + ctrl = pmic_reg_read(dev->parent, > + d->enable_reg); > + if (ctrl < 0) > + return ctrl; > + > + ctrl |= d->sel_mask; > + return pmic_reg_write(dev->parent, > + d->enable_reg, ctrl); > + } > + return 0; > + } > + } > + > + pr_err("Unknown regulator '%s'\n", dev->name); > + > + return -EINVAL; -ENOENT ? > +} > + > +static const struct dm_regulator_ops bd71837_regulator_ops = { > + .get_value = bd71837_get_value, > + .set_value = bd71837_set_value, > + .get_enable = bd71837_get_enable, > + .set_enable = bd71837_set_enable, > +}; > + > +U_BOOT_DRIVER(bd71837_regulator) = { > + .name = "bd71837_regulator", > + .id = UCLASS_REGULATOR, > + .ops = &bd71837_regulator_ops, > + .probe = bd71837_regulator_probe, > + .platdata_auto_alloc_size = sizeof(struct bd71837_data), > +}; > diff --git a/include/power/bd71837.h b/include/power/bd71837.h > index 9c74f6fc61..da14dc9bb1 100644 > --- a/include/power/bd71837.h > +++ b/include/power/bd71837.h > @@ -59,6 +59,26 @@ enum { > BD71837_REG_NUM, > }; > > +#define BD71837_BUCK_EN 0x01 > +#define BD71837_LDO_EN 0x40 > +#define BD71837_BUCK_SEL 0x02 > +#define BD71837_LDO_SEL 0x80 > + > +#define DVS_BUCK_RUN_MASK 0x3F > +#define BD71837_BUCK5_MASK 0x07 > +#define BD71837_BUCK6_MASK 0x03 > +#define BD71837_BUCK7_MASK 0x07 > +#define BD71837_BUCK8_MASK 0x3F > + > +#define BD71837_LDO1_MASK 0x03 > +#define BD71837_LDO1_RANGE_MASK 0x20 > +#define BD71837_LDO2_MASK 0x20 > +#define BD71837_LDO3_MASK 0x0F > +#define BD71837_LDO4_MASK 0x0F > +#define BD71837_LDO5_MASK 0x0F > +#define BD71837_LDO6_MASK 0x0F > +#define BD71837_LDO7_MASK 0x0F > + > int power_bd71837_init(unsigned char bus); > > #endif > -- > 2.17.2 > > > -- > Matti Vaittinen, Linux device drivers > ROHM Semiconductors, Finland SWDC > Kiviharjunlenkki 1E > 90220 OULU > FINLAND > > ~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~ This would be better in Latin. Regards, Simon
Hello Simon, Thanks a bunch for taking the time and reviewing this! I do appreciate! On Tue, 2019-04-23 at 21:54 -0600, Simon Glass wrote: > Hi Matti, > > On Wed, 27 Mar 2019 at 06:40, Matti Vaittinen > <matti.vaittinen@fi.rohmeurope.com> wrote: > > > > Add regulator driver for ROHM BD71837 PMIC. BD71837 contains > > 8 bucks and 7 LDOS. Voltages for bucks 1-4 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. > > > > BD71837 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 bucks3 and 4 by > > default. > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > --- > > drivers/power/regulator/Kconfig | 15 ++ > > drivers/power/regulator/Makefile | 1 + > > drivers/power/regulator/bd71837.c | 373 > > ++++++++++++++++++++++++++++++ > > include/power/bd71837.h | 20 ++ > > 4 files changed, 409 insertions(+) > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > But please see nits below. I see you reviewed the RFC version. I would like to ask you to see also the non RFC version https://patchwork.ozlabs.org/patch/1080860/ which supports also BD71847. I see that most (maybe all) of these comments apply to that patch too - but you might have some additional ideas too. I will create v2 of this non RFC patch based on these comments (and fix your comments to patch 1/2 too) but I won't add your reviewed-by just yet - I hope you can also check the pieces adding BD71847 support and give me a nudge if you see there something to improve. =) > > --- /dev/null > > +++ b/drivers/power/regulator/bd71837.c > > @@ -0,0 +1,373 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > +// > > +// Copyright (C) 2019 ROHM Semiconductors > > +// > > +// ROHM BD71837 regulator driver > > The SPDX line has a // comment but everything else should use C > comments: > > /* > * Copyright ... > * > * ROHM ... > */ This is fine for me. But just to note that this differs from Linux notation which accepts also the copyright block being // - comments. I am not sure how closely u-Boot follows (or wants to follow) kernel coding guidelines. But as I said, I don't mind changing this. > > + > > +static int bd71837_regulator_probe(struct udevice *dev) > > +{ > > + struct bd71837_data *d = dev_get_platdata(dev); > > + int i, ret; > > + struct dm_regulator_uclass_platdata *uc_pdata; > > + > > + for (i = 0; i < ARRAY_SIZE(bd71837_reg_data); i++) { > > + if (!strcmp(dev->name, bd71837_reg_data[i].name)) { > > + *d = bd71837_reg_data[i]; > > + if (d->enablemask != HW_STATE_CONTROL) { > > + u8 ctrl; > > + > > + /* Take the regulator under SW > > control. Ensure > > + * the initial state matches dt > > flags and then > > + * write the SEL bit > > + */ > > + uc_pdata = > > dev_get_uclass_platdata(dev); > > + ret = bd71837_set_enable(dev, > > + !!(uc_pdat > > a->boot_on || > > + uc_pdata- > > >always_on)); > > + if (ret) > > + return ret; > > + > > + ctrl = pmic_reg_read(dev->parent, > > + d- > > >enable_reg); > > + if (ctrl < 0) > > + return ctrl; > > + > > + ctrl |= d->sel_mask; > > + return pmic_reg_write(dev->parent, > > + d- > > >enable_reg, ctrl); > > + } > > + return 0; > > + } > > + } > > + > > + pr_err("Unknown regulator '%s'\n", dev->name); > > + > > + return -EINVAL; > > -ENOENT ? At first the -ENOENT sounded to me like a regulator/device is missing. I thought that here we have extra (invalid/unknown) regulator in the device-tree. Thus I used the -EINVAL. But I think you are right, we can think that DT is correct and the driver lacks of correct regulator entity. So -ENOENT can be appropriate. => I'll change this too. > > -- > > Matti Vaittinen, Linux device drivers > > ROHM Semiconductors, Finland SWDC > > Kiviharjunlenkki 1E > > 90220 OULU > > FINLAND > > > > ~~~ "I don't think so," said Rene Descartes. Just then he vanished > > ~~~ > > This would be better in Latin. Unfortunately that's far beyond my skills =) I can do Finnish though ;) Rest of the comments seemed all like trivial fixes and I will apply them =) > > Regards, > Simon
Hi Matti, On Tue, 23 Apr 2019 at 23:59, Vaittinen, Matti <Matti.Vaittinen@fi.rohmeurope.com> wrote: > > Hello Simon, > > Thanks a bunch for taking the time and reviewing this! I do appreciate! > > On Tue, 2019-04-23 at 21:54 -0600, Simon Glass wrote: > > Hi Matti, > > > > On Wed, 27 Mar 2019 at 06:40, Matti Vaittinen > > <matti.vaittinen@fi.rohmeurope.com> wrote: > > > > > > Add regulator driver for ROHM BD71837 PMIC. BD71837 contains > > > 8 bucks and 7 LDOS. Voltages for bucks 1-4 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. > > > > > > BD71837 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 bucks3 and 4 by > > > default. > > > > > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > > > --- > > > drivers/power/regulator/Kconfig | 15 ++ > > > drivers/power/regulator/Makefile | 1 + > > > drivers/power/regulator/bd71837.c | 373 > > > ++++++++++++++++++++++++++++++ > > > include/power/bd71837.h | 20 ++ > > > 4 files changed, 409 insertions(+) > > > > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > But please see nits below. > > I see you reviewed the RFC version. I would like to ask you to see also > the non RFC version https://patchwork.ozlabs.org/patch/1080860/ which > supports also BD71847. I see that most (maybe all) of these comments > apply to that patch too - but you might have some additional ideas > too. > > I will create v2 of this non RFC patch based on these comments (and fix > your comments to patch 1/2 too) but I won't add your reviewed-by just > yet - I hope you can also check the pieces adding BD71847 support and > give me a nudge if you see there something to improve. =) > > > > --- /dev/null > > > +++ b/drivers/power/regulator/bd71837.c > > > @@ -0,0 +1,373 @@ > > > +// SPDX-License-Identifier: GPL-2.0-or-later > > > +// > > > +// Copyright (C) 2019 ROHM Semiconductors > > > +// > > > +// ROHM BD71837 regulator driver > > > > The SPDX line has a // comment but everything else should use C > > comments: > > > > /* > > * Copyright ... > > * > > * ROHM ... > > */ > > This is fine for me. But just to note that this differs from Linux > notation which accepts also the copyright block being // - comments. I > am not sure how closely u-Boot follows (or wants to follow) kernel > coding guidelines. But as I said, I don't mind changing this. U-Boot mostly follows Linux, but you can see conventions by looking at the code, generally. > > > > + > > > +static int bd71837_regulator_probe(struct udevice *dev) > > > +{ > > > + struct bd71837_data *d = dev_get_platdata(dev); > > > + int i, ret; > > > + struct dm_regulator_uclass_platdata *uc_pdata; > > > + > > > + for (i = 0; i < ARRAY_SIZE(bd71837_reg_data); i++) { > > > + if (!strcmp(dev->name, bd71837_reg_data[i].name)) { > > > + *d = bd71837_reg_data[i]; > > > + if (d->enablemask != HW_STATE_CONTROL) { > > > + u8 ctrl; > > > + > > > + /* Take the regulator under SW > > > control. Ensure > > > + * the initial state matches dt > > > flags and then > > > + * write the SEL bit > > > + */ > > > + uc_pdata = > > > dev_get_uclass_platdata(dev); > > > + ret = bd71837_set_enable(dev, > > > + !!(uc_pdat > > > a->boot_on || > > > + uc_pdata- > > > >always_on)); > > > + if (ret) > > > + return ret; > > > + > > > + ctrl = pmic_reg_read(dev->parent, > > > + d- > > > >enable_reg); > > > + if (ctrl < 0) > > > + return ctrl; > > > + > > > + ctrl |= d->sel_mask; > > > + return pmic_reg_write(dev->parent, > > > + d- > > > >enable_reg, ctrl); > > > + } > > > + return 0; > > > + } > > > + } > > > + > > > + pr_err("Unknown regulator '%s'\n", dev->name); > > > + > > > + return -EINVAL; > > > > -ENOENT ? > > At first the -ENOENT sounded to me like a regulator/device is missing. > I thought that here we have extra (invalid/unknown) regulator in the > device-tree. Thus I used the -EINVAL. But I think you are right, we can > think that DT is correct and the driver lacks of correct regulator > entity. So -ENOENT can be appropriate. => I'll change this too. > > > > -- > > > Matti Vaittinen, Linux device drivers > > > ROHM Semiconductors, Finland SWDC > > > Kiviharjunlenkki 1E > > > 90220 OULU > > > FINLAND > > > > > > ~~~ "I don't think so," said Rene Descartes. Just then he vanished > > > ~~~ > > > > This would be better in Latin. > > Unfortunately that's far beyond my skills =) I can do Finnish though ;) Me also, but maybe something like this is close? "non cogito me" dixit Rene Descarte, deinde evanescavit Regards, Simon
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..5b32425ba9 --- /dev/null +++ b/drivers/power/regulator/bd71837.c @@ -0,0 +1,373 @@ +// 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> +#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/bd71837.h> + +#define HW_STATE_CONTROL 0 + +struct bd71837_vrange { + unsigned int min_volt; + unsigned int step; + u8 min_sel; + u8 max_sel; + u8 rangeval; +}; + +struct bd71837_data { + 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 buck1to4_vranges[] = { + BD_RANGE(700000, 10000, 0, 0x3C, 0), + BD_RANGE(1300000, 0, 0x3D, 0x3F, 0), +}; + +static struct bd71837_vrange 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 buck6_vranges[] = { + BD_RANGE(3000000, 100000, 0x00, 0x03, 0), +}; + +static struct bd71837_vrange buck7_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 buck8_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 ldo5_vranges[] = { + BD_RANGE(1800000, 100000, 0x00, 0x0F, 0), +}; + +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_data bd71837_reg_data[] = { + BD_DATA("BUCK1", BD71837_BUCK1_CTRL, HW_STATE_CONTROL, + BD71837_BUCK1_VOLT_RUN, DVS_BUCK_RUN_MASK, buck1to4_vranges, 0, + true, BD71837_BUCK_SEL), + BD_DATA("BUCK2", BD71837_BUCK2_CTRL, HW_STATE_CONTROL, + BD71837_BUCK2_VOLT_RUN, DVS_BUCK_RUN_MASK, buck1to4_vranges, 0, + true, BD71837_BUCK_SEL), + BD_DATA("BUCK3", BD71837_BUCK3_CTRL, BD71837_BUCK_EN, + BD71837_BUCK3_VOLT_RUN, DVS_BUCK_RUN_MASK, buck1to4_vranges, 0, + true, BD71837_BUCK_SEL), + BD_DATA("BUCK4", BD71837_BUCK4_CTRL, BD71837_BUCK_EN, + BD71837_BUCK4_VOLT_RUN, DVS_BUCK_RUN_MASK, buck1to4_vranges, 0, + true, BD71837_BUCK_SEL), + BD_DATA("BUCK5", BD71837_BUCK5_CTRL, HW_STATE_CONTROL, + BD71837_BUCK5_VOLT, BD71837_BUCK5_MASK, buck5_vranges, 0x80, + false, BD71837_BUCK_SEL), + BD_DATA("BUCK6", BD71837_BUCK6_CTRL, HW_STATE_CONTROL, + BD71837_BUCK6_VOLT, BD71837_BUCK6_MASK, buck6_vranges, 0, + false, BD71837_BUCK_SEL), + BD_DATA("BUCK7", BD71837_BUCK7_CTRL, HW_STATE_CONTROL, + BD71837_BUCK7_VOLT, BD71837_BUCK7_MASK, buck7_vranges, 0, + false, BD71837_BUCK_SEL), + BD_DATA("BUCK8", BD71837_BUCK8_CTRL, HW_STATE_CONTROL, + BD71837_BUCK8_VOLT, BD71837_BUCK8_MASK, buck8_vranges, 0, + false, BD71837_BUCK_SEL), + BD_DATA("LDO1", BD71837_LDO1_VOLT, HW_STATE_CONTROL, BD71837_LDO1_VOLT, + BD71837_LDO1_MASK, ldo1_vranges, 0x20, false, BD71837_LDO_SEL), + BD_DATA("LDO2", BD71837_LDO2_VOLT, HW_STATE_CONTROL, BD71837_LDO2_VOLT, + BD71837_LDO2_MASK, ldo2_vranges, 0, false, BD71837_LDO_SEL), + BD_DATA("LDO3", BD71837_LDO3_VOLT, HW_STATE_CONTROL, BD71837_LDO3_VOLT, + BD71837_LDO3_MASK, ldo3_vranges, 0, false, BD71837_LDO_SEL), + BD_DATA("LDO4", BD71837_LDO4_VOLT, HW_STATE_CONTROL, BD71837_LDO4_VOLT, + BD71837_LDO4_MASK, ldo4_vranges, 0, false, BD71837_LDO_SEL), + BD_DATA("LDO5", BD71837_LDO5_VOLT, HW_STATE_CONTROL, BD71837_LDO5_VOLT, + BD71837_LDO5_MASK, ldo5_vranges, 0, false, BD71837_LDO_SEL), + BD_DATA("LDO6", BD71837_LDO6_VOLT, HW_STATE_CONTROL, BD71837_LDO6_VOLT, + BD71837_LDO6_MASK, ldo6_vranges, 0, false, BD71837_LDO_SEL), + BD_DATA("LDO7", BD71837_LDO7_VOLT, HW_STATE_CONTROL, BD71837_LDO7_VOLT, + BD71837_LDO7_MASK, ldo7_vranges, 0, false, BD71837_LDO_SEL), +}; + +static int vrange_find_value(struct bd71837_vrange *r, u8 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 bd71837_vrange *r, int val, u8 *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 bd71837_get_enable(struct udevice *dev) +{ + int val; + struct bd71837_data *d = 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 (d->enablemask == HW_STATE_CONTROL) + return 1; + + val = pmic_reg_read(dev->parent, d->enable_reg); + + if (val < 0) + return val; + + return (val & d->enablemask); +} + +static int bd71837_set_enable(struct udevice *dev, bool enable) +{ + int val; + struct bd71837_data *d = 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 (d->enablemask == HW_STATE_CONTROL) + return -EINVAL; + + val = pmic_reg_read(dev->parent, d->enable_reg); + + if (val < 0) + return val; + + if (enable) + val |= d->enablemask; + else + val &= ~d->enablemask; + + return pmic_reg_write(dev->parent, d->enable_reg, val); +} + +static int bd71837_set_value(struct udevice *dev, int uvolt) +{ + u8 sel, reg; + u8 range; + int i; + int not_found = 1; + struct bd71837_data *d = 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 (!d->dvs) + if (bd71837_get_enable(dev)) { + pr_err("Only buck1-4 can be changed when enabled\n"); + return -EINVAL; + } + + for (i = 0; i < d->numranges; i++) { + struct bd71837_vrange *r = &d->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(d->volt_mask) - 1; + + reg = pmic_reg_read(dev->parent, d->volt_reg); + if (reg < 0) + return reg; + + reg &= ~d->volt_mask; + reg |= sel; + if (d->rangemask) { + reg &= ~d->rangemask; + reg |= range; + } + + return pmic_reg_write(dev->parent, d->volt_reg, reg); +} + +static int bd71837_get_value(struct udevice *dev) +{ + u8 reg, range; + unsigned int tmp; + struct bd71837_data *d = dev_get_platdata(dev); + int i; + + reg = pmic_reg_read(dev->parent, d->volt_reg); + if (reg < 0) + return reg; + + range = reg & d->rangemask; + + reg &= d->volt_mask; + reg >>= ffs(d->volt_mask) - 1; + + for (i = 0; i < d->numranges; i++) { + struct bd71837_vrange *r = &d->ranges[i]; + + if (d->rangemask && ((d->rangemask & range) != r->rangeval)) + continue; + + if (!vrange_find_value(r, reg, &tmp)) + return tmp; + } + + pr_err("Unknown voltage value read from pmic\n"); + + return -EINVAL; +} + +static int bd71837_regulator_probe(struct udevice *dev) +{ + struct bd71837_data *d = dev_get_platdata(dev); + int i, ret; + struct dm_regulator_uclass_platdata *uc_pdata; + + for (i = 0; i < ARRAY_SIZE(bd71837_reg_data); i++) { + if (!strcmp(dev->name, bd71837_reg_data[i].name)) { + *d = bd71837_reg_data[i]; + if (d->enablemask != HW_STATE_CONTROL) { + u8 ctrl; + + /* Take the regulator under SW control. Ensure + * the initial state matches dt flags and then + * write the SEL bit + */ + uc_pdata = dev_get_uclass_platdata(dev); + ret = bd71837_set_enable(dev, + !!(uc_pdata->boot_on || + uc_pdata->always_on)); + if (ret) + return ret; + + ctrl = pmic_reg_read(dev->parent, + d->enable_reg); + if (ctrl < 0) + return ctrl; + + ctrl |= d->sel_mask; + return pmic_reg_write(dev->parent, + d->enable_reg, ctrl); + } + return 0; + } + } + + pr_err("Unknown regulator '%s'\n", dev->name); + + return -EINVAL; +} + +static const struct dm_regulator_ops bd71837_regulator_ops = { + .get_value = bd71837_get_value, + .set_value = bd71837_set_value, + .get_enable = bd71837_get_enable, + .set_enable = bd71837_set_enable, +}; + +U_BOOT_DRIVER(bd71837_regulator) = { + .name = "bd71837_regulator", + .id = UCLASS_REGULATOR, + .ops = &bd71837_regulator_ops, + .probe = bd71837_regulator_probe, + .platdata_auto_alloc_size = sizeof(struct bd71837_data), +}; diff --git a/include/power/bd71837.h b/include/power/bd71837.h index 9c74f6fc61..da14dc9bb1 100644 --- a/include/power/bd71837.h +++ b/include/power/bd71837.h @@ -59,6 +59,26 @@ enum { BD71837_REG_NUM, }; +#define BD71837_BUCK_EN 0x01 +#define BD71837_LDO_EN 0x40 +#define BD71837_BUCK_SEL 0x02 +#define BD71837_LDO_SEL 0x80 + +#define DVS_BUCK_RUN_MASK 0x3F +#define BD71837_BUCK5_MASK 0x07 +#define BD71837_BUCK6_MASK 0x03 +#define BD71837_BUCK7_MASK 0x07 +#define BD71837_BUCK8_MASK 0x3F + +#define BD71837_LDO1_MASK 0x03 +#define BD71837_LDO1_RANGE_MASK 0x20 +#define BD71837_LDO2_MASK 0x20 +#define BD71837_LDO3_MASK 0x0F +#define BD71837_LDO4_MASK 0x0F +#define BD71837_LDO5_MASK 0x0F +#define BD71837_LDO6_MASK 0x0F +#define BD71837_LDO7_MASK 0x0F + int power_bd71837_init(unsigned char bus); #endif
Add regulator driver for ROHM BD71837 PMIC. BD71837 contains 8 bucks and 7 LDOS. Voltages for bucks 1-4 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. BD71837 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 bucks3 and 4 by default. Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> --- drivers/power/regulator/Kconfig | 15 ++ drivers/power/regulator/Makefile | 1 + drivers/power/regulator/bd71837.c | 373 ++++++++++++++++++++++++++++++ include/power/bd71837.h | 20 ++ 4 files changed, 409 insertions(+)