[U-Boot] [PATCH 0/4] Add adc uclass's channel mask and conversion helpers and enhance adc cmd

[U-Boot] [PATCH 0/4] Add adc uclass's channel mask and conversion helpers and enhance adc cmd

[Fabrice Gasnier]

This series adds ADC uclass's helpers to
- retrieve the ADC device available channels
- ease conversions to standard unit (µV)
- enhance 'adc' command to print information on available ADC channels
- enhance 'adc' command to print conversion result both as raw and µV
- enhance 'adc' command to scan once several or all channels


Fabrice Gasnier (4):
  dm: adc: add uclass's mask and conversion helpers
  cmd: adc: add info on channel mask
  cmd: adc: print single conversion also in uV
  cmd: adc: add an option to scan some or all available channels

 cmd/adc.c                | 70 +++++++++++++++++++++++++++++++++++++++++++++---
 drivers/adc/adc-uclass.c | 37 +++++++++++++++++++++++++
 include/adc.h            | 21 +++++++++++++++
 3 files changed, 124 insertions(+), 4 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH 1/4] dm: adc: add uclass's mask and conversion helpers

[Fabrice Gasnier]

Add two functions to ADC uclass's:
- adc_raw_to_uV() to ease ADC raw value conversion to microvolts
- adc_channel_mask() to get channels on consumer side

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 drivers/adc/adc-uclass.c | 37 +++++++++++++++++++++++++++++++++++++
 include/adc.h            | 21 +++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/drivers/adc/adc-uclass.c b/drivers/adc/adc-uclass.c
index 738c1ea..0a492eb 100644
--- a/drivers/adc/adc-uclass.c
+++ b/drivers/adc/adc-uclass.c
@@ -6,6 +6,7 @@
 
 #include <common.h>
 #include <errno.h>
+#include <div64.h>
 #include <dm.h>
 #include <dm/lists.h>
 #include <dm/device-internal.h>
@@ -77,6 +78,18 @@ int adc_data_mask(struct udevice *dev, unsigned int *data_mask)
 	return 0;
 }
 
+int adc_channel_mask(struct udevice *dev, unsigned int *channel_mask)
+{
+	struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
+
+	if (!uc_pdata)
+		return -ENOSYS;
+
+	*channel_mask = uc_pdata->channel_mask;
+
+	return 0;
+}
+
 int adc_stop(struct udevice *dev)
 {
 	const struct adc_ops *ops = dev_get_driver_ops(dev);
@@ -329,6 +342,30 @@ int adc_vss_value(struct udevice *dev, int *uV)
 	return 0;
 }
 
+int adc_raw_to_uV(struct udevice *dev, unsigned int raw, int *uV)
+{
+	unsigned int data_mask;
+	int ret, val, vref;
+	u64 raw64 = raw;
+
+	ret = adc_vdd_value(dev, &vref);
+	if (ret)
+		return ret;
+
+	if (!adc_vss_value(dev, &val))
+		vref -= val;
+
+	ret = adc_data_mask(dev, &data_mask);
+	if (ret)
+		return ret;
+
+	raw64 *= vref;
+	do_div(raw64, data_mask);
+	*uV = raw64;
+
+	return 0;
+}
+
 static int adc_vdd_platdata_set(struct udevice *dev)
 {
 	struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
diff --git a/include/adc.h b/include/adc.h
index d04c9c4..5841dfb 100644
--- a/include/adc.h
+++ b/include/adc.h
@@ -219,6 +219,17 @@ int adc_channels_data(struct udevice *dev, unsigned int channel_mask,
 int adc_data_mask(struct udevice *dev, unsigned int *data_mask);
 
 /**
+ * adc_channel_mask() - get channel mask for given ADC device
+ *
+ * This can be used if adc uclass platform data is filled.
+ *
+ * @dev:       ADC device to check
+ * @channel_mask: pointer to the returned channel bitmask
+ * @return: 0 if OK, -ve on error
+ */
+int adc_channel_mask(struct udevice *dev, unsigned int *channel_mask);
+
+/**
  * adc_channel_single_shot() - get output data of conversion for the ADC
  * device's channel. This function searches for the device with the given name,
  * starts the given channel conversion and returns the output data.
@@ -284,4 +295,14 @@ int adc_vss_value(struct udevice *dev, int *uV);
  */
 int adc_stop(struct udevice *dev);
 
+/**
+ * adc_raw_to_uV() - converts raw value to microvolts for given ADC device.
+ *
+ * @dev:     ADC device used from conversion
+ * @raw:     raw value to convert
+ * @uV:	     converted value in microvolts
+ * @return:  0 on success or -ve on error
+ */
+int adc_raw_to_uV(struct udevice *dev, unsigned int raw, int *uV);
+
 #endif
-- 
1.9.1

[U-Boot] [PATCH 2/4] cmd: adc: add info on channel mask

[Fabrice Gasnier]

Enhance adc info command to report also the channel mask.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 cmd/adc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/cmd/adc.c b/cmd/adc.c
index c8857ed..39f61c1 100644
--- a/cmd/adc.c
+++ b/cmd/adc.c
@@ -35,7 +35,7 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
 		       char *const argv[])
 {
 	struct udevice *dev;
-	unsigned int data_mask;
+	unsigned int data_mask, ch_mask;
 	int ret, vss, vdd;
 
 	if (argc < 2)
@@ -49,6 +49,10 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
 
 	printf("ADC Device '%s' :\n", argv[1]);
 
+	ret = adc_channel_mask(dev, &ch_mask);
+	if (!ret)
+		printf("channel mask: %x\n", ch_mask);
+
 	ret = adc_data_mask(dev, &data_mask);
 	if (!ret)
 		printf("data mask: %x\n", data_mask);
-- 
1.9.1

[U-Boot] [PATCH 3/4] cmd: adc: print single conversion also in uV

[Fabrice Gasnier]

Use newly introduced adc_raw_to_uV() API to print conversion result
both as raw value and micro-volts by default.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 cmd/adc.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/cmd/adc.c b/cmd/adc.c
index 39f61c1..7360a96 100644
--- a/cmd/adc.c
+++ b/cmd/adc.c
@@ -71,8 +71,9 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
 static int do_adc_single(cmd_tbl_t *cmdtp, int flag, int argc,
 			 char *const argv[])
 {
+	struct udevice *dev;
 	unsigned int data;
-	int ret;
+	int ret, uV;
 
 	if (argc < 3)
 		return CMD_RET_USAGE;
@@ -85,7 +86,11 @@ static int do_adc_single(cmd_tbl_t *cmdtp, int flag, int argc,
 		return CMD_RET_FAILURE;
 	}
 
-	printf("%u\n", data);
+	ret = uclass_get_device_by_name(UCLASS_ADC, argv[1], &dev);
+	if (!ret && !adc_raw_to_uV(dev, data, &uV))
+		printf("%u, %d uV\n", data, uV);
+	else
+		printf("%u\n", data);
 
 	return CMD_RET_SUCCESS;
 }
-- 
1.9.1

[U-Boot] [PATCH 4/4] cmd: adc: add an option to scan some or all available channels

[Fabrice Gasnier]

Add new option to 'adc' command to do a single scan of:
- some channel(s), using mask argument
- all channels available on an ADC device (when optional mask is omitted).

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---

 cmd/adc.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/cmd/adc.c b/cmd/adc.c
index 7360a96..2d635ac 100644
--- a/cmd/adc.c
+++ b/cmd/adc.c
@@ -95,10 +95,62 @@ static int do_adc_single(cmd_tbl_t *cmdtp, int flag, int argc,
 	return CMD_RET_SUCCESS;
 }
 
+static int do_adc_scan(cmd_tbl_t *cmdtp, int flag, int argc,
+		       char *const argv[])
+{
+	struct adc_channel ch[ADC_MAX_CHANNEL];
+	struct udevice *dev;
+	unsigned int ch_mask;
+	int i, chan, ret, uV;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	ret = uclass_get_device_by_name(UCLASS_ADC, argv[1], &dev);
+	if (ret) {
+		pr_err("Can't get the ADC %s: %d\n", argv[1], ret);
+		return CMD_RET_FAILURE;
+	}
+
+	switch (argc) {
+	case 3:
+		ch_mask = simple_strtoul(argv[2], NULL, 0);
+		if (ch_mask)
+			break;
+	case 2:
+		ret = adc_channel_mask(dev, &ch_mask);
+		if (ret) {
+			pr_err("Can't get mask for %s: %d\n", dev->name, ret);
+			return CMD_RET_FAILURE;
+		}
+		break;
+	}
+
+	ret = adc_channels_single_shot(dev->name, ch_mask, ch);
+	if (ret) {
+		pr_err("Can't get single shot for %s (chans mask: 0x%x): %d\n",
+		       dev->name, ch_mask, ret);
+		return CMD_RET_FAILURE;
+	}
+
+	for (chan = 0, i = 0; chan < ADC_MAX_CHANNEL; chan++) {
+		if (!(ch_mask & ADC_CHANNEL(chan)))
+			continue;
+		if (!adc_raw_to_uV(dev, ch[i].data, &uV))
+			printf("[%02d]: %u, %d uV\n", ch[i].id, ch[i].data, uV);
+		else
+			printf("[%02d]: %u\n", ch[i].id, ch[i].data);
+		i++;
+	}
+
+	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, "", ""),
+	U_BOOT_CMD_MKENT(scan, 3, 1, do_adc_scan, "", ""),
 };
 
 static int do_adc(cmd_tbl_t *cmdtp, int flag, int argc,
@@ -124,6 +176,7 @@ static int do_adc(cmd_tbl_t *cmdtp, int flag, int argc,
 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";
+	"adc single <name> <channel> - Get Single data of ADC device channel\n"
+	"adc scan <name> [channel mask] - Scan all [or masked] ADC channels";
 
 U_BOOT_CMD(adc, 4, 1, do_adc, "ADC sub-system", adc_help_text);
-- 
1.9.1

[U-Boot] [PATCH 3/4] cmd: adc: print single conversion also in uV

[Simon Glass]

On 11 October 2018 at 10:00, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> Use newly introduced adc_raw_to_uV() API to print conversion result
> both as raw value and micro-volts by default.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>
>  cmd/adc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>

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

[U-Boot] [PATCH 0/4] Add adc uclass's channel mask and conversion helpers and enhance adc cmd

[Simon Glass]

Hi Fabrice,

On 11 October 2018 at 10:00, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> This series adds ADC uclass's helpers to
> - retrieve the ADC device available channels
> - ease conversions to standard unit (µV)
> - enhance 'adc' command to print information on available ADC channels
> - enhance 'adc' command to print conversion result both as raw and µV
> - enhance 'adc' command to scan once several or all channels
>
>
> Fabrice Gasnier (4):
>   dm: adc: add uclass's mask and conversion helpers
>   cmd: adc: add info on channel mask
>   cmd: adc: print single conversion also in uV
>   cmd: adc: add an option to scan some or all available channels
>
>  cmd/adc.c                | 70 +++++++++++++++++++++++++++++++++++++++++++++---
>  drivers/adc/adc-uclass.c | 37 +++++++++++++++++++++++++
>  include/adc.h            | 21 +++++++++++++++
>  3 files changed, 124 insertions(+), 4 deletions(-)

Please also update test/dm/adc.c to call your new functions.

Regards,
Simon

[U-Boot] [PATCH 2/4] cmd: adc: add info on channel mask

[Simon Glass]

Hi Fabrice,

On 11 October 2018 at 10:00, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> Enhance adc info command to report also the channel mask.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>
>  cmd/adc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/adc.c b/cmd/adc.c
> index c8857ed..39f61c1 100644
> --- a/cmd/adc.c
> +++ b/cmd/adc.c
> @@ -35,7 +35,7 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
>                        char *const argv[])
>  {
>         struct udevice *dev;
> -       unsigned int data_mask;
> +       unsigned int data_mask, ch_mask;
>         int ret, vss, vdd;
>
>         if (argc < 2)
> @@ -49,6 +49,10 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
>
>         printf("ADC Device '%s' :\n", argv[1]);
>
> +       ret = adc_channel_mask(dev, &ch_mask);

Could this just return the channel mask in the return value (or -ve on error)?

> +       if (!ret)
> +               printf("channel mask: %x\n", ch_mask);
> +
>         ret = adc_data_mask(dev, &data_mask);
>         if (!ret)
>                 printf("data mask: %x\n", data_mask);
> --
> 1.9.1
>

Regards,
Simon

[U-Boot] [PATCH 4/4] cmd: adc: add an option to scan some or all available channels

[Simon Glass]

Hi Fabrice,

On 11 October 2018 at 10:00, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> Add new option to 'adc' command to do a single scan of:
> - some channel(s), using mask argument
> - all channels available on an ADC device (when optional mask is omitted).
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> ---
>
>  cmd/adc.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 54 insertions(+), 1 deletion(-)
>

For this you could add a test in test/py/tests/test_adc.py perhaps.

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

[U-Boot] [PATCH 2/4] cmd: adc: add info on channel mask

[Fabrice Gasnier]

On 10/19/2018 05:25 AM, Simon Glass wrote:
> Hi Fabrice,
> 
> On 11 October 2018 at 10:00, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>> Enhance adc info command to report also the channel mask.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> ---
>>
>>  cmd/adc.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/cmd/adc.c b/cmd/adc.c
>> index c8857ed..39f61c1 100644
>> --- a/cmd/adc.c
>> +++ b/cmd/adc.c
>> @@ -35,7 +35,7 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
>>                        char *const argv[])
>>  {
>>         struct udevice *dev;
>> -       unsigned int data_mask;
>> +       unsigned int data_mask, ch_mask;
>>         int ret, vss, vdd;
>>
>>         if (argc < 2)
>> @@ -49,6 +49,10 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
>>
>>         printf("ADC Device '%s' :\n", argv[1]);
>>
>> +       ret = adc_channel_mask(dev, &ch_mask);
> 
> Could this just return the channel mask in the return value (or -ve on error)?

Hi Simon,

Channel mask is unsigned, so I would prefer to rely on (signed) ret to
return -ve on error, or success. And use pointer for the unsigned mask.

Do you agree to keep this approach ?

Thanks for reviewing,
Best Regards,
Fabrice

> 
>> +       if (!ret)
>> +               printf("channel mask: %x\n", ch_mask);
>> +
>>         ret = adc_data_mask(dev, &data_mask);
>>         if (!ret)
>>                 printf("data mask: %x\n", data_mask);
>> --
>> 1.9.1
>>
> 
> Regards,
> Simon
> 

[U-Boot] [PATCH 2/4] cmd: adc: add info on channel mask

[Simon Glass]

Hi Fabrice,

On 19 October 2018 at 01:40, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
>
> On 10/19/2018 05:25 AM, Simon Glass wrote:
> > Hi Fabrice,
> >
> > On 11 October 2018 at 10:00, Fabrice Gasnier <fabrice.gasnier@st.com> wrote:
> >> Enhance adc info command to report also the channel mask.
> >>
> >> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> >> ---
> >>
> >>  cmd/adc.c | 6 +++++-
> >>  1 file changed, 5 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/cmd/adc.c b/cmd/adc.c
> >> index c8857ed..39f61c1 100644
> >> --- a/cmd/adc.c
> >> +++ b/cmd/adc.c
> >> @@ -35,7 +35,7 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
> >>                        char *const argv[])
> >>  {
> >>         struct udevice *dev;
> >> -       unsigned int data_mask;
> >> +       unsigned int data_mask, ch_mask;
> >>         int ret, vss, vdd;
> >>
> >>         if (argc < 2)
> >> @@ -49,6 +49,10 @@ static int do_adc_info(cmd_tbl_t *cmdtp, int flag, int argc,
> >>
> >>         printf("ADC Device '%s' :\n", argv[1]);
> >>
> >> +       ret = adc_channel_mask(dev, &ch_mask);
> >
> > Could this just return the channel mask in the return value (or -ve on error)?
>
> Hi Simon,
>
> Channel mask is unsigned, so I would prefer to rely on (signed) ret to
> return -ve on error, or success. And use pointer for the unsigned mask.
>
> Do you agree to keep this approach ?

Yes that's OK with me.

Regards,
Simon