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

Przemyslaw Marczak
2015-04-03T16:08:33Z

Hello Simon,

On 03/29/2015 03:07 PM, Simon Glass wrote:
> 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
>

Ok

>> + 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?
>
>

Right, I will extend the uclass.c API.

>> + 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.
>

Good idea, will move to uclass API.

>> +
>> + 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++) {
>

Ok.

>> + 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' ?
>

Ok

>> + "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
>

Thanks,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com