On 02/28/2017 04:49 PM, Joel Stanley wrote:
> On Wed, Mar 1, 2017 at 6:44 AM, Rick Altherr <[email protected]> wrote:
>> Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. This
>> driver implements reading the ADC values, enabling channels via device
>> tree, and optionally providing channel labels via device tree. Low and
>> high threshold interrupts are supported by the hardware but not
>> implemented.
>>
>> Signed-off-by: Rick Altherr <[email protected]>
>
> Looks good. Some minor comments below.
>
> Is there a reason you wrote a hwmon driver instead of an iio driver? I
> wasn't sure what the recommended subsystem is.
Excellent point. Question is really if there is a plan to add support for
thresholds. If not, an iio driver might be more appropriate.
Guenter
>
>> ---
>> drivers/hwmon/Kconfig | 10 ++
>> drivers/hwmon/Makefile | 1 +
>> drivers/hwmon/aspeed_adc.c | 383 +++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 394 insertions(+)
>> create mode 100644 drivers/hwmon/aspeed_adc.c
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index 0649d53f3d16..c99a67b4afe4 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -261,6 +261,16 @@ config SENSORS_ASC7621
>> This driver can also be built as a module. If so, the module
>> will be called asc7621.
>>
>> +config SENSORS_ASPEED_ADC
>> + tristate "Aspeed AST2400/AST2500 ADC"
>> + depends on ARCH_ASPEED
>
> depends on ARCH_ASPEED || COMPILE_TEST.
>
>> + help
>> + If you say yes here you get support for the Aspeed AST2400/AST2500
>> + ADC.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called aspeed_adc.
>> +
>> config SENSORS_K8TEMP
>> tristate "AMD Athlon64/FX or Opteron temperature sensor"
>> depends on X86 && PCI
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index 5509edf6186a..eede049c9d0d 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -46,6 +46,7 @@ obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o
>> obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
>> obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
>> obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
>> +obj-$(CONFIG_SENSORS_ASPEED_ADC) += aspeed_adc.o
>> obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
>> obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
>> obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
>> diff --git a/drivers/hwmon/aspeed_adc.c b/drivers/hwmon/aspeed_adc.c
>> new file mode 100644
>> index 000000000000..098e32315264
>> --- /dev/null
>> +++ b/drivers/hwmon/aspeed_adc.c
>> @@ -0,0 +1,383 @@
>> +/*
>> + * Aspeed AST2400/2500 ADC
>> + *
>> + * Copyright (C) 2017 Google, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <asm/io.h>
>> +#include <linux/clk.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/types.h>
>> +
>> +#define ASPEED_ADC_NUM_CHANNELS 16
>> +#define ASPEED_ADC_REF_VOLTAGE 2500 /* millivolts */
>> +
>> +#define ASPEED_ADC_REG_ADC00 0x00
>> +#define ASPEED_ADC_REG_ADC04 0x04
>> +#define ASPEED_ADC_REG_ADC08 0x08
>> +#define ASPEED_ADC_REG_ADC0C 0x0C
>> +#define ASPEED_ADC_REG_ADC10 0x10
>> +#define ASPEED_ADC_REG_ADC14 0x14
>> +#define ASPEED_ADC_REG_ADC18 0x18
>> +#define ASPEED_ADC_REG_ADC1C 0x1C
>> +#define ASPEED_ADC_REG_ADC20 0x20
>> +#define ASPEED_ADC_REG_ADC24 0x24
>> +#define ASPEED_ADC_REG_ADC28 0x28
>> +#define ASPEED_ADC_REG_ADC2C 0x2C
>
> I'm not sure that these defines add any value. I'd either give them
> names such as "ASPEED_ADC_ENGINE_CTRL". or just use the register
> offset directly.
>
>> +
>> +#define ASPEED_ADC_OPERATION_MODE_POWER_DOWN (0x0 << 1)
>> +#define ASPEED_ADC_OPERATION_MODE_STANDBY (0x1 << 1)
>> +#define ASPEED_ADC_OPERATION_MODE_NORMAL (0x7 << 1)
>> +
>> +#define ASPEED_ADC_ENGINE_ENABLE BIT(0)
>> +
>> +struct aspeed_adc_data {
>> + struct device *dev;
>> + void __iomem *base;
>> + struct clk *pclk;
>> + struct mutex lock;
>> + unsigned long update_interval_ms;
>> + u32 enabled_channel_mask;
>> + const char* channel_labels[ASPEED_ADC_NUM_CHANNELS];
>> +};
>> +
>> +static int aspeed_adc_set_adc_clock_frequency(
>> + struct aspeed_adc_data *data,
>> + unsigned long desired_update_interval_ms)
>> +{
>> + long pclk_hz = clk_get_rate(data->pclk);
>> + long adc_combined_divisor;
>> + long adc_pre_divisor;
>> + long adc_divisor;
>> + long adc_clock_control_reg_val;
>> + long num_enabled_channels = hweight32(data->enabled_channel_mask);
>
> Some whitespace damage here.
>
>> +
>> + if (desired_update_interval_ms < 1)
>> + return -EINVAL;
>> +
>> + /* From the AST2400 datasheet:
>
> Nit: kernel style is to leave a blank line on the first line of
> multi-line comments:
>
> /*
> * Foo
> * etc
>
>> + * adc_period_s = pclk_period_s * 2 * (pre_divisor+1) * (divisor+1)
>> + *
>> + * and
>> + *
>> + * sample_period_s = 12 * adc_period_s
>> + *
>> + * Substitute pclk_period_s = (1 / pclk_hz)
>> + *
>> + * Solve for (pre-divisor+1) * (divisor+1) as those are settings we need
>> + * to program:
>> + * (pre-divisor+1) * (divisor+1) = (sample_period_s * pclk_hz) / 24
>> + */
>> +
>> + /* Assume PCLK runs at a fairly high frequency so dividing it first
>> + * loses little accuracy. Also note that the above formulas have
>> + * sample_period in seconds while our desired_update_interval is in
>> + * milliseconds, hence the divide by 1000.
>> + */
>> + adc_combined_divisor = desired_update_interval_ms *
>> + (pclk_hz / 24 / 1000 / num_enabled_channels);
>> +
>> + /* Prefer to use the ADC divisor over the ADC pre-divisor. ADC divisor
>> + * is 10-bits wide so anything over 1024 must use the pre-divisor.
>> + */
>> + adc_pre_divisor = 1;
>> + while (adc_combined_divisor/adc_pre_divisor > (1<<10)) {
>> + adc_pre_divisor += 1;
>> + };
>> + adc_divisor = adc_combined_divisor / adc_pre_divisor;
>> +
>> + /* Since ADC divisor and pre-divisor are used in division, the register
>> + * value is implicitly incremented by one before use. This means we
>> + * need to subtract one from our calculated values above.
>> + */
>> + adc_pre_divisor -= 1;
>> + adc_divisor -= 1;
>> +
>> + adc_clock_control_reg_val = (adc_pre_divisor << 17) | adc_divisor;
>> +
>> + mutex_lock(&data->lock);
>> + data->update_interval_ms =
>> + (adc_pre_divisor + 1) * (adc_divisor + 1)
>> + / (pclk_hz / 24 / 1000);
>> + writel(adc_clock_control_reg_val, data->base + ASPEED_ADC_REG_ADC0C);
>> + mutex_unlock(&data->lock);
>> +
>> + return 0;
>> +}
>> +
>> +static int aspeed_adc_get_channel_reading(struct aspeed_adc_data *data,
>> + int channel, long *val)
>> +{
>> + u32 data_reg;
>> + u32 data_reg_val;
>> + long adc_val;
>> +
>> + /* Each 32-bit data register contains 2 10-bit ADC values. */
>> + data_reg = ASPEED_ADC_REG_ADC10 + (channel / 2) * 4;
>> + data_reg_val = readl(data->base + data_reg);
>> + if (channel % 2 == 0) {
>> + adc_val = data_reg_val & 0x3FF;
>> + } else {
>> + adc_val = (data_reg_val >> 16) & 0x3FF;
>> + }
>
> I wonder if you could replace the above block with:
>
> adc_val = readw(data->base + ASPEED_ADC_REG_ADC10 + channel);
>
>> +
>> + /* Scale 10-bit input reading to millivolts. */
>> + *val = adc_val * ASPEED_ADC_REF_VOLTAGE / 1024;
>> +
>> + return 0;
>> +}
>> +
>> +static umode_t aspeed_adc_hwmon_is_visible(const void *_data,
>> + enum hwmon_sensor_types type,
>> + u32 attr, int channel)
>> +{
>> + const struct aspeed_adc_data* data = _data;
>> +
>> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
>> + return S_IRUGO | S_IWUSR;
>> + } else if (type == hwmon_in) {
>> + /* Only channels that are enabled should be visible. */
>> + if (channel >= 0 && channel <= ASPEED_ADC_NUM_CHANNELS &&
>> + (data->enabled_channel_mask & BIT(channel))) {
>> +
>> + /* All enabled channels have an input but labels are
>> + * optional in the device tree.
>> + */
>> + if (attr == hwmon_in_input) {
>> + return S_IRUGO;
>> + } else if (attr == hwmon_in_label &&
>> + data->channel_labels[channel] != NULL) {
>> + return S_IRUGO;
>> + }
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int aspeed_adc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>> + u32 attr, int channel, long *val)
>> +{
>> + struct aspeed_adc_data *data = dev_get_drvdata(dev);
>> +
>> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
>> + *val = data->update_interval_ms;
>> + return 0;
>> + } else if (type == hwmon_in && attr == hwmon_in_input) {
>> + return aspeed_adc_get_channel_reading(data, channel, val);
>> + }
>> +
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static int aspeed_adc_hwmon_read_string(struct device *dev,
>> + enum hwmon_sensor_types type,
>> + u32 attr, int channel, char **str)
>> +{
>> + struct aspeed_adc_data *data = dev_get_drvdata(dev);
>> +
>> + if (type == hwmon_in && attr == hwmon_in_label) {
>> + *str = (char*)data->channel_labels[channel];
>> + return 0;
>> + }
>> +
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static int aspeed_adc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>> + u32 attr, int channel, long val)
>> +{
>> + struct aspeed_adc_data *data = dev_get_drvdata(dev);
>> +
>> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
>> + return aspeed_adc_set_adc_clock_frequency(data, val);
>> + }
>> +
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static const struct hwmon_ops aspeed_adc_hwmon_ops = {
>> + .is_visible = aspeed_adc_hwmon_is_visible,
>> + .read = aspeed_adc_hwmon_read,
>> + .read_string = aspeed_adc_hwmon_read_string,
>> + .write = aspeed_adc_hwmon_write,
>> +};
>> +
>> +static const u32 aspeed_adc_hwmon_chip_config[] = {
>> + HWMON_C_UPDATE_INTERVAL,
>> + 0
>> +};
>> +
>> +static const struct hwmon_channel_info aspeed_adc_hwmon_chip_channel = {
>> + .type = hwmon_chip,
>> + .config = aspeed_adc_hwmon_chip_config,
>> +};
>> +
>> +static const u32 aspeed_adc_hwmon_in_config[] = {
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + HWMON_I_INPUT | HWMON_I_LABEL,
>> + 0
>> +};
>> +
>> +static const struct hwmon_channel_info aspeed_adc_hwmon_in_channel = {
>> + .type = hwmon_in,
>> + .config = aspeed_adc_hwmon_in_config,
>> +};
>> +
>> +static const struct hwmon_channel_info *aspeed_adc_hwmon_channel_info[] = {
>> + &aspeed_adc_hwmon_chip_channel,
>> + &aspeed_adc_hwmon_in_channel,
>> + NULL
>> +};
>> +
>> +static const struct hwmon_chip_info aspeed_adc_hwmon_chip_info = {
>> + .ops = &aspeed_adc_hwmon_ops,
>> + .info = aspeed_adc_hwmon_channel_info,
>> +};
>> +
>> +static int aspeed_adc_probe(struct platform_device *pdev)
>> +{
>> + struct aspeed_adc_data *data;
>> + struct resource *res;
>> + int ret;
>> + struct device *hwmon_dev;
>> + u32 desired_update_interval_ms;
>> + u32 adc_engine_control_reg_val;
>> + struct device_node* node;
>> +
>> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + data->pclk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(data->pclk)) {
>> + dev_err(&pdev->dev, "clk_get failed\n");
>> + return PTR_ERR(data->pclk);
>> + }
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + data->base = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(data->base))
>> + return PTR_ERR(data->base);
>> +
>> + ret = of_property_read_u32(pdev->dev.of_node,
>> + "update-interval-ms", &desired_update_interval_ms);
>> + if (ret < 0 || desired_update_interval_ms == 0) {
>> + dev_err(&pdev->dev,
>> + "Missing or zero update-interval-ms property, "
>> + "defaulting to 100ms\n");
>
> Put the string on one line so it can be easily searched for.
>
>> + desired_update_interval_ms = 100;
>> + }
>> +
>> + mutex_init(&data->lock);
>> +
>> + ret = clk_prepare_enable(data->pclk);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to enable clock\n");
>> + mutex_destroy(&data->lock);
>> + return ret;
>
> Strange whitespace here.
>
>> + }
>> +
>> + /* Figure out which channels are marked available in the device tree. */
>> + data->enabled_channel_mask = 0;
>> + for_each_available_child_of_node(pdev->dev.of_node, node) {
>> + u32 pval;
>> + unsigned int channel;
>> +
>> + if (of_property_read_u32(node, "reg", &pval)) {
>> + dev_err(&pdev->dev, "invalid reg on %s\n",
>> + node->full_name);
>> + continue;
>> + }
>> +
>> + channel = pval;
>> + if (channel >= ASPEED_ADC_NUM_CHANNELS) {
>> + dev_err(&pdev->dev,
>> + "invalid channel index %d on %s\n",
>> + channel, node->full_name);
>> + continue;
>> + }
>> +
>> + data->enabled_channel_mask |= BIT(channel);
>> +
>> + /* Cache a pointer to the label if one is specified. Ignore any
>> + * errors as the label property is optional.
>> + */
>> + of_property_read_string(node, "label", &data->channel_labels[channel]);
>
> I was wondering how long we could keep that pointer around. I think we
> are ok for the lifetime of the driver, as we hold a reference to the
> node in dev.of_node.
>
>> + }
>> +
>> + platform_set_drvdata(pdev, data);
>> + aspeed_adc_set_adc_clock_frequency(data, desired_update_interval_ms);
>
> This reads funny. aspeed_adc_set_clock_frequency instead?
>
>> +
>> + /* Start all the requested channels in normal mode. */
>> + adc_engine_control_reg_val = (data->enabled_channel_mask << 16) |
>> + ASPEED_ADC_OPERATION_MODE_NORMAL | ASPEED_ADC_ENGINE_ENABLE;
>> + writel(adc_engine_control_reg_val, data->base + ASPEED_ADC_REG_ADC00);
>> +
>> + /* Register sysfs hooks */
>> + hwmon_dev = devm_hwmon_device_register_with_info(
>> + &pdev->dev, "aspeed_adc", data,
>> + &aspeed_adc_hwmon_chip_info, NULL);
>> + if (IS_ERR(hwmon_dev)) {
>> + clk_disable_unprepare(data->pclk);
>> + mutex_destroy(&data->lock);
>> + return PTR_ERR(hwmon_dev);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int aspeed_adc_remove(struct platform_device *pdev) {
>> + struct aspeed_adc_data *data = dev_get_drvdata(&pdev->dev);
>> + clk_disable_unprepare(data->pclk);
>> + mutex_destroy(&data->lock);
>> + return 0;
>> +}
>> +
>> +const struct of_device_id aspeed_adc_matches[] = {
>> + { .compatible = "aspeed,ast2400-adc" },
>> + { .compatible = "aspeed,ast2500-adc" },
>> +};
>> +MODULE_DEVICE_TABLE(of, aspeed_adc_matches);
>> +
>> +static struct platform_driver aspeed_adc_driver = {
>> + .probe = aspeed_adc_probe,
>> + .remove = aspeed_adc_remove,
>> + .driver = {
>> + .name = KBUILD_MODNAME,
>> + .of_match_table = aspeed_adc_matches,
>> + }
>> +};
>> +
>> +module_platform_driver(aspeed_adc_driver);
>> +
>> +MODULE_AUTHOR("Rick Altherr <[email protected]>");
>> +MODULE_DESCRIPTION("Aspeed AST2400/2500 ADC Driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.11.0.483.g087da7b7c-goog
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Resending in plain text.
On Tue, Feb 28, 2017 at 7:45 PM, Guenter Roeck <[email protected]> wrote:
> On 02/28/2017 04:49 PM, Joel Stanley wrote:
>>
>> On Wed, Mar 1, 2017 at 6:44 AM, Rick Altherr <[email protected]> wrote:
>>>
>>> Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. This
>>> driver implements reading the ADC values, enabling channels via device
>>> tree, and optionally providing channel labels via device tree. Low and
>>> high threshold interrupts are supported by the hardware but not
>>> implemented.
>>>
>>> Signed-off-by: Rick Altherr <[email protected]>
>>
>>
>> Looks good. Some minor comments below.
>>
>> Is there a reason you wrote a hwmon driver instead of an iio driver? I
>> wasn't sure what the recommended subsystem is.
>
>
> Excellent point. Question is really if there is a plan to add support for
> thresholds. If not, an iio driver might be more appropriate.
>
> Guenter
>
The hardware supports firing interrupts on high and low thresholds.
I'm not planning to use that feature yet so I didn't implement it.
Would you prefer that I leave it as is (maybe with a TODO), implement
the thresholding, or change to iio?
Rick
>>
>>> ---
>>> drivers/hwmon/Kconfig | 10 ++
>>> drivers/hwmon/Makefile | 1 +
>>> drivers/hwmon/aspeed_adc.c | 383
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 394 insertions(+)
>>> create mode 100644 drivers/hwmon/aspeed_adc.c
>>>
>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>>> index 0649d53f3d16..c99a67b4afe4 100644
>>> --- a/drivers/hwmon/Kconfig
>>> +++ b/drivers/hwmon/Kconfig
>>> @@ -261,6 +261,16 @@ config SENSORS_ASC7621
>>> This driver can also be built as a module. If so, the module
>>> will be called asc7621.
>>>
>>> +config SENSORS_ASPEED_ADC
>>> + tristate "Aspeed AST2400/AST2500 ADC"
>>> + depends on ARCH_ASPEED
>>
>>
>> depends on ARCH_ASPEED || COMPILE_TEST.
>>
>>> + help
>>> + If you say yes here you get support for the Aspeed
>>> AST2400/AST2500
>>> + ADC.
>>> +
>>> + This driver can also be built as a module. If so, the module
>>> + will be called aspeed_adc.
>>> +
>>> config SENSORS_K8TEMP
>>> tristate "AMD Athlon64/FX or Opteron temperature sensor"
>>> depends on X86 && PCI
>>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>>> index 5509edf6186a..eede049c9d0d 100644
>>> --- a/drivers/hwmon/Makefile
>>> +++ b/drivers/hwmon/Makefile
>>> @@ -46,6 +46,7 @@ obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o
>>> obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
>>> obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
>>> obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
>>> +obj-$(CONFIG_SENSORS_ASPEED_ADC) += aspeed_adc.o
>>> obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
>>> obj-$(CONFIG_SENSORS_CORETEMP) += coretemp.o
>>> obj-$(CONFIG_SENSORS_DA9052_ADC)+= da9052-hwmon.o
>>> diff --git a/drivers/hwmon/aspeed_adc.c b/drivers/hwmon/aspeed_adc.c
>>> new file mode 100644
>>> index 000000000000..098e32315264
>>> --- /dev/null
>>> +++ b/drivers/hwmon/aspeed_adc.c
>>> @@ -0,0 +1,383 @@
>>> +/*
>>> + * Aspeed AST2400/2500 ADC
>>> + *
>>> + * Copyright (C) 2017 Google, Inc.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + */
>>> +
>>> +#include <asm/io.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/err.h>
>>> +#include <linux/errno.h>
>>> +#include <linux/hwmon.h>
>>> +#include <linux/hwmon-sysfs.h>
>>> +#include <linux/module.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/types.h>
>>> +
>>> +#define ASPEED_ADC_NUM_CHANNELS 16
>>> +#define ASPEED_ADC_REF_VOLTAGE 2500 /* millivolts */
>>> +
>>> +#define ASPEED_ADC_REG_ADC00 0x00
>>> +#define ASPEED_ADC_REG_ADC04 0x04
>>> +#define ASPEED_ADC_REG_ADC08 0x08
>>> +#define ASPEED_ADC_REG_ADC0C 0x0C
>>> +#define ASPEED_ADC_REG_ADC10 0x10
>>> +#define ASPEED_ADC_REG_ADC14 0x14
>>> +#define ASPEED_ADC_REG_ADC18 0x18
>>> +#define ASPEED_ADC_REG_ADC1C 0x1C
>>> +#define ASPEED_ADC_REG_ADC20 0x20
>>> +#define ASPEED_ADC_REG_ADC24 0x24
>>> +#define ASPEED_ADC_REG_ADC28 0x28
>>> +#define ASPEED_ADC_REG_ADC2C 0x2C
>>
>>
>> I'm not sure that these defines add any value. I'd either give them
>> names such as "ASPEED_ADC_ENGINE_CTRL". or just use the register
>> offset directly.
>>
>>> +
>>> +#define ASPEED_ADC_OPERATION_MODE_POWER_DOWN (0x0 << 1)
>>> +#define ASPEED_ADC_OPERATION_MODE_STANDBY (0x1 << 1)
>>> +#define ASPEED_ADC_OPERATION_MODE_NORMAL (0x7 << 1)
>>> +
>>> +#define ASPEED_ADC_ENGINE_ENABLE BIT(0)
>>> +
>>> +struct aspeed_adc_data {
>>> + struct device *dev;
>>> + void __iomem *base;
>>> + struct clk *pclk;
>>> + struct mutex lock;
>>> + unsigned long update_interval_ms;
>>> + u32 enabled_channel_mask;
>>> + const char* channel_labels[ASPEED_ADC_NUM_CHANNELS];
>>> +};
>>> +
>>> +static int aspeed_adc_set_adc_clock_frequency(
>>> + struct aspeed_adc_data *data,
>>> + unsigned long desired_update_interval_ms)
>>> +{
>>> + long pclk_hz = clk_get_rate(data->pclk);
>>> + long adc_combined_divisor;
>>> + long adc_pre_divisor;
>>> + long adc_divisor;
>>> + long adc_clock_control_reg_val;
>>> + long num_enabled_channels =
>>> hweight32(data->enabled_channel_mask);
>>
>>
>> Some whitespace damage here.
>>
>>> +
>>> + if (desired_update_interval_ms < 1)
>>> + return -EINVAL;
>>> +
>>> + /* From the AST2400 datasheet:
>>
>>
>> Nit: kernel style is to leave a blank line on the first line of
>> multi-line comments:
>>
>> /*
>> * Foo
>> * etc
>>
>>> + * adc_period_s = pclk_period_s * 2 * (pre_divisor+1) *
>>> (divisor+1)
>>> + *
>>> + * and
>>> + *
>>> + * sample_period_s = 12 * adc_period_s
>>> + *
>>> + * Substitute pclk_period_s = (1 / pclk_hz)
>>> + *
>>> + * Solve for (pre-divisor+1) * (divisor+1) as those are settings
>>> we need
>>> + * to program:
>>> + * (pre-divisor+1) * (divisor+1) = (sample_period_s * pclk_hz)
>>> / 24
>>> + */
>>> +
>>> + /* Assume PCLK runs at a fairly high frequency so dividing it
>>> first
>>> + * loses little accuracy. Also note that the above formulas have
>>> + * sample_period in seconds while our desired_update_interval is
>>> in
>>> + * milliseconds, hence the divide by 1000.
>>> + */
>>> + adc_combined_divisor = desired_update_interval_ms *
>>> + (pclk_hz / 24 / 1000 / num_enabled_channels);
>>> +
>>> + /* Prefer to use the ADC divisor over the ADC pre-divisor. ADC
>>> divisor
>>> + * is 10-bits wide so anything over 1024 must use the
>>> pre-divisor.
>>> + */
>>> + adc_pre_divisor = 1;
>>> + while (adc_combined_divisor/adc_pre_divisor > (1<<10)) {
>>> + adc_pre_divisor += 1;
>>> + };
>>> + adc_divisor = adc_combined_divisor / adc_pre_divisor;
>>> +
>>> + /* Since ADC divisor and pre-divisor are used in division, the
>>> register
>>> + * value is implicitly incremented by one before use. This means
>>> we
>>> + * need to subtract one from our calculated values above.
>>> + */
>>> + adc_pre_divisor -= 1;
>>> + adc_divisor -= 1;
>>> +
>>> + adc_clock_control_reg_val = (adc_pre_divisor << 17) |
>>> adc_divisor;
>>> +
>>> + mutex_lock(&data->lock);
>>> + data->update_interval_ms =
>>> + (adc_pre_divisor + 1) * (adc_divisor + 1)
>>> + / (pclk_hz / 24 / 1000);
>>> + writel(adc_clock_control_reg_val, data->base +
>>> ASPEED_ADC_REG_ADC0C);
>>> + mutex_unlock(&data->lock);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int aspeed_adc_get_channel_reading(struct aspeed_adc_data *data,
>>> + int channel, long *val)
>>> +{
>>> + u32 data_reg;
>>> + u32 data_reg_val;
>>> + long adc_val;
>>> +
>>> + /* Each 32-bit data register contains 2 10-bit ADC values. */
>>> + data_reg = ASPEED_ADC_REG_ADC10 + (channel / 2) * 4;
>>> + data_reg_val = readl(data->base + data_reg);
>>> + if (channel % 2 == 0) {
>>> + adc_val = data_reg_val & 0x3FF;
>>> + } else {
>>> + adc_val = (data_reg_val >> 16) & 0x3FF;
>>> + }
>>
>>
>> I wonder if you could replace the above block with:
>>
>> adc_val = readw(data->base + ASPEED_ADC_REG_ADC10 + channel);
>>
>>> +
>>> + /* Scale 10-bit input reading to millivolts. */
>>> + *val = adc_val * ASPEED_ADC_REF_VOLTAGE / 1024;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static umode_t aspeed_adc_hwmon_is_visible(const void *_data,
>>> + enum hwmon_sensor_types
>>> type,
>>> + u32 attr, int channel)
>>> +{
>>> + const struct aspeed_adc_data* data = _data;
>>> +
>>> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
>>> + return S_IRUGO | S_IWUSR;
>>> + } else if (type == hwmon_in) {
>>> + /* Only channels that are enabled should be visible. */
>>> + if (channel >= 0 && channel <= ASPEED_ADC_NUM_CHANNELS &&
>>> + (data->enabled_channel_mask & BIT(channel))) {
>>> +
>>> + /* All enabled channels have an input but labels
>>> are
>>> + * optional in the device tree.
>>> + */
>>> + if (attr == hwmon_in_input) {
>>> + return S_IRUGO;
>>> + } else if (attr == hwmon_in_label &&
>>> + data->channel_labels[channel] !=
>>> NULL) {
>>> + return S_IRUGO;
>>> + }
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int aspeed_adc_hwmon_read(struct device *dev, enum
>>> hwmon_sensor_types type,
>>> + u32 attr, int channel, long *val)
>>> +{
>>> + struct aspeed_adc_data *data = dev_get_drvdata(dev);
>>> +
>>> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
>>> + *val = data->update_interval_ms;
>>> + return 0;
>>> + } else if (type == hwmon_in && attr == hwmon_in_input) {
>>> + return aspeed_adc_get_channel_reading(data, channel,
>>> val);
>>> + }
>>> +
>>> + return -EOPNOTSUPP;
>>> +}
>>> +
>>> +static int aspeed_adc_hwmon_read_string(struct device *dev,
>>> + enum hwmon_sensor_types type,
>>> + u32 attr, int channel, char
>>> **str)
>>> +{
>>> + struct aspeed_adc_data *data = dev_get_drvdata(dev);
>>> +
>>> + if (type == hwmon_in && attr == hwmon_in_label) {
>>> + *str = (char*)data->channel_labels[channel];
>>> + return 0;
>>> + }
>>> +
>>> + return -EOPNOTSUPP;
>>> +}
>>> +
>>> +static int aspeed_adc_hwmon_write(struct device *dev, enum
>>> hwmon_sensor_types type,
>>> + u32 attr, int channel, long val)
>>> +{
>>> + struct aspeed_adc_data *data = dev_get_drvdata(dev);
>>> +
>>> + if (type == hwmon_chip && attr == hwmon_chip_update_interval) {
>>> + return aspeed_adc_set_adc_clock_frequency(data, val);
>>> + }
>>> +
>>> + return -EOPNOTSUPP;
>>> +}
>>> +
>>> +static const struct hwmon_ops aspeed_adc_hwmon_ops = {
>>> + .is_visible = aspeed_adc_hwmon_is_visible,
>>> + .read = aspeed_adc_hwmon_read,
>>> + .read_string = aspeed_adc_hwmon_read_string,
>>> + .write = aspeed_adc_hwmon_write,
>>> +};
>>> +
>>> +static const u32 aspeed_adc_hwmon_chip_config[] = {
>>> + HWMON_C_UPDATE_INTERVAL,
>>> + 0
>>> +};
>>> +
>>> +static const struct hwmon_channel_info aspeed_adc_hwmon_chip_channel = {
>>> + .type = hwmon_chip,
>>> + .config = aspeed_adc_hwmon_chip_config,
>>> +};
>>> +
>>> +static const u32 aspeed_adc_hwmon_in_config[] = {
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + HWMON_I_INPUT | HWMON_I_LABEL,
>>> + 0
>>> +};
>>> +
>>> +static const struct hwmon_channel_info aspeed_adc_hwmon_in_channel = {
>>> + .type = hwmon_in,
>>> + .config = aspeed_adc_hwmon_in_config,
>>> +};
>>> +
>>> +static const struct hwmon_channel_info *aspeed_adc_hwmon_channel_info[]
>>> = {
>>> + &aspeed_adc_hwmon_chip_channel,
>>> + &aspeed_adc_hwmon_in_channel,
>>> + NULL
>>> +};
>>> +
>>> +static const struct hwmon_chip_info aspeed_adc_hwmon_chip_info = {
>>> + .ops = &aspeed_adc_hwmon_ops,
>>> + .info = aspeed_adc_hwmon_channel_info,
>>> +};
>>> +
>>> +static int aspeed_adc_probe(struct platform_device *pdev)
>>> +{
>>> + struct aspeed_adc_data *data;
>>> + struct resource *res;
>>> + int ret;
>>> + struct device *hwmon_dev;
>>> + u32 desired_update_interval_ms;
>>> + u32 adc_engine_control_reg_val;
>>> + struct device_node* node;
>>> +
>>> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>>> + if (!data)
>>> + return -ENOMEM;
>>> +
>>> + data->pclk = devm_clk_get(&pdev->dev, NULL);
>>> + if (IS_ERR(data->pclk)) {
>>> + dev_err(&pdev->dev, "clk_get failed\n");
>>> + return PTR_ERR(data->pclk);
>>> + }
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>> + data->base = devm_ioremap_resource(&pdev->dev, res);
>>> + if (IS_ERR(data->base))
>>> + return PTR_ERR(data->base);
>>> +
>>> + ret = of_property_read_u32(pdev->dev.of_node,
>>> + "update-interval-ms",
>>> &desired_update_interval_ms);
>>> + if (ret < 0 || desired_update_interval_ms == 0) {
>>> + dev_err(&pdev->dev,
>>> + "Missing or zero update-interval-ms
>>> property, "
>>> + "defaulting to 100ms\n");
>>
>>
>> Put the string on one line so it can be easily searched for.
>>
>>> + desired_update_interval_ms = 100;
>>> + }
>>> +
>>> + mutex_init(&data->lock);
>>> +
>>> + ret = clk_prepare_enable(data->pclk);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to enable clock\n");
>>> + mutex_destroy(&data->lock);
>>> + return ret;
>>
>>
>> Strange whitespace here.
>>
>>> + }
>>> +
>>> + /* Figure out which channels are marked available in the device
>>> tree. */
>>> + data->enabled_channel_mask = 0;
>>> + for_each_available_child_of_node(pdev->dev.of_node, node) {
>>> + u32 pval;
>>> + unsigned int channel;
>>> +
>>> + if (of_property_read_u32(node, "reg", &pval)) {
>>> + dev_err(&pdev->dev, "invalid reg on %s\n",
>>> + node->full_name);
>>> + continue;
>>> + }
>>> +
>>> + channel = pval;
>>> + if (channel >= ASPEED_ADC_NUM_CHANNELS) {
>>> + dev_err(&pdev->dev,
>>> + "invalid channel index %d on %s\n",
>>> + channel, node->full_name);
>>> + continue;
>>> + }
>>> +
>>> + data->enabled_channel_mask |= BIT(channel);
>>> +
>>> + /* Cache a pointer to the label if one is specified.
>>> Ignore any
>>> + * errors as the label property is optional.
>>> + */
>>> + of_property_read_string(node, "label",
>>> &data->channel_labels[channel]);
>>
>>
>> I was wondering how long we could keep that pointer around. I think we
>> are ok for the lifetime of the driver, as we hold a reference to the
>> node in dev.of_node.
>>
>>> + }
>>> +
>>> + platform_set_drvdata(pdev, data);
>>> + aspeed_adc_set_adc_clock_frequency(data,
>>> desired_update_interval_ms);
>>
>>
>> This reads funny. aspeed_adc_set_clock_frequency instead?
>>
>>> +
>>> + /* Start all the requested channels in normal mode. */
>>> + adc_engine_control_reg_val = (data->enabled_channel_mask << 16) |
>>> + ASPEED_ADC_OPERATION_MODE_NORMAL |
>>> ASPEED_ADC_ENGINE_ENABLE;
>>> + writel(adc_engine_control_reg_val, data->base +
>>> ASPEED_ADC_REG_ADC00);
>>> +
>>> + /* Register sysfs hooks */
>>> + hwmon_dev = devm_hwmon_device_register_with_info(
>>> + &pdev->dev, "aspeed_adc", data,
>>> + &aspeed_adc_hwmon_chip_info, NULL);
>>> + if (IS_ERR(hwmon_dev)) {
>>> + clk_disable_unprepare(data->pclk);
>>> + mutex_destroy(&data->lock);
>>> + return PTR_ERR(hwmon_dev);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int aspeed_adc_remove(struct platform_device *pdev) {
>>> + struct aspeed_adc_data *data = dev_get_drvdata(&pdev->dev);
>>> + clk_disable_unprepare(data->pclk);
>>> + mutex_destroy(&data->lock);
>>> + return 0;
>>> +}
>>> +
>>> +const struct of_device_id aspeed_adc_matches[] = {
>>> + { .compatible = "aspeed,ast2400-adc" },
>>> + { .compatible = "aspeed,ast2500-adc" },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, aspeed_adc_matches);
>>> +
>>> +static struct platform_driver aspeed_adc_driver = {
>>> + .probe = aspeed_adc_probe,
>>> + .remove = aspeed_adc_remove,
>>> + .driver = {
>>> + .name = KBUILD_MODNAME,
>>> + .of_match_table = aspeed_adc_matches,
>>> + }
>>> +};
>>> +
>>> +module_platform_driver(aspeed_adc_driver);
>>> +
>>> +MODULE_AUTHOR("Rick Altherr <[email protected]>");
>>> +MODULE_DESCRIPTION("Aspeed AST2400/2500 ADC Driver");
>>> +MODULE_LICENSE("GPL");
>>> --
>>> 2.11.0.483.g087da7b7c-goog
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
On Tue, Feb 28, 2017 at 07:45:23PM -0800, Guenter Roeck wrote:
> On 02/28/2017 04:49 PM, Joel Stanley wrote:
> > On Wed, Mar 1, 2017 at 6:44 AM, Rick Altherr <[email protected]> wrote:
> > > Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. This
> > > driver implements reading the ADC values, enabling channels via device
> > > tree, and optionally providing channel labels via device tree. Low and
> > > high threshold interrupts are supported by the hardware but not
> > > implemented.
> > >
> > > Signed-off-by: Rick Altherr <[email protected]>
> >
> > Looks good. Some minor comments below.
> >
> > Is there a reason you wrote a hwmon driver instead of an iio driver? I
> > wasn't sure what the recommended subsystem is.
>
> Excellent point. Question is really if there is a plan to add support for
> thresholds. If not, an iio driver might be more appropriate.
Sigh. We have ADCs in 2 places? Fine for the kernel I guess, but not
bindings.
Rob
On 03/02/2017 10:21 PM, Rob Herring wrote:
> On Tue, Feb 28, 2017 at 07:45:23PM -0800, Guenter Roeck wrote:
>> On 02/28/2017 04:49 PM, Joel Stanley wrote:
>>> On Wed, Mar 1, 2017 at 6:44 AM, Rick Altherr <[email protected]> wrote:
>>>> Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. This
>>>> driver implements reading the ADC values, enabling channels via device
>>>> tree, and optionally providing channel labels via device tree. Low and
>>>> high threshold interrupts are supported by the hardware but not
>>>> implemented.
>>>>
>>>> Signed-off-by: Rick Altherr <[email protected]>
>>>
>>> Looks good. Some minor comments below.
>>>
>>> Is there a reason you wrote a hwmon driver instead of an iio driver? I
>>> wasn't sure what the recommended subsystem is.
>>
>> Excellent point. Question is really if there is a plan to add support for
>> thresholds. If not, an iio driver might be more appropriate.
>
> Sigh. We have ADCs in 2 places? Fine for the kernel I guess, but not
> bindings.
>
Almost every {voltage, current, power, temperature} sensor chip is implemented
as ADC. Given that, we have (at least) three places. hwmon, iio, thermal.
Guenter
On 03/01/2017 07:29 PM, Rick Altherr wrote:
> Resending in plain text.
>
> On Tue, Feb 28, 2017 at 7:45 PM, Guenter Roeck <[email protected]> wrote:
>> On 02/28/2017 04:49 PM, Joel Stanley wrote:
>>>
>>> On Wed, Mar 1, 2017 at 6:44 AM, Rick Altherr <[email protected]> wrote:
>>>>
>>>> Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. This
>>>> driver implements reading the ADC values, enabling channels via device
>>>> tree, and optionally providing channel labels via device tree. Low and
>>>> high threshold interrupts are supported by the hardware but not
>>>> implemented.
>>>>
>>>> Signed-off-by: Rick Altherr <[email protected]>
>>>
>>>
>>> Looks good. Some minor comments below.
>>>
>>> Is there a reason you wrote a hwmon driver instead of an iio driver? I
>>> wasn't sure what the recommended subsystem is.
>>
>>
>> Excellent point. Question is really if there is a plan to add support for
>> thresholds. If not, an iio driver might be more appropriate.
>>
>> Guenter
>>
>
> The hardware supports firing interrupts on high and low thresholds.
> I'm not planning to use that feature yet so I didn't implement it.
> Would you prefer that I leave it as is (maybe with a TODO), implement
> the thresholding, or change to iio?
>
Let's try to determine the intended use of the ADC. I don't find the datasheet
online; all I can find is brief summaries which don't me tell much, but suggest
that it is a general purpose ADC and not specifically intended for hardware
monitoring. What is the minimum sampling rate ? That should give us an idea.
If it is in the uS range, iio would be more appropriate (and the iio-hwmon
bridge could be used if a channel is in fact used for hardware monitoring).
Thanks,
Guenter
On Sun, Mar 5, 2017 at 8:28 AM, Guenter Roeck <[email protected]> wrote:
> On 03/01/2017 07:29 PM, Rick Altherr wrote:
>>
>> Resending in plain text.
>>
>> On Tue, Feb 28, 2017 at 7:45 PM, Guenter Roeck <[email protected]> wrote:
>>>
>>> On 02/28/2017 04:49 PM, Joel Stanley wrote:
>>>>
>>>>
>>>> On Wed, Mar 1, 2017 at 6:44 AM, Rick Altherr <[email protected]>
>>>> wrote:
>>>>>
>>>>>
>>>>> Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. This
>>>>> driver implements reading the ADC values, enabling channels via device
>>>>> tree, and optionally providing channel labels via device tree. Low and
>>>>> high threshold interrupts are supported by the hardware but not
>>>>> implemented.
>>>>>
>>>>> Signed-off-by: Rick Altherr <[email protected]>
>>>>
>>>>
>>>>
>>>> Looks good. Some minor comments below.
>>>>
>>>> Is there a reason you wrote a hwmon driver instead of an iio driver? I
>>>> wasn't sure what the recommended subsystem is.
>>>
>>>
>>>
>>> Excellent point. Question is really if there is a plan to add support for
>>> thresholds. If not, an iio driver might be more appropriate.
>>>
>>> Guenter
>>>
>>
>> The hardware supports firing interrupts on high and low thresholds.
>> I'm not planning to use that feature yet so I didn't implement it.
>> Would you prefer that I leave it as is (maybe with a TODO), implement
>> the thresholding, or change to iio?
>>
>
> Let's try to determine the intended use of the ADC. I don't find the
> datasheet
> online; all I can find is brief summaries which don't me tell much, but
> suggest
> that it is a general purpose ADC and not specifically intended for hardware
> monitoring. What is the minimum sampling rate ? That should give us an idea.
> If it is in the uS range, iio would be more appropriate (and the iio-hwmon
> bridge could be used if a channel is in fact used for hardware monitoring).
>
> Thanks,
> Guenter
>
AST2500 is a BMC SoC from Aspeed
(https://www.aspeedtech.com/products.php?fPath=20&rId=440). The BMC
is a separate, always-on computer that manages the health and remote
management for a server. The driver I sent is for the ADCs included
in the SoC that are intended to monitor power rails on the server
motherboard but are really just general-purpose ADCs. According to
the (not public) datasheet, the sampling rate is 10-500kHz, resolution
is 10-bit, and precision +/- 2%. This is my first time writing a
linux driver for ADCs. My cursory look at iio indicates that that
will work fine for this driver and the hwmon-iio bridge will suffice
for the userspace stack which is currently expecting hwmon APIs.