linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>,
	knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net,
	linux-iio@vger.kernel.org, William.Markezana@meas-spec.com
Subject: Re: [PATCH v3 3/6] Add tsys02d meas-spec driver support
Date: Sun, 27 Sep 2015 18:51:22 +0100	[thread overview]
Message-ID: <56082C9A.3030106@kernel.org> (raw)
In-Reply-To: <1443189405-18516-4-git-send-email-ludovic.tancerel@maplehightech.com>

On 25/09/15 14:56, Ludovic Tancerel wrote:
> Support for TSYS02D temperature sensor
> 
> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
Nice straight forward driver.  I'm happy with this, though one
possible suggestion to consider for the battery low interface.

Amazing how different two devices that sound roughly the same from the same
company can be!

Jonathan
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-meas-spec |   7 +
>  drivers/iio/temperature/Kconfig                   |  11 ++
>  drivers/iio/temperature/Makefile                  |   1 +
>  drivers/iio/temperature/tsys02d.c                 | 192 ++++++++++++++++++++++
>  4 files changed, 211 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>  create mode 100644 drivers/iio/temperature/tsys02d.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
> new file mode 100644
> index 0000000..6d47e54
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
> @@ -0,0 +1,7 @@
> +What:           /sys/bus/iio/devices/iio:deviceX/battery_low
> +KernelVersion:  4.1.0
> +Contact:        linux-iio@vger.kernel.org
> +Description:
> +                Reading returns either '1' or '0'. '1' means that the
> +                battery level supplied to sensor is below 2.25V.
> +                This ABI is available for tsys02d, htu21, ms8607
I'm wondering if we can come up with a nicer interface for this.  Either
support it as a polled threshold event or consider other options.
I guess it's a one off at the moment and hardly painful to keep around
for compatability if we come up with something else better later.
So inconclusion, I'm fine for now with leaving this as it is.


> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 35712032..c4664e5 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -34,4 +34,15 @@ config TSYS01
>  	  This driver can also be built as a module. If so, the module will
>  	  be called tsys01.
>  
> +config TSYS02D
> +	tristate "Measurement Specialties TSYS02D temperature sensor"
> +	depends on I2C
> +	select IIO_MS_SENSORS_I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  TSYS02D temperature sensor.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called tsys02d.
> +
>  endmenu
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 368a2a2..02bc79d 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -5,3 +5,4 @@
>  obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
>  obj-$(CONFIG_TSYS01) += tsys01.o
> +obj-$(CONFIG_TSYS02D) += tsys02d.o
> diff --git a/drivers/iio/temperature/tsys02d.c b/drivers/iio/temperature/tsys02d.c
> new file mode 100644
> index 0000000..f8940d6
> --- /dev/null
> +++ b/drivers/iio/temperature/tsys02d.c
> @@ -0,0 +1,192 @@
> +/*
> + * tsys02d.c - Support for Measurement-Specialties tsys02d temperature sensor
> + *
> + * Copyright (c) 2015 Measurement-Specialties
> + *
> + * Licensed under the GPL-2.
> + *
> + * (7-bit I2C slave address 0x40)
> + *
> + * Datasheet:
> + *  http://www.meas-spec.com/downloads/Digital_Sensor_TSYS02D.pdf
> + *
> + */
> +
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/stat.h>
> +#include <linux/module.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include "../common/ms_sensors/ms_sensors_i2c.h"
> +
> +#define TSYS02D_RESET				0xFE
> +
> +static const int tsys02d_samp_freq[4] = { 20, 40, 70, 140 };
> +/* String copy of the above const for readability purpose */
> +static const char tsys02d_show_samp_freq[] = "20 40 70 140";
> +
> +static int tsys02d_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *channel, int *val,
> +			    int *val2, long mask)
> +{
> +	int ret;
> +	s32 temperature;
> +	struct ms_ht_dev *dev_data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED:
> +		switch (channel->type) {
> +		case IIO_TEMP:	/* in milli °C */
> +			ret = ms_sensors_i2c_ht_read_temperature(dev_data,
> +								 &temperature);
> +			if (ret)
> +				return ret;
> +			*val = temperature;
> +
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = tsys02d_samp_freq[dev_data->res_index];
> +
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int tsys02d_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct ms_ht_dev *dev_data = iio_priv(indio_dev);
> +	int i, ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		i = ARRAY_SIZE(tsys02d_samp_freq);
> +		while (i-- > 0)
> +			if (val == tsys02d_samp_freq[i])
> +				break;
> +		if (i < 0)
> +			return -EINVAL;
> +		mutex_lock(&dev_data->lock);
> +		dev_data->res_index = i;
> +		ret = ms_sensors_i2c_write_resolution(dev_data, i);
> +		mutex_unlock(&dev_data->lock);
> +
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_chan_spec tsys02d_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	}
> +};
> +
> +static ssize_t tsys02_read_battery_low(struct device *dev,
> +				       struct device_attribute *attr,
> +				       char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct ms_ht_dev *dev_data = iio_priv(indio_dev);
> +
> +	return ms_sensors_i2c_show_battery_low(dev_data, buf);
> +}
> +
> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(tsys02d_show_samp_freq);
> +static IIO_DEVICE_ATTR(battery_low, S_IRUGO,
> +		       tsys02_read_battery_low, NULL, 0);
> +
> +static struct attribute *tsys02d_attributes[] = {
> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_battery_low.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group tsys02d_attribute_group = {
> +	.attrs = tsys02d_attributes,
> +};
> +
> +static const struct iio_info tsys02d_info = {
> +	.read_raw = tsys02d_read_raw,
> +	.write_raw = tsys02d_write_raw,
> +	.attrs = &tsys02d_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +int tsys02d_probe(struct i2c_client *client,
> +		  const struct i2c_device_id *id)
> +{
> +	struct ms_ht_dev *dev_data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +	u64 serial_number;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_WRITE_BYTE |
> +				     I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> +		dev_err(&client->dev,
> +			"Adapter does not support some i2c transaction\n");
> +		return -ENODEV;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*dev_data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	dev_data = iio_priv(indio_dev);
> +	dev_data->client = client;
> +	dev_data->res_index = 0;
> +	mutex_init(&dev_data->lock);
> +
> +	indio_dev->info = &tsys02d_info;
> +	indio_dev->name = id->name;
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = tsys02d_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(tsys02d_channels);
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	ret = ms_sensors_i2c_reset(client, TSYS02D_RESET, 15000);
> +	if (ret)
> +		return ret;
> +
> +	ret = ms_sensors_i2c_read_serial(client, &serial_number);
> +	if (ret)
> +		return ret;
> +	dev_info(&client->dev, "Serial number : %llx", serial_number);
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id tsys02d_id[] = {
> +	{"tsys02d", 0},
> +	{}
> +};
> +
> +static struct i2c_driver tsys02d_driver = {
> +	.probe = tsys02d_probe,
> +	.id_table = tsys02d_id,
> +	.driver = {
> +		   .name = "tsys02d",
> +		   },
> +};
> +
> +module_i2c_driver(tsys02d_driver);
> +
> +MODULE_DESCRIPTION("Measurement-Specialties tsys02d temperature driver");
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
> +MODULE_LICENSE("GPL v2");
> 


  reply	other threads:[~2015-09-27 17:51 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-25 13:56 [PATCH v3 0/6] iio: TSYS01, TSYS02D, HTU21, MS5637, MS8607, Measurement Specialties driver developments Ludovic Tancerel
2015-09-25 13:56 ` [PATCH v3 1/6] Add meas-spec sensors common part Ludovic Tancerel
2015-09-27 16:23   ` Jonathan Cameron
2015-09-29  7:59     ` ludovic.tancerel
2015-09-29  8:03       ` ludovic.tancerel
2015-09-29 17:20         ` Jonathan Cameron
2015-09-25 13:56 ` [PATCH v3 2/6] Add tsys01 meas-spec driver support Ludovic Tancerel
2015-09-27 16:55   ` Jonathan Cameron
2015-09-29  9:36     ` ludovic.tancerel
2015-09-29 17:21       ` Jonathan Cameron
2015-09-25 13:56 ` [PATCH v3 3/6] Add tsys02d " Ludovic Tancerel
2015-09-27 17:51   ` Jonathan Cameron [this message]
2015-09-29  9:40     ` ludovic.tancerel
2015-09-25 13:56 ` [PATCH v3 4/6] Add htu21 " Ludovic Tancerel
2015-09-27 17:54   ` Jonathan Cameron
2015-09-25 13:56 ` [PATCH v3 5/6] Add ms5637 " Ludovic Tancerel
2015-09-27 17:57   ` Jonathan Cameron
2015-09-29  9:45     ` ludovic.tancerel
2015-09-25 13:56 ` [PATCH v3 6/6] Add ms8607 " Ludovic Tancerel
2015-09-27 18:00   ` Jonathan Cameron
2015-09-29 10:00     ` ludovic.tancerel
2015-09-29 17:27       ` Jonathan Cameron

Reply instructions:

You may reply publicly 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=56082C9A.3030106@kernel.org \
    --to=jic23@kernel.org \
    --cc=William.Markezana@meas-spec.com \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=ludovic.tancerel@maplehightech.com \
    --cc=pmeerw@pmeerw.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).