[U-Boot] [PATCH v4 07/16] dm: regulator: add regulator command
Simon Glass
sjg at chromium.org
Wed Apr 22 18:30:51 CEST 2015
Hi Przemyslaw,
On 20 April 2015 at 12:07, Przemyslaw Marczak <p.marczak at samsung.com> wrote:
> This command is based on driver model regulator's API.
> The user interface provides:
> - list UCLASS regulator devices
> - show or [set] operating regulator device
> - print constraints info
> - print operating status
> - print/[set] voltage value [uV] (force)
> - print/[set] current value [uA]
> - print/[set] operating mode id
> - enable the regulator output
> - disable the regulator output
>
> The 'force' option can be used for setting the value which exceeds
> the constraints min/max limits.
>
> Signed-off-by: Przemyslaw Marczak <p.marczak at samsung.com>
> ---
> Changes v3:
> - new file
> - Kconfig entry
>
> Changes V4:
> - cmd regulator: move platdata to uc pdata
> - cmd_regulator: includes cleanup
> - cmd_regulator: add get_curr_dev_and_pl() check type
> - move config name: CONFIG_DM_REGULATOR_CMD to CONFIG_CMD_REGULATOR
> - common/Kconfig - cleanup
> ---
> common/Kconfig | 22 +++
> common/Makefile | 1 +
> common/cmd_regulator.c | 403 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 426 insertions(+)
> create mode 100644 common/cmd_regulator.c
Acked-by: Simon Glass <sjg at chromium.org>
I have a few nits that could be dealt with by a follow-on patch.
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 4666f8e..52f8bb1 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -470,5 +470,27 @@ config CMD_PMIC
> - pmic read address - read byte of register at address
> - pmic write address - write byte to register at address
> The only one change for this command is 'dev' subcommand.
> +
> +config CMD_REGULATOR
> + bool "Enable Driver Model REGULATOR command"
> + depends on DM_REGULATOR
> + help
> + This command is based on driver model regulator's API.
> + User interface features:
> + - list - list regulator devices
> + - regulator dev <id> - show or [set] operating regulator device
> + - regulator info - print constraints info
> + - regulator status - print operating status
> + - regulator value <val] <-f> - print/[set] voltage value [uV]
> + - regulator current <val> - print/[set] current value [uA]
> + - regulator mode <id> - print/[set] operating mode id
> + - regulator enable - enable the regulator output
> + - regulator disable - disable the regulator output
> +
> + The '-f' (force) option can be used for set the value which exceeds
> + the limits, which are found in device-tree and are kept in regulator's
> + uclass platdata structure.
> +
> endmenu
> +
> endmenu
> diff --git a/common/Makefile b/common/Makefile
> index 87a3efe..93bded3 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -212,6 +212,7 @@ obj-$(CONFIG_CMD_GPT) += cmd_gpt.o
>
> # Power
> obj-$(CONFIG_CMD_PMIC) += cmd_pmic.o
> +obj-$(CONFIG_CMD_REGULATOR) += cmd_regulator.o
> endif
>
> ifdef CONFIG_SPL_BUILD
> diff --git a/common/cmd_regulator.c b/common/cmd_regulator.c
> new file mode 100644
> index 0000000..b1b9e87
> --- /dev/null
> +++ b/common/cmd_regulator.c
> @@ -0,0 +1,403 @@
> +/*
> + * Copyright (C) 2014-2015 Samsung Electronics
> + * Przemyslaw Marczak <p.marczak at samsung.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +#include <common.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <dm/uclass-internal.h>
> +#include <power/regulator.h>
> +
> +#define LIMIT_SEQ 3
> +#define LIMIT_DEVNAME 20
> +#define LIMIT_OFNAME 20
> +#define LIMIT_INFO 16
> +
> +static struct udevice *currdev;
> +
> +static int failed(const char *getset, const char *thing,
> + const char *for_dev, int ret)
> +{
> + printf("Can't %s %s %s.\nError: %d (%s)\n", getset, thing, for_dev,
> + ret, errno_str(ret));
blank line here.
I worry that if someone gets one of these messages they will not be
able to find it in the source code. How about passing in the full
printf() string in each case, or just using printf() in situ? I don't
think the code space saving is significant.
> + return CMD_RET_FAILURE;
> +}
> +
> +static int regulator_get(bool list_only, int get_seq, struct udevice **devp)
This function seems to do multiple things (find and list). Should we
split it into two?
> +{
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + struct udevice *dev;
> + int ret;
> +
> + if (devp)
> + *devp = NULL;
> +
> + for (ret = uclass_first_device(UCLASS_REGULATOR, &dev); dev;
> + ret = uclass_next_device(&dev)) {
This will probe all regulators that it checks. I think it should avoid
that. Do you mean to use
> + if (list_only) {
> + uc_pdata = dev_get_uclass_platdata(dev);
> + printf("|%*d | %*.*s @ %-*.*s| %s @ %s\n",
> + LIMIT_SEQ, dev->seq,
> + LIMIT_DEVNAME, LIMIT_DEVNAME, dev->name,
> + LIMIT_OFNAME, LIMIT_OFNAME, uc_pdata->name,
> + dev->parent->name,
> + dev_get_uclass_name(dev->parent));
> + continue;
> + }
> +
> + if (dev->seq == get_seq) {
> + if (devp)
> + *devp = dev;
> + else
> + return -EINVAL;
> +
> + return 0;
> + }
> + }
> +
> + if (list_only)
> + return ret;
> +
> + return -ENODEV;
> +}
> +
> +static int do_dev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + int seq, ret = -ENXIO;
> +
> + switch (argc) {
> + case 2:
> + seq = simple_strtoul(argv[1], NULL, 0);
> + ret = uclass_get_device_by_seq(UCLASS_REGULATOR, seq, &currdev);
> + if (ret && (ret = regulator_get(false, seq, &currdev)))
> + goto failed;
> + case 1:
> + uc_pdata = dev_get_uclass_platdata(currdev);
> + if (!uc_pdata)
> + goto failed;
> +
> + printf("dev: %d @ %s\n", currdev->seq, uc_pdata->name);
> + }
> +
> + return CMD_RET_SUCCESS;
> +failed:
> + return failed("get", "the", "device", ret);
> +}
> +
> +static int get_curr_dev_and_pl(struct udevice **devp,
What is pl? The name does not seem very meaningful to me.
> + struct dm_regulator_uclass_platdata **uc_pdata,
> + bool allow_type_fixed)
> +{
> + *devp = NULL;
> + *uc_pdata = NULL;
> +
> + if (!currdev)
> + return failed("get", "current", "device", -ENODEV);
> +
> + *devp = currdev;
> +
> + *uc_pdata = dev_get_uclass_platdata(*devp);
> + if (!*uc_pdata)
> + return failed("get", "regulator", "platdata", -ENXIO);
> +
> + if (!allow_type_fixed && (*uc_pdata)->type == REGULATOR_TYPE_FIXED) {
> + printf("Operation not allowed for fixed regulator!\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_list(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + int ret;
> +
> + printf("|%*s | %*.*s @ %-*.*s| %s @ %s\n",
> + LIMIT_SEQ, "Seq",
> + LIMIT_DEVNAME, LIMIT_DEVNAME, "Name",
> + LIMIT_OFNAME, LIMIT_OFNAME, "fdtname",
> + "Parent", "uclass");
> +
> + ret = regulator_get(true, 0, NULL);
> + if (ret)
> + return CMD_RET_FAILURE;
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int constraint(const char *name, int val, const char *val_name)
> +{
> + printf("%-*s", LIMIT_INFO, name);
> + if (val < 0) {
> + printf(" %s (err: %d)\n", errno_str(val), val);
> + return val;
> + }
> +
> + if (val_name)
> + printf(" %d (%s)\n", val, val_name);
> + else
> + printf(" %d\n", val);
> +
> + return 0;
> +}
> +
> +static const char *get_mode_name(struct dm_regulator_mode *mode,
> + int mode_count,
> + int mode_id)
> +{
> + while (mode_count--) {
> + if (mode->id == mode_id)
> + return mode->name;
> + mode++;
> + }
> +
> + return NULL;
> +}
> +
> +static int do_info(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + struct dm_regulator_mode *modes;
> + const char *parent_uc;
> + int mode_count;
> + int ret;
> + int i;
> +
> + ret = get_curr_dev_and_pl(&dev, &uc_pdata, true);
> + if (ret)
> + return ret;
> +
> + parent_uc = dev_get_uclass_name(dev->parent);
> +
> + printf("Uclass regulator dev %d info:\n", dev->seq);
> + printf("%-*s %s @ %s\n%-*s %s\n%-*s %s\n%-*s\n",
> + LIMIT_INFO, "* parent:", dev->parent->name, parent_uc,
> + LIMIT_INFO, "* dev name:", dev->name,
> + LIMIT_INFO, "* fdt name:", uc_pdata->name,
> + LIMIT_INFO, "* constraints:");
> +
> + constraint(" - min uV:", uc_pdata->min_uV, NULL);
> + constraint(" - max uV:", uc_pdata->max_uV, NULL);
> + constraint(" - min uA:", uc_pdata->min_uA, NULL);
> + constraint(" - max uA:", uc_pdata->max_uA, NULL);
> + constraint(" - always on:", uc_pdata->always_on,
> + uc_pdata->always_on ? "true" : "false");
> + constraint(" - boot on:", uc_pdata->boot_on,
> + uc_pdata->boot_on ? "true" : "false");
> +
> + mode_count = regulator_mode(dev, &modes);
> + constraint("* op modes:", mode_count, NULL);
> +
> + for (i = 0; i < mode_count; i++, modes++)
> + constraint(" - mode id:", modes->id, modes->name);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_status(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + int current, value, mode, ret;
> + const char *mode_name = NULL;
> + struct udevice *dev;
> + bool enabled;
> +
> + ret = get_curr_dev_and_pl(&dev, &uc_pdata, true);
> + if (ret)
> + return ret;
> +
> + enabled = regulator_get_enable(dev);
> + constraint(" * enable:", enabled, enabled ? "true" : "false");
> +
> + value = regulator_get_value(dev);
> + constraint(" * value uV:", value, NULL);
> +
> + current = regulator_get_current(dev);
> + constraint(" * current uA:", current, NULL);
> +
> + mode = regulator_get_mode(dev);
> + mode_name = get_mode_name(uc_pdata->mode, uc_pdata->mode_count, mode);
> + constraint(" * mode id:", mode, mode_name);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_value(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + int value;
> + int force;
> + int ret;
> +
> + ret = get_curr_dev_and_pl(&dev, &uc_pdata, argc == 1);
> + if (ret)
> + return ret;
> +
> + if (argc == 1) {
> + value = regulator_get_value(dev);
> + if (value < 0)
> + return failed("get", uc_pdata->name, "voltage", value);
> +
> + printf("%d uV\n", value);
> + return CMD_RET_SUCCESS;
> + }
> +
> + if (argc == 3)
> + force = !strcmp("-f", argv[2]);
> + else
> + force = 0;
> +
> + value = simple_strtoul(argv[1], NULL, 0);
> + if ((value < uc_pdata->min_uV || value > uc_pdata->max_uV) && !force) {
> + printf("Value exceeds regulator constraint limits\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + ret = regulator_set_value(dev, value);
> + if (ret)
> + return failed("set", uc_pdata->name, "voltage value", ret);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_current(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + int current;
> + int ret;
> +
> + ret = get_curr_dev_and_pl(&dev, &uc_pdata, argc == 1);
> + if (ret)
> + return ret;
> +
> + if (argc == 1) {
> + current = regulator_get_current(dev);
> + if (current < 0)
> + return failed("get", uc_pdata->name, "current", current);
> +
> + printf("%d uA\n", current);
> + return CMD_RET_SUCCESS;
> + }
> +
> + current = simple_strtoul(argv[1], NULL, 0);
> + if (current < uc_pdata->min_uA || current > uc_pdata->max_uA) {
> + printf("Current exceeds regulator constraint limits\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + ret = regulator_set_current(dev, current);
> + if (ret)
> + return failed("set", uc_pdata->name, "current value", ret);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_mode(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + int new_mode;
> + int mode;
> + int ret;
> +
> + ret = get_curr_dev_and_pl(&dev, &uc_pdata, false);
> + if (ret)
> + return ret;
> +
> + if (argc == 1) {
> + mode = regulator_get_mode(dev);
> + if (mode < 0)
> + return failed("get", uc_pdata->name, "mode", mode);
> +
> + printf("mode id: %d\n", mode);
> + return CMD_RET_SUCCESS;
> + }
> +
> + new_mode = simple_strtoul(argv[1], NULL, 0);
> +
> + ret = regulator_set_mode(dev, new_mode);
> + if (ret)
> + return failed("set", uc_pdata->name, "mode", ret);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_enable(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + int ret;
> +
> + ret = get_curr_dev_and_pl(&dev, &uc_pdata, true);
> + if (ret)
> + return ret;
> +
> + ret = regulator_set_enable(dev, true);
> + if (ret)
> + return failed("enable", "regulator", uc_pdata->name, ret);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_disable(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + struct dm_regulator_uclass_platdata *uc_pdata;
> + int ret;
> +
> + ret = get_curr_dev_and_pl(&dev, &uc_pdata, true);
> + if (ret)
> + return ret;
> +
> + ret = regulator_set_enable(dev, false);
> + if (ret)
> + return failed("disable", "regulator", uc_pdata->name, ret);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static cmd_tbl_t subcmd[] = {
> + U_BOOT_CMD_MKENT(dev, 2, 1, do_dev, "", ""),
> + U_BOOT_CMD_MKENT(list, 1, 1, do_list, "", ""),
> + U_BOOT_CMD_MKENT(info, 2, 1, do_info, "", ""),
> + U_BOOT_CMD_MKENT(status, 2, 1, do_status, "", ""),
> + U_BOOT_CMD_MKENT(value, 3, 1, do_value, "", ""),
> + U_BOOT_CMD_MKENT(current, 3, 1, do_current, "", ""),
> + U_BOOT_CMD_MKENT(mode, 2, 1, do_mode, "", ""),
> + U_BOOT_CMD_MKENT(enable, 1, 1, do_enable, "", ""),
> + U_BOOT_CMD_MKENT(disable, 1, 1, do_disable, "", ""),
> +};
> +
> +static int do_regulator(cmd_tbl_t *cmdtp, int flag, int argc,
> + char * const argv[])
> +{
> + cmd_tbl_t *cmd;
> +
> + argc--;
> + argv++;
> +
> + cmd = find_cmd_tbl(argv[0], subcmd, ARRAY_SIZE(subcmd));
> + if (cmd == NULL || argc > cmd->maxargs)
> + return CMD_RET_USAGE;
> +
> + return cmd->cmd(cmdtp, flag, argc, argv);
> +}
> +
> +U_BOOT_CMD(regulator, CONFIG_SYS_MAXARGS, 1, do_regulator,
> + "uclass operations",
> + "list - list UCLASS regulator devices\n"
> + "regulator dev [id] - show or [set] operating regulator device\n"
> + "regulator [info] - print constraints info\n"
> + "regulator [status] - print operating status\n"
> + "regulator [value] [-f] - print/[set] voltage value [uV] (force)\n"
> + "regulator [current] - print/[set] current value [uA]\n"
> + "regulator [mode_id] - print/[set] operating mode id\n"
> + "regulator [enable] - enable the regulator output\n"
> + "regulator [disable] - disable the regulator output\n"
> +);
> --
> 1.9.1
>
Regards,
Simon
More information about the U-Boot
mailing list