* [PATCH v2 0/2] power: regulator: add driver for ANATOP regulator @ 2021-03-23 13:48 Ying-Chun Liu 2021-03-23 13:48 ` [PATCH v2 1/2] " Ying-Chun Liu 2021-03-23 13:48 ` [PATCH v2 2/2] doc: device-tree-bindings: regulator: anatop regulator Ying-Chun Liu 0 siblings, 2 replies; 6+ messages in thread From: Ying-Chun Liu @ 2021-03-23 13:48 UTC (permalink / raw) To: u-boot From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org> Anatop is an integrated regulator inside i.MX6 SoC. There are 3 digital regulators which controls PU, CORE (ARM), and SOC. And 3 analog regulators which controls 1P1, 2P5, 3P0 (USB). This patch adds the Anatop regulator driver. Ying-Chun Liu (PaulLiu) (2): power: regulator: add driver for ANATOP regulator doc: device-tree-bindings: regulator: anatop regulator v2: add functions for set selector and delay. Define ANATOP_REGULATOR_STEP .../regulator/fsl,anatop-regulator.txt | 45 +++ drivers/power/regulator/Kconfig | 10 + drivers/power/regulator/Makefile | 1 + drivers/power/regulator/anatop_regulator.c | 299 ++++++++++++++++++ 4 files changed, 355 insertions(+) create mode 100644 doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt create mode 100644 drivers/power/regulator/anatop_regulator.c -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] power: regulator: add driver for ANATOP regulator 2021-03-23 13:48 [PATCH v2 0/2] power: regulator: add driver for ANATOP regulator Ying-Chun Liu @ 2021-03-23 13:48 ` Ying-Chun Liu 2021-03-23 15:06 ` Sean Anderson 2021-03-23 13:48 ` [PATCH v2 2/2] doc: device-tree-bindings: regulator: anatop regulator Ying-Chun Liu 1 sibling, 1 reply; 6+ messages in thread From: Ying-Chun Liu @ 2021-03-23 13:48 UTC (permalink / raw) To: u-boot From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org> Anatop is an integrated regulator inside i.MX6 SoC. There are 3 digital regulators which controls PU, CORE (ARM), and SOC. And 3 analog regulators which controls 1P1, 2P5, 3P0 (USB). This patch adds the Anatop regulator driver. Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> --- v2: add functions for set selector and delay. Define ANATOP_REGULATOR_STEP --- drivers/power/regulator/Kconfig | 10 + drivers/power/regulator/Makefile | 1 + drivers/power/regulator/anatop_regulator.c | 299 +++++++++++++++++++++ 3 files changed, 310 insertions(+) create mode 100644 drivers/power/regulator/anatop_regulator.c diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig index fbbea18c7d..1cd1f3f5ed 100644 --- a/drivers/power/regulator/Kconfig +++ b/drivers/power/regulator/Kconfig @@ -312,6 +312,16 @@ config DM_REGULATOR_STPMIC1 by the PMIC device. This driver is controlled by a device tree node which includes voltage limits. +config DM_REGULATOR_ANATOP + bool "Enable driver for ANATOP regulators" + depends on DM_REGULATOR + select REGMAP + select SYSCON + help + Enable support for the Freescale i.MX on-chip ANATOP LDOs + regulators. It is recommended that this option be enabled on + i.MX6 platform. + config SPL_DM_REGULATOR_STPMIC1 bool "Enable driver for STPMIC1 regulators in SPL" depends on SPL_DM_REGULATOR && PMIC_STPMIC1 diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile index 9d58112dcb..e7198da911 100644 --- a/drivers/power/regulator/Makefile +++ b/drivers/power/regulator/Makefile @@ -30,3 +30,4 @@ obj-$(CONFIG_DM_REGULATOR_TPS65910) += tps65910_regulator.o obj-$(CONFIG_DM_REGULATOR_TPS62360) += tps62360_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_STPMIC1) += stpmic1.o obj-$(CONFIG_DM_REGULATOR_TPS65941) += tps65941_regulator.o +obj-$(CONFIG_$(SPL_)DM_REGULATOR_ANATOP) += anatop_regulator.o diff --git a/drivers/power/regulator/anatop_regulator.c b/drivers/power/regulator/anatop_regulator.c new file mode 100644 index 0000000000..d824f4acc4 --- /dev/null +++ b/drivers/power/regulator/anatop_regulator.c @@ -0,0 +1,299 @@ +// SPDX-License-Identifier: GPL-2.0+ +// +// Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved. +// Copyright (C) 2021 Linaro + +#include <common.h> +#include <errno.h> +#include <dm.h> +#include <log.h> +#include <linux/delay.h> +#include <linux/err.h> +#include <power/pmic.h> +#include <power/regulator.h> +#include <regmap.h> +#include <syscon.h> +#include <linux/bitops.h> +#include <linux/ioport.h> +#include <dm/device-internal.h> +#include <dm/device_compat.h> + +#define LDO_RAMP_UP_UNIT_IN_CYCLES 64 /* 64 cycles per step */ +#define LDO_RAMP_UP_FREQ_IN_MHZ 24 /* cycle based on 24M OSC */ + +#define LDO_POWER_GATE 0x00 +#define LDO_FET_FULL_ON 0x1f + +#define BIT_WIDTH_MAX 32 + +#define ANATOP_REGULATOR_STEP 25000 + +struct anatop_regulator { + const char *name; + u32 control_reg; + u32 vol_bit_shift; + u32 vol_bit_width; + u32 min_bit_val; + u32 min_voltage; + u32 max_voltage; + u32 delay_reg; + u32 delay_bit_shift; + u32 delay_bit_width; +}; + +static u32 anatop_get_bits(struct udevice *dev, u32 addr, int bit_shift, + int bit_width) +{ + struct udevice *syscon; + struct regmap *regmap; + int err; + u32 val, mask; + + syscon = dev_get_parent(dev); + if (!syscon) { + dev_err(dev, "%s: unable to find syscon device\n", __func__); + return -ENODEV; + } + + regmap = syscon_get_regmap(syscon); + if (IS_ERR(regmap)) { + dev_err(dev, "%s: unable to find regmap (%ld)\n", __func__, + PTR_ERR(regmap)); + return PTR_ERR(regmap); + } + + if (bit_width == BIT_WIDTH_MAX) + mask = ~0; + else + mask = (1 << bit_width) - 1; + + err = regmap_read(regmap, addr, &val); + if (err) + return err; + + val = (val >> bit_shift) & mask; + + return val; +} + +static int anatop_set_bits(struct udevice *dev, u32 addr, int bit_shift, + int bit_width, u32 data) +{ + struct udevice *syscon; + struct regmap *regmap; + int err; + u32 val, mask; + + syscon = dev_get_parent(dev); + if (!syscon) { + dev_err(dev, "%s: unable to find syscon device\n", __func__); + return -ENODEV; + } + + regmap = syscon_get_regmap(syscon); + if (IS_ERR(regmap)) { + dev_err(dev, + "%s: unable to find regmap (%ld)\n", __func__, + PTR_ERR(regmap)); + return PTR_ERR(regmap); + } + + if (bit_width == 32) + mask = ~0; + else + mask = (1 << bit_width) - 1; + + err = regmap_read(regmap, addr, &val); + if (err) { + dev_err(dev, + "%s: cannot read reg (%d)\n", __func__, + err); + return err; + } + val = val & ~(mask << bit_shift); + err = regmap_write(regmap, addr, (data << bit_shift) | val); + if (err) { + dev_err(dev, + "%s: cannot wrie reg (%d)\n", __func__, + err); + return err; + } + + return 0; +} + +static u32 anatop_get_selector(struct udevice *dev, + const struct anatop_regulator *anatop_reg) +{ + u32 val = anatop_get_bits(dev, + anatop_reg->control_reg, + anatop_reg->vol_bit_shift, + anatop_reg->vol_bit_width); + + val = val - anatop_reg->min_bit_val; + + return val; +} + +static int anatop_set_voltage(struct udevice *dev, int uV) +{ + const struct anatop_regulator *anatop_reg = dev_get_plat(dev); + u32 val; + u32 sel; + int delayus = 0; + int ret = 0; + int uv = uV; + + dev_dbg(dev, "%s: uv %d, min %d, max %d\n", __func__, + uv, anatop_reg->min_voltage, + anatop_reg->max_voltage); + + if (uv < anatop_reg->min_voltage) + return -EINVAL; + + if (!anatop_reg->control_reg) + return -ENOTSUPP; + + sel = DIV_ROUND_UP(uv - anatop_reg->min_voltage, + ANATOP_REGULATOR_STEP); + if (sel * ANATOP_REGULATOR_STEP + anatop_reg->min_voltage > + anatop_reg->max_voltage) + return -EINVAL; + val = anatop_reg->min_bit_val + sel; + dev_dbg(dev, "%s: calculated val %d\n", __func__, val); + + /* check whether need to care about LDO ramp up speed */ + if (anatop_reg->delay_bit_width) { + /* + * the delay for LDO ramp up time is + * based on the register setting, we need + * to calculate how many steps LDO need to + * ramp up, and how much delay needed. (us) + */ + u32 old_sel; + u32 new_sel = sel; + + old_sel = anatop_get_selector(dev, anatop_reg); + + if (new_sel > old_sel) { + val = anatop_get_bits(dev, + anatop_reg->delay_reg, + anatop_reg->delay_bit_shift, + anatop_reg->delay_bit_width); + delayus = (new_sel - old_sel) * + (LDO_RAMP_UP_UNIT_IN_CYCLES << val) / + LDO_RAMP_UP_FREQ_IN_MHZ + 1; + } + } + + anatop_set_bits(dev, + anatop_reg->control_reg, + anatop_reg->vol_bit_shift, + anatop_reg->vol_bit_width, + val); + + if (delayus) + udelay(delayus); + + return ret; +} + +static int anatop_get_voltage(struct udevice *dev) +{ + const struct anatop_regulator *anatop_reg = dev_get_plat(dev); + u32 sel; + + sel = anatop_get_selector(dev, anatop_reg); + + return sel * ANATOP_REGULATOR_STEP + anatop_reg->min_voltage; +} + +static const struct dm_regulator_ops anatop_regulator_ops = { + .set_value = anatop_set_voltage, + .get_value = anatop_get_voltage, +}; + +static int anatop_regulator_probe(struct udevice *dev) +{ + struct anatop_regulator *sreg; + int ret = 0; + + sreg = dev_get_plat(dev); + if (!sreg) { + dev_err(dev, "failed to get plat data\n"); + return -ENOMEM; + } + + sreg->name = ofnode_read_string(dev_ofnode(dev), "regulator-name"); + if (!sreg->name) { + dev_err(dev, "failed to get a regulator-name\n"); + return -EINVAL; + } + + ret = ofnode_read_u32(dev_ofnode(dev), + "anatop-reg-offset", + &sreg->control_reg); + if (ret) { + dev_err(dev, "no anatop-reg-offset property set\n"); + return ret; + } + ret = ofnode_read_u32(dev_ofnode(dev), + "anatop-vol-bit-width", + &sreg->vol_bit_width); + if (ret) { + dev_err(dev, "no anatop-vol-bit-width property set\n"); + return ret; + } + ret = ofnode_read_u32(dev_ofnode(dev), + "anatop-vol-bit-shift", + &sreg->vol_bit_shift); + if (ret) { + dev_err(dev, "no anatop-vol-bit-shift property set\n"); + return ret; + } + ret = ofnode_read_u32(dev_ofnode(dev), + "anatop-min-bit-val", + &sreg->min_bit_val); + if (ret) { + dev_err(dev, "no anatop-min-bit-val property set\n"); + return ret; + } + ret = ofnode_read_u32(dev_ofnode(dev), + "anatop-min-voltage", + &sreg->min_voltage); + if (ret) { + dev_err(dev, "no anatop-min-voltage property set\n"); + return ret; + } + ret = ofnode_read_u32(dev_ofnode(dev), + "anatop-max-voltage", + &sreg->max_voltage); + if (ret) { + dev_err(dev, "no anatop-max-voltage property set\n"); + return ret; + } + + /* read LDO ramp up setting, only for core reg */ + ofnode_read_u32(dev_ofnode(dev), "anatop-delay-reg-offset", + &sreg->delay_reg); + ofnode_read_u32(dev_ofnode(dev), "anatop-delay-bit-width", + &sreg->delay_bit_width); + ofnode_read_u32(dev_ofnode(dev), "anatop-delay-bit-shift", + &sreg->delay_bit_shift); + + return 0; +} + +static const struct udevice_id of_anatop_regulator_match_tbl[] = { + { .compatible = "fsl,anatop-regulator", }, + { /* end */ } +}; + +U_BOOT_DRIVER(anatop_regulator) = { + .name = "anatop_regulator", + .id = UCLASS_REGULATOR, + .ops = &anatop_regulator_ops, + .of_match = of_anatop_regulator_match_tbl, + .plat_auto = sizeof(struct anatop_regulator), + .probe = anatop_regulator_probe, +}; -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] power: regulator: add driver for ANATOP regulator 2021-03-23 13:48 ` [PATCH v2 1/2] " Ying-Chun Liu @ 2021-03-23 15:06 ` Sean Anderson 2021-03-24 2:01 ` Ying-Chun Liu 0 siblings, 1 reply; 6+ messages in thread From: Sean Anderson @ 2021-03-23 15:06 UTC (permalink / raw) To: u-boot On 3/23/21 9:48 AM, Ying-Chun Liu wrote: > From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org> > > Anatop is an integrated regulator inside i.MX6 SoC. > There are 3 digital regulators which controls PU, CORE (ARM), and SOC. > And 3 analog regulators which controls 1P1, 2P5, 3P0 (USB). > This patch adds the Anatop regulator driver. +CC Peng Fan, Fabio Estevam I don't see support for vin-supply. I have made comments below showing the (relatively-minimal IMO) changes necessary to support it, along with some additional comments. > > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> > --- > v2: add functions for set selector and delay. Define ANATOP_REGULATOR_STEP > --- > drivers/power/regulator/Kconfig | 10 + > drivers/power/regulator/Makefile | 1 + > drivers/power/regulator/anatop_regulator.c | 299 +++++++++++++++++++++ > 3 files changed, 310 insertions(+) > create mode 100644 drivers/power/regulator/anatop_regulator.c > > diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig > index fbbea18c7d..1cd1f3f5ed 100644 > --- a/drivers/power/regulator/Kconfig > +++ b/drivers/power/regulator/Kconfig > @@ -312,6 +312,16 @@ config DM_REGULATOR_STPMIC1 > by the PMIC device. This driver is controlled by a device tree node > which includes voltage limits. > > +config DM_REGULATOR_ANATOP > + bool "Enable driver for ANATOP regulators" > + depends on DM_REGULATOR > + select REGMAP > + select SYSCON > + help > + Enable support for the Freescale i.MX on-chip ANATOP LDOs > + regulators. It is recommended that this option be enabled on > + i.MX6 platform. > + > config SPL_DM_REGULATOR_STPMIC1 > bool "Enable driver for STPMIC1 regulators in SPL" > depends on SPL_DM_REGULATOR && PMIC_STPMIC1 > diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile > index 9d58112dcb..e7198da911 100644 > --- a/drivers/power/regulator/Makefile > +++ b/drivers/power/regulator/Makefile > @@ -30,3 +30,4 @@ obj-$(CONFIG_DM_REGULATOR_TPS65910) += tps65910_regulator.o > obj-$(CONFIG_DM_REGULATOR_TPS62360) += tps62360_regulator.o > obj-$(CONFIG_$(SPL_)DM_REGULATOR_STPMIC1) += stpmic1.o > obj-$(CONFIG_DM_REGULATOR_TPS65941) += tps65941_regulator.o > +obj-$(CONFIG_$(SPL_)DM_REGULATOR_ANATOP) += anatop_regulator.o > diff --git a/drivers/power/regulator/anatop_regulator.c b/drivers/power/regulator/anatop_regulator.c > new file mode 100644 > index 0000000000..d824f4acc4 > --- /dev/null > +++ b/drivers/power/regulator/anatop_regulator.c > @@ -0,0 +1,299 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +// > +// Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved. > +// Copyright (C) 2021 Linaro Use C-style comments (/* */) for everything other than the initial SPDX line. > + > +#include <common.h> > +#include <errno.h> > +#include <dm.h> > +#include <log.h> > +#include <linux/delay.h> > +#include <linux/err.h> These should be below with the other linux headers. > +#include <power/pmic.h> > +#include <power/regulator.h> > +#include <regmap.h> > +#include <syscon.h> > +#include <linux/bitops.h> > +#include <linux/ioport.h> > +#include <dm/device-internal.h> > +#include <dm/device_compat.h> These should be above the linux headers. > + > +#define LDO_RAMP_UP_UNIT_IN_CYCLES 64 /* 64 cycles per step */ > +#define LDO_RAMP_UP_FREQ_IN_MHZ 24 /* cycle based on 24M OSC */ > + > +#define LDO_POWER_GATE 0x00 > +#define LDO_FET_FULL_ON 0x1f > + > +#define BIT_WIDTH_MAX 32 > + > +#define ANATOP_REGULATOR_STEP 25000 > + > +struct anatop_regulator { > + const char *name; struct udevice *supply; > + u32 control_reg; > + u32 vol_bit_shift; > + u32 vol_bit_width; > + u32 min_bit_val; > + u32 min_voltage; > + u32 max_voltage; > + u32 delay_reg; > + u32 delay_bit_shift; > + u32 delay_bit_width; > +}; > + > +static u32 anatop_get_bits(struct udevice *dev, u32 addr, int bit_shift, > + int bit_width) > +{ > + struct udevice *syscon; > + struct regmap *regmap; > + int err; > + u32 val, mask; > + > + syscon = dev_get_parent(dev); > + if (!syscon) { > + dev_err(dev, "%s: unable to find syscon device\n", __func__); Use dev_dbg, per [1]. This applies to most other logs in this file as well. > + return -ENODEV; Use ENOENT, per [1]. [1] https://lists.denx.de/pipermail/u-boot/2021-March/445207.html > + } > + > + regmap = syscon_get_regmap(syscon); > + if (IS_ERR(regmap)) { > + dev_err(dev, "%s: unable to find regmap (%ld)\n", __func__, > + PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } This should be done in probe(). > + > + if (bit_width == BIT_WIDTH_MAX) > + mask = ~0; > + else > + mask = (1 << bit_width) - 1; > + > + err = regmap_read(regmap, addr, &val); > + if (err) > + return err; > + > + val = (val >> bit_shift) & mask; > + > + return val; > +} > + > +static int anatop_set_bits(struct udevice *dev, u32 addr, int bit_shift, > + int bit_width, u32 data) > +{ > + struct udevice *syscon; > + struct regmap *regmap; > + int err; > + u32 val, mask; > + > + syscon = dev_get_parent(dev); > + if (!syscon) { > + dev_err(dev, "%s: unable to find syscon device\n", __func__); > + return -ENODEV; > + } > + > + regmap = syscon_get_regmap(syscon); > + if (IS_ERR(regmap)) { > + dev_err(dev, > + "%s: unable to find regmap (%ld)\n", __func__, > + PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } Again, should be done in probe. > + > + if (bit_width == 32) > + mask = ~0; > + else > + mask = (1 << bit_width) - 1; > + > + err = regmap_read(regmap, addr, &val); > + if (err) { > + dev_err(dev, > + "%s: cannot read reg (%d)\n", __func__, > + err); > + return err; > + } > + val = val & ~(mask << bit_shift); > + err = regmap_write(regmap, addr, (data << bit_shift) | val); > + if (err) { > + dev_err(dev, > + "%s: cannot wrie reg (%d)\n", __func__, > + err); > + return err; > + } > + > + return 0; > +} > + > +static u32 anatop_get_selector(struct udevice *dev, > + const struct anatop_regulator *anatop_reg) > +{ > + u32 val = anatop_get_bits(dev, > + anatop_reg->control_reg, > + anatop_reg->vol_bit_shift, > + anatop_reg->vol_bit_width); > + > + val = val - anatop_reg->min_bit_val; > + > + return val; > +} > + > +static int anatop_set_voltage(struct udevice *dev, int uV) > +{ > + const struct anatop_regulator *anatop_reg = dev_get_plat(dev); > + u32 val; > + u32 sel; > + int delayus = 0; > + int ret = 0; This does not need to be set to 0. > + int uv = uV; > + > + dev_dbg(dev, "%s: uv %d, min %d, max %d\n", __func__, > + uv, anatop_reg->min_voltage, > + anatop_reg->max_voltage); > + > + if (uv < anatop_reg->min_voltage) > + return -EINVAL; > + > + if (!anatop_reg->control_reg) > + return -ENOTSUPP; > + > + sel = DIV_ROUND_UP(uv - anatop_reg->min_voltage, > + ANATOP_REGULATOR_STEP); > + if (sel * ANATOP_REGULATOR_STEP + anatop_reg->min_voltage > > + anatop_reg->max_voltage) > + return -EINVAL; > + val = anatop_reg->min_bit_val + sel; > + dev_dbg(dev, "%s: calculated val %d\n", __func__, val); if (anatop_reg->supply) { ret = regulator_set_value(anatop_reg->supply, uV + 150000); if (ret) return ret; } > + > + /* check whether need to care about LDO ramp up speed */ Can this be handled by setting dm_regulator_uclass_platdata.ramp_delay? > + if (anatop_reg->delay_bit_width) { > + /* > + * the delay for LDO ramp up time is > + * based on the register setting, we need > + * to calculate how many steps LDO need to > + * ramp up, and how much delay needed. (us) > + */ > + u32 old_sel; > + u32 new_sel = sel; > + > + old_sel = anatop_get_selector(dev, anatop_reg); > + > + if (new_sel > old_sel) { > + val = anatop_get_bits(dev, > + anatop_reg->delay_reg, > + anatop_reg->delay_bit_shift, > + anatop_reg->delay_bit_width); > + delayus = (new_sel - old_sel) * > + (LDO_RAMP_UP_UNIT_IN_CYCLES << val) / > + LDO_RAMP_UP_FREQ_IN_MHZ + 1; > + } > + } > + > + anatop_set_bits(dev, > + anatop_reg->control_reg, > + anatop_reg->vol_bit_shift, > + anatop_reg->vol_bit_width, > + val); > + > + if (delayus) > + udelay(delayus); > + > + return ret; > +} > + > +static int anatop_get_voltage(struct udevice *dev) > +{ > + const struct anatop_regulator *anatop_reg = dev_get_plat(dev); > + u32 sel; > + > + sel = anatop_get_selector(dev, anatop_reg); > + > + return sel * ANATOP_REGULATOR_STEP + anatop_reg->min_voltage; > +} > + > +static const struct dm_regulator_ops anatop_regulator_ops = { > + .set_value = anatop_set_voltage, > + .get_value = anatop_get_voltage, > +}; > + > +static int anatop_regulator_probe(struct udevice *dev) > +{ > + struct anatop_regulator *sreg; Please use consistent names for this structure. > + int ret = 0; > + > + sreg = dev_get_plat(dev); > + if (!sreg) { > + dev_err(dev, "failed to get plat data\n"); The driver model checks for this. Do not check it again. > + return -ENOMEM; > + } > + > + sreg->name = ofnode_read_string(dev_ofnode(dev), "regulator-name"); > + if (!sreg->name) { > + dev_err(dev, "failed to get a regulator-name\n"); > + return -EINVAL; > + } ret = device_get_supply_regulator(dev, "vin-supply", &sreg->supply) if (ret) return ret; ret = regulator_set_enable(sreg->supply, true); if (ret) return ret; > + > + ret = ofnode_read_u32(dev_ofnode(dev), use dev_read_u32. > + "anatop-reg-offset", > + &sreg->control_reg); > + if (ret) { > + dev_err(dev, "no anatop-reg-offset property set\n"); > + return ret; > + } Perhaps consider something like ret = dev_read_u32(dev, "some-prop", &sreg->some_prop); if (ret) return log_msg_ret("some-prop", ret); > + ret = ofnode_read_u32(dev_ofnode(dev), > + "anatop-vol-bit-width", > + &sreg->vol_bit_width); > + if (ret) { > + dev_err(dev, "no anatop-vol-bit-width property set\n"); > + return ret; > + } > + ret = ofnode_read_u32(dev_ofnode(dev), > + "anatop-vol-bit-shift", > + &sreg->vol_bit_shift); > + if (ret) { > + dev_err(dev, "no anatop-vol-bit-shift property set\n"); > + return ret; > + } > + ret = ofnode_read_u32(dev_ofnode(dev), > + "anatop-min-bit-val", > + &sreg->min_bit_val); > + if (ret) { > + dev_err(dev, "no anatop-min-bit-val property set\n"); > + return ret; > + } > + ret = ofnode_read_u32(dev_ofnode(dev), > + "anatop-min-voltage", > + &sreg->min_voltage); > + if (ret) { > + dev_err(dev, "no anatop-min-voltage property set\n"); > + return ret; > + } > + ret = ofnode_read_u32(dev_ofnode(dev), > + "anatop-max-voltage", > + &sreg->max_voltage); > + if (ret) { > + dev_err(dev, "no anatop-max-voltage property set\n"); > + return ret; > + } > + > + /* read LDO ramp up setting, only for core reg */ > + ofnode_read_u32(dev_ofnode(dev), "anatop-delay-reg-offset", > + &sreg->delay_reg); > + ofnode_read_u32(dev_ofnode(dev), "anatop-delay-bit-width", > + &sreg->delay_bit_width); > + ofnode_read_u32(dev_ofnode(dev), "anatop-delay-bit-shift", > + &sreg->delay_bit_shift); > + > + return 0; > +} > + > +static const struct udevice_id of_anatop_regulator_match_tbl[] = { > + { .compatible = "fsl,anatop-regulator", }, > + { /* end */ } > +}; > + > +U_BOOT_DRIVER(anatop_regulator) = { > + .name = "anatop_regulator", > + .id = UCLASS_REGULATOR, > + .ops = &anatop_regulator_ops, > + .of_match = of_anatop_regulator_match_tbl, > + .plat_auto = sizeof(struct anatop_regulator), > + .probe = anatop_regulator_probe, > +}; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] power: regulator: add driver for ANATOP regulator 2021-03-23 15:06 ` Sean Anderson @ 2021-03-24 2:01 ` Ying-Chun Liu 2021-03-25 14:20 ` Sean Anderson 0 siblings, 1 reply; 6+ messages in thread From: Ying-Chun Liu @ 2021-03-24 2:01 UTC (permalink / raw) To: u-boot Hi Sean, Thanks for the review. I fix almost of the issues. Will upload the v3 soon. Still have some questions. Sean Anderson ? 2021/3/23 ??11:06 ??: > > > > ????if (anatop_reg->supply) { > ??????? ret = regulator_set_value(anatop_reg->supply, uV + 150000); > ??????? if (ret) > ??????????? return ret; > ????} > What is 150000? Is it the min_dropout_uV? Should I set it to 125000 instead? > > > ???? > ????ret = regulator_set_enable(sreg->supply, true); > ????if (ret) > ??????? return ret; > Since vin-supply is optional, I change it to ??????? ret = device_get_supply_regulator(dev, "vin-supply", ????????????????????????????????????????? &anatop_reg->supply); ??????? if (!ret) { ??????????????? ret = regulator_set_enable(anatop_reg->supply, true); ??????????????? if (ret) ??????????????????????? return ret; ??????? } Is this ok? > > + > > +??? ret = ofnode_read_u32(dev_ofnode(dev), > > > +U_BOOT_DRIVER(anatop_regulator) = { > > +??? .name = "anatop_regulator", > > +??? .id = UCLASS_REGULATOR, > > +??? .ops = &anatop_regulator_ops, > > +??? .of_match = of_anatop_regulator_match_tbl, > > +??? .plat_auto = sizeof(struct anatop_regulator), > > +??? .probe = anatop_regulator_probe, > > +}; > > Yours, Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] power: regulator: add driver for ANATOP regulator 2021-03-24 2:01 ` Ying-Chun Liu @ 2021-03-25 14:20 ` Sean Anderson 0 siblings, 0 replies; 6+ messages in thread From: Sean Anderson @ 2021-03-25 14:20 UTC (permalink / raw) To: u-boot On 3/23/21 10:01 PM, Ying-Chun Liu (PaulLiu) wrote: > Hi Sean, > > Thanks for the review. I fix almost of the issues. Will upload the v3 soon. > > Still have some questions. > > > Sean Anderson ? 2021/3/23 ??11:06 ??: >> >> >> >> ????if (anatop_reg->supply) { >> ??????? ret = regulator_set_value(anatop_reg->supply, uV + 150000); >> ??????? if (ret) >> ??????????? return ret; >> ????} >> > > What is 150000? Is it the min_dropout_uV? Yes. > > Should I set it to 125000 instead? Yes. > > > >> >> >> >> ????ret = regulator_set_enable(sreg->supply, true); >> ????if (ret) >> ??????? return ret; >> > > Since vin-supply is optional, I change it to > > ??????? ret = device_get_supply_regulator(dev, "vin-supply", > ????????????????????????????????????????? &anatop_reg->supply); > ??????? if (!ret) { > ??????????????? ret = regulator_set_enable(anatop_reg->supply, true); > ??????????????? if (ret) > ??????????????????????? return ret; > ??????? } > > Is this ok? Yes. --Sean > >>> + >>> +??? ret = ofnode_read_u32(dev_ofnode(dev), >> >>> +U_BOOT_DRIVER(anatop_regulator) = { >>> +??? .name = "anatop_regulator", >>> +??? .id = UCLASS_REGULATOR, >>> +??? .ops = &anatop_regulator_ops, >>> +??? .of_match = of_anatop_regulator_match_tbl, >>> +??? .plat_auto = sizeof(struct anatop_regulator), >>> +??? .probe = anatop_regulator_probe, >>> +}; >>> > > > Yours, > Paul > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] doc: device-tree-bindings: regulator: anatop regulator 2021-03-23 13:48 [PATCH v2 0/2] power: regulator: add driver for ANATOP regulator Ying-Chun Liu 2021-03-23 13:48 ` [PATCH v2 1/2] " Ying-Chun Liu @ 2021-03-23 13:48 ` Ying-Chun Liu 1 sibling, 0 replies; 6+ messages in thread From: Ying-Chun Liu @ 2021-03-23 13:48 UTC (permalink / raw) To: u-boot From: "Ying-Chun Liu (PaulLiu)" <paul.liu@linaro.org> Document the bindings for fsl,anatop-regulator Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> --- .../regulator/fsl,anatop-regulator.txt | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt diff --git a/doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt b/doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt new file mode 100644 index 0000000000..2a60e4941b --- /dev/null +++ b/doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt @@ -0,0 +1,45 @@ +ANATOP REGULATOR + +Anatop is an integrated regulator inside i.MX6 SoC. + +Required properties: +- compatible: Must be "fsl,anatop-regulator". +- regulator-name: Name of the regulator +- anatop-reg-offset: u32 value representing the anatop MFD register offset. +- anatop-vol-bit-shift: u32 value representing the bit shift for the register. +- anatop-vol-bit-width: u32 value representing the number of bits used in the + register. +- anatop-min-bit-val: u32 value representing the minimum value of this + register. +- anatop-min-voltage: u32 value representing the minimum voltage of this + regulator. +- anatop-max-voltage: u32 value representing the maximum voltage of this + regulator. + +Optional properties: +- anatop-delay-reg-offset: u32 value representing the anatop MFD step time + register offset. +- anatop-delay-bit-shift: u32 value representing the bit shift for the step + time register. +- anatop-delay-bit-width: u32 value representing the number of bits used in + the step time register. +- anatop-enable-bit: u32 value representing regulator enable bit offset. +- vin-supply: input supply phandle. + +Example: + regulator-vddpu { + compatible = "fsl,anatop-regulator"; + regulator-name = "vddpu"; + regulator-min-microvolt = <725000>; + regulator-max-microvolt = <1300000>; + regulator-always-on; + anatop-reg-offset = <0x140>; + anatop-vol-bit-shift = <9>; + anatop-vol-bit-width = <5>; + anatop-delay-reg-offset = <0x170>; + anatop-delay-bit-shift = <24>; + anatop-delay-bit-width = <2>; + anatop-min-bit-val = <1>; + anatop-min-voltage = <725000>; + anatop-max-voltage = <1300000>; + }; -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-25 14:20 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-23 13:48 [PATCH v2 0/2] power: regulator: add driver for ANATOP regulator Ying-Chun Liu 2021-03-23 13:48 ` [PATCH v2 1/2] " Ying-Chun Liu 2021-03-23 15:06 ` Sean Anderson 2021-03-24 2:01 ` Ying-Chun Liu 2021-03-25 14:20 ` Sean Anderson 2021-03-23 13:48 ` [PATCH v2 2/2] doc: device-tree-bindings: regulator: anatop regulator Ying-Chun Liu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.