Message-ID: <574B132F.9010809@roeck-us.net>
Date:	Sun, 29 May 2016 09:05:03 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Pascal Sachs <pascalsachs@...il.com>, jdelvare@...e.com
Cc:	corbet@....net, pascal.sachs@...sirion.com,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	David Frey <david.frey@...sirion.com>
Subject: Re: [PATCH 1/1 v3] hwmon: add support for Sensirion SHT3x sensors

On 05/26/2016 12:25 AM, Pascal Sachs wrote:
> From: David Frey <david.frey@...sirion.com>
>
> This driver implements support for the Sensirion SHT3x-DIS chip,
> a humidity and temperature sensor. Temperature is measured
> in degrees celsius, relative humidity is expressed as a percentage.
> In the sysfs interface, all values are scaled by 1000,
> i.e. the value for 31.5 degrees celsius is 31500.
>
> Signed-off-by: Pascal Sachs <pascal.sachs@...sirion.com>
> ---
> Patch was generated against kernel v4.6-rc7
>
>   Documentation/hwmon/sht3x           |  72 ++++
>   drivers/hwmon/Kconfig               |  11 +
>   drivers/hwmon/Makefile              |   1 +
>   drivers/hwmon/sht3x.c               | 701 ++++++++++++++++++++++++++++++++++++
>   include/linux/platform_data/sht3x.h |  25 ++
>   5 files changed, 810 insertions(+)
>   create mode 100644 Documentation/hwmon/sht3x
>   create mode 100644 drivers/hwmon/sht3x.c
>   create mode 100644 include/linux/platform_data/sht3x.h
>
> diff --git a/Documentation/hwmon/sht3x b/Documentation/hwmon/sht3x
> new file mode 100644
> index 0000000..f5aa633
> --- /dev/null
> +++ b/Documentation/hwmon/sht3x
> @@ -0,0 +1,72 @@
> +Kernel driver sht3x
> +===================
> +
> +Supported chips:
> +  * Sensirion SHT3x-DIS
> +    Prefix: 'sht3x'
> +    Addresses scanned: none
> +    Datasheet: http://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/Humidity/Sensirion_Humidity_Datasheet_SHT3x_DIS.pdf
> +
> +Author:
> +  David Frey <david.frey@...sirion.com>
> +  Pascal Sachs <pascal.sachs@...sirion.com>
> +
> +Description
> +-----------
> +
> +This driver implements support for the Sensirion SHT3x-DIS chip, a humidity
> +and temperature sensor. Temperature is measured in degrees celsius, relative
> +humidity is expressed as a percentage. In the sysfs interface, all values are
> +scaled by 1000, i.e. the value for 31.5 degrees celsius is 31500.
> +
> +The device communicates with the I2C protocol. Sensors can have the I2C
> +addresses 0x44 or 0x45, depending on the wiring. See
> +Documentation/i2c/instantiating-devices for methods to instantiate the device.
> +
> +There are two options configurable by means of sht3x_platform_data:
> +1. blocking (pull the I2C clock line down while performing the measurement) or
> +   non-blocking mode. Blocking mode will guarantee the fastest result but
> +   the I2C bus will be busy during that time. By default, non-blocking mode
> +   is used. Make sure clock-stretching works properly on your device if you
> +   want to use blocking mode.
> +2. high or low accuracy. High accuracy is used by default and using it is
> +   strongly recommended.
> +
> +The sht3x sensor supports a single shot mode as well as 5 periodic measure
> +modes, which can be controlled with the update_interval sysfs interface.
> +The allowed update_interval in milliseconds are as follows:
> +  *     0   single shot mode
> +  *  2000   0.5 Hz periodic measurement
> +  *  1000   1   Hz periodic measurement
> +  *   500   2   Hz periodic measurement
> +  *   250   4   Hz periodic measurement
> +  *   100  10   Hz periodic measurement
> +
> +In the periodic measure mode, the sensor automatically triggers a measurement
> +with the configured update interval on the chip. When a temperature or humidity
> +reading exceeds the configured limits, the alert attribute is set to 1 and
> +the alert pin on the sensor is set to high.
> +When the temperature and humidity readings move back between the hysteresis
> +values, the alert bit is set to 0 and the alert pin on the sensor is set to
> +low.
> +
> +sysfs-Interface
> +---------------
> +
> +temp1_input:        temperature input
> +humidity1_input:    humidity input
> +temp1_max:          temperature max value
> +temp1_max_hyst:     temperature hysteresis value for max limit
> +humidity1_max:      humidity max value
> +humidity1_max_hyst: humidity hysteresis value for max limit
> +temp1_min:          temperature min value
> +temp1_min_hyst:     temperature hysteresis value for min limit
> +humidity1_min:      humidity min value
> +humidity1_min_hyst: humidity hysteresis value for min limit
> +temp1_alarm:        alarm flag is set to 1 if the temperature is outside the
> +                    configured limits. Alarm only works in periodic measure mode
> +humidity1_alarm:    alarm flag is set to 1 if the humidity is outside the
> +                    configured limits. Alarm only works in periodic measure mode
> +update_interval:    update interval, 0 for single shot, interval in msec
> +                    for periodic measurement. If the interval is not supported
> +                    by the sensor, the next faster interval is chosen
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 5c2d13a..222c483 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1254,6 +1254,17 @@ config SENSORS_SHT21
>   	  This driver can also be built as a module.  If so, the module
>   	  will be called sht21.
>
> +config SENSORS_SHT3x
> +	tristate "Sensiron humidity and temperature sensors. SHT3x and compat."
> +	depends on I2C
> +	select CRC8
> +	help
> +	  If you say yes here you get support for the Sensiron SHT30 and SHT31
> +	  humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called sht3x.
> +
>   config SENSORS_SHTC1
>   	tristate "Sensiron humidity and temperature sensors. SHTC1 and compat."
>   	depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 58cc3ac..999ff2d 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -137,6 +137,7 @@ obj-$(CONFIG_SENSORS_SCH5627)	+= sch5627.o
>   obj-$(CONFIG_SENSORS_SCH5636)	+= sch5636.o
>   obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
>   obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
> +obj-$(CONFIG_SENSORS_SHT3x)	+= sht3x.o
>   obj-$(CONFIG_SENSORS_SHTC1)	+= shtc1.o
>   obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
>   obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
> diff --git a/drivers/hwmon/sht3x.c b/drivers/hwmon/sht3x.c
> new file mode 100644
> index 0000000..a3d2130
> --- /dev/null
> +++ b/drivers/hwmon/sht3x.c
> @@ -0,0 +1,701 @@
> +/* Sensirion SHT3x-DIS humidity and temperature sensor driver.
> + * The SHT3x comes in many different versions, this driver is for the
> + * I2C version only.
> + *
> + * Copyright (C) 2016 Sensirion AG, Switzerland
> + * Author: David Frey <david.frey@...sirion.com>
> + * Author: Pascal Sachs <pascal.sachs@...sirion.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <asm/page.h>
> +#include <linux/crc8.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/platform_data/sht3x.h>
> +
> +/* commands (high precision mode) */
> +static const unsigned char sht3x_cmd_measure_blocking_hpm[]    = { 0x2c, 0x06 };
> +static const unsigned char sht3x_cmd_measure_nonblocking_hpm[] = { 0x24, 0x00 };
> +
> +/* commands (low power mode) */
> +static const unsigned char sht3x_cmd_measure_blocking_lpm[]    = { 0x2c, 0x10 };
> +static const unsigned char sht3x_cmd_measure_nonblocking_lpm[] = { 0x24, 0x16 };
> +
> +/* commands for periodic mode */
> +static const unsigned char sht3x_cmd_measure_periodic_mode[]   = { 0xe0, 0x00 };
> +static const unsigned char sht3x_cmd_break[]                   = { 0x30, 0x93 };
> +
> +/* other commands */
> +static const unsigned char sht3x_cmd_read_status_reg[]         = { 0xf3, 0x2d };
> +static const unsigned char sht3x_cmd_clear_status_reg[]        = { 0x30, 0x41 };
> +
> +/* delays for non-blocking i2c commands, both in us */
> +#define SHT3X_NONBLOCKING_WAIT_TIME_HPM  15000
> +#define SHT3X_NONBLOCKING_WAIT_TIME_LPM   4000
> +
> +#define SHT3X_WORD_LEN         2
> +#define SHT3X_CMD_LENGTH       2
> +#define SHT3X_CRC8_LEN         1
> +#define SHT3X_RESPONSE_LENGTH  6
> +#define SHT3X_CRC8_POLYNOMIAL  0x31
> +#define SHT3X_CRC8_INIT        0xFF
> +
> +enum sht3x_chips {
> +	sht3x,
> +	sts3x,
> +};
> +
> +DECLARE_CRC8_TABLE(sht3x_crc8_table);
> +
> +/* periodic measure commands (high precision mode) */
> +static const char periodic_measure_commands_hpm[][SHT3X_CMD_LENGTH] = {
> +	/* 0.5 measurements per second */
> +	{0x20, 0x32},
> +	/* 1 measurements per second */
> +	{0x21, 0x30},
> +	/* 2 measurements per second */
> +	{0x22, 0x36},
> +	/* 4 measurements per second */
> +	{0x23, 0x34},
> +	/* 10 measurements per second */
> +	{0x27, 0x37},
> +};
> +
> +/* periodic measure commands (low power mode) */
> +static const char periodic_measure_commands_lpm[][SHT3X_CMD_LENGTH] = {
> +	/* 0.5 measurements per second */
> +	{0x20, 0x2f},
> +	/* 1 measurements per second */
> +	{0x21, 0x2d},
> +	/* 2 measurements per second */
> +	{0x22, 0x2b},
> +	/* 4 measurements per second */
> +	{0x23, 0x29},
> +	/* 10 measurements per second */
> +	{0x27, 0x2a},
> +};
> +
> +struct sht3x_limit_commands {
> +	const char read_command[SHT3X_CMD_LENGTH];
> +	const char write_command[SHT3X_CMD_LENGTH];
> +};
> +
> +static const struct sht3x_limit_commands limit_commands[] = {
> +	/* temp1_max, humidity1_max */
> +	{ {0xe1, 0x1f}, {0x61, 0x1d} },
> +	/* temp_1_max_hyst, humidity1_max_hyst */
> +	{ {0xe1, 0x14}, {0x61, 0x16} },
> +	/* temp1_min, humidity1_min */
> +	{ {0xe1, 0x02}, {0x61, 0x00} },
> +	/* temp_1_min_hyst, humidity1_min_hyst */
> +	{ {0xe1, 0x09}, {0x61, 0x0B} },
> +};
> +
> +#define SHT3X_NUM_LIMIT_CMD  ARRAY_SIZE(limit_commands)
> +
> +static const u16 mode_to_update_interval[] = {
> +	   0,
> +	2000,
> +	1000,
> +	 500,
> +	 250,
> +	 100,
> +};
> +
> +struct sht3x_data {
> +	struct i2c_client *client;
> +	struct mutex i2c_lock; /* lock for sending i2c commands */
> +	struct mutex data_lock; /* lock for updating driver data */
> +
> +	u8 mode;
> +	const unsigned char *command;
> +	u32 wait_time;			/* in us*/
> +	unsigned long last_update;	/* last update in periodic mode*/
> +
> +	struct sht3x_platform_data setup;
> +
> +	/*
> +	 * cached values for temperature and humidity and limits
> +	 * the limits arrays have the following order:
> +	 * max, max_hyst, min, min_hyst
> +	 */
> +	int temperature;
> +	int temperature_limits[SHT3X_NUM_LIMIT_CMD];
> +	u32 humidity;
> +	u32 humidity_limits[SHT3X_NUM_LIMIT_CMD];
> +};
> +
> +static u8 get_mode_from_update_interval(u16 value)
> +{
> +	size_t index;
> +	u8 number_of_modes = ARRAY_SIZE(mode_to_update_interval);
> +
> +	if (value == 0)
> +		return 0;
> +
> +	/* find next faster update interval */
> +	for (index = 1; index < number_of_modes; index++) {
> +		if (mode_to_update_interval[index] <= value)
> +			return index;
> +	}
> +
> +	return number_of_modes - 1;
> +}
> +
> +static int sht3x_read_from_command(struct i2c_client *client,
> +				   struct sht3x_data *data,
> +				   const char *command,
> +				   char *buf, int length, u32 wait_time)
> +{
> +	int ret;
> +
> +	mutex_lock(&data->i2c_lock);
> +	ret = i2c_master_send(client, command, SHT3X_CMD_LENGTH);
> +
> +	if (ret != SHT3X_CMD_LENGTH) {
> +		ret = ret < 0 ? ret : -EIO;
> +		goto out;
> +	}
> +
> +	if (wait_time)
> +		usleep_range(wait_time, wait_time + 1000);
> +
> +	ret = i2c_master_recv(client, buf, length);
> +	if (ret != length) {
> +		ret = ret < 0 ? ret : -EIO;
> +		goto out;
> +	}
> +
> +	ret = 0;
> +out:
> +	mutex_unlock(&data->i2c_lock);
> +	return ret;
> +}
> +
> +static int sht3x_extract_temperature(u16 raw)
> +{
> +	/*
> +	 * From datasheet:
> +	 * T = -45 + 175 * ST / 2^16
> +	 * Adapted for integer fixed point (3 digit) arithmetic.
> +	 */
> +	return ((21875 * (int)raw) >> 13) - 45000;
> +}
> +
> +static u32 sht3x_extract_humidity(u16 raw)
> +{
> +	/*
> +	 * From datasheet:
> +	 * RH = 100 * SRH / 2^16
> +	 * Adapted for integer fixed point (3 digit) arithmetic.
> +	 */
> +	return (12500 * (int)raw) >> 13;
> +}
> +
> +static struct sht3x_data *sht3x_update_client(struct device *dev)
> +{
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +	struct i2c_client *client = data->client;
> +	u16 interval_ms = mode_to_update_interval[data->mode];
> +	unsigned long interval_jiffies = msecs_to_jiffies(interval_ms);
> +	unsigned char buf[SHT3X_RESPONSE_LENGTH];
> +	u16 val;
> +	int ret;
> +
<<<<
> +	/* only update once per interval in periodic mode */
> +	if (time_after(jiffies, data->last_update + interval_jiffies)) {
> +		ret = sht3x_read_from_command(client, data, data->command, buf,
> +					      sizeof(buf), data->wait_time);
> +		if (ret)
> +			return ERR_PTR(ret);
> +
> +		mutex_lock(&data->data_lock);
> +		val = be16_to_cpup((__be16 *)buf);
> +		data->temperature = sht3x_extract_temperature(val);
> +		val = be16_to_cpup((__be16 *)(buf + 3));
> +		data->humidity = sht3x_extract_humidity(val);
> +		data->last_update = jiffies;
> +		mutex_unlock(&data->data_lock);

Locking is incomplete. data_lock needs to be acquired at <<<< to be useful
(ie it needs to cover last_update).

> +	}
> +
> +	return data;
> +}
> +
> +/* sysfs attributes */
> +static ssize_t temp1_input_show(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct sht3x_data *data = sht3x_update_client(dev);
> +
> +	if (IS_ERR(data))
> +		return PTR_ERR(data);
> +
> +	return sprintf(buf, "%d\n", data->temperature);
> +}
> +
> +static ssize_t humidity1_input_show(struct device *dev,
> +				    struct device_attribute *attr, char *buf)
> +{
> +	struct sht3x_data *data = sht3x_update_client(dev);
> +
> +	if (IS_ERR(data))
> +		return PTR_ERR(data);
> +
> +	return sprintf(buf, "%d\n", data->humidity);
> +}
> +
> +static int limits_update(struct sht3x_data *data)
> +{
> +	int ret;
> +	u8 index;
> +	int temperature;
> +	u32 humidity;
> +	u16 raw;
> +	char buffer[SHT3X_RESPONSE_LENGTH];
> +	const struct sht3x_limit_commands *commands;
> +	struct i2c_client *client = data->client;
> +
> +	for (index = 0; index < SHT3X_NUM_LIMIT_CMD; index++) {
> +		commands = &limit_commands[index];
> +		ret = sht3x_read_from_command(client, data,
> +					      commands->read_command, buffer,
> +					      SHT3X_RESPONSE_LENGTH, 0);
> +
> +		if (ret)
> +			return ret;
> +
> +		raw = be16_to_cpup((__be16 *)buffer);
> +		temperature = sht3x_extract_temperature((raw & 0x01ff) << 7);
> +		humidity = sht3x_extract_humidity(raw & 0xfe00);
> +		mutex_lock(&data->data_lock);
> +		data->temperature_limits[index] = temperature;
> +		data->humidity_limits[index] = humidity;
> +		mutex_unlock(&data->data_lock);

Since the function is only called during probe, you don't need locks here.
Just mark in the header that the function must only be called from probe
or with data_lock held. Alternatively, data_lock would have to cover
the entire function for it to be useful.

> +	}
> +
> +	return ret;
> +}
> +
> +static ssize_t temp1_limit_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	int temperature_limit;
> +	u8 index;
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +
> +	index = to_sensor_dev_attr(attr)->index;
> +	temperature_limit = data->temperature_limits[index];
> +
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", temperature_limit);
> +}
> +
> +static ssize_t humidity1_limit_show(struct device *dev,
> +				    struct device_attribute *attr,
> +				    char *buf)
> +{
> +	int humidity_limit;
> +	u8 index;
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +
> +	index = to_sensor_dev_attr(attr)->index;
> +	humidity_limit = data->humidity_limits[index];
> +
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", humidity_limit);
> +}
> +
> +static size_t limit_store(struct device *dev,
> +			  struct device_attribute *attr,
> +			  size_t count,
> +			  int temperature,
> +			  u32 humidity)
> +{
> +	char buffer[SHT3X_CMD_LENGTH + SHT3X_WORD_LEN + SHT3X_CRC8_LEN];
> +	char *position = buffer;
> +	int ret;
> +	u16 raw;
> +	u8 index;
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +	struct i2c_client *client = data->client;
> +	const struct sht3x_limit_commands *commands;
> +
> +	index = to_sensor_dev_attr(attr)->index;
> +	commands = &limit_commands[index];
> +
> +	memcpy(position, commands->write_command, SHT3X_CMD_LENGTH);
> +	position += SHT3X_CMD_LENGTH;
> +	/*
> +	 * ST = (T + 45) / 175 * 2^16
> +	 * SRH = RH / 100 * 2^16
> +	 * adapted for fixed point arithmetic and packed the same as
> +	 * in limit_show()
> +	 */
> +	raw = ((u32)(temperature + 45000) * 24543) >> (16 + 7);
> +	raw |= ((humidity * 42950) >> 16) & 0xfe00;
> +
> +	*(__be16 *)(position) = cpu_to_be16(raw);

(position) seems unnecessary. Did you mean
	*((__be16 *)position) ?


> +	position += SHT3X_WORD_LEN;
> +	*position = crc8(sht3x_crc8_table,
> +			 position - SHT3X_WORD_LEN,
> +			 SHT3X_WORD_LEN,
> +			 SHT3X_CRC8_INIT);
> +
> +	mutex_lock(&data->i2c_lock);
> +	ret = i2c_master_send(client, buffer, sizeof(buffer));
> +	mutex_unlock(&data->i2c_lock);
> +	if (ret != sizeof(buffer)) {
> +		ret =  ret < 0 ? ret : -EIO;
> +	} else {
> +		mutex_lock(&data->data_lock);
> +		data->temperature_limits[index] = temperature;
> +		data->humidity_limits[index] = humidity;
> +		mutex_unlock(&data->data_lock);
> +	}
> +		ret = count;
> +
'}' at the wrong location ? Note that per coding style you should use
goto for error handling.

The locking is incomplete. You have to acquire data_lock before sending
the i2c data, and hold it until the data is stored in the local variables.

> +	return ret;
> +}
> +
> +static ssize_t temp1_limit_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf,
> +				 size_t count)
> +{
> +	int temperature;
> +	u32 humidity;
> +	u8 index;
> +	int ret;
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +
> +	ret = kstrtoint(buf, 0, &temperature);
> +
> +	if (ret)
> +		return -EINVAL;
> +
> +	temperature = clamp_val(temperature, -45000, 130000);
> +
> +	index = to_sensor_dev_attr(attr)->index;
> +	humidity = data->humidity_limits[index];
> +
> +	ret = limit_store(dev, attr, count, temperature, humidity);
> +
> +	return ret;
> +}
> +
> +static ssize_t humidity1_limit_store(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf,
> +				     size_t count)
> +{
> +	int temperature;
> +	u32 humidity;
> +	u8 index;
> +	int ret;
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +
> +	ret = kstrtou32(buf, 0, &humidity);
> +
> +	if (ret)
> +		return -EINVAL;
> +
> +	humidity = clamp_val(humidity, 0, 100000);
> +	index = to_sensor_dev_attr(attr)->index;
> +	temperature = data->temperature_limits[index];
> +
> +	ret = limit_store(dev, attr, count, temperature, humidity);
> +
> +	return ret;
> +}
> +
> +static void sht3x_select_command(struct sht3x_data *data)
> +{
> +	/*
> +	 * In blocking mode (clock stretching mode) the I2C bus
> +	 * is blocked for other traffic, thus the call to i2c_master_recv()
> +	 * will wait until the data is ready. For non blocking mode, we
> +	 * have to wait ourselves.
> +	 */
> +	if (data->mode > 0) {
> +		data->command = sht3x_cmd_measure_periodic_mode;
> +		data->wait_time = 0;
> +	} else if (data->setup.blocking_io) {
> +		data->command = data->setup.high_precision ?
> +				sht3x_cmd_measure_blocking_hpm :
> +				sht3x_cmd_measure_blocking_lpm;
> +		data->wait_time = 0;
> +	} else {
> +		if (data->setup.high_precision) {
> +			data->command = sht3x_cmd_measure_nonblocking_hpm;
> +			data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_HPM;
> +		} else {
> +			data->command = sht3x_cmd_measure_nonblocking_lpm;
> +			data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_LPM;
> +		}
> +	}
> +}
> +
> +static int status_register_read(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buffer, int length)
> +{
> +	int ret;
> +
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +	struct i2c_client *client = data->client;
> +
> +	ret = sht3x_read_from_command(client, data, sht3x_cmd_read_status_reg,
> +				      buffer, length, 0);
> +
> +	return ret;
> +}
> +
> +static ssize_t temp1_alarm_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	int ret;
> +	int length = SHT3X_WORD_LEN + SHT3X_CRC8_LEN;
> +	char buffer[length];
> +
> +	ret = status_register_read(dev, attr, buffer, length);
> +
> +	if (ret)
> +		return ret;
> +
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", (buffer[0] & 0x04) == 0x04);

		!!(buffer[0] & 0x04)

would be a bit easier to understand.

> +}
> +
> +static ssize_t humidity1_alarm_show(struct device *dev,
> +				    struct device_attribute *attr,
> +				    char *buf)
> +{
> +	int ret;
> +	int length = SHT3X_WORD_LEN + SHT3X_CRC8_LEN;
> +	char buffer[length];
> +
> +	ret = status_register_read(dev, attr, buffer, length);
> +
> +	if (ret)
> +		return ret;
> +
> +	return scnprintf(buf, PAGE_SIZE, "%d\n", (buffer[0] & 0x08) == 0x08);

			!!(buffer[0] & 0x08)

would be a bit easier to understand.

> +}
> +
> +static ssize_t update_interval_show(struct device *dev,
> +				    struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +
> +	return scnprintf(buf, PAGE_SIZE, "%d\n",
> +			 mode_to_update_interval[data->mode]);
> +}
> +
> +static ssize_t update_interval_store(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf,
> +				     size_t count)
> +{
> +	u16 update_interval;
> +	u8 mode;
> +	int ret;
> +	const char *command;
> +	struct sht3x_data *data = dev_get_drvdata(dev);
> +	struct i2c_client *client = data->client;
> +
> +	ret = kstrtou16(buf, 0, &update_interval);
> +
> +	if (ret)
> +		return -EINVAL;
> +
> +	mode = get_mode_from_update_interval(update_interval);
> +
> +	/* mode did not change */
> +	if (mode == data->mode)
> +		return count;
> +
> +	mutex_lock(&data->i2c_lock);
> +	mutex_lock(&data->data_lock);
> +	/* abort periodic measure */
> +	ret = i2c_master_send(client, sht3x_cmd_break, SHT3X_CMD_LENGTH);
> +	if (ret != SHT3X_CMD_LENGTH)
> +		goto out;
> +	data->mode = 0;
> +
> +	if (mode > 0) {
> +		if (data->setup.high_precision)
> +			command = periodic_measure_commands_hpm[mode - 1];
> +		else
> +			command = periodic_measure_commands_lpm[mode - 1];
> +
> +		/* select mode */
> +		ret = i2c_master_send(client, command, SHT3X_CMD_LENGTH);
> +		if (ret != SHT3X_CMD_LENGTH)
> +			goto out;
> +	}
> +
> +	/* select mode and command */
> +	data->mode = mode;
> +	sht3x_select_command(data);
> +
> +out:
> +	mutex_unlock(&data->i2c_lock);
> +	mutex_unlock(&data->data_lock);

You need to unlock in the order you locked.

> +	if (ret != SHT3X_CMD_LENGTH)
> +		return ret < 0 ? ret : -EIO;
> +
> +	return count;
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, temp1_input_show, NULL, 0);
> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, humidity1_input_show,
> +			  NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO | S_IWUSR, temp1_limit_show,
> +			  temp1_limit_store, 0);
> +static SENSOR_DEVICE_ATTR(humidity1_max, S_IRUGO | S_IWUSR,
> +			  humidity1_limit_show, humidity1_limit_store, 0);
> +static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IRUGO | S_IWUSR, temp1_limit_show,
> +			  temp1_limit_store, 1);
> +static SENSOR_DEVICE_ATTR(humidity1_max_hyst, S_IRUGO | S_IWUSR,
> +			  humidity1_limit_show, humidity1_limit_store, 1);
> +static SENSOR_DEVICE_ATTR(temp1_min, S_IRUGO | S_IWUSR, temp1_limit_show,
> +			  temp1_limit_store, 2);
> +static SENSOR_DEVICE_ATTR(humidity1_min, S_IRUGO | S_IWUSR,
> +			  humidity1_limit_show, humidity1_limit_store, 2);
> +static SENSOR_DEVICE_ATTR(temp1_min_hyst, S_IRUGO | S_IWUSR,
> +			  temp1_limit_show, temp1_limit_store, 3);
> +static SENSOR_DEVICE_ATTR(humidity1_min_hyst, S_IRUGO | S_IWUSR,
> +			  humidity1_limit_show, humidity1_limit_store, 3);
> +static SENSOR_DEVICE_ATTR(temp1_alarm, S_IRUGO, temp1_alarm_show, NULL, 3);
> +static SENSOR_DEVICE_ATTR(humidity1_alarm, S_IRUGO, humidity1_alarm_show,
> +			  NULL, 3);
> +static SENSOR_DEVICE_ATTR(update_interval, S_IRUGO | S_IWUSR,
> +			  update_interval_show, update_interval_store, 0);
> +
> +static struct attribute *sht3x_attrs[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	&sensor_dev_attr_temp1_max.dev_attr.attr,
> +	&sensor_dev_attr_temp1_max_hyst.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_max.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_max_hyst.dev_attr.attr,
> +	&sensor_dev_attr_temp1_min.dev_attr.attr,
> +	&sensor_dev_attr_temp1_min_hyst.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_min.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_min_hyst.dev_attr.attr,
> +	&sensor_dev_attr_temp1_alarm.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_alarm.dev_attr.attr,
> +	&sensor_dev_attr_update_interval.dev_attr.attr,
> +	NULL
> +};
> +
> +static struct attribute *sts3x_attrs[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +ATTRIBUTE_GROUPS(sht3x);
> +ATTRIBUTE_GROUPS(sts3x);
> +
> +static int sht3x_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	int ret;
> +	struct sht3x_data *data;
> +	struct device *hwmon_dev;
> +	struct i2c_adapter *adap = client->adapter;
> +	struct device *dev = &client->dev;
> +	const struct attribute_group **attribute_groups;
> +
> +	/*
> +	 * we require full i2c support since the sht3x uses multi-byte read and
> +	 * writes as well as multi-byte commands which are not supported by
> +	 * the smbus protocol
> +	 */
> +	if (!i2c_check_functionality(adap, I2C_FUNC_I2C))
> +		return -ENODEV;
> +
> +	ret = i2c_master_send(client, sht3x_cmd_clear_status_reg,
> +			      SHT3X_CMD_LENGTH);
> +	if (ret != SHT3X_CMD_LENGTH)
> +		return ret < 0 ? ret : -ENODEV;
> +
> +	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->setup.blocking_io = false;
> +	data->setup.high_precision = true;
> +	data->mode = 0;
> +	data->last_update = 0;
> +	data->client = client;
> +	crc8_populate_msb(sht3x_crc8_table, SHT3X_CRC8_POLYNOMIAL);
> +
> +	if (client->dev.platform_data)
> +		data->setup = *(struct sht3x_platform_data *)dev->platform_data;
> +
> +	sht3x_select_command(data);
> +
> +	mutex_init(&data->i2c_lock);
> +	mutex_init(&data->data_lock);
> +
> +	ret = limits_update(data);
> +	if (ret)
> +		return ret;
> +
> +	if (id->driver_data == sts3x)
> +		attribute_groups = sts3x_groups;
> +	else
> +		attribute_groups = sht3x_groups;
> +
> +	hwmon_dev = devm_hwmon_device_register_with_groups(dev,
> +							   client->name,
> +							   data,
> +							   attribute_groups);
> +
> +	if (IS_ERR(hwmon_dev))
> +		dev_dbg(dev, "unable to register hwmon device\n");
> +
> +	return PTR_ERR_OR_ZERO(hwmon_dev);
> +}
> +
> +/* device ID table */
> +static const struct i2c_device_id sht3x_ids[] = {
> +	{"sht3x", sht3x},
> +	{"sts3x", sts3x},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, sht3x_ids);
> +
> +static struct i2c_driver sht3x_i2c_driver = {
> +	.driver.name = "sht3x",
> +	.probe       = sht3x_probe,
> +	.id_table    = sht3x_ids,
> +};
> +
> +module_i2c_driver(sht3x_i2c_driver);
> +
> +MODULE_AUTHOR("David Frey <david.frey@...sirion.com>");
> +MODULE_AUTHOR("Pascal Sachs <pascal.sachs@...sirion.com>");
> +MODULE_DESCRIPTION("Sensirion SHT3x humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/platform_data/sht3x.h b/include/linux/platform_data/sht3x.h
> new file mode 100644
> index 0000000..46af9a0
> --- /dev/null
> +++ b/include/linux/platform_data/sht3x.h
> @@ -0,0 +1,25 @@
> +/*
> + * Copyright (C) 2015 Sensirion AG, Switzerland
> + * Author: David Frey <david.frey@...sirion.com>
> + * Author: Pascal Sachs <pascal.sachs@...sirion.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifndef __SHT3X_H_
> +#define __SHT3X_H_
> +
> +struct sht3x_platform_data {
> +	bool blocking_io;
> +	bool high_precision;
> +};
> +#endif /* __SHT3X_H_ */
>

Powered by blists - more mailing lists