Message ID | 1524493099-13054-1-git-send-email-narmstrong@baylibre.com |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
Series | [U-Boot] cmd: add ADC cli commands | expand |
Hi Neil, On 23 April 2018 at 08:18, Neil Armstrong <narmstrong@baylibre.com> wrote: > Add an 'adc' cli command to get adc devices informations and read single > shots datas. > > Signed-off-by: Neil Armstrong <narmstrong@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@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@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
Hi, On 26/04/2018 16:40, Simon Glass wrote: > Hi Neil, > > On 23 April 2018 at 08:18, Neil Armstrong <narmstrong@baylibre.com> wrote: >> Add an 'adc' cli command to get adc devices informations and read single >> shots datas. >> >> Signed-off-by: Neil Armstrong <narmstrong@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@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? I'll add more text. > >> + >> 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@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? Is there another way ? I mean you need to probe to know if the device is actually usable, no ? > >> + 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(). Ok > >> + printf("No available ADC device\n"); > > return? Oops > >> + } >> + >> + 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. I can drop printing the error, only printing what's available. > >> + >> + 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 > Thanks, Neil
Hi Neil, On 27 April 2018 at 04:07, Neil Armstrong <narmstrong@baylibre.com> wrote: > Hi, > > On 26/04/2018 16:40, Simon Glass wrote: >> Hi Neil, >> >> On 23 April 2018 at 08:18, Neil Armstrong <narmstrong@baylibre.com> wrote: >>> Add an 'adc' cli command to get adc devices informations and read single >>> shots datas. >>> >>> Signed-off-by: Neil Armstrong <narmstrong@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@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? > > I'll add more text. > >> >>> + >>> 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@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? > > Is there another way ? > I mean you need to probe to know if the device is actually usable, no ? You can use something like uclass_find_first_device() which doesn't probe. Regards, Simon
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; + 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@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); + if (ret || !dev) { + printf("No available ADC device\n"); + } + + 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); + + 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; +} + +static int do_adc_single(cmd_tbl_t *cmdtp, int flag, int argc, + char *const argv[]) +{ + unsigned int data; + int ret; + + if (argc < 3) + return CMD_RET_USAGE; + + ret = adc_channel_single_shot(argv[1], simple_strtol(argv[2], NULL, 0), + &data); + if (ret) { + printf("Error getting single shot for device %s channel %s\n", + argv[1], argv[2]); + return CMD_RET_FAILURE; + } + + printf("%u\n", data); + + return CMD_RET_SUCCESS; +} + +static cmd_tbl_t cmd_adc_sub[] = { + U_BOOT_CMD_MKENT(list, 1, 1, do_adc_list, "", ""), + U_BOOT_CMD_MKENT(info, 2, 1, do_adc_info, "", ""), + U_BOOT_CMD_MKENT(single, 3, 1, do_adc_single, "", ""), +}; + +static int do_adc(cmd_tbl_t *cmdtp, int flag, int argc, + char *const argv[]) +{ + cmd_tbl_t *c; + + if (argc < 2) + return CMD_RET_USAGE; + + /* Strip off leading 'adc' command argument */ + argc--; + argv++; + + c = find_cmd_tbl(argv[0], &cmd_adc_sub[0], ARRAY_SIZE(cmd_adc_sub)); + + if (c) + return c->cmd(cmdtp, flag, argc, argv); + else + return CMD_RET_USAGE; +} + +static char adc_help_text[] = + "list - list ADC devices\n" + "adc info <name> - Get ADC device info\n" + "adc single <name> <channel> - Get Single data of ADC device channel"; + +U_BOOT_CMD(adc, 4, 1, do_adc, "ADC sub-system", adc_help_text);
Add an 'adc' cli command to get adc devices informations and read single shots datas. Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> --- cmd/Kconfig | 7 ++++ cmd/Makefile | 1 + cmd/adc.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 cmd/adc.c