| Message ID | 20210307181849.83627-2-grandpaul@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | ANATOP regulator driver | expand |
Dear Ying-Chun On 3/8/21 3:18 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. > > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> > --- > drivers/power/regulator/Kconfig | 10 + > drivers/power/regulator/Makefile | 1 + > drivers/power/regulator/anatop_regulator.c | 289 +++++++++++++++++++++ > 3 files changed, 300 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..2bb5cdbac5 > --- /dev/null > +++ b/drivers/power/regulator/anatop_regulator.c > @@ -0,0 +1,289 @@ > +// 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> > +#include <dm/read.h> Well, i think that it can be removed more than now. > + > +#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 > + > +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 err; > + } > + > + 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) Use macro instead of 32, plz. > + 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; > +} > + > +void anatop_set_bits(struct udevice *dev, u32 addr, int bit_shift, > + int bit_width, u32 data) static? doesn't it need to return int type? If there is a somehting failed, it seems that it needs to pass the error number. (get_bits is returend to error..but set_bits doesn't return. It's strange.) And How about passing the struct anatop_regulator instead of each values? anatop_get/set_bits(struct anatop_regulator *regulator) { .. } > +{ > + 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; > + } > + > + regmap = syscon_get_regmap(syscon); > + if (IS_ERR(regmap)) { > + dev_err(dev, > + "%s: unable to find regmap (%ld)\n", __func__, > + PTR_ERR(regmap)); > + return; > + } > + > + 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; > + } > + 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; > + } > +} > + > +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 = uV; Not need to assign at here. Assign to above. 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, 25000); What is 25000? If it's min value, use MACRO, plz. > + if (sel * 25000 + 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_bits(dev, > + anatop_reg->control_reg, > + anatop_reg->vol_bit_shift, > + anatop_reg->vol_bit_width); > + old_sel = old_sel - anatop_reg->min_bit_val; > + > + 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 val; > + u32 sel; > + > + val = anatop_get_bits(dev, > + anatop_reg->control_reg, > + anatop_reg->vol_bit_shift, > + anatop_reg->vol_bit_width); > + sel = val - anatop_reg->min_bit_val; > + > + return sel * 25000 + 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); I don't know why anatop-min-voltage is need? Doesn't it use "regulator-min-microvolt"? > + 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); Ditto. Best Regards, Jaehoon Chung > + 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, > +}; >
Hi Jaehoon, Thanks for the review. On Mon, 8 Mar 2021 at 07:03, Jaehoon Chung <jh80.chung@samsung.com> wrote: > Dear Ying-Chun > > On 3/8/21 3:18 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. > > > > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> > > --- > > drivers/power/regulator/Kconfig | 10 + > > drivers/power/regulator/Makefile | 1 + > > drivers/power/regulator/anatop_regulator.c | 289 +++++++++++++++++++++ > > 3 files changed, 300 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..2bb5cdbac5 > > --- /dev/null > > +++ b/drivers/power/regulator/anatop_regulator.c > > @@ -0,0 +1,289 @@ > > +// 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> > > +#include <dm/read.h> > > Well, i think that it can be removed more than now. > > Will fix it in v2. > > + > > +#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 > > + > > +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 err; > > + } > > + > > + 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) > > Use macro instead of 32, plz. > > Will fix it in v2. > > + 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; > > +} > > + > > +void anatop_set_bits(struct udevice *dev, u32 addr, int bit_shift, > > + int bit_width, u32 data) > > static? > doesn't it need to return int type? > If there is a somehting failed, it seems that it needs to pass the error > number. > (get_bits is returend to error..but set_bits doesn't return. It's strange.) > > Will fix it in v2. > And How about passing the struct anatop_regulator instead of each values? > anatop_get/set_bits(struct anatop_regulator *regulator) { > .. > } > > Will fix it in v2. > > +{ > > + 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; > > + } > > + > > + regmap = syscon_get_regmap(syscon); > > + if (IS_ERR(regmap)) { > > + dev_err(dev, > > + "%s: unable to find regmap (%ld)\n", __func__, > > + PTR_ERR(regmap)); > > + return; > > + } > > + > > + 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; > > + } > > + 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; > > + } > > +} > > + > > +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 = uV; > > Not need to assign at here. Assign to above. > int uv = uV; > > Will fix it in v2. > > > + 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, 25000); > > What is 25000? If it's min value, use MACRO, plz. > > It is "STEP" or "RESOLUTION". Sorry for my bad English. 25000 uV is the anatop regulator's "STEP" or "RESOLUTION". So the uV is register value * 25000 uV + min uV. I'll change it to use MACRO with a proper name to avoid confusion. > > + if (sel * 25000 + 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_bits(dev, > > + anatop_reg->control_reg, > > + anatop_reg->vol_bit_shift, > > + anatop_reg->vol_bit_width); > > + old_sel = old_sel - anatop_reg->min_bit_val; > > + > > + 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 val; > > + u32 sel; > > + > > + val = anatop_get_bits(dev, > > + anatop_reg->control_reg, > > + anatop_reg->vol_bit_shift, > > + anatop_reg->vol_bit_width); > > + sel = val - anatop_reg->min_bit_val; > > + > > + return sel * 25000 + 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); > > I don't know why anatop-min-voltage is need? > Doesn't it use "regulator-min-microvolt"? > > It is needed. It is because the anatop-min-voltage is used to calculate the register value of anatop. But regulator-min-microvolt might not be equal to anatop-min-voltage. Because regulator-min-microvolt describes the device that consumes the regulator. So the device might need higher min-microvolt based on the hardware design and will be described in device tree settings. But the value to be written in the anatop register must be based on anatop-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); > > Ditto. > > Best Regards, > Jaehoon Chung > > > + 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, > > +}; > > > > Yours, Paul
On 3/8/21 10:04 PM, Paul Liu wrote: > Hi Jaehoon, > > Thanks for the review. > > On Mon, 8 Mar 2021 at 07:03, Jaehoon Chung <jh80.chung@samsung.com> wrote: > >> Dear Ying-Chun >> >> On 3/8/21 3:18 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. >>> >>> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org> >>> --- >>> drivers/power/regulator/Kconfig | 10 + >>> drivers/power/regulator/Makefile | 1 + >>> drivers/power/regulator/anatop_regulator.c | 289 +++++++++++++++++++++ >>> 3 files changed, 300 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..2bb5cdbac5 >>> --- /dev/null >>> +++ b/drivers/power/regulator/anatop_regulator.c >>> @@ -0,0 +1,289 @@ >>> +// 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> >>> +#include <dm/read.h> >> >> Well, i think that it can be removed more than now. >> >> > Will fix it in v2. > > >>> + >>> +#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 >>> + >>> +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 err; >>> + } >>> + >>> + 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) >> >> Use macro instead of 32, plz. >> >> > Will fix it in v2. > > >>> + 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; >>> +} >>> + >>> +void anatop_set_bits(struct udevice *dev, u32 addr, int bit_shift, >>> + int bit_width, u32 data) >> >> static? >> doesn't it need to return int type? >> If there is a somehting failed, it seems that it needs to pass the error >> number. >> (get_bits is returend to error..but set_bits doesn't return. It's strange.) >> >> > Will fix it in v2. > > >> And How about passing the struct anatop_regulator instead of each values? >> anatop_get/set_bits(struct anatop_regulator *regulator) { >> .. >> } >> >> > Will fix it in v2. > > >>> +{ >>> + 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; >>> + } >>> + >>> + regmap = syscon_get_regmap(syscon); >>> + if (IS_ERR(regmap)) { >>> + dev_err(dev, >>> + "%s: unable to find regmap (%ld)\n", __func__, >>> + PTR_ERR(regmap)); >>> + return; >>> + } >>> + >>> + 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; >>> + } >>> + 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; >>> + } >>> +} >>> + >>> +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 = uV; >> >> Not need to assign at here. Assign to above. >> int uv = uV; >> >> Will fix it in v2. > >> >>> + 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, 25000); >> >> What is 25000? If it's min value, use MACRO, plz. >> >> > It is "STEP" or "RESOLUTION". Sorry for my bad English. 25000 uV is the > anatop regulator's "STEP" or "RESOLUTION". So the uV is register value * > 25000 uV + min uV. > I'll change it to use MACRO with a proper name to avoid confusion. > > > >>> + if (sel * 25000 + 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_bits(dev, >>> + anatop_reg->control_reg, >>> + anatop_reg->vol_bit_shift, >>> + anatop_reg->vol_bit_width); >>> + old_sel = old_sel - anatop_reg->min_bit_val; >>> + >>> + 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 val; >>> + u32 sel; >>> + >>> + val = anatop_get_bits(dev, >>> + anatop_reg->control_reg, >>> + anatop_reg->vol_bit_shift, >>> + anatop_reg->vol_bit_width); >>> + sel = val - anatop_reg->min_bit_val; >>> + >>> + return sel * 25000 + 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); >> >> I don't know why anatop-min-voltage is need? >> Doesn't it use "regulator-min-microvolt"? >> >> > It is needed. It is because the anatop-min-voltage is used to calculate the > register value of anatop. > But regulator-min-microvolt might not be equal to anatop-min-voltage. > Because regulator-min-microvolt describes the device that consumes the > regulator. > So the device might need higher min-microvolt based on the hardware design > and will be described in device tree settings. > But the value to be written in the anatop register must be based > on anatop-min-voltage. Thanks for explanation. I guessed that it can be used regulator-min-microvolt after checked your [PATCH 2/2]. Because there are same value with regulator-microvolt and antop-min-voltage. Best Regards, Jaehoon Chung > > >> >>> + 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); >> >> Ditto. >> >> Best Regards, >> Jaehoon Chung >> >>> + 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, >>> +}; >>> >> >> Yours, > Paul >
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..2bb5cdbac5 --- /dev/null +++ b/drivers/power/regulator/anatop_regulator.c @@ -0,0 +1,289 @@ +// 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> +#include <dm/read.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 + +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 err; + } + + 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) + return err; + + val = (val >> bit_shift) & mask; + + return val; +} + +void 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; + } + + regmap = syscon_get_regmap(syscon); + if (IS_ERR(regmap)) { + dev_err(dev, + "%s: unable to find regmap (%ld)\n", __func__, + PTR_ERR(regmap)); + return; + } + + 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; + } + 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; + } +} + +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 = 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, 25000); + if (sel * 25000 + 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_bits(dev, + anatop_reg->control_reg, + anatop_reg->vol_bit_shift, + anatop_reg->vol_bit_width); + old_sel = old_sel - anatop_reg->min_bit_val; + + 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 val; + u32 sel; + + val = anatop_get_bits(dev, + anatop_reg->control_reg, + anatop_reg->vol_bit_shift, + anatop_reg->vol_bit_width); + sel = val - anatop_reg->min_bit_val; + + return sel * 25000 + 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, +};