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

[Przemyslaw Marczak <p.marczak@samsung.com>]

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