[U-Boot] [PATCH v2 0/2] support for ROHM BD71837 and BD71847 PMICs
Patch series to support for ROHM BD71837 and BD71847 PMICs.
ROHM 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.
Series adds PMIC driver with register read and write support, and regulator driver with enable/disable and voltage changing support with certain limitations: - Enable/Disable state control is by default only allowed for BD71837 bucks 3 and 4. This is due to a HW feature which leaves SW controlled power rails unpowered after reset which leads to SNVS state. - Voltage control for enabled regulator is only allowed for "DVS" regulators (bucks 1-4 on BD71837, bucks 1 and 2 on BD71847). This is done because changing voltage on other regulators may cause under-/over shoot when regulator is enabled.
Drivers expect to see PMIC node with proper regulator sub-nodes given from device tree but.
Driver is tested using BD71837 and BD71847 break-out board connected to Beagle Bone Black and device-tree entry which was created as desctibed in dt-binding document for BD718x7 shipped with Linux.
This patch series does not support DT properties which allow changing the reset target between SNVS and READY or identifying boot-critical regulators and disabling SW control based on reset target as the Linux driver does.
Changelog v2: (fix issues pointed by Simon Glass) - Fix typo in patch series name (BD71827 => BD71837) - Drop drivers/power/pmic/pmic_bd71837.c (the old PMIC interface) - Fix characters in hex numbers to lowercase - use pmic_clrsetbits() instead of using separate reads and writes - fix styling issues
Changelog v1: - This version is created based on the RFC v1. https://lists.denx.de/pipermail/u-boot/2019-March/363076.html https://lists.denx.de/pipermail/u-boot/2019-March/363077.html - Support BD71847. - Unlock the PMIC protection register
Patch 1: - Cherry picks the initial PMIC driver for BD71837 from NXP's i.MX repository at: https://source.codeaurora.org/external/imx/uboot-imx (commits e9a3bec2e95a and acdc5c297a96). Fixes checkpatch issues.
Patch 2: - Support BD71847 PMIC. - Add support for BD71837 and BD71847 regulators.
---
Matti Vaittinen (2): regulator: bd71837: copy the bd71837 pmic driver from NXP imx u-boot regulator: bd718x7: support ROHM BD71837 and BD71847 PMICs
drivers/power/pmic/Kconfig | 7 + drivers/power/pmic/Makefile | 1 + drivers/power/pmic/bd71837.c | 122 ++++++++ drivers/power/regulator/Kconfig | 15 + drivers/power/regulator/Makefile | 1 + drivers/power/regulator/bd71837.c | 469 ++++++++++++++++++++++++++++++ include/power/bd71837.h | 103 +++++++ 7 files changed, 718 insertions(+) create mode 100644 drivers/power/pmic/bd71837.c create mode 100644 drivers/power/regulator/bd71837.c create mode 100644 include/power/bd71837.h
https://source.codeaurora.org/external/imx/uboot-imx
cherry picked, styled and merged commits: - MLK-18387 pmic: Add pmic driver for BD71837: e9a3bec2e95a - MLK-18590 pmic: bd71837: Change to use new fdt API: acdc5c297a96
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Matti Vaittinen matti.vaittinen@fi.rohmeurope.com ---
Changelog v1 => v2: - Drop drivers/power/pmic/pmic_bd71837.c (the old PMIC interface) - Fix characters in hex numbers to lowercase
drivers/power/pmic/Kconfig | 7 +++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/bd71837.c | 90 ++++++++++++++++++++++++++++++++++++ include/power/bd71837.h | 62 +++++++++++++++++++++++++ 4 files changed, 160 insertions(+) create mode 100644 drivers/power/pmic/bd71837.c create mode 100644 include/power/bd71837.h
diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig index 8cf60ebcf3..e154d0a57b 100644 --- a/drivers/power/pmic/Kconfig +++ b/drivers/power/pmic/Kconfig @@ -48,6 +48,13 @@ config PMIC_AS3722 interface and is designs to cover most of the power managementment required for a tablets or laptop.
+config DM_PMIC_BD71837 + bool "Enable Driver Model for PMIC BD71837" + depends on DM_PMIC + help + This config enables implementation of driver-model pmic uclass features + for PMIC BD71837. The driver implements read/write operations. + config DM_PMIC_FAN53555 bool "Enable support for OnSemi FAN53555" depends on DM_PMIC && DM_REGULATOR && DM_I2C diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile index 637352ab2b..e605a88196 100644 --- a/drivers/power/pmic/Makefile +++ b/drivers/power/pmic/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_DM_PMIC_FAN53555) += fan53555.o obj-$(CONFIG_DM_PMIC_MAX77686) += max77686.o obj-$(CONFIG_DM_PMIC_MAX8998) += max8998.o obj-$(CONFIG_DM_PMIC_MC34708) += mc34708.o +obj-$(CONFIG_$(SPL_)DM_PMIC_BD71837) += bd71837.o obj-$(CONFIG_$(SPL_)DM_PMIC_PFUZE100) += pfuze100.o obj-$(CONFIG_PMIC_S2MPS11) += s2mps11.o obj-$(CONFIG_DM_PMIC_SANDBOX) += sandbox.o i2c_pmic_emul.o diff --git a/drivers/power/pmic/bd71837.c b/drivers/power/pmic/bd71837.c new file mode 100644 index 0000000000..b749f9430a --- /dev/null +++ b/drivers/power/pmic/bd71837.c @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2018 NXP + */ + +#include <common.h> +#include <fdtdec.h> +#include <errno.h> +#include <dm.h> +#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/bd71837.h> + +DECLARE_GLOBAL_DATA_PTR; + +static const struct pmic_child_info pmic_children_info[] = { + /* buck */ + { .prefix = "b", .driver = BD71837_REGULATOR_DRIVER}, + /* ldo */ + { .prefix = "l", .driver = BD71837_REGULATOR_DRIVER}, + { }, +}; + +static int bd71837_reg_count(struct udevice *dev) +{ + return BD71837_REG_NUM; +} + +static int bd71837_write(struct udevice *dev, uint reg, const uint8_t *buff, + int len) +{ + if (dm_i2c_write(dev, reg, buff, len)) { + pr_err("write error to device: %p register: %#x!", dev, reg); + return -EIO; + } + + return 0; +} + +static int bd71837_read(struct udevice *dev, uint reg, uint8_t *buff, int len) +{ + if (dm_i2c_read(dev, reg, buff, len)) { + pr_err("read error from device: %p register: %#x!", dev, reg); + return -EIO; + } + + return 0; +} + +static int bd71837_bind(struct udevice *dev) +{ + int children; + ofnode regulators_node; + + regulators_node = dev_read_subnode(dev, "regulators"); + if (!ofnode_valid(regulators_node)) { + debug("%s: %s regulators subnode not found!", __func__, + dev->name); + return -ENXIO; + } + + debug("%s: '%s' - found regulators subnode\n", __func__, dev->name); + + children = pmic_bind_children(dev, regulators_node, pmic_children_info); + if (!children) + debug("%s: %s - no child found\n", __func__, dev->name); + + /* Always return success for this device */ + return 0; +} + +static struct dm_pmic_ops bd71837_ops = { + .reg_count = bd71837_reg_count, + .read = bd71837_read, + .write = bd71837_write, +}; + +static const struct udevice_id bd71837_ids[] = { + { .compatible = "rohm,bd71837", .data = 0x4b, }, + { } +}; + +U_BOOT_DRIVER(pmic_bd71837) = { + .name = "bd71837 pmic", + .id = UCLASS_PMIC, + .of_match = bd71837_ids, + .bind = bd71837_bind, + .ops = &bd71837_ops, +}; diff --git a/include/power/bd71837.h b/include/power/bd71837.h new file mode 100644 index 0000000000..38c69b2b90 --- /dev/null +++ b/include/power/bd71837.h @@ -0,0 +1,62 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* Copyright (C) 2018 ROHM Semiconductors */ + +#ifndef BD71837_H_ +#define BD71837_H_ + +#define BD71837_REGULATOR_DRIVER "bd71837_regulator" + +enum { + BD71837_REV = 0x00, + BD71837_SWRESET = 0x01, + BD71837_I2C_DEV = 0x02, + BD71837_PWRCTRL0 = 0x03, + BD71837_PWRCTRL1 = 0x04, + BD71837_BUCK1_CTRL = 0x05, + BD71837_BUCK2_CTRL = 0x06, + BD71837_BUCK3_CTRL = 0x07, + BD71837_BUCK4_CTRL = 0x08, + BD71837_BUCK5_CTRL = 0x09, + BD71837_BUCK6_CTRL = 0x0a, + BD71837_BUCK7_CTRL = 0x0b, + BD71837_BUCK8_CTRL = 0x0c, + BD71837_BUCK1_VOLT_RUN = 0x0d, + BD71837_BUCK1_VOLT_IDLE = 0x0e, + BD71837_BUCK1_VOLT_SUSP = 0x0f, + BD71837_BUCK2_VOLT_RUN = 0x10, + BD71837_BUCK2_VOLT_IDLE = 0x11, + BD71837_BUCK3_VOLT_RUN = 0x12, + BD71837_BUCK4_VOLT_RUN = 0x13, + BD71837_BUCK5_VOLT = 0x14, + BD71837_BUCK6_VOLT = 0x15, + BD71837_BUCK7_VOLT = 0x16, + BD71837_BUCK8_VOLT = 0x17, + BD71837_LDO1_VOLT = 0x18, + BD71837_LDO2_VOLT = 0x19, + BD71837_LDO3_VOLT = 0x1a, + BD71837_LDO4_VOLT = 0x1b, + BD71837_LDO5_VOLT = 0x1c, + BD71837_LDO6_VOLT = 0x1d, + BD71837_LDO7_VOLT = 0x1e, + BD71837_TRANS_COND0 = 0x1f, + BD71837_TRANS_COND1 = 0x20, + BD71837_VRFAULTEN = 0x21, + BD71837_MVRFLTMASK0 = 0x22, + BD71837_MVRFLTMASK1 = 0x23, + BD71837_MVRFLTMASK2 = 0x24, + BD71837_RCVCFG = 0x25, + BD71837_RCVNUM = 0x26, + BD71837_PWRONCONFIG0 = 0x27, + BD71837_PWRONCONFIG1 = 0x28, + BD71837_RESETSRC = 0x29, + BD71837_MIRQ = 0x2a, + BD71837_IRQ = 0x2b, + BD71837_IN_MON = 0x2c, + BD71837_POW_STATE = 0x2d, + BD71837_OUT32K = 0x2e, + BD71837_REGLOCK = 0x2f, + BD71837_MUXSW_EN = 0x30, + BD71837_REG_NUM, +}; + +#endif
On Wed, 24 Apr 2019 at 06:35, Matti Vaittinen matti.vaittinen@fi.rohmeurope.com wrote:
https://source.codeaurora.org/external/imx/uboot-imx
cherry picked, styled and merged commits:
- MLK-18387 pmic: Add pmic driver for BD71837: e9a3bec2e95a
- MLK-18590 pmic: bd71837: Change to use new fdt API: acdc5c297a96
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Matti Vaittinen matti.vaittinen@fi.rohmeurope.com
Changelog v1 => v2:
- Drop drivers/power/pmic/pmic_bd71837.c (the old PMIC interface)
- Fix characters in hex numbers to lowercase
drivers/power/pmic/Kconfig | 7 +++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/bd71837.c | 90 ++++++++++++++++++++++++++++++++++++ include/power/bd71837.h | 62 +++++++++++++++++++++++++ 4 files changed, 160 insertions(+) create mode 100644 drivers/power/pmic/bd71837.c create mode 100644 include/power/bd71837.h
Reviewed-by: Simon Glass sjg@chromium.org
Can you drop fdtdec.h?
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 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 matti.vaittinen@fi.rohmeurope.com ---
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); + 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> +#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/bd71837.h> + +#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, + 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_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; + int i; + int not_found = 1; + 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; + unsigned int tmp; + struct bd71837_platdata *plat = dev_get_platdata(dev); + int i; + + reg = pmic_reg_read(dev->parent, plat->volt_reg); + if (reg < 0) + return reg; + + range = reg & plat->rangemask; + + reg &= plat->volt_mask; + reg >>= ffs(plat->volt_mask) - 1; + + for (i = 0; i < plat->numranges; i++) { + struct bd71837_vrange *r = &plat->ranges[i]; + + if (plat->rangemask && ((plat->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_platdata *plat = dev_get_platdata(dev); + int i, ret; + struct dm_regulator_uclass_platdata *uc_pdata; + int type; + struct bd71837_platdata *init_data; + int data_amnt; + + type = dev_get_driver_data(dev_get_parent(dev)); + + switch (type) { + case ROHM_CHIP_TYPE_BD71837: + init_data = bd71837_reg_data; + data_amnt = ARRAY_SIZE(bd71837_reg_data); + break; + case ROHM_CHIP_TYPE_BD71847: + init_data = bd71847_reg_data; + data_amnt = ARRAY_SIZE(bd71847_reg_data); + break; + default: + debug("Unknown PMIC type\n"); + init_data = NULL; + data_amnt = 0; + break; + } + + for (i = 0; i < data_amnt; i++) { + if (!strcmp(dev->name, init_data[i].name)) { + *plat = init_data[i]; + if (plat->enablemask != HW_STATE_CONTROL) { + /* + * 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; + + return pmic_clrsetbits(dev->parent, + plat->enable_reg, + plat->sel_mask, + plat->sel_mask); + } + return 0; + } + } + + pr_err("Unknown regulator '%s'\n", dev->name); + + return -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 = BD718XX_REGULATOR_DRIVER, + .id = UCLASS_REGULATOR, + .ops = &bd71837_regulator_ops, + .probe = bd71837_regulator_probe, + .platdata_auto_alloc_size = sizeof(struct bd71837_platdata), +}; diff --git a/include/power/bd71837.h b/include/power/bd71837.h index 38c69b2b90..75e07e1de3 100644 --- a/include/power/bd71837.h +++ b/include/power/bd71837.h @@ -1,62 +1,103 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ /* Copyright (C) 2018 ROHM Semiconductors */
-#ifndef BD71837_H_ -#define BD71837_H_ +#ifndef BD718XX_H_ +#define BD718XX_H_
-#define BD71837_REGULATOR_DRIVER "bd71837_regulator" +#define BD718XX_REGULATOR_DRIVER "bd718x7_regulator"
enum { - BD71837_REV = 0x00, - BD71837_SWRESET = 0x01, - BD71837_I2C_DEV = 0x02, - BD71837_PWRCTRL0 = 0x03, - BD71837_PWRCTRL1 = 0x04, - BD71837_BUCK1_CTRL = 0x05, - BD71837_BUCK2_CTRL = 0x06, - BD71837_BUCK3_CTRL = 0x07, - BD71837_BUCK4_CTRL = 0x08, - BD71837_BUCK5_CTRL = 0x09, - BD71837_BUCK6_CTRL = 0x0a, - BD71837_BUCK7_CTRL = 0x0b, - BD71837_BUCK8_CTRL = 0x0c, - BD71837_BUCK1_VOLT_RUN = 0x0d, - BD71837_BUCK1_VOLT_IDLE = 0x0e, - BD71837_BUCK1_VOLT_SUSP = 0x0f, - BD71837_BUCK2_VOLT_RUN = 0x10, - BD71837_BUCK2_VOLT_IDLE = 0x11, - BD71837_BUCK3_VOLT_RUN = 0x12, - BD71837_BUCK4_VOLT_RUN = 0x13, - BD71837_BUCK5_VOLT = 0x14, - BD71837_BUCK6_VOLT = 0x15, - BD71837_BUCK7_VOLT = 0x16, - BD71837_BUCK8_VOLT = 0x17, - BD71837_LDO1_VOLT = 0x18, - BD71837_LDO2_VOLT = 0x19, - BD71837_LDO3_VOLT = 0x1a, - BD71837_LDO4_VOLT = 0x1b, - BD71837_LDO5_VOLT = 0x1c, - BD71837_LDO6_VOLT = 0x1d, - BD71837_LDO7_VOLT = 0x1e, - BD71837_TRANS_COND0 = 0x1f, - BD71837_TRANS_COND1 = 0x20, - BD71837_VRFAULTEN = 0x21, - BD71837_MVRFLTMASK0 = 0x22, - BD71837_MVRFLTMASK1 = 0x23, - BD71837_MVRFLTMASK2 = 0x24, - BD71837_RCVCFG = 0x25, - BD71837_RCVNUM = 0x26, - BD71837_PWRONCONFIG0 = 0x27, - BD71837_PWRONCONFIG1 = 0x28, - BD71837_RESETSRC = 0x29, - BD71837_MIRQ = 0x2a, - BD71837_IRQ = 0x2b, - BD71837_IN_MON = 0x2c, - BD71837_POW_STATE = 0x2d, - BD71837_OUT32K = 0x2e, - BD71837_REGLOCK = 0x2f, - BD71837_MUXSW_EN = 0x30, - BD71837_REG_NUM, + ROHM_CHIP_TYPE_BD71837 = 0, + ROHM_CHIP_TYPE_BD71847, + ROHM_CHIP_TYPE_BD70528, + ROHM_CHIP_TYPE_AMOUNT };
+enum { + BD718XX_REV = 0x00, + BD718XX_SWRESET = 0x01, + BD718XX_I2C_DEV = 0x02, + BD718XX_PWRCTRL0 = 0x03, + BD718XX_PWRCTRL1 = 0x04, + BD718XX_BUCK1_CTRL = 0x05, + BD718XX_BUCK2_CTRL = 0x06, + BD71837_BUCK3_CTRL = 0x07, + BD71837_BUCK4_CTRL = 0x08, + BD718XX_1ST_NODVS_BUCK_CTRL = 0x09, + BD718XX_2ND_NODVS_BUCK_CTRL = 0x0a, + BD718XX_3RD_NODVS_BUCK_CTRL = 0x0b, + BD718XX_4TH_NODVS_BUCK_CTRL = 0x0c, + BD718XX_BUCK1_VOLT_RUN = 0x0d, + BD718XX_BUCK1_VOLT_IDLE = 0x0e, + BD718XX_BUCK1_VOLT_SUSP = 0x0f, + BD718XX_BUCK2_VOLT_RUN = 0x10, + BD718XX_BUCK2_VOLT_IDLE = 0x11, + BD71837_BUCK3_VOLT_RUN = 0x12, + BD71837_BUCK4_VOLT_RUN = 0x13, + BD718XX_1ST_NODVS_BUCK_VOLT = 0x14, + BD718XX_2ND_NODVS_BUCK_VOLT = 0x15, + BD718XX_3RD_NODVS_BUCK_VOLT = 0x16, + BD718XX_4TH_NODVS_BUCK_VOLT = 0x17, + BD718XX_LDO1_VOLT = 0x18, + BD718XX_LDO2_VOLT = 0x19, + BD718XX_LDO3_VOLT = 0x1a, + BD718XX_LDO4_VOLT = 0x1b, + BD718XX_LDO5_VOLT = 0x1c, + BD718XX_LDO6_VOLT = 0x1d, + BD71837_LDO7_VOLT = 0x1e, + BD718XX_TRANS_COND0 = 0x1f, + BD718XX_TRANS_COND1 = 0x20, + BD718XX_VRFAULTEN = 0x21, + BD718XX_MVRFLTMASK0 = 0x22, + BD718XX_MVRFLTMASK1 = 0x23, + BD718XX_MVRFLTMASK2 = 0x24, + BD718XX_RCVCFG = 0x25, + BD718XX_RCVNUM = 0x26, + BD718XX_PWRONCONFIG0 = 0x27, + BD718XX_PWRONCONFIG1 = 0x28, + BD718XX_RESETSRC = 0x29, + BD718XX_MIRQ = 0x2a, + BD718XX_IRQ = 0x2b, + BD718XX_IN_MON = 0x2c, + BD718XX_POW_STATE = 0x2d, + BD718XX_OUT32K = 0x2e, + BD718XX_REGLOCK = 0x2f, + BD718XX_MUXSW_EN = 0x30, + BD718XX_REG_OTPVER = 0xff, + BD718XX_MAX_REGISTER = 0x100, +}; + +#define BD718XX_REGLOCK_PWRSEQ 0x1 +#define BD718XX_REGLOCK_VREG 0x10 + +#define BD718XX_BUCK_EN 0x01 +#define BD718XX_LDO_EN 0x40 +#define BD718XX_BUCK_SEL 0x02 +#define BD718XX_LDO_SEL 0x80 + +#define DVS_BUCK_RUN_MASK 0x3f +#define BD718XX_1ST_NODVS_BUCK_MASK 0x07 +#define BD718XX_3RD_NODVS_BUCK_MASK 0x07 +#define BD718XX_4TH_NODVS_BUCK_MASK 0x3f + +#define BD71847_BUCK3_MASK 0x07 +#define BD71847_BUCK3_RANGE_MASK 0xc0 +#define BD71847_BUCK4_MASK 0x03 +#define BD71847_BUCK4_RANGE_MASK 0x40 + +#define BD71837_BUCK5_RANGE_MASK 0x80 +#define BD71837_BUCK6_MASK 0x03 + +#define BD718XX_LDO1_MASK 0x03 +#define BD718XX_LDO1_RANGE_MASK 0x20 +#define BD718XX_LDO2_MASK 0x20 +#define BD718XX_LDO3_MASK 0x0f +#define BD718XX_LDO4_MASK 0x0f +#define BD718XX_LDO6_MASK 0x0f + +#define BD71837_LDO5_MASK 0x0f +#define BD71847_LDO5_MASK 0x0f +#define BD71847_LDO5_RANGE_MASK 0x20 +#define BD71837_LDO7_MASK 0x0f + #endif
HI Matti,
On Wed, 24 Apr 2019 at 06:37, Matti Vaittinen matti.vaittinen@fi.rohmeurope.com 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 matti.vaittinen@fi.rohmeurope.com
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_BD71837helpThis config enables implementation of driver-model regulator uclassfeatures 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_BD71837helpThis config enables implementation of driver-model regulator uclassfeatures 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
Hello Simon and thanks again for taking the time to check this =)
On Wed, 2019-04-24 at 17:58 -0600, Simon Glass wrote:
HI Matti,
On Wed, 24 Apr 2019 at 06:37, Matti Vaittinen matti.vaittinen@fi.rohmeurope.com 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.
Good idea. I'll do so in the next version.
+static int bd718x7_probe(struct udevice *dev) +{
int ret;u8 unlock;/* Unlock the PMIC regulator control before probing thechildren */
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() ?
Sure. I'll fix this too. Makes this much nicer.
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?
errno.h? I return -EINVAL from few of the functions. Or do you mean i2c.h? I think that can be dropped, thanks.
+#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/bd71837.h>
Put above power/pmic to keep ordering
I'll do that.
+static int vrange_find_value(struct bd71837_vrange *r, u8 sel,
Can you use uint instea of u8?
I'll replace u8 with uint8_t for all occurrences in this file. I personally prefer uint8_t. I've got this u8 infection from the linux kernel code where u8 seems to be preferred =)
+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'
I see your point =) not_found became not_found just because return value 0 from vrange_find_selector() (or pretty much any other function I write) means success. So direct assignment to variable made it 'not_found' :] But "double negation" (!not_....) is indeed a bit difficult. I'll change this too.
struct bd71837_platdata *plat = dev_get_platdata(dev);/** An under/overshooting may occur if voltage is changedfor other
* regulators but buck 1,2,3 or 4 when regulator isenabled. Prevent
* change to protect the HW*/if (!plat->dvs)if (bd71837_get_enable(dev)) {pr_err("Only DVS bucks can be changed whenenabled\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 valueto be
* supported - this can be changed later ifneeded
*/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);+}
Best Regards Matti Vaittinen
-- 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 ~~~
Simon says - in Latin please. "non cogito me" dixit Rene Descarte, deinde evanescavit
(Thanks for the translation Simon)
Hi Matti,
On Wed, 24 Apr 2019 at 23:58, Vaittinen, Matti Matti.Vaittinen@fi.rohmeurope.com wrote:
Hello Simon and thanks again for taking the time to check this =)
On Wed, 2019-04-24 at 17:58 -0600, Simon Glass wrote:
HI Matti,
On Wed, 24 Apr 2019 at 06:37, Matti Vaittinen matti.vaittinen@fi.rohmeurope.com 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.
Good idea. I'll do so in the next version.
+static int bd718x7_probe(struct udevice *dev) +{
int ret;u8 unlock;/* Unlock the PMIC regulator control before probing thechildren */
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() ?
Sure. I'll fix this too. Makes this much nicer.
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?
errno.h? I return -EINVAL from few of the functions. Or do you mean i2c.h? I think that can be dropped, thanks.
I mean that errno.h should be included already?
+#include <i2c.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <power/bd71837.h>
Put above power/pmic to keep ordering
I'll do that.
+static int vrange_find_value(struct bd71837_vrange *r, u8 sel,
Can you use uint instea of u8?
I'll replace u8 with uint8_t for all occurrences in this file. I personally prefer uint8_t. I've got this u8 infection from the linux kernel code where u8 seems to be preferred =)
No, u8 is preferred over uint8_t.
I mean that you shouldn't be using u8 for arguments. You should use uint (unsigned int).
+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'
I see your point =) not_found became not_found just because return value 0 from vrange_find_selector() (or pretty much any other function I write) means success. So direct assignment to variable made it 'not_found' :] But "double negation" (!not_....) is indeed a bit difficult. I'll change this too.
[..]
Simon says - in Latin please. "non cogito me" dixit Rene Descarte, deinde evanescavit
(Thanks for the translation Simon)
I hope it is close :-)
- Simon
participants (3)
-
Matti Vaittinen -
Simon Glass -
Vaittinen, Matti