[PATCH v3 1/2] power: regulator: add driver for ANATOP regulator
Jaehoon Chung
jh80.chung at samsung.com
Thu Mar 25 23:46:55 CET 2021
On 3/26/21 5:32 AM, Sean Anderson wrote:
>
>
> 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.
If prevent to access to wrong register, also need to check other location where it's used.
Best Regards,
Jaehoon Chung
>
>> +
>> + 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