[U-Boot] [PATCH v3 07/17] dm: pmic: add pmic command

Simon Glass
2015-03-29T13:07:54Z

Hi Prazemyslaw,

On 24 March 2015 at 14:30, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> This is new command for the pmic devices based on driver model pmic api.
> Command features are unchanged:
> - list - list UCLASS pmic devices
> - pmic dev [id] - show or [set] operating pmic device (NEW)
> - pmic dump - dump registers
> - 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.
>
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
>
> Changes v3:
> - new file
> - add Kconfig
> ---
> common/Kconfig | 14 ++++
> common/Makefile | 3 +
> common/cmd_pmic.c | 210 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 227 insertions(+)
> create mode 100644 common/cmd_pmic.c
>
> diff --git a/common/Kconfig b/common/Kconfig
> index e662774..1125e6d 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -335,4 +335,18 @@ config CMD_SETGETDCR
>
> endmenu
>
> +menu "Power commands"
> +config DM_PMIC_CMD

CMD_DM_PMIC

since this fits better with the other ones

> + bool "Enable Driver Model PMIC command"
> + depends on DM_PMIC
> + help
> + This is new command for the pmic devices based on driver model pmic api.
> + Command features are unchanged:
> + - list - list UCLASS pmic devices
> + - pmic dev [id] - show or [set] operating pmic device (NEW)
> + - pmic dump - dump registers
> + - 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.
> +endmenu
> endmenu
> diff --git a/common/Makefile b/common/Makefile
> index 7216a13..d908851 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -208,6 +208,9 @@ obj-$(CONFIG_UPDATE_TFTP) += update.o
> obj-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
> obj-$(CONFIG_CMD_DFU) += cmd_dfu.o
> obj-$(CONFIG_CMD_GPT) += cmd_gpt.o
> +
> +# Power
> +obj-$(CONFIG_DM_PMIC_CMD) += cmd_pmic.o
> endif
>
> ifdef CONFIG_SPL_BUILD
> diff --git a/common/cmd_pmic.c b/common/cmd_pmic.c
> new file mode 100644
> index 0000000..978a94a
> --- /dev/null
> +++ b/common/cmd_pmic.c
> @@ -0,0 +1,210 @@
> +/*
> + * Copyright (C) 2014-2015 Samsung Electronics
> + * Przemyslaw Marczak <p.marczak@samsung.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +#include <common.h>
> +#include <linux/types.h>
> +#include <linux/ctype.h>
> +#include <fdtdec.h>
> +#include <dm.h>
> +#include <power/pmic.h>
> +#include <power/regulator.h>
> +#include <dm/device-internal.h>
> +#include <dm/uclass-internal.h>
> +#include <dm/root.h>
> +#include <dm/lists.h>
> +#include <i2c.h>
> +#include <compiler.h>
> +#include <errno.h>
> +
> +#define LIMIT_SEQ 3
> +#define LIMIT_DEVNAME 20
> +
> +static struct udevice *pmic_curr;
> +
> +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));
> + return CMD_RET_FAILURE;
> +}
> +
> +static int do_dev(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + int seq, ret = -ENODEV;
> +
> + switch (argc) {
> + case 2:
> + seq = simple_strtoul(argv[1], NULL, 0);
> + ret = uclass_get_device_by_seq(UCLASS_PMIC, seq, &pmic_curr);
> + case 1:
> + if (!pmic_curr)
> + return failed("get", "the", "device", ret);
> +
> + printf("dev: %d @ %s\n", pmic_curr->seq, pmic_curr->name);
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_list(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + const char *parent_uc;
> + int ret;
> +
> + printf("|%*s | %-*.*s| %-*.*s| %s @ %s\n",
> + LIMIT_SEQ, "Seq",
> + LIMIT_DEVNAME, LIMIT_DEVNAME, "Name",
> + LIMIT_DEVNAME, LIMIT_DEVNAME, "Parent name",
> + "Parent uclass", "seq");
> +
> + for (ret = uclass_first_device(UCLASS_PMIC, &dev); dev;
> + ret = uclass_next_device(&dev)) {

Note this will probe everything.

Perhaps we need uclass_find_first_device() and
uclass_find_next_device() which don't probe before returning each
device?

> + if (!dev)
> + continue;
> +
> + /* Parent uclass name*/
> + parent_uc = dev->parent->uclass->uc_drv->name;

What do you think about a new function at some point, so you can call
dev_uclass_name(dev_get_parent(dev))? We want to avoid digging around
in the driver model data structures outside drivers/core.

> +
> + printf("|%*d | %-*.*s| %-*.*s| %s @ %d\n",
> + LIMIT_SEQ, dev->seq,
> + LIMIT_DEVNAME, LIMIT_DEVNAME, dev->name,
> + LIMIT_DEVNAME, LIMIT_DEVNAME, dev->parent->name,
> + parent_uc, dev->parent->seq);
> + }
> +
> + if (ret)
> + return CMD_RET_FAILURE;
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_dump(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + int regs, ret;
> + uint8_t value;
> + uint reg;
> +
> + if (!pmic_curr)
> + return failed("get", "current", "device", -ENODEV);
> +
> + dev = pmic_curr;
> +
> + printf("Dump pmic: %s registers\n", dev->name);
> +
> + regs = pmic_reg_count(dev);
> + reg = 0;
> + while (reg < regs) {

for (ret = 0; ret < regs; reg++) {

> + ret = pmic_read(dev, reg, &value, 1);
> + if (ret)
> + return failed("read", dev->name, "register", ret);
> +
> + if (!(reg % 16))
> + printf("\n0x%02x: ", reg);
> +
> + printf("%2.2x ", value);
> + reg++;
> + }
> + printf("\n");
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_read(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + int regs, ret;
> + uint8_t value;
> + uint reg;
> +
> + if (!pmic_curr)
> + return failed("get", "current", "device", -ENODEV);
> +
> + dev = pmic_curr;
> +
> + if (argc != 2)
> + return CMD_RET_USAGE;
> +
> + reg = simple_strtoul(argv[1], NULL, 0);
> + regs = pmic_reg_count(dev);
> + if (reg > regs) {
> + printf("Pmic max reg: %d\n", regs);
> + return failed("read", "given", "address", -EFAULT);
> + }
> +
> + ret = pmic_read(dev, reg, &value, 1);
> + if (ret)
> + return failed("read", dev->name, "register", ret);
> +
> + printf("0x%02x: 0x%2.2x\n", reg, value);
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int do_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct udevice *dev;
> + int regs, ret;
> + uint8_t value;
> + uint reg;
> +
> + if (!pmic_curr)
> + return failed("get", "current", "device", -ENODEV);
> +
> + dev = pmic_curr;
> +
> + if (argc != 3)
> + return CMD_RET_USAGE;
> +
> + reg = simple_strtoul(argv[1], NULL, 0);
> + regs = pmic_reg_count(dev);
> + if (reg > regs) {
> + printf("Pmic max reg: %d\n", regs);
> + return failed("write", "given", "address", -EFAULT);
> + }
> +
> + value = simple_strtoul(argv[2], NULL, 0);
> +
> + ret = pmic_write(dev, reg, &value, 1);
> + if (ret)
> + return failed("write", dev->name, "register", 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(dump, 1, 1, do_dump, "", ""),
> + U_BOOT_CMD_MKENT(read, 2, 1, do_read, "", ""),
> + U_BOOT_CMD_MKENT(write, 3, 1, do_write, "", ""),
> +};
> +
> +static int do_pmic(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(pmic, CONFIG_SYS_MAXARGS, 1, do_pmic,
> + "uclass operations",
> + "list - list UCLASS pmic devices\n"

Can we drop the 'UCLASS' ?

> + "pmic dev [id] - show or [set] operating pmic device\n"
> + "pmic dump - dump registers\n"
> + "pmic read address - read byte of register at address\n"
> + "pmic write address - write byte to register at address\n"
> +);
> --
> 1.9.1
>

Regards,
Simon