[U-Boot] [PATCH] cmd: add ADC cli commands

[Simon Glass]

Hi Neil,

On 23 April 2018 at 08:18, Neil Armstrong <narmstrong at baylibre.com> wrote:
> Add an 'adc' cli command to get adc devices informations and read single
> shots datas.
>
> Signed-off-by: Neil Armstrong <narmstrong at baylibre.com>
> ---
>  cmd/Kconfig  |   7 ++++
>  cmd/Makefile |   1 +
>  cmd/adc.c    | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 134 insertions(+)
>  create mode 100644 cmd/adc.c

Reviewed-by: Simon Glass <sjg at chromium.org>

Nits below

>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index bc1d2f3..631daee 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -601,6 +601,13 @@ config CMD_ARMFLASH
>         help
>           ARM Ltd reference designs flash partition access
>
> +config CMD_ADC
> +       bool "adc - Access ADC info and data"
> +       select ADC
> +       select DM_REGULATOR
> +       help
> +         Shows ADC device info and get single shot data;

Please spell out ADC in the help. Also, what is single-shot data?

> +
>  config CMD_CLK
>         bool "clk - Show clock frequencies"
>         help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index c4269ac..4c66353 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -14,6 +14,7 @@ obj-y += version.o
>
>  # command
>  obj-$(CONFIG_CMD_AES) += aes.o
> +obj-$(CONFIG_CMD_ADC) += adc.o
>  obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
>  obj-y += blk_common.o
>  obj-$(CONFIG_SOURCE) += source.o
> diff --git a/cmd/adc.c b/cmd/adc.c
> new file mode 100644
> index 0000000..f82305c
> --- /dev/null
> +++ b/cmd/adc.c
> @@ -0,0 +1,126 @@
> +/*
> + * Copyright (C) 2018 BayLibre, SAS
> + *  Author: Neil Armstrong <narmstrong at baylibre.com>
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +#include <common.h>
> +#include <command.h>
> +#include <dm.h>
> +#include <adc.h>
> +
> +static int do_adc_list(cmd_tbl_t *cmdtp, int flag, int argc,
> +                      char *const argv[])
> +{
> +       struct udevice *dev;
> +       int ret;
> +
> +       ret = uclass_first_device_err(UCLASS_ADC, &dev);

This will probe the device. Is that what you want?

> +       if (ret || !dev) {

You don't need to check dev, since ret will return -ENODEV if there is
no device. See the docs for uclass_first_device_err().

> +               printf("No available ADC device\n");

return?

> +       }
> +
> +       do {
> +               printf("- %s\n", dev->name);
> +
> +               ret = uclass_next_device(&dev);
> +               if (ret)
> +                       return CMD_RET_FAILURE;
> +       } while (dev);
> +
> +       return CMD_RET_SUCCESS;
> +}
> +
> +static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
> +                      char *const argv[])
> +{
> +       struct udevice *dev;
> +       unsigned int data_mask;
> +       int ret, vss, vdd;
> +
> +       if (argc < 2)
> +               return CMD_RET_USAGE;
> +
> +       ret = uclass_get_device_by_name(UCLASS_ADC, argv[1], &dev);
> +       if (ret) {
> +               printf("Unknown ADC device %s\n", argv[1]);
> +               return CMD_RET_FAILURE;
> +       }
> +
> +       printf("ADC Device '%s' :\n", argv[1]);
> +
> +       ret = adc_data_mask(dev, &data_mask);
> +       if (ret)
> +               printf("data mask: Error %d\n", ret);
> +       else
> +               printf("data mask: %x\n", data_mask);

You could perhaps have a function to print the error or the value. I'm
not sure if it is worth it.

> +
> +       ret = adc_vdd_value(dev, &vdd);
> +       if (ret)
> +               printf("vdd: Error %d\n", ret);
> +       else
> +               printf("vdd: %duV\n", vdd);
> +
> +       ret = adc_vss_value(dev, &vss);
> +       if (ret)
> +               printf("vss: Error %d\n", ret);
> +       else
> +               printf("vss: %duV\n", vss);
> +
> +       return CMD_RET_SUCCESS;
> +}
> +

[..]

Regards,
Simon