[PATCH v3 3/7] power: regulator: max77663: add regulator support
Simon Glass
sjg at chromium.org
Mon Jul 24 04:28:17 CEST 2023
Hi Svyatoslav,
On Sun, 23 Jul 2023 at 06:28, Svyatoslav Ryhel <clamor95 at gmail.com> wrote:
>
> The driver provides regulator set/get voltage
> enable/disable functions for MAXIM MAX77663 PMICs.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95 at gmail.com>
> ---
> drivers/power/regulator/Kconfig | 8 +
> drivers/power/regulator/Makefile | 1 +
> drivers/power/regulator/max77663_regulator.c | 358 +++++++++++++++++++
> 3 files changed, 367 insertions(+)
> create mode 100644 drivers/power/regulator/max77663_regulator.c
Reviewed-by: Simon Glass <sjg at chromium.org>
nits below
>
> diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig
> index eb5aa38c1c..571debd54e 100644
> --- a/drivers/power/regulator/Kconfig
> +++ b/drivers/power/regulator/Kconfig
> @@ -141,6 +141,14 @@ config SPL_REGULATOR_PWM
> This config enables implementation of driver-model regulator uclass
> features for PWM regulators in SPL.
>
> +config DM_REGULATOR_MAX77663
> + bool "Enable Driver Model for REGULATOR MAX77663"
> + depends on DM_REGULATOR && DM_PMIC_MAX77663
> + ---help---
> + This config enables implementation of driver-model regulator uclass
> + features for REGULATOR MAX77663. The driver implements get/set api for:
> + value and enable.
any thoughts on the feature set of this regulator?
> +
> config DM_REGULATOR_MAX77686
> bool "Enable Driver Model for REGULATOR MAX77686"
> depends on DM_REGULATOR && DM_PMIC_MAX77686
> diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile
> index d9e0cd5949..8d73169b50 100644
> --- a/drivers/power/regulator/Makefile
> +++ b/drivers/power/regulator/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_REGULATOR_AS3722) += as3722_regulator.o
> obj-$(CONFIG_$(SPL_)REGULATOR_AXP) += axp_regulator.o
> obj-$(CONFIG_$(SPL_)REGULATOR_AXP_USB_POWER) += axp_usb_power.o
> obj-$(CONFIG_$(SPL_)DM_REGULATOR_DA9063) += da9063.o
> +obj-$(CONFIG_$(SPL_)DM_REGULATOR_MAX77663) += max77663_regulator.o
> obj-$(CONFIG_DM_REGULATOR_MAX77686) += max77686.o
> obj-$(CONFIG_DM_REGULATOR_NPCM8XX) += npcm8xx_regulator.o
> obj-$(CONFIG_$(SPL_)DM_PMIC_PFUZE100) += pfuze100.o
> diff --git a/drivers/power/regulator/max77663_regulator.c b/drivers/power/regulator/max77663_regulator.c
> new file mode 100644
> index 0000000000..cbc275b30b
> --- /dev/null
> +++ b/drivers/power/regulator/max77663_regulator.c
> @@ -0,0 +1,358 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright(C) 2023 Svyatoslav Ryhel <clamor95 at gmail.com>
> + */
> +
> +#include <common.h>
> +#include <fdtdec.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <power/pmic.h>
> +#include <power/regulator.h>
> +#include <power/max77663.h>
> +
> +/* fist row is control registers, second is voltage registers */
> +static const char max77663_sd_reg[][MAX77663_SD_NUM] = {
> + { 0x1d, 0x1e, 0x1f, 0x20, 0x21 },
> + { 0x16, 0x17, 0x18, 0x19, 0x2a },
> +};
> +
> +static const char max77663_ldo_reg[MAX77663_LDO_NUM] = {
> + 0x23, 0x25, 0x27, 0x29, 0x2b, 0x2d, 0x2f, 0x31, 0x33
> +};
> +
> +static int max77663_sd_enable(struct udevice *dev, int op, bool *enable)
> +{
> + struct dm_regulator_uclass_plat *uc_pdata =
> + dev_get_uclass_plat(dev);
> + u32 adr = uc_pdata->ctrl_reg;
> + int ret;
> +
> + ret = pmic_reg_read(dev->parent, adr);
> + if (ret < 0)
> + return ret;
> +
> + if (op == PMIC_OP_GET) {
> + if (ret & MAX77663_SD_STATUS_MASK)
> + *enable = true;
> + else
> + *enable = false;
> +
> + return 0;
> + } else if (op == PMIC_OP_SET) {
> + ret &= ~(MAX77663_SD_STATUS_MASK);
If that macro is set up correct you should not need the ()
> +
> + if (*enable)
> + ret |= MAX77663_SD_STATUS_MASK;
> +
> + ret = pmic_reg_write(dev->parent, adr, ret);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int max77663_sd_volt2hex(int sd, int uV)
> +{
> + switch (sd) {
> + case 0:
> + /* SD0 has max voltage 1.4V */
> + if (uV > MAX77663_SD0_VOLT_MAX)
> + return -EINVAL;
> + break;
> + case 1:
> + /* SD1 has max voltage 1.55V */
> + if (uV > MAX77663_SD1_VOLT_MAX)
> + return -EINVAL;
> + break;
> + default:
> + /* SD2 and SD3 have max voltage 3.79V */
> + if (uV > MAX77663_SD_VOLT_MAX)
> + return -EINVAL;
> + break;
> + };
> +
> + if (uV < MAX77663_SD_VOLT_MIN)
> + uV = MAX77663_SD_VOLT_MIN;
> +
> + return (uV - MAX77663_SD_VOLT_BASE) / 12500;
If the header file with these defines is only included by this driver,
you can drop the MAX77663_ prefix and make everything shorter.
> +}
> +
> +static int max77663_sd_hex2volt(int sd, int hex)
What is sd? What is hex? Please add a function comment.
> +{
> + switch (sd) {
> + case 0:
> + /* SD0 has max voltage 1.4V */
> + if (hex > MAX77663_SD0_VOLT_MAX_HEX)
> + return -EINVAL;
> + break;
> + case 1:
> + /* SD1 has max voltage 1.55V */
> + if (hex > MAX77663_SD1_VOLT_MAX_HEX)
> + return -EINVAL;
> + break;
> + default:
> + /* SD2 and SD3 have max voltage 3.79V */
> + if (hex > MAX77663_SD_VOLT_MAX_HEX)
> + return -EINVAL;
> + break;
> + };
> +
> + if (hex < MAX77663_SD_VOLT_MIN_HEX)
> + hex = MAX77663_SD_VOLT_MIN_HEX;
> +
> + return MAX77663_SD_VOLT_BASE + hex * 12500;
> +}
> +
> +static int max77663_sd_val(struct udevice *dev, int op, int *uV)
> +{
> + struct dm_regulator_uclass_plat *uc_pdata =
> + dev_get_uclass_plat(dev);
> + u32 adr = uc_pdata->volt_reg;
> + int id = dev->driver_data;
> + int ret;
> +
> + if (op == PMIC_OP_GET) {
> + ret = pmic_reg_read(dev->parent, adr);
> + if (ret < 0)
> + return ret;
> +
> + *uV = 0;
> +
> + ret = max77663_sd_hex2volt(id, ret);
> + if (ret < 0)
> + return ret;
> + *uV = ret;
> +
> + return 0;
> + }
> +
> + /* SD regulators use entire register for voltage */
> + ret = max77663_sd_volt2hex(id, *uV);
> + if (ret < 0)
> + return ret;
> +
> + return pmic_reg_write(dev->parent, adr, ret);
> +}
> +
> +static int max77663_sd_probe(struct udevice *dev)
> +{
> + struct dm_regulator_uclass_plat *uc_pdata =
> + dev_get_uclass_plat(dev);
> +
> + uc_pdata->type = REGULATOR_TYPE_BUCK;
> + uc_pdata->ctrl_reg = max77663_sd_reg[0][dev->driver_data];
> + uc_pdata->volt_reg = max77663_sd_reg[1][dev->driver_data];
> +
> + return 0;
> +}
> +
> +static int sd_get_value(struct udevice *dev)
> +{
> + int uV;
> + int ret;
> +
> + ret = max77663_sd_val(dev, PMIC_OP_GET, &uV);
> + if (ret)
> + return ret;
> +
> + return uV;
> +}
> +
> +static int sd_set_value(struct udevice *dev, int uV)
> +{
> + return max77663_sd_val(dev, PMIC_OP_SET, &uV);
> +}
> +
> +static int sd_get_enable(struct udevice *dev)
> +{
> + bool enable = false;
> + int ret;
> +
> + ret = max77663_sd_enable(dev, PMIC_OP_GET, &enable);
> + if (ret)
> + return ret;
> +
> + return enable;
> +}
> +
> +static int sd_set_enable(struct udevice *dev, bool enable)
> +{
> + return max77663_sd_enable(dev, PMIC_OP_SET, &enable);
> +}
> +
> +static const struct dm_regulator_ops max77663_sd_ops = {
> + .get_value = sd_get_value,
> + .set_value = sd_set_value,
> + .get_enable = sd_get_enable,
> + .set_enable = sd_set_enable,
> +};
> +
> +U_BOOT_DRIVER(max77663_sd) = {
> + .name = MAX77663_SD_DRIVER,
> + .id = UCLASS_REGULATOR,
> + .ops = &max77663_sd_ops,
> + .probe = max77663_sd_probe,
> +};
> +
> +static int max77663_ldo_enable(struct udevice *dev, int op, bool *enable)
> +{
> + struct dm_regulator_uclass_plat *uc_pdata =
> + dev_get_uclass_plat(dev);
> + u32 adr = uc_pdata->ctrl_reg;
> + int ret;
> +
> + ret = pmic_reg_read(dev->parent, adr);
> + if (ret < 0)
> + return ret;
> +
> + if (op == PMIC_OP_GET) {
> + if (ret & MAX77663_LDO_STATUS_MASK)
> + *enable = true;
> + else
> + *enable = false;
> +
> + return 0;
> + } else if (op == PMIC_OP_SET) {
> + ret &= ~(MAX77663_LDO_STATUS_MASK);
> +
> + if (*enable)
> + ret |= MAX77663_LDO_STATUS_MASK;
> +
> + ret = pmic_reg_write(dev->parent, adr, ret);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int max77663_ldo_volt2hex(int ldo, int uV)
> +{
> + switch (ldo) {
> + case 0:
> + case 1:
> + if (uV > MAX77663_LDO01_VOLT_MAX)
> + return -EINVAL;
> +
> + return (uV - MAX77663_LDO_VOLT_BASE) / 25000;
> + case 4:
> + if (uV > MAX77663_LDO4_VOLT_MAX)
> + return -EINVAL;
> +
> + return (uV - MAX77663_LDO_VOLT_BASE) / 12500;
> + default:
> + if (uV > MAX77663_LDO_VOLT_MAX)
> + return -EINVAL;
> +
> + return (uV - MAX77663_LDO_VOLT_BASE) / 50000;
> + };
> +}
> +
> +static int max77663_ldo_hex2volt(int ldo, int hex)
> +{
> + if (hex > MAX77663_LDO_VOLT_MAX_HEX)
> + return -EINVAL;
> +
> + switch (ldo) {
> + case 0:
> + case 1:
> + return (hex * 25000) + MAX77663_LDO_VOLT_BASE;
> + case 4:
> + return (hex * 12500) + MAX77663_LDO_VOLT_BASE;
> + default:
> + return (hex * 50000) + MAX77663_LDO_VOLT_BASE;
> + };
> +}
> +
> +static int max77663_ldo_val(struct udevice *dev, int op, int *uV)
We tend to add a 'p' suffix to return values and use snake case, i.e int *uvp
> +{
> + struct dm_regulator_uclass_plat *uc_pdata =
> + dev_get_uclass_plat(dev);
> + u32 adr = uc_pdata->ctrl_reg;
> + int id = dev->driver_data;
> + int hex, ret;
hex is a strange varname. Does it have an actual meaning? Perhaps it
is the code for the microvolts?
> +
> + ret = pmic_reg_read(dev->parent, adr);
> + if (ret < 0)
> + return ret;
> +
> + if (op == PMIC_OP_GET) {
> + *uV = 0;
> +
> + ret = max77663_ldo_hex2volt(id, ret & MAX77663_LDO_VOLT_MASK);
> + if (ret < 0)
> + return ret;
> +
> + *uV = ret;
> + return 0;
> + }
> +
> + hex = max77663_ldo_volt2hex(id, *uV);
> + if (hex < 0)
> + return hex;
> +
> + ret &= ~(MAX77663_LDO_VOLT_MASK);
> +
> + return pmic_reg_write(dev->parent, adr, ret | hex);
> +}
> +
> +static int max77663_ldo_probe(struct udevice *dev)
> +{
> + struct dm_regulator_uclass_plat *uc_pdata =
> + dev_get_uclass_plat(dev);
> +
> + uc_pdata->type = REGULATOR_TYPE_LDO;
> + uc_pdata->ctrl_reg = max77663_ldo_reg[dev->driver_data];
> +
> + return 0;
> +}
> +
> +static int ldo_get_value(struct udevice *dev)
> +{
> + int uV;
> + int ret;
> +
> + ret = max77663_ldo_val(dev, PMIC_OP_GET, &uV);
> + if (ret)
> + return ret;
> +
> + return uV;
> +}
> +
> +static int ldo_set_value(struct udevice *dev, int uV)
> +{
> + return max77663_ldo_val(dev, PMIC_OP_SET, &uV);
> +}
> +
> +static int ldo_get_enable(struct udevice *dev)
> +{
> + bool enable = false;
> + int ret;
> +
> + ret = max77663_ldo_enable(dev, PMIC_OP_GET, &enable);
> + if (ret)
> + return ret;
> +
> + return enable;
> +}
> +
> +static int ldo_set_enable(struct udevice *dev, bool enable)
> +{
> + return max77663_ldo_enable(dev, PMIC_OP_SET, &enable);
> +}
> +
> +static const struct dm_regulator_ops max77663_ldo_ops = {
> + .get_value = ldo_get_value,
> + .set_value = ldo_set_value,
> + .get_enable = ldo_get_enable,
> + .set_enable = ldo_set_enable,
> +};
> +
> +U_BOOT_DRIVER(max77663_ldo) = {
> + .name = MAX77663_LDO_DRIVER,
> + .id = UCLASS_REGULATOR,
> + .ops = &max77663_ldo_ops,
> + .probe = max77663_ldo_probe,
> +};
> --
> 2.39.2
>
Regards,
Simon
More information about the U-Boot
mailing list