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

Message ID 20210325184407.37915-2-grandpaul@gmail.com
State New
Headers show
Series
  • power: regulator: add driver for ANATOP regulator
Related show

Commit Message

Ying-Chun Liu March 25, 2021, 6:44 p.m.
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>

Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <sean.anderson@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

-- 
2.30.2

Comments

Sean Anderson March 25, 2021, 8:32 p.m. | #1
On 3/25/21 2:44 PM, 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>

 > Cc: Fabio Estevam <fabio.estevam@nxp.com>

 > Cc: Jaehoon Chung <jh80.chung@samsung.com>

 > Cc: Peng Fan <peng.fan@nxp.com>

 > Cc: Sean Anderson <sean.anderson@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,

 > +};

 >
Jaehoon Chung March 25, 2021, 10:46 p.m. | #2
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@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>

>> Cc: Fabio Estevam <fabio.estevam@nxp.com>

>> Cc: Jaehoon Chung <jh80.chung@samsung.com>

>> Cc: Peng Fan <peng.fan@nxp.com>

>> Cc: Sean Anderson <sean.anderson@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,

>> +};

>>

>

Patch

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..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);
+	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__,
+			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;
+
+	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) {
+		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,
+};