[PATCH v3 1/2] power: regulator: add driver for ANATOP regulator

Sean Anderson sean.anderson at seco.com
Thu Mar 25 21:32:12 CET 2021



On 3/25/21 2:44 PM, Ying-Chun Liu wrote:
 > From: "Ying-Chun Liu (PaulLiu)" <paul.liu at 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 at linaro.org>
 > Cc: Fabio Estevam <fabio.estevam at nxp.com>
 > Cc: Jaehoon Chung <jh80.chung at samsung.com>
 > Cc: Peng Fan <peng.fan at nxp.com>
 > Cc: Sean Anderson <sean.anderson at seco.com>
 > ---
 > v2: add functions for set selector and delay. Define ANATOP_REGULATOR_STEP
 > v3: add vin-supply. move regmap retrival to probe
 > ---
 >   drivers/power/regulator/Kconfig            |  10 +
 >   drivers/power/regulator/Makefile           |   1 +
 >   drivers/power/regulator/anatop_regulator.c | 287 +++++++++++++++++++++
 >   3 files changed, 298 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

nit: LDO

 > +	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..af5f9d2a2b
 > --- /dev/null
 > +++ b/drivers/power/regulator/anatop_regulator.c
 > @@ -0,0 +1,287 @@
 > +// SPDX-License-Identifier: GPL-2.0+
 > +/*
 > + * Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved.
 > + * Copyright (C) 2021 Linaro
 > + */
 > +
 > +#include <common.h>
 > +#include <dm.h>
 > +#include <errno.h>
 > +#include <log.h>
 > +#include <regmap.h>
 > +#include <syscon.h>
 > +#include <dm/device-internal.h>
 > +#include <dm/device_compat.h>
 > +#include <linux/bitops.h>
 > +#include <linux/delay.h>
 > +#include <linux/err.h>
 > +#include <linux/ioport.h>
 > +#include <power/pmic.h>
 > +#include <power/regulator.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
 > +#define MIN_DROPOUT_UV			125000
 > +
 > +struct anatop_regulator {
 > +	const char *name;
 > +	struct regmap *regmap;
 > +	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)
 > +{
 > +	const struct anatop_regulator *anatop_reg = dev_get_plat(dev);
 > +	struct regmap *regmap;
 > +	int err;
 > +	u32 val, mask;
 > +
 > +	regmap = anatop_reg->regmap;
 > +
 > +	if (bit_width == BIT_WIDTH_MAX)
 > +		mask = ~0;
 > +	else
 > +		mask = (1 << bit_width) - 1;
 > +
 > +	err = regmap_read(regmap, addr, &val);

Just use anatop_reg->regmap here.

 > +	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)
 > +{
 > +	const struct anatop_regulator *anatop_reg = dev_get_plat(dev);
 > +	struct regmap *regmap;
 > +	int err;
 > +	u32 val, mask;
 > +
 > +	regmap = anatop_reg->regmap;
 > +
 > +	if (bit_width == 32)
 > +		mask = ~0;
 > +	else
 > +		mask = (1 << bit_width) - 1;
 > +
 > +	err = regmap_read(regmap, addr, &val);
 > +	if (err) {
 > +		dev_dbg(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_dbg(dev,
 > +			"%s: cannot wrie reg (%d)\n", __func__,

nit: write

You do not need to add the function name, since you can enable
CONFIG_LOGF_FUNC instead. This goes for the rest of your debug messages
as well.

 > +			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 ret;
 > +
 > +	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;

Use -ENOSYS to match what regulator_set_value returns.

 > +
 > +	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 + MIN_DROPOUT_UV);
 > +		if (ret)
 > +			return ret;
 > +	}
 > +
 > +	ret = anatop_set_bits(dev,
 > +			      anatop_reg->control_reg,
 > +			      anatop_reg->vol_bit_shift,
 > +			      anatop_reg->vol_bit_width,
 > +			      val);
 > +
 > +	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 *anatop_reg;
 > +	struct dm_regulator_uclass_plat *uc_pdata;
 > +	struct udevice *syscon;
 > +	int ret = 0;
 > +	u32 val;
 > +
 > +	anatop_reg = dev_get_plat(dev);
 > +	uc_pdata = dev_get_uclass_plat(dev);
 > +
 > +	anatop_reg->name = ofnode_read_string(dev_ofnode(dev),
 > +					      "regulator-name");
 > +	if (!anatop_reg->name)
 > +		return log_msg_ret("regulator-name", -EINVAL);
 > +
 > +	ret = device_get_supply_regulator(dev, "vin-supply",
 > +					  &anatop_reg->supply);
 > +	if (!ret) {

perhaps

	if (ret != -ENODEV) {
		if (ret)
			return log_msg_ret("get vin-supply", ret);
		ret = regulator_set_enable(...);
		/* etc */
	}

It would be nice to find out if the supply exists but could not be
probed (since that is likely a programmer error)

 > +		ret = regulator_set_enable(anatop_reg->supply, true);
 > +		if (ret)
 > +			return ret;
 > +	}
 > +
 > +	ret = dev_read_u32(dev,
 > +			   "anatop-reg-offset",
 > +			   &anatop_reg->control_reg);
 > +	if (ret)
 > +		return log_msg_ret("anatop-reg-offset", ret);
 > +
 > +	ret = dev_read_u32(dev,
 > +			   "anatop-vol-bit-width",
 > +			   &anatop_reg->vol_bit_width);
 > +	if (ret)
 > +		return log_msg_ret("anatop-vol-bit-width", ret);
 > +
 > +	ret = dev_read_u32(dev,
 > +			   "anatop-vol-bit-shift",
 > +			   &anatop_reg->vol_bit_shift);
 > +	if (ret)
 > +		return log_msg_ret("anatop-vol-bit-shift", ret);
 > +
 > +	ret = dev_read_u32(dev,
 > +			   "anatop-min-bit-val",
 > +			   &anatop_reg->min_bit_val);
 > +	if (ret)
 > +		return log_msg_ret("anatop-min-bit-val", ret);
 > +
 > +	ret = dev_read_u32(dev,
 > +			   "anatop-min-voltage",
 > +			   &anatop_reg->min_voltage);
 > +	if (ret)
 > +		return log_msg_ret("anatop-min-voltage", ret);
 > +
 > +	ret = dev_read_u32(dev,
 > +			   "anatop-max-voltage",
 > +			   &anatop_reg->max_voltage);
 > +	if (ret)
 > +		return log_msg_ret("anatop-max-voltage", ret);
 > +
 > +	/* read LDO ramp up setting, only for core reg */
 > +	dev_read_u32(dev, "anatop-delay-reg-offset",
 > +		     &anatop_reg->delay_reg);
 > +	dev_read_u32(dev, "anatop-delay-bit-width",
 > +		     &anatop_reg->delay_bit_width);
 > +	dev_read_u32(dev, "anatop-delay-bit-shift",
 > +		     &anatop_reg->delay_bit_shift);
 > +
 > +	syscon = dev_get_parent(dev);
 > +	if (!syscon) {
 > +		dev_dbg(dev, "%s: unable to find syscon device\n", __func__);
 > +		return -ENOENT;
 > +	}
 > +
 > +	anatop_reg->regmap = syscon_get_regmap(syscon);
 > +	if (IS_ERR(anatop_reg->regmap)) {
 > +		dev_dbg(dev, "%s: unable to find regmap (%ld)\n", __func__,
 > +			PTR_ERR(anatop_reg->regmap));
 > +		return -ENOENT;
 > +	}
 > +
 > +	/* 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)
 > +		 */
 > +		val = anatop_get_bits(dev,
 > +				      anatop_reg->delay_reg,
 > +				      anatop_reg->delay_bit_shift,
 > +				      anatop_reg->delay_bit_width);
 > +		uc_pdata->ramp_delay = (LDO_RAMP_UP_UNIT_IN_CYCLES << val)
 > +			/ LDO_RAMP_UP_FREQ_IN_MHZ + 1;
 > +	}
 > +
 > +	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,
 > +};
 >


More information about the U-Boot mailing list