Re: [PATCH v2 4/8] power: regulator: Add AB8500 AUX3 support

On Mon, Aug 17, 2026 at 11:31:53AM +0200, Linus Walleij wrote:
> Add regulator support for LDO AUX3 on AB8500 and AB8505 PMICs.
> AUX3 supplies the removable SD card on the upstream Ux500 Samsung
> device trees, and both PMIC variants use the same control registers.
> 
> Imply the regulator core for ARCH_U8500 so the driver can instantiate
> from the upstream device trees.
> 
> Signed-off-by: Linus Walleij <[email protected]>
> ---
>  MAINTAINERS                      |   1 +
>  arch/arm/Kconfig                 |   2 +
>  drivers/power/pmic/ab8500.c      |   1 +
>  drivers/power/regulator/Kconfig  |   7 +++
>  drivers/power/regulator/Makefile |   1 +
>  drivers/power/regulator/ab8500.c | 111 +++++++++++++++++++++++++++++++++++++++
>  6 files changed, 123 insertions(+)
> 
> [...]
> diff --git a/drivers/power/regulator/ab8500.c b/drivers/power/regulator/ab8500.c
> new file mode 100644
> index 000000000000..03beb0834d53
> --- /dev/null
> +++ b/drivers/power/regulator/ab8500.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* ST-Ericsson AB8500/AB8505 LDO AUX3 regulator */
> +
> +#include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <linux/errno.h>
> +#include <power/ab8500.h>
> +#include <power/pmic.h>
> +#include <power/regulator.h>
> +
> +#define AB8500_VAUX3_REGU	AB8500_REGU_CTRL2(0x0a)
> +#define AB8500_VAUX3_SEL	AB8500_REGU_CTRL2(0x21)
> +#define AB8500_VAUX3_EN_MASK	GENMASK(1, 0)
> +#define AB8500_VAUX3_EN		BIT(0)

Nitpick: Can you call this _REGU_EN maybe to make it clear that this
belongs to the REGU register?

> +#define AB8500_VAUX3_SEL_MASK	GENMASK(2, 0)
> +
> +static const int ab8500_vaux3_voltages[] = {
> +	1200000, 1500000, 1800000, 2100000,
> +	2500000, 2750000, 2790000, 2910000,
> +};
> +
> +static struct udevice *ab8500_regulator_pmic(struct udevice *dev)
> +{
> +	return dev->parent->parent;
> +}
> +
> +static int ab8500_regulator_get_value(struct udevice *dev)
> +{
> +	int ret;
> +
> +	ret = pmic_reg_read(ab8500_regulator_pmic(dev), AB8500_VAUX3_SEL);
> +	if (ret < 0)
> +		return ret;
> +	ret &= AB8500_VAUX3_SEL_MASK;
> +
> +	return ab8500_vaux3_voltages[ret];
> +}
> +
> +static int ab8500_regulator_set_value(struct udevice *dev, int uV)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ab8500_vaux3_voltages); i++) {
> +		if (ab8500_vaux3_voltages[i] == uV)
> +			return pmic_clrsetbits(ab8500_regulator_pmic(dev),
> +					       AB8500_VAUX3_SEL,
> +					       AB8500_VAUX3_SEL_MASK, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int ab8500_regulator_get_enable(struct udevice *dev)
> +{
> +	int ret;
> +
> +	ret = pmic_reg_read(ab8500_regulator_pmic(dev), AB8500_VAUX3_REGU);
> +	if (ret < 0)
> +		return ret;
> +
> +	return (ret & AB8500_VAUX3_EN_MASK) == AB8500_VAUX3_EN;
> +}
> +
> +static int ab8500_regulator_set_enable(struct udevice *dev, bool enable)
> +{
> +	return pmic_clrsetbits(ab8500_regulator_pmic(dev), AB8500_VAUX3_REGU,
> +			       AB8500_VAUX3_EN_MASK,
> +			       enable ? AB8500_VAUX3_EN : 0);
> +}
> +
> +static const struct dm_regulator_ops ab8500_regulator_ops = {
> +	.get_value = ab8500_regulator_get_value,
> +	.set_value = ab8500_regulator_set_value,
> +	.get_enable = ab8500_regulator_get_enable,
> +	.set_enable = ab8500_regulator_set_enable,
> +};
> +
> +U_BOOT_DRIVER(ab8500_ldo_aux3) = {
> +	.name = "ab8500_ldo_aux3",
> +	.id = UCLASS_REGULATOR,
> +	.ops = &ab8500_regulator_ops,
> +};
> +
> +static int ab8500_regulators_bind(struct udevice *dev)
> +{
> +	struct driver *drv = lists_driver_lookup_name("ab8500_ldo_aux3");
> +	ofnode node;
> +
> +	dev_for_each_subnode(node, dev) {
> +		if (!strcmp(ofnode_get_name(node), "ab8500_ldo_aux3"))
> +			return device_bind_with_driver_data(dev, drv,
> +						    ofnode_get_name(node), 0,
> +						    node, NULL);
> +	}

dev_read_subnode(node, "ab8500_ldo_aux3") and
device_bind_driver_to_node() instead of lists_driver_lookup_name() +
device_bind_with_driver_data() should be a bit simpler.

Thanks,
Stephan