[U-Boot] [PATCH V3 07/14] dm: adc: add simple ADC uclass implementation

Simon Glass
2015-11-05T18:25:20Z

Hi Przemyslaw,

On 27 October 2015 at 06:08, Przemyslaw Marczak <p.marczak@samsung.com> wrote:
> This commit adds:
> - new uclass id: UCLASS_ADC
> - new uclass driver: drivers/adc/adc-uclass.c
>
> The new uclass's API allows for ADC operation on:
> * single-channel with channel selection by a number
> * multti-channel with channel selection by bit mask
>
> ADC uclass's functions:
> * single-channel:
> - adc_start_channel() - start channel conversion
> - adc_channel_data() - get conversion data
> - adc_channel_single_shot() - start/get conversion data
> * multi-channel:
> - adc_start_channels() - start selected channels conversion
> - adc_channels_data() - get conversion data
> - adc_channels_single_shot() - start/get conversion data for channels
> selected by bit mask
> * general:
> - adc_stop() - stop the conversion
> - adc_vdd_value() - positive reference Voltage value with polarity [uV]
> - adc_vss_value() - negative reference Voltage value with polarity [uV]
> - adc_data_mask() - conversion data bit mask
>
> The device tree can provide below constraints/properties:
> - vdd-polarity-negative: if true: Vdd = vdd-microvolts * (-1)
> - vss-polarity-negative: if true: Vss = vss-microvolts * (-1)
> - vdd-supply: phandle to Vdd regulator's node
> - vss-supply: phandle to Vss regulator's node
> And optional, checked only if the above corresponding, doesn't exist:
> - vdd-microvolts: positive reference Voltage [uV]
> - vss-microvolts: negative reference Voltage [uV]
>
> Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
> Cc: Simon Glass <sjg@chromium.org>

Some comments below.

> ---
> Changes V2:
> - new commit - introduce ADC uclass driver
> Changes V3:
> - Add binding info
> - ADC uclass's code rework, add single/multi-channel API
> - Select single channel by a number and multi, by a bit mask
> - Wait for conversion end in uclass's internal function
> - Add ADC supply polarity constraint
> - Add function for getting supply Voltage with polarity
> ---
> doc/device-tree-bindings/adc/adc.txt | 62 ++++++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/adc/Kconfig | 12 +
> drivers/adc/Makefile | 8 +
> drivers/adc/adc-uclass.c | 409 +++++++++++++++++++++++++++++++++++
> include/adc.h | 288 ++++++++++++++++++++++++
> include/dm/uclass-id.h | 1 +
> 8 files changed, 783 insertions(+)
> create mode 100644 doc/device-tree-bindings/adc/adc.txt
> create mode 100644 drivers/adc/Kconfig
> create mode 100644 drivers/adc/Makefile
> create mode 100644 drivers/adc/adc-uclass.c
> create mode 100644 include/adc.h
>
> diff --git a/doc/device-tree-bindings/adc/adc.txt b/doc/device-tree-bindings/adc/adc.txt
> new file mode 100644
> index 0000000..463de3c
> --- /dev/null
> +++ b/doc/device-tree-bindings/adc/adc.txt
> @@ -0,0 +1,62 @@
> +ADC device binding
> +
> +There are no mandatory properties for ADC. However, if Voltage info is required,
> +then there are two options:
> +- use microvolts constraint or
> +- use regulator phandle to enable/read supply's Voltage
> +
> +Properties and constraints:
> +*optional and always checked, Voltage polarity info:
> +- vdd-polarity-negative: positive reference Voltage has a negative polarity
> +- vss-polarity-negative: negative reference Voltage has a negative polarity
> +
> +Chose one option, for each supply (Vdd/Vss):
> +
> +*optional and always checked, supply Voltage constants:
> +- vdd-supply: phandle to Vdd regulator's node
> +- vss-supply: phandle to Vss regulator's node
> +
> +*optional and checked only if the above corresponding, doesn't exist:
> +- vdd-microvolts: positive reference Voltage value [uV]
> +- vss-microvolts: negative reference Voltage value [uV]
> +
> +Example with constant 'Vdd' value:
> +adc at 1000000 {
> + compatible = "some-adc";
> + reg = <0xaabb000 0x100>;
> + status = "enabled";
> + vdd-microvolts = <1800000>;
> +};
> +
> +Example of supply phandle usage, for the ADC's VDD/VSS references as below:
> + _______ _______
> + |Sandbox| |Sandbox|
> + : PMIC : : ADC :
> + . . . .
> + | | (Vdd) | AIN0|-->
> + | BUCK2|-------|VDDref |
> + | (3.3V)| _|VSSref |
> + |_______| | |_______|
> + _|_
> +
> +For the above PMIC, the node can be defined as follows:
> +sandbox_pmic {
> + compatible = "sandbox,pmic";
> + ...
> + buck2: buck2 {
> + regulator-name = "SUPPLY_3.3V";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> + ...
> +};
> +
> +For the above ADC, the node can be defined as follows:
> +adc at 0 {
> + compatible = "sandbox,adc";
> + vdd-supply = <&buck2>;
> + vss-microvolts = <0>;
> +};
> +
> +The ADC uclass code, will enable the supply before start of the conversion,
> +but it will not configure the regulator settings.
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index ba88b5e..c481e93 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -4,6 +4,8 @@ source "drivers/core/Kconfig"
>
> # types of drivers sorted in alphabetical order
>
> +source "drivers/adc/Kconfig"
> +
> source "drivers/block/Kconfig"
>
> source "drivers/clk/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 4f49bfd..ad29a4f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_SPL_SATA_SUPPORT) += block/
>
> else
>
> +obj-y += adc/
> obj-$(CONFIG_DM_DEMO) += demo/
> obj-$(CONFIG_BIOSEMU) += bios_emulator/
> obj-y += block/
> diff --git a/drivers/adc/Kconfig b/drivers/adc/Kconfig
> new file mode 100644
> index 0000000..b6e226a
> --- /dev/null
> +++ b/drivers/adc/Kconfig
> @@ -0,0 +1,12 @@
> +config ADC
> + bool "Enable ADC drivers using Driver Model"
> + help
> + This enables ADC API for drivers, which allows driving ADC features
> + by single and multi-channel methods for:
> + - start/stop/get data for conversion of a single-channel selected by
> + a number or multi-channels selected by a bitmask
> + - get data mask (ADC resolution)
> + ADC reference Voltage supply options:
> + - methods for get Vdd/Vss reference Voltage values with polarity
> + - support supply's phandle with auto-enable
> + - supply polarity setting in fdt
> diff --git a/drivers/adc/Makefile b/drivers/adc/Makefile
> new file mode 100644
> index 0000000..c4d9618
> --- /dev/null
> +++ b/drivers/adc/Makefile
> @@ -0,0 +1,8 @@
> +#
> +# Copyright (C) 2015 Samsung Electronics
> +# Przemyslaw Marczak <p.marczak@samsung.com>
> +#
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +
> +obj-$(CONFIG_ADC) += adc-uclass.o
> diff --git a/drivers/adc/adc-uclass.c b/drivers/adc/adc-uclass.c
> new file mode 100644
> index 0000000..9233fcd
> --- /dev/null
> +++ b/drivers/adc/adc-uclass.c
> @@ -0,0 +1,409 @@
> +/*
> + * Copyright (C) 2015 Samsung Electronics
> + * Przemyslaw Marczak <p.marczak@samsung.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <dm/lists.h>
> +#include <dm/device-internal.h>
> +#include <dm/uclass-internal.h>
> +#include <adc.h>

That should go below common.h

> +#include <power/regulator.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define ADC_UCLASS_PLATDATA_SIZE sizeof(struct adc_uclass_platdata)

Please drop this and just use sizeof() where needed.

> +#define CHECK_NUMBER true
> +#define CHECK_MASK (!CHECK_NUMBER)

What are those for? I think you should convert this to an enum:

enum some_name_t {
CHECK_NUMBER,
CHECK_MASK
}

and use an enum parameter instead of bool.

> +
> +/* TODO: add support for timer uclass (for early calls) */

TODO(email):

so we know who is going to send the follow-up patch.

> +#ifdef CONFIG_SANDBOX_ARCH
> +#define sdelay(x) udelay(x)
> +#else
> +extern void sdelay(unsigned long loops);
> +#endif
> +
> +static int check_channel(struct udevice *dev, int value, bool number_or_mask,
> + const char *caller_function)

What does this function do? I think it needs a function comment.

> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + unsigned mask = number_or_mask ? (1 << value) : value;
> +
> + /* For the real ADC hardware, some ADC channels can be inactive.

Comment style

/*
* For the....

> + * For example if device has 4 analog channels, and only channels
> + * 1-st and 3-rd are valid, then channel mask is: 0b1010, so request
> + * with mask 0b1110 should return an error.
> + */
> + if ((uc_pdata->channel_mask >= mask) && (uc_pdata->channel_mask & mask))
> + return 0;
> +
> + printf("Error in %s/%s().\nWrong channel selection for device: %s\n",
> + __FILE__, caller_function, dev->name);
> +
> + return -EINVAL;
> +}
> +
> +static int adc_supply_enable(struct udevice *dev)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + const char *supply_type;
> + int ret = 0;
> +
> + if (uc_pdata->vdd_supply) {
> + supply_type = "vdd";
> + ret = regulator_set_enable(uc_pdata->vdd_supply, true);
> + }
> +
> + if (!ret && uc_pdata->vss_supply) {
> + supply_type = "vss";
> + ret = regulator_set_enable(uc_pdata->vss_supply, true);
> + }
> +
> + if (ret)
> + error("%s: can't enable %s-supply!", dev->name, supply_type);
> +
> + return ret;
> +}
> +
> +int adc_data_mask(struct udevice *dev, unsigned int *data_mask)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> +
> + if (!uc_pdata)
> + return -ENOSYS;
> +
> + *data_mask = uc_pdata->data_mask;
> + return 0;
> +}
> +
> +int adc_stop(struct udevice *dev)
> +{
> + const struct adc_ops *ops = dev_get_driver_ops(dev);
> +
> + if (!ops->stop)
> + return -ENOSYS;
> +
> + return ops->stop(dev);
> +}
> +
> +int adc_start_channel(struct udevice *dev, int channel)
> +{
> + const struct adc_ops *ops = dev_get_driver_ops(dev);
> + int ret;
> +
> + if (!ops->start_channel)
> + return -ENOSYS;
> +
> + ret = check_channel(dev, channel, CHECK_NUMBER, __func__);
> + if (ret)
> + return ret;
> +
> + ret = adc_supply_enable(dev);
> + if (ret)
> + return ret;
> +
> + return ops->start_channel(dev, channel);
> +}
> +
> +int adc_start_channels(struct udevice *dev, unsigned int channel_mask)
> +{
> + const struct adc_ops *ops = dev_get_driver_ops(dev);
> + int ret;
> +
> + if (!ops->start_channels)
> + return -ENOSYS;
> +
> + ret = check_channel(dev, channel_mask, CHECK_MASK, __func__);
> + if (ret)
> + return ret;
> +
> + ret = adc_supply_enable(dev);
> + if (ret)
> + return ret;
> +
> + return ops->start_channels(dev, channel_mask);
> +}
> +
> +int adc_channel_data(struct udevice *dev, int channel, unsigned int *data)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + const struct adc_ops *ops = dev_get_driver_ops(dev);
> + unsigned int timeout_us = uc_pdata->data_timeout_us;
> + int ret;
> +
> + if (!ops->channel_data)
> + return -ENOSYS;
> +
> + ret = check_channel(dev, channel, CHECK_NUMBER, __func__);
> + if (ret)
> + return ret;
> +
> + do {
> + ret = ops->channel_data(dev, channel, data);
> + if (!ret || ret != -EBUSY)
> + break;
> +
> + /* TODO: use timer uclass (for early calls). */

Remove '.'

> + sdelay(5);
> + } while (timeout_us--);
> +
> + return ret;
> +}
> +
> +int adc_channels_data(struct udevice *dev, unsigned int channel_mask,
> + struct adc_channel *channels)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + unsigned int timeout_us = uc_pdata->multidata_timeout_us;
> + const struct adc_ops *ops = dev_get_driver_ops(dev);
> + int ret;
> +
> + if (!ops->channels_data)
> + return -ENOSYS;
> +
> + ret = check_channel(dev, channel_mask, CHECK_MASK, __func__);
> + if (ret)
> + return ret;
> +
> + do {
> + ret = ops->channels_data(dev, channel_mask, channels);
> + if (!ret || ret != -EBUSY)
> + break;
> +
> + /* TODO: use timer uclass (for early calls). */
> + sdelay(5);
> + } while (timeout_us--);
> +
> + return ret;
> +}
> +
> +int adc_channel_single_shot(const char *name, int channel, unsigned int *data)
> +{
> + struct udevice *dev;
> + int ret;
> +
> + ret = uclass_get_device_by_name(UCLASS_ADC, name, &dev);
> + if (ret)
> + return ret;
> +
> + ret = adc_start_channel(dev, channel);
> + if (ret)
> + return ret;
> +
> + ret = adc_channel_data(dev, channel, data);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static int _adc_channels_single_shot(struct udevice *dev,
> + unsigned int channel_mask,
> + struct adc_channel *channels)
> +{
> + unsigned int data;
> + int channel, ret;
> +
> + for (channel = 0; channel <= ADC_MAX_CHANNEL; channel++) {
> + /* Check channel bit. */
> + if (!((channel_mask >> channel) & 0x1))
> + continue;
> +
> + ret = adc_start_channel(dev, channel);
> + if (ret)
> + return ret;
> +
> + ret = adc_channel_data(dev, channel, &data);
> + if (ret)
> + return ret;
> +
> + channels->id = channel;
> + channels->data = data;
> + channels++;
> + }
> +
> + return 0;
> +}
> +
> +int adc_channels_single_shot(const char *name, unsigned int channel_mask,
> + struct adc_channel *channels)
> +{
> + struct udevice *dev;
> + int ret;
> +
> + ret = uclass_get_device_by_name(UCLASS_ADC, name, &dev);
> + if (ret)
> + return ret;
> +
> + ret = adc_start_channels(dev, channel_mask);
> + if (ret)
> + goto try_manual;
> +
> + ret = adc_channels_data(dev, channel_mask, channels);
> + if (ret)
> + return ret;
> +
> + return 0;
> +
> +try_manual:
> + if (ret != -ENOSYS)
> + return ret;
> +
> + return _adc_channels_single_shot(dev, channel_mask, channels);
> +}
> +
> +static int adc_vdd_platdata_update(struct udevice *dev)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + int ret;
> +
> + /* Warning!
> + * This function can't return supply device before its bind.
> + * Please pay attention to proper fdt scan sequence. If ADC device
> + * will bind before its supply regulator device, then the below 'get'
> + * will return an error.
> + */

How could this happen? Is this function called before the device is probed?

It seems to be called from adc_pre_probe(), by which time all devices
should be bound.

> + ret = device_get_supply_regulator(dev, "vdd-supply",
> + &uc_pdata->vdd_supply);
> + if (ret)
> + return ret;
> +
> + ret = regulator_get_value(uc_pdata->vdd_supply);
> + if (ret < 0)
> + return ret;
> +
> + uc_pdata->vdd_microvolts = ret;
> +
> + return 0;
> +}
> +
> +static int adc_vss_platdata_update(struct udevice *dev)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + int ret;
> +
> + ret = device_get_supply_regulator(dev, "vss-supply",
> + &uc_pdata->vss_supply);
> + if (ret)
> + return ret;
> +
> + ret = regulator_get_value(uc_pdata->vss_supply);
> + if (ret < 0)
> + return ret;
> +
> + uc_pdata->vss_microvolts = ret;
> +
> + return 0;
> +}
> +
> +int adc_vdd_value(struct udevice *dev, int *uV)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + int ret, value_sign = uc_pdata->vdd_polarity_negative ? -1 : 1;
> +
> + if (!uc_pdata->vdd_supply)
> + goto nodev;
> +
> + /* Update the regulator Value. */
> + ret = adc_vdd_platdata_update(dev);
> + if (ret)
> + return ret;
> +nodev:
> + if (uc_pdata->vdd_microvolts == -ENODATA)
> + return -ENODATA;
> +
> + *uV = uc_pdata->vdd_microvolts * value_sign;
> +
> + return 0;
> +}
> +
> +int adc_vss_value(struct udevice *dev, int *uV)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + int ret, value_sign = uc_pdata->vss_polarity_negative ? -1 : 1;
> +
> + if (!uc_pdata->vss_supply)
> + goto nodev;
> +
> + /* Update the regulator Value. */
> + ret = adc_vss_platdata_update(dev);
> + if (ret)
> + return ret;
> +nodev:
> + if (uc_pdata->vss_microvolts == -ENODATA)
> + return -ENODATA;
> +
> + *uV = uc_pdata->vss_microvolts * value_sign;
> +
> + return 0;
> +}
> +
> +static int adc_vdd_platdata_set(struct udevice *dev)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + int ret, offset = dev->of_offset;
> + const void *fdt = gd->fdt_blob;
> + char *prop;
> +
> + prop = "vdd-polarity-negative";

Can you just drop this variable and use the string?

> + uc_pdata->vdd_polarity_negative = fdtdec_get_bool(fdt, offset, prop);
> +
> + ret = adc_vdd_platdata_update(dev);
> + if (ret != -ENOENT)
> + return ret;
> +
> + /* No vdd-supply phandle. */
> + prop = "vdd-microvolts";
> + uc_pdata->vdd_microvolts = fdtdec_get_int(fdt, offset, prop, -ENODATA);
> +
> + return 0;
> +}
> +
> +static int adc_vss_platdata_set(struct udevice *dev)
> +{
> + struct adc_uclass_platdata *uc_pdata = dev_get_uclass_platdata(dev);
> + int ret, offset = dev->of_offset;
> + const void *fdt = gd->fdt_blob;
> + char *prop;
> +
> + prop = "vss-polarity-negative";

Can you just drop this variable and use the string?

> + uc_pdata->vss_polarity_negative = fdtdec_get_bool(fdt, offset, prop);
> +
> + ret = adc_vss_platdata_update(dev);
> + if (ret != -ENOENT)
> + return ret;
> +
> + /* No vss-supply phandle. */
> + prop = "vss-microvolts";
> + uc_pdata->vss_microvolts = fdtdec_get_int(fdt, offset, prop, -ENODATA);
> +
> + return 0;
> +}
> +
> +static int adc_pre_probe(struct udevice *dev)
> +{
> + int ret;
> +
> + /* Set ADC VDD platdata: polarity, uV, regulator (phandle). */
> + ret = adc_vdd_platdata_set(dev);
> + if (ret)
> + error("%s: Can't update Vdd. Error: %d", dev->name, ret);

Shouldn't this return ret?

> +
> + /* Set ADC VSS platdata: polarity, uV, regulator (phandle). */
> + ret = adc_vss_platdata_set(dev);
> + if (ret)
> + error("%s: Can't update Vss. Error: %d", dev->name, ret);

and here?

> +
> + return 0;
> +}
> +
> +UCLASS_DRIVER(adc) = {
> + .id = UCLASS_ADC,
> + .name = "adc",
> + .pre_probe = adc_pre_probe,
> + .per_device_platdata_auto_alloc_size = ADC_UCLASS_PLATDATA_SIZE,
> +};
> diff --git a/include/adc.h b/include/adc.h
> new file mode 100644
> index 0000000..4b14017
> --- /dev/null
> +++ b/include/adc.h
> @@ -0,0 +1,288 @@
> +/*
> + * Copyright (C) 2015 Samsung Electronics
> + * Przemyslaw Marczak <p.marczak@samsung.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef _ADC_H_
> +#define _ADC_H_
> +
> +/* ADC_CHANNEL() - ADC channel bit mask, to select only required channels */
> +#define ADC_CHANNEL(x) (1 << x)

(1U << (x))

> +
> +/* The last possible selected channel with 32-bit mask */
> +#define ADC_MAX_CHANNEL 31

Can we use ADC_MAX_CHANNELS = 32 instead? You can adjust to for loop
to use < instead of <=.

> +
> +/**
> + * adc_data_format: define the ADC output data format, can be useful when
> + * the device's input Voltage range is bipolar.
> + * - ADC_DATA_FORMAT_BIN - binary offset
> + * - ADC_DATA_FORMAT_2S - two's complement
> + *
> + * Note: Device's driver should fill the 'data_format' field of its uclass's
> + * platform data using one of the above data format types.
> + */
> +enum adc_data_format {
> + ADC_DATA_FORMAT_BIN,
> + ADC_DATA_FORMAT_2S,
> +};
> +
> +/**
> + * struct adc_channel - structure to hold channel conversion data.
> + * Useful to keep the result of a multi-channel conversion output.
> + *
> + * @id - channel id
> + * @data - channel conversion data
> + */
> +struct adc_channel {
> + int id;
> + unsigned int data;
> +};
> +
> +/**
> + * struct adc_uclass_platdata - basic ADC info
> + *
> + * Note: The positive/negative reference Voltage is only a name and it doesn't
> + * provide an information about the value polarity. It is possible, for both
> + * values to be a negative or positive. For this purpose the uclass's platform
> + * data provides a bool fields: 'vdd/vss_supply_is_negative'. This is useful,
> + * since the regulator API returns only a positive Voltage values.
> + *
> + * To get the reference Voltage values with polarity, use functions:
> + * - adc_vdd_value()
> + * - adc_vss_value()
> + * Those are useful for some cases of ADC's references, e.g.:
> + * * Vdd: +3.3V; Vss: -3.3V -> 6.6 Vdiff
> + * * Vdd: +3.3V; Vss: +0.3V -> 3.0 Vdiff
> + * * Vdd: +3.3V; Vss: 0.0V -> 3.3 Vdiff
> + * The last one is usually standard and doesn't require the fdt polarity info.
> + *
> + * For more informations read binding info:

information

> + * - doc/device-tree-bindings/adc/adc.txt
> + *
> + * @data_mask - conversion output data mask

Use : rather than -

@data_mask: conversion output data mask

> + * @data_timeout_us - single channel conversion timeout
> + * @multidata_timeout_us - multi channel conversion timeout
> + * @channel_mask - bit mask of available channels [0:31]
> + * @vdd_supply - positive reference Voltage supply (regulator)
> + * @vss_supply - negative reference Voltage supply (regulator)
> + * @vdd_polarity_negative - positive reference Voltage has negative polarity
> + * @vss_polarity_negative - negative reference Voltage has negative polarity
> + * @vdd_microvolts - positive reference Voltage value
> + * @vss_microvolts - negative reference Voltage value
> + */
> +struct adc_uclass_platdata {
> + int data_format;
> + unsigned int data_mask;
> + unsigned int data_timeout_us;
> + unsigned int multidata_timeout_us;
> + unsigned int channel_mask;
> + struct udevice *vdd_supply;
> + struct udevice *vss_supply;
> + bool vdd_polarity_negative;
> + bool vss_polarity_negative;
> + int vdd_microvolts;

Would vdd_uv be good enough?

> + int vss_microvolts;
> +};
> +
> +/**
> + * struct adc_ops - ADC device operations for single/multi-channel operation.
> + */
> +struct adc_ops {
> + /**
> + * start_channel() - start conversion with its default parameters
> + * for the given channel number.
> + *
> + * @dev: ADC device to init
> + * @channel: analog channel number
> + * @return: 0 if OK, -ve on error
> + */
> + int (*start_channel)(struct udevice *dev, int channel);
> +
> + /**
> + * start_channels() - start conversion with its default parameters
> + * for the channel numbers selected by the bit mask.
> + *
> + * This is optional, useful when the hardware supports multichannel
> + * conversion by the single software trigger.
> + *
> + * @dev: ADC device to init
> + * @channel_mask: bit mask of selected analog channels
> + * @return: 0 if OK, -ve on error
> + */
> + int (*start_channels)(struct udevice *dev, unsigned int channel_mask);
> +
> + /**
> + * channel_data() - get conversion output data for the given channel.
> + *
> + * Note: The implementation of this function should only check, that
> + * the conversion data is available at the call time. If the hardware
> + * requires some delay to get the data, then this function should
> + * return with -EBUSY value. The ADC API will call it in a loop,
> + * until the data is available or the timeout expires. The maximum
> + * timeout for this operation is defined by the field 'data_timeout_us'
> + * in ADC uclasses platform data structure.
> + *
> + * @dev: ADC device to trigger
> + * @channel: selected analog channel number
> + * @data: returned pointer to selected channel's output data
> + * @return: 0 if OK, -EBUSY if busy, and other negative on error
> + */
> + int (*channel_data)(struct udevice *dev, int channel,
> + unsigned int *data);
> +
> + /**
> + * channels_data() - get conversion data for the selected channels.
> + *
> + * This is optional, useful when multichannel conversion is supported
> + * by the hardware, by the single software trigger.
> + *
> + * For the proper implementation, please look at the 'Note' for the
> + * above method. The only difference is in used timeout value, which
> + * is defined by field 'multidata_timeout_us'.
> + *
> + * @dev: ADC device to trigger
> + * @channel_mask: bit mask of selected analog channels
> + * @channels: returned pointer to array of output data for channels
> + * selected by the given mask
> + * @return: 0 if OK, -ve on error
> + */
> + int (*channels_data)(struct udevice *dev, unsigned int channel_mask,
> + struct adc_channel *channels);
> +
> + /**
> + * stop() - stop conversion of the given ADC device
> + *
> + * @dev: ADC device to stop
> + * @return: 0 if OK, -ve on error
> + */
> + int (*stop)(struct udevice *dev);

Does this stop all channels? Should update the comment to explain the
API better here.

> +};
> +
> +/**
> + * adc_start_channel() - start conversion for given device/channel and exit.
> + *
> + * @dev: ADC device
> + * @channel: analog channel number
> + * @return: 0 if OK, -ve on error
> + */
> +int adc_start_channel(struct udevice *dev, int channel);
> +
> +/**
> + * adc_start_channels() - start conversion for given device/channels and exit.
> + *
> + * Note:
> + * To use this function, device must implement method: start_channels().
> + *
> + * @dev: ADC device to start
> + * @channel_mask: channel selection - a bit mask
> + * @channel_mask: bit mask of analog channels
> + * @return: 0 if OK, -ve on error
> + */
> +int adc_start_channels(struct udevice *dev, unsigned int channel_mask);
> +
> +/**
> + * adc_channel_data() - get conversion data for the given device channel number.
> + *
> + * @dev: ADC device to read
> + * @channel: analog channel number
> + * @data: pointer to returned channel's data
> + * @return: 0 if OK, -ve on error
> + */
> +int adc_channel_data(struct udevice *dev, int channel, unsigned int *data);
> +
> +/**
> + * adc_channels_data() - get conversion data for the channels selected by mask
> + *
> + * Note:
> + * To use this function, device must implement methods:
> + * - start_channels()
> + * - channels_data()
> + *
> + * @dev: ADC device to read
> + * @channel_mask: channel selection - a bit mask
> + * @channels: pointer to structure array of returned data for each channel
> + * @return: 0 if OK, -ve on error
> + */
> +int adc_channels_data(struct udevice *dev, unsigned int channel_mask,
> + struct adc_channel *channels);
> +
> +/**
> + * adc_data_mask() - get data mask (ADC resolution bitmask) for given ADC device
> + *
> + * This can be used if adc uclass platform data is filled.
> + *
> + * @dev: ADC device to check
> + * @data_mask: pointer to the returned data bitmask
> + * @return: 0 if OK, -ve on error
> + */
> +int adc_data_mask(struct udevice *dev, unsigned int *data_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.
> + *
> + * Note: To use this function, device must implement metods:
> + * - start_channel()
> + * - channel_data()
> + *
> + * @name: device's name to search
> + * @channel: device's input channel to init
> + * @data: pointer to conversion output data
> + * @return: 0 if OK, -ve on error
> + */
> +int adc_channel_single_shot(const char *name, int channel, unsigned int *data);
> +
> +/**
> + * adc_channels_single_shot() - get ADC conversion output data for the selected
> + * device's channels. This function searches for the device by the given name,
> + * starts the selected channels conversion and returns the output data as array
> + * of type 'struct adc_channel'.
> + *
> + * Note: This function can be used if device implements one of ADC's single
> + * or multi-channel operation API. If multi-channel operation is not supported,
> + * then each selected channel is triggered by the sequence start/data in a loop.
> + *
> + * @name: device's name to search
> + * @channel_mask: channel selection - a bit mask
> + * @channels: pointer to conversion output data for the selected channels
> + * @return: 0 if OK, -ve on error
> + */
> +int adc_channels_single_shot(const char *name, unsigned int channel_mask,
> + struct adc_channel *channels);
> +
> +/**
> + * adc_vdd_value() - get the ADC device's positive reference Voltage value
> + *
> + * Note: Depending on bool value 'vdd_supply_is_negative' of platform data,
> + * the returned uV value can be negative, and it's not an error.
> + *
> + * @dev: ADC device to check
> + * @uV: Voltage value with polarization sign (uV)
> + * @return: 0 on success or -ve on error
> +*/
> +int adc_vdd_value(struct udevice *dev, int *uV);
> +
> +/**
> + * adc_vss_value() - get the ADC device's negative reference Voltage value
> + *
> + * Note: Depending on bool value 'vdd_supply_is_negative' of platform data,
> + * the returned uV value can be negative, and it's not an error.
> + *
> + * @dev: ADC device to check
> + * @uV: Voltage value with polarization sign (uV)
> + * @return: 0 on success or -ve on error
> +*/
> +int adc_vss_value(struct udevice *dev, int *uV);
> +
> +/**
> + * adc_stop() - stop operation for given ADC device.
> + *
> + * @dev: ADC device to stop
> + * @return: 0 if OK, -ve on error
> + */
> +int adc_stop(struct udevice *dev);
> +
> +#endif
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 886a44c..d0cf4ce 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -25,6 +25,7 @@ enum uclass_id {
> UCLASS_SIMPLE_BUS, /* bus with child devices */
>
> /* U-Boot uclasses start here - in alphabetical order */
> + UCLASS_ADC, /* Analog-to-digital converter */
> UCLASS_CLK, /* Clock source, e.g. used by peripherals */
> UCLASS_CPU, /* CPU, typically part of an SoC */
> UCLASS_CROS_EC, /* Chrome OS EC */
> --
> 1.9.1
>

Regards,
Simon