Linux-Hwmon Archive on lore.kernel.org
 help / color / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Serge Semin <fancer.lancer@gmail.com>
Cc: Jean Delvare <jdelvare@suse.com>,
	Serge Semin <Sergey.Semin@t-platforms.ru>,
	linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
	Jonathan Cameron <jic23@kernel.org>,
	linux-iio@vger.kernel.org
Subject: Re: [PATCH 2/2] hwmon: Add ads1000/ads1100 voltage ADCs driver
Date: Thu, 30 May 2019 05:55:10 -0700
Message-ID: <20190530125510.GA26072@roeck-us.net> (raw)
In-Reply-To: <20190514225810.12591-3-fancer.lancer@gmail.com>

Hi,

On Wed, May 15, 2019 at 01:58:09AM +0300, Serge Semin wrote:
> These are simple Texas Instruments ADC working over i2c-interface with
> just one differential input and with configurable 12-16 bits resolution.
> Sample rate is fixed to 128 for ads1000 and can vary from 8 to 128 for
> ads1100. Vdd value reference value must be supplied so to properly
> translate the sampled code to the real voltage. All of these configs are
> implemented in the device drivers for hwmon subsystem. The next dts
> properties should be specified to comply the device platform setup:
>  - vdd-supply - voltage regulator connected to the Vdd pin of the device
>  - ti,gain - programmable gain amplifier
>  - ti,datarate - converter data rate
>  - ti,voltage-divider - possible resistors-base external divider
> See bindings documentation file for details.
> 
> Even though these devices seem more like ads1015 series, they
> in fact pretty much different. First of all ads1000/ads1100 got less
> capabilities: just one port, no configurations of digital comparator, no
> input multi-channel multiplexer, smaller PGA and data-rate ranges.
> In addition they haven't got internal voltage reference, but instead
> are created to use Vdd pin voltage. Finally the output code value is
> provided in different format. As a result it was much easier for
> development and for future support to create a separate driver.
> 

This chicp doesn't have any real hardware monitoring characteristics
(no limit registers). It seems to be better suited to be implemented
as iio driver. If it is used as hardware monitor, the iio-hwmon bridge
should work just fine.

Jonathan, what do you think ?

Thanks,
Guenter

> Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
> ---
>  MAINTAINERS                           |   8 +
>  drivers/hwmon/Kconfig                 |  10 +
>  drivers/hwmon/Makefile                |   1 +
>  drivers/hwmon/ads1000.c               | 320 ++++++++++++++++++++++++++
>  include/linux/platform_data/ads1000.h |  20 ++
>  5 files changed, 359 insertions(+)
>  create mode 100644 drivers/hwmon/ads1000.c
>  create mode 100644 include/linux/platform_data/ads1000.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ce573aaa04df..5c3a8107ef1a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -517,6 +517,14 @@ W:	http://ez.analog.com/community/linux-device-drivers
>  S:	Supported
>  F:	drivers/video/backlight/adp8860_bl.c
>  
> +ADS1000 HARDWARE MONITOR DRIVER
> +M:	Serge Semin <fancer.lancer@gmail.com>
> +L:	linux-hwmon@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/hwmon/ads1000.rst
> +F:	drivers/hwmon/ads1000.c
> +F:	include/linux/platform_data/ads1000.h
> +
>  ADS1015 HARDWARE MONITOR DRIVER
>  M:	Dirk Eibach <eibach@gdsys.de>
>  L:	linux-hwmon@vger.kernel.org
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 1915a18b537b..a1220cc48f2f 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1569,6 +1569,16 @@ config SENSORS_ADC128D818
>  	  This driver can also be built as a module. If so, the module
>  	  will be called adc128d818.
>  
> +config SENSORS_ADS1000
> +	tristate "Texas Instruments ADS1000"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for Texas Instruments
> +	  ADS1000/ADS1100 12-16-bit single channel ADC device.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ads1000.
> +
>  config SENSORS_ADS1015
>  	tristate "Texas Instruments ADS1015"
>  	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 8db472ea04f0..2cd82f6c651e 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_SENSORS_ADM1026)	+= adm1026.o
>  obj-$(CONFIG_SENSORS_ADM1029)	+= adm1029.o
>  obj-$(CONFIG_SENSORS_ADM1031)	+= adm1031.o
>  obj-$(CONFIG_SENSORS_ADM9240)	+= adm9240.o
> +obj-$(CONFIG_SENSORS_ADS1000)	+= ads1000.o
>  obj-$(CONFIG_SENSORS_ADS1015)	+= ads1015.o
>  obj-$(CONFIG_SENSORS_ADS7828)	+= ads7828.o
>  obj-$(CONFIG_SENSORS_ADS7871)	+= ads7871.o
> diff --git a/drivers/hwmon/ads1000.c b/drivers/hwmon/ads1000.c
> new file mode 100644
> index 000000000000..a88b738f56bd
> --- /dev/null
> +++ b/drivers/hwmon/ads1000.c
> @@ -0,0 +1,320 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for ADS1000/ADS1100 12-16-bit ADC
> + *
> + * Copyright (C) 2019 T-platforms JSC (fancer.lancer@gmail.com)
> + *
> + * Based on the ads1015 driver by Dirk Eibach.
> + *
> + * Datasheet available at: http://focus.ti.com/lit/ds/symlink/ads1000.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/platform_data/ads1000.h>
> +
> +/* Data rates scale table */
> +static const unsigned int scale_table[4] = {
> +	2048, 8192, 16384, 32768
> +};
> +
> +/* Minimal data rates in samples per second */
> +static const unsigned int data_rate_table[4] = {
> +	100, 25, 12, 5
> +};
> +
> +#define ADS1000_DEFAULT_PGA 0
> +#define ADS1000_DEFAULT_DATA_RATE 0
> +#define ADS1000_DEFAULT_R1_DIVIDER 0
> +#define ADS1000_DEFAULT_R2_DIVIDER 0
> +
> +enum ads1000_chips {
> +	ads1000,
> +	ads1100,
> +};
> +
> +struct ads1000 {
> +	struct device *hwmon_dev;
> +	struct mutex update_lock;
> +	struct i2c_client *client;
> +	struct ads1000_platform_data data;
> +	enum ads1000_chips id;
> +};
> +
> +static inline int ads1000_enable_vdd(struct ads1000 *priv)
> +{
> +	return regulator_enable(priv->data.vdd);
> +}
> +
> +static inline int ads1000_get_vdd(struct ads1000 *priv)
> +{
> +	return regulator_get_voltage(priv->data.vdd);
> +}
> +
> +static int ads1000_read_adc(struct ads1000 *priv)
> +{
> +	struct i2c_client *client = priv->client;
> +	unsigned int delay_ms;
> +	u8 data[3] = {0};
> +	int res;
> +
> +	mutex_lock(&priv->update_lock);
> +
> +	delay_ms = DIV_ROUND_UP(1000, data_rate_table[priv->data.data_rate]);
> +
> +	/* setup and start single conversion */
> +	data[2] |= (1 << 7) | (1 << 4);
> +	data[2] |= priv->data.pga;
> +	data[2] |= priv->data.data_rate << 2;
> +
> +	res = i2c_master_send(client, &data[2], 1);
> +	if (res < 0)
> +		goto err_unlock;
> +
> +	/* wait until conversion finished */
> +	msleep(delay_ms);
> +	res = i2c_master_recv(client, data, 3);
> +	if (res < 0)
> +		goto err_unlock;
> +
> +	if (data[2] & (1 << 7)) {
> +		res = -EIO;
> +		goto err_unlock;
> +	}
> +
> +	res = ((u16)data[0] << 8) | data[1];
> +
> +err_unlock:
> +	mutex_unlock(&priv->update_lock);
> +
> +	return res;
> +}
> +
> +static int ads1000_reg_to_mv(struct ads1000 *priv, s16 reg)
> +{
> +	unsigned int *divider = priv->data.divider;
> +	int voltage = ads1000_get_vdd(priv);
> +	int gain = 1 << priv->data.pga;
> +	int c = 0;
> +
> +	voltage = reg*DIV_ROUND_CLOSEST(voltage, 1000);
> +	gain = gain*scale_table[priv->data.data_rate];
> +	voltage = DIV_ROUND_CLOSEST(voltage, gain);
> +
> +	if (divider[0] && divider[1]) {
> +		c = divider[0]*voltage;
> +		c = DIV_ROUND_CLOSEST(c, (int)divider[1]);
> +	}
> +
> +	return voltage + c;
> +}
> +
> +static ssize_t show_in(struct device *dev, struct device_attribute *da,
> +		       char *buf)
> +{
> +	struct ads1000 *priv = dev_get_drvdata(dev);
> +	int res;
> +
> +	res = ads1000_read_adc(priv);
> +	if (res < 0)
> +		return res;
> +
> +	return sprintf(buf, "%d\n", ads1000_reg_to_mv(priv, res));
> +}
> +
> +static SENSOR_DEVICE_ATTR(in0_input, 0444, show_in, NULL, 0);
> +
> +static struct attribute *ads1000_attrs[] = {
> +	&sensor_dev_attr_in0_input.dev_attr.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(ads1000);
> +
> +static struct ads1000 *ads1000_create_priv(struct i2c_client *client,
> +					   const struct i2c_device_id *id)
> +{
> +	struct ads1000 *priv;
> +
> +	priv = devm_kzalloc(&client->dev, sizeof(struct ads1000),
> +			    GFP_KERNEL);
> +	if (!priv)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (client->dev.of_node)
> +		priv->id = (enum ads1000_chips)
> +			of_device_get_match_data(&client->dev);
> +	else
> +		priv->id = id->driver_data;
> +
> +	i2c_set_clientdata(client, priv);
> +	priv->client = client;
> +	mutex_init(&priv->update_lock);
> +
> +	return priv;
> +}
> +
> +#ifdef CONFIG_OF
> +static int ads1000_get_config_of(struct ads1000 *priv)
> +{
> +	struct i2c_client *client = priv->client;
> +	struct device_node *node = client->dev.of_node;
> +	u32 divider[2];
> +	u32 val;
> +
> +	if (!node)
> +		return -EINVAL;
> +
> +	if (!of_property_read_u32(node, "ti,gain", &val))
> +		priv->data.pga = val;
> +
> +	if (!of_property_read_u32(node, "ti,datarate", &val))
> +		priv->data.data_rate = val;
> +
> +	if (!of_property_read_u32_array(node, "ti,voltage-divider",
> +					divider, 2)) {
> +		priv->data.divider[0] = divider[0];
> +		priv->data.divider[1] = divider[1];
> +	}
> +
> +	priv->data.vdd = devm_regulator_get(&client->dev, "vdd");
> +	if (IS_ERR(priv->data.vdd))
> +		return PTR_ERR(priv->data.vdd);
> +
> +	return 0;
> +}
> +#endif
> +
> +static int ads1000_get_config(struct ads1000 *priv)
> +{
> +	struct i2c_client *client = priv->client;
> +	struct ads1000_platform_data *pdata = dev_get_platdata(&client->dev);
> +
> +	priv->data.pga = ADS1000_DEFAULT_PGA;
> +	priv->data.data_rate = ADS1000_DEFAULT_DATA_RATE;
> +	priv->data.divider[0] = ADS1000_DEFAULT_R1_DIVIDER;
> +	priv->data.divider[1] = ADS1000_DEFAULT_R2_DIVIDER;
> +
> +	/* prefer platform data */
> +	if (pdata) {
> +		memcpy(&priv->data, pdata, sizeof(priv->data));
> +	} else {
> +#ifdef CONFIG_OF
> +		int ret;
> +
> +		ret = ads1000_get_config_of(priv);
> +		if (ret)
> +			return ret;
> +#endif
> +	}
> +
> +	if (!priv->data.vdd) {
> +		dev_err(&client->dev, "No VDD regulator\n");
> +		return -EINVAL;
> +	}
> +
> +	if (priv->data.pga > 4) {
> +		dev_err(&client->dev, "Invalid gain, using default\n");
> +		priv->data.pga = ADS1000_DEFAULT_PGA;
> +	}
> +
> +	if (priv->data.data_rate > 4) {
> +		dev_err(&client->dev, "Invalid datarate, using default\n");
> +		priv->data.data_rate = ADS1000_DEFAULT_DATA_RATE;
> +	}
> +
> +	if (priv->id == ads1000 && priv->data.data_rate != 0) {
> +		dev_warn(&client->dev, "ADC data rate can be 128SPS only\n");
> +		priv->data.data_rate = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ads1000_set_config(struct ads1000 *priv)
> +{
> +	u8 data = 0;
> +	int ret;
> +
> +	/* disable continuous conversion */
> +	data |= (1 << 4);
> +	data |= priv->data.pga;
> +	data |= priv->data.data_rate << 2;
> +
> +	ret = i2c_master_send(priv->client, &data, 1);
> +
> +	return ret < 0 ? ret : 0;
> +}
> +
> +static int ads1000_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct ads1000 *priv;
> +	int ret;
> +
> +	priv = ads1000_create_priv(client, id);
> +	if (IS_ERR(priv))
> +		return PTR_ERR(priv);
> +
> +	ret = ads1000_get_config(priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads1000_enable_vdd(priv);
> +	if (ret)
> +		return ret;
> +
> +	ret = ads1000_set_config(priv);
> +	if (ret)
> +		return ret;
> +
> +	priv->hwmon_dev = devm_hwmon_device_register_with_groups(&client->dev,
> +				client->name, priv, ads1000_groups);
> +	if (IS_ERR(priv->hwmon_dev))
> +		return PTR_ERR(priv->hwmon_dev);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id ads1000_id[] = {
> +	{ "ads1000",  ads1000},
> +	{ "ads1100",  ads1100},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ads1000_id);
> +
> +static const struct of_device_id ads1000_of_match[] = {
> +	{
> +		.compatible = "ti,ads1000",
> +		.data = (void *)ads1000
> +	},
> +	{
> +		.compatible = "ti,ads1100",
> +		.data = (void *)ads1100
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, ads1000_of_match);
> +
> +static struct i2c_driver ads1000_driver = {
> +	.driver = {
> +		.name = "ads1000",
> +		.of_match_table = of_match_ptr(ads1000_of_match),
> +	},
> +	.probe = ads1000_probe,
> +	.id_table = ads1000_id,
> +};
> +module_i2c_driver(ads1000_driver);
> +
> +MODULE_AUTHOR("Serge Semin <fancer.lancer@gmail.com>");
> +MODULE_DESCRIPTION("ADS1000 driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/platform_data/ads1000.h b/include/linux/platform_data/ads1000.h
> new file mode 100644
> index 000000000000..979670483537
> --- /dev/null
> +++ b/include/linux/platform_data/ads1000.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Platform Data for ADS1000/ADS1100 12-16-bit ADC
> + *
> + * Copyright (C) 2019 T-platforms JSC (fancer.lancer@gmail.com)
> + */
> +
> +#ifndef LINUX_ADS1000_H
> +#define LINUX_ADS1000_H
> +
> +#include <linux/regulator/consumer.h>
> +
> +struct ads1000_platform_data {
> +	unsigned int pga;
> +	unsigned int data_rate;
> +	struct regulator *vdd;
> +	unsigned int divider[2];
> +};
> +
> +#endif /* LINUX_ADS1000_H */

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-14 22:58 [PATCH 0/2] hwmon: Add TI ads1000/ads1100 driver package Serge Semin
2019-05-14 22:58 ` [PATCH 1/2] dt-bindings: hwmon: Add DT bindings for TI ads1000/ads1100 ADCs Serge Semin
2019-06-13 20:33   ` Rob Herring
2019-06-14  6:04     ` Serge Semin
2019-05-14 22:58 ` [PATCH 2/2] hwmon: Add ads1000/ads1100 voltage ADCs driver Serge Semin
2019-05-30 12:55   ` Guenter Roeck [this message]
2019-06-03 11:11     ` Jonathan Cameron
2019-06-05 20:55       ` Guenter Roeck
2019-06-07 23:01         ` Serge Semin
2019-06-08  1:02           ` Guenter Roeck
2019-06-10 14:57             ` Serge Semin

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190530125510.GA26072@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=Sergey.Semin@t-platforms.ru \
    --cc=fancer.lancer@gmail.com \
    --cc=jdelvare@suse.com \
    --cc=jic23@kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Hwmon Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hwmon/0 linux-hwmon/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hwmon linux-hwmon/ https://lore.kernel.org/linux-hwmon \
		linux-hwmon@vger.kernel.org linux-hwmon@archiver.kernel.org
	public-inbox-index linux-hwmon


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hwmon


AGPL code for this site: git clone https://public-inbox.org/ public-inbox