[PATCH v3 0/6] iio: TSYS01, TSYS02D, HTU21, MS5637, MS8607, Measurement Specialties driver developments

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH v3 0/6] iio: TSYS01, TSYS02D, HTU21, MS5637, MS8607, Measurement Specialties driver developments
@ 2015-09-25 13:56 Ludovic Tancerel
  2015-09-25 13:56 ` [PATCH v3 1/6] Add meas-spec sensors common part Ludovic Tancerel
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Ludovic Tancerel @ 2015-09-25 13:56 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, ludovic.tancerel,
	William.Markezana

This is the resubmission of patches for measurement specialties drivers.
	- TSYS01 : Temperature sensor
	- TSYS02D : Temperature sensor
	- HTU21 : Temperature & Humidity sensor
	- MS5637 : Temperature & Pressure sensor
	- MS8607 : Temperature, Pressure & Humidity sensor

The list of changes are :
	- Remove ms8607 from Kconfig and document ms5637 and htu21 accordingly
	- Move heater_enable in sysfs-bus-iio
	- Minor corrections
	- Modify patchset presentation

Thanks a lot for your feedback.

Regards,
Ludovic

revision history
----------------
v3      Minor corrections. Remove ms8607 Kconfig
v2      Split ms8607/Align units/Document private ABI/ kernel-doc ABI
v1      initial post

Ludovic Tancerel (6):
  Add meas-spec sensors common part
  Add tsys01 meas-spec driver support
  Add tsys02d meas-spec driver support
  Add htu21 meas-spec driver support
  Add ms5637 meas-spec driver support
  Add ms8607 meas-spec driver support

 Documentation/ABI/testing/sysfs-bus-iio           |  10 +
 Documentation/ABI/testing/sysfs-bus-iio-meas-spec |   8 +
 drivers/iio/common/Kconfig                        |   1 +
 drivers/iio/common/Makefile                       |   1 +
 drivers/iio/common/ms_sensors/Kconfig             |   6 +
 drivers/iio/common/ms_sensors/Makefile            |   5 +
 drivers/iio/common/ms_sensors/ms_sensors_i2c.c    | 645 ++++++++++++++++++++++
 drivers/iio/common/ms_sensors/ms_sensors_i2c.h    |  53 ++
 drivers/iio/humidity/Kconfig                      |  13 +
 drivers/iio/humidity/Makefile                     |   1 +
 drivers/iio/humidity/htu21.c                      | 254 +++++++++
 drivers/iio/pressure/Kconfig                      |  17 +-
 drivers/iio/pressure/Makefile                     |   1 +
 drivers/iio/pressure/ms5637.c                     | 191 +++++++
 drivers/iio/temperature/Kconfig                   |  22 +
 drivers/iio/temperature/Makefile                  |   2 +
 drivers/iio/temperature/tsys01.c                  | 231 ++++++++
 drivers/iio/temperature/tsys02d.c                 | 192 +++++++
 18 files changed, 1651 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-meas-spec
 create mode 100644 drivers/iio/common/ms_sensors/Kconfig
 create mode 100644 drivers/iio/common/ms_sensors/Makefile
 create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.c
 create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.h
 create mode 100644 drivers/iio/humidity/htu21.c
 create mode 100644 drivers/iio/pressure/ms5637.c
 create mode 100644 drivers/iio/temperature/tsys01.c
 create mode 100644 drivers/iio/temperature/tsys02d.c

-- 
2.3.7


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 1/6] Add meas-spec sensors common part
  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 ` Ludovic Tancerel
  2015-09-27 16:23   ` Jonathan Cameron
  2015-09-25 13:56 ` [PATCH v3 2/6] Add tsys01 meas-spec driver support Ludovic Tancerel
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Ludovic Tancerel @ 2015-09-25 13:56 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, ludovic.tancerel,
	William.Markezana

Measurement specialties drivers common part.
These functions are used by further drivers in the patchset: TSYS01, TSYS02D, HTU21, MS5637, MS8607

Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
---
 drivers/iio/common/Kconfig                     |   1 +
 drivers/iio/common/Makefile                    |   1 +
 drivers/iio/common/ms_sensors/Kconfig          |   6 +
 drivers/iio/common/ms_sensors/Makefile         |   5 +
 drivers/iio/common/ms_sensors/ms_sensors_i2c.c | 645 +++++++++++++++++++++++++
 drivers/iio/common/ms_sensors/ms_sensors_i2c.h |  53 ++
 6 files changed, 711 insertions(+)
 create mode 100644 drivers/iio/common/ms_sensors/Kconfig
 create mode 100644 drivers/iio/common/ms_sensors/Makefile
 create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.c
 create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.h

diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
index 790f106..26a6026 100644
--- a/drivers/iio/common/Kconfig
+++ b/drivers/iio/common/Kconfig
@@ -3,5 +3,6 @@
 #
 
 source "drivers/iio/common/hid-sensors/Kconfig"
+source "drivers/iio/common/ms_sensors/Kconfig"
 source "drivers/iio/common/ssp_sensors/Kconfig"
 source "drivers/iio/common/st_sensors/Kconfig"
diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
index b1e4d9c..585da6a 100644
--- a/drivers/iio/common/Makefile
+++ b/drivers/iio/common/Makefile
@@ -8,5 +8,6 @@
 
 # When adding new entries keep the list in alphabetical order
 obj-y += hid-sensors/
+obj-y += ms_sensors/
 obj-y += ssp_sensors/
 obj-y += st_sensors/
diff --git a/drivers/iio/common/ms_sensors/Kconfig b/drivers/iio/common/ms_sensors/Kconfig
new file mode 100644
index 0000000..b28a92b
--- /dev/null
+++ b/drivers/iio/common/ms_sensors/Kconfig
@@ -0,0 +1,6 @@
+#
+# Measurements Specialties sensors common library
+#
+
+config IIO_MS_SENSORS_I2C
+        tristate
diff --git a/drivers/iio/common/ms_sensors/Makefile b/drivers/iio/common/ms_sensors/Makefile
new file mode 100644
index 0000000..7846428
--- /dev/null
+++ b/drivers/iio/common/ms_sensors/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the Measurement Specialties sensor common modules.
+#
+
+obj-$(CONFIG_IIO_MS_SENSORS_I2C) += ms_sensors_i2c.o
diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
new file mode 100644
index 0000000..4b1bc31
--- /dev/null
+++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
@@ -0,0 +1,645 @@
+/*
+ * Measurements Specialties driver common i2c functions
+ *
+ * Copyright (c) 2015 Measurement-Specialties
+ *
+ * Licensed under the GPL-2.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/iio/iio.h>
+#include <linux/device.h>
+#include <linux/delay.h>
+
+#include "ms_sensors_i2c.h"
+
+/* Conversion times in us */
+static const u16 ms_sensors_ht_t_conversion_time[] = { 50000, 25000,
+						       13000, 7000 };
+static const u16 ms_sensors_ht_h_conversion_time[] = { 16000, 3000,
+						       5000, 8000 };
+static const u16 ms_sensors_tp_conversion_time[] = { 500, 1100, 2100,
+						     4100, 8220, 16440 };
+
+#define MS_SENSORS_SERIAL_READ_MSB		0xFA0F
+#define MS_SENSORS_SERIAL_READ_LSB		0xFCC9
+#define MS_SENSORS_CONFIG_REG_WRITE		0xE6
+#define MS_SENSORS_CONFIG_REG_READ		0xE7
+#define MS_SENSORS_HT_T_CONVERSION_START	0xF3
+#define MS_SENSORS_HT_H_CONVERSION_START	0xF5
+
+#define MS_SENSORS_TP_PROM_READ			0xA0
+#define MS_SENSORS_TP_T_CONVERSION_START	0x50
+#define MS_SENSORS_TP_P_CONVERSION_START	0x40
+#define MS_SENSORS_TP_ADC_READ			0x00
+
+#define MS_SENSORS_NO_READ_CMD			0xFF
+
+/**
+ * ms_sensors_i2c_reset() - i2c reset function
+ * @cli:	pointer to i2c client
+ * @cmd:	reset cmd. Depends on device in use
+ * @delay:	usleep minimal delay after reset command is issued
+ *
+ * Generic I2C reset function for Measurement Specialties devices.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int ms_sensors_i2c_reset(void *cli, u8 cmd, unsigned int delay)
+{
+	int ret;
+	struct i2c_client *client = cli;
+
+	ret = i2c_smbus_write_byte(client, cmd);
+	if (ret) {
+		dev_err(&client->dev, "Failed to reset device\n");
+		return ret;
+	}
+	usleep_range(delay, delay + 1000);
+
+	return 0;
+}
+EXPORT_SYMBOL(ms_sensors_i2c_reset);
+
+/**
+ * ms_sensors_i2c_read_prom_word() - i2c prom word read function
+ * @cli:	pointer to i2c client
+ * @cmd:	PROM read cmd. Depends on device and prom id
+ * @word:	pointer to word destination value
+ *
+ * Generic i2c prom word read function for Measurement Specialties devices.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word)
+{
+	int ret;
+	struct i2c_client *client = (struct i2c_client *)cli;
+
+	ret = i2c_smbus_read_word_swapped(client, cmd);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to read prom word\n");
+		return ret;
+	}
+	*word = ret;
+
+	return 0;
+}
+EXPORT_SYMBOL(ms_sensors_i2c_read_prom_word);
+
+/**
+ * ms_sensors_i2c_convert_and_read() - i2c ADC conversion & read function
+ * @cli:	pointer to i2c client
+ * @conv:	ADC conversion command. Depends on device in use
+ * @rd:		ADC read command. Depends on device in use
+ * @delay:	usleep minimal delay after conversion command is issued
+ * @adc:	pointer to ADC destination value
+ *
+ * Generic i2c ADC conversion & read function for Measurement Specialties
+ * devices.
+ * The function will issue conversion command, sleep appopriate delay, and
+ * issue command to read ADC.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
+				    unsigned int delay, u32 *adc)
+{
+	int ret;
+	u32 buf = 0;
+	struct i2c_client *client = (struct i2c_client *)cli;
+
+	/* Trigger conversion */
+	ret = i2c_smbus_write_byte(client, conv);
+	if (ret)
+		goto err;
+	usleep_range(delay, delay + 1000);
+
+	/* Retrieve ADC value */
+	if (rd != MS_SENSORS_NO_READ_CMD)
+		ret = i2c_smbus_read_i2c_block_data(client, rd, 3, (u8 *)&buf);
+	else
+		ret = i2c_master_recv(client, (u8 *)&buf, 3);
+	if (ret < 0)
+		goto err;
+
+	dev_dbg(&client->dev, "ADC raw value : %x\n", be32_to_cpu(buf) >> 8);
+	*adc = be32_to_cpu(buf) >> 8;
+
+	return 0;
+err:
+	dev_err(&client->dev, "Unable to make sensor adc conversion\n");
+	return ret;
+}
+EXPORT_SYMBOL(ms_sensors_i2c_convert_and_read);
+
+/**
+ * ms_sensors_crc_valid() - CRC check function
+ * @value:	input and CRC compare value
+ *
+ * Cyclic Redundancy Check function used in TSYS02D, HTU21, MS8607.
+ * This function performs a x^8 + x^5 + x^4 + 1 polynomial CRC.
+ * The argument contains CRC value in LSB byte while the bytes 1 and 2
+ * are used for CRC computation.
+ *
+ * Return: 1 if CRC is valid, 0 otherwise.
+ */
+static bool ms_sensors_crc_valid(u32 value)
+{
+	u32 polynom = 0x988000;	/* x^8 + x^5 + x^4 + 1 */
+	u32 msb = 0x800000;
+	u32 mask = 0xFF8000;
+	u32 result = value & 0xFFFF00;
+	u8 crc = value & 0xFF;
+
+	while (msb != 0x80) {
+		if (result & msb)
+			result = ((result ^ polynom) & mask)
+				| (result & ~mask);
+		msb >>= 1;
+		mask >>= 1;
+		polynom >>= 1;
+	}
+
+	return result == crc;
+}
+
+/**
+ * ms_sensors_i2c_read_serial() - i2c serial number read function
+ * @cli:	pointer to i2c client
+ * @sn:		pointer to 64-bits destination value
+ *
+ * Generic i2c serial number read function for Measurement Specialties devices.
+ * This function is used for TSYS02d, HTU21, MS8607 chipset.
+ * Refer to datasheet:
+ *	http://www.meas-spec.com/downloads/HTU2X_Serial_Number_Reading.pdf
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn)
+{
+	u8 i;
+	u64 rcv_buf = 0;
+	u16 send_buf;
+	int ret;
+
+	struct i2c_msg msg[2] = {
+		{
+		 .addr = client->addr,
+		 .flags = client->flags,
+		 .len = 2,
+		 .buf = (__u8 *)&send_buf,
+		 },
+		{
+		 .addr = client->addr,
+		 .flags = client->flags | I2C_M_RD,
+		 .buf = (__u8 *)&rcv_buf,
+		 },
+	};
+
+	/* Read MSB part of serial number */
+	send_buf = cpu_to_be16(MS_SENSORS_SERIAL_READ_MSB);
+	msg[1].len = 8;
+	ret = i2c_transfer(client->adapter, msg, 2);
+	if (ret < 0) {
+		dev_err(&client->dev, "Unable to read device serial number");
+		return ret;
+	}
+
+	rcv_buf = be64_to_cpu(rcv_buf);
+	dev_dbg(&client->dev, "Serial MSB raw : %llx\n", rcv_buf);
+
+	for (i = 0; i < 64; i += 16) {
+		if (!ms_sensors_crc_valid((rcv_buf >> i) & 0xFFFF))
+			return -ENODEV;
+	}
+
+	*sn = (((rcv_buf >> 32) & 0xFF000000) |
+	       ((rcv_buf >> 24) & 0x00FF0000) |
+	       ((rcv_buf >> 16) & 0x0000FF00) |
+	       ((rcv_buf >> 8) & 0x000000FF)) << 16;
+
+	/* Read LSB part of serial number */
+	send_buf = cpu_to_be16(MS_SENSORS_SERIAL_READ_LSB);
+	msg[1].len = 6;
+	rcv_buf = 0;
+	ret = i2c_transfer(client->adapter, msg, 2);
+	if (ret < 0) {
+		dev_err(&client->dev, "Unable to read device serial number");
+		return ret;
+	}
+
+	rcv_buf = be64_to_cpu(rcv_buf) >> 16;
+	dev_dbg(&client->dev, "Serial MSB raw : %llx\n", rcv_buf);
+
+	for (i = 0; i < 48; i += 24) {
+		if (!ms_sensors_crc_valid((rcv_buf >> i) & 0xFFFFFF))
+			return -ENODEV;
+	}
+
+	*sn |= (rcv_buf & 0xFFFF00) << 40 | (rcv_buf >> 32);
+
+	return 0;
+}
+EXPORT_SYMBOL(ms_sensors_i2c_read_serial);
+
+static int ms_sensors_i2c_read_config_reg(struct i2c_client *client,
+					  u8 *config_reg)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte(client, MS_SENSORS_CONFIG_REG_READ);
+	if (ret) {
+		dev_err(&client->dev, "Unable to read config register");
+		return ret;
+	}
+
+	ret = i2c_master_recv(client, config_reg, 1);
+	if (ret < 0) {
+		dev_err(&client->dev, "Unable to read config register");
+		return ret;
+	}
+	dev_dbg(&client->dev, "Config register :%x\n", *config_reg);
+
+	return 0;
+}
+
+/**
+ * ms_sensors_i2c_write_resolution() - set resolution i2c function
+ * @dev_data:	pointer to temperature/humidity device data
+ * @i:		resolution index to set
+ *
+ * This function will program the appropriate resolution based on the index
+ * provided when user space will set samp_freq channel.
+ * This function is used for TSYS02D, HTU21 and MS8607 chipsets.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+ssize_t ms_sensors_i2c_write_resolution(struct ms_ht_dev *dev_data,
+					u8 i)
+{
+	u8 config_reg;
+	int ret;
+
+	ret = ms_sensors_i2c_read_config_reg(dev_data->client, &config_reg);
+	if (ret)
+		return ret;
+
+	config_reg &= 0x7E;
+	config_reg |= ((i & 1) << 7) + ((i & 2) >> 1);
+
+	return i2c_smbus_write_byte_data(dev_data->client,
+					 MS_SENSORS_CONFIG_REG_WRITE,
+					 config_reg);
+}
+EXPORT_SYMBOL(ms_sensors_i2c_write_resolution);
+
+/**
+ * ms_sensors_i2c_show_battery_low() - show device battery low indicator
+ * @dev_data:	pointer to temperature/humidity device data
+ * @buf:	pointer to char buffer to write result
+ *
+ * This function will read battery indicator value in the device and
+ * return 1 if the device voltage is below 2.25V.
+ * This function is used for TSYS02D, HTU21 and MS8607 chipsets.
+ *
+ * Return: length of sprintf on success, negative errno otherwise.
+ */
+ssize_t ms_sensors_i2c_show_battery_low(struct ms_ht_dev *dev_data,
+					char *buf)
+{
+	int ret;
+	u8 config_reg;
+
+	mutex_lock(&dev_data->lock);
+	ret = ms_sensors_i2c_read_config_reg(dev_data->client, &config_reg);
+	mutex_unlock(&dev_data->lock);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", (config_reg & 0x40) >> 6);
+}
+EXPORT_SYMBOL(ms_sensors_i2c_show_battery_low);
+
+/**
+ * ms_sensors_i2c_show_heater() - show device heater
+ * @dev_data:	pointer to temperature/humidity device data
+ * @buf:	pointer to char buffer to write result
+ *
+ * This function will read heater enable value in the device and
+ * return 1 if the heater is enabled.
+ * This function is used for HTU21 and MS8607 chipsets.
+ *
+ * Return: length of sprintf on success, negative errno otherwise.
+ */
+ssize_t ms_sensors_i2c_show_heater(struct ms_ht_dev *dev_data,
+				   char *buf)
+{
+	u8 config_reg;
+	int ret;
+
+	mutex_lock(&dev_data->lock);
+	ret = ms_sensors_i2c_read_config_reg(dev_data->client, &config_reg);
+	mutex_unlock(&dev_data->lock);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "%d\n", (config_reg & 0x4) >> 2);
+}
+EXPORT_SYMBOL(ms_sensors_i2c_show_heater);
+
+/**
+ * ms_sensors_i2c_write_heater() - write device heater
+ * @dev_data:	pointer to temperature/humidity device data
+ * @buf:	pointer to char buffer from user space
+ * @len:	length of buf
+ *
+ * This function will write 1 or 0 value in the device
+ * to enable or disable heater.
+ * This function is used for HTU21 and MS8607 chipsets.
+ *
+ * Return: length of buffer, negative errno otherwise.
+ */
+ssize_t ms_sensors_i2c_write_heater(struct ms_ht_dev *dev_data,
+				    const char *buf, size_t len)
+{
+	u8 val, config_reg;
+	int ret;
+
+	ret = kstrtou8(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	if (val > 1)
+		return -EINVAL;
+
+	mutex_lock(&dev_data->lock);
+	ret = ms_sensors_i2c_read_config_reg(dev_data->client, &config_reg);
+	if (ret) {
+		mutex_unlock(&dev_data->lock);
+		return ret;
+	}
+
+	config_reg &= 0xFB;
+	config_reg |= val << 2;
+
+	ret = i2c_smbus_write_byte_data(dev_data->client,
+					MS_SENSORS_CONFIG_REG_WRITE,
+					config_reg);
+	mutex_unlock(&dev_data->lock);
+	if (ret) {
+		dev_err(&dev_data->client->dev, "Unable to write config register\n");
+		return ret;
+	}
+
+	return len;
+}
+EXPORT_SYMBOL(ms_sensors_i2c_write_heater);
+
+/**
+ * ms_sensors_i2c_ht_read_temperature() - i2c read temperature
+ * @dev_data:	pointer to temperature/humidity device data
+ * @temperature:pointer to temperature destination value
+ *
+ * This function will get temperature ADC value from the device,
+ * check the CRC and compute the temperature value.
+ * This function is used for TSYS02D, HTU21 and MS8607 chipsets.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int ms_sensors_i2c_ht_read_temperature(struct ms_ht_dev *dev_data,
+				       s32 *temperature)
+{
+	int ret;
+	u32 adc;
+	u16 delay;
+
+	mutex_lock(&dev_data->lock);
+	delay = ms_sensors_ht_t_conversion_time[dev_data->res_index];
+	ret = ms_sensors_i2c_convert_and_read(dev_data->client,
+					      MS_SENSORS_HT_T_CONVERSION_START,
+					      MS_SENSORS_NO_READ_CMD,
+					      delay, &adc);
+	mutex_unlock(&dev_data->lock);
+	if (ret)
+		return ret;
+
+	if (!ms_sensors_crc_valid(adc)) {
+		dev_err(&dev_data->client->dev,
+			"Temperature read crc check error\n");
+		return -ENODEV;
+	}
+
+	/* Temperature algorithm */
+	*temperature = (((s64)(adc >> 8) * 175720) >> 16) - 46850;
+
+	return 0;
+}
+EXPORT_SYMBOL(ms_sensors_i2c_ht_read_temperature);
+
+/**
+ * ms_sensors_i2c_ht_read_humidity() - i2c read humidity
+ * @dev_data:	pointer to temperature/humidity device data
+ * @humidity:	pointer to humidity destination value
+ *
+ * This function will get humidity ADC value from the device,
+ * check the CRC and compute the temperature value.
+ * This function is used for HTU21 and MS8607 chipsets.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int ms_sensors_i2c_ht_read_humidity(struct ms_ht_dev *dev_data,
+				    u32 *humidity)
+{
+	int ret;
+	u32 adc;
+	u16 delay;
+
+	mutex_lock(&dev_data->lock);
+	delay = ms_sensors_ht_h_conversion_time[dev_data->res_index];
+	ret = ms_sensors_i2c_convert_and_read(dev_data->client,
+					      MS_SENSORS_HT_H_CONVERSION_START,
+					      MS_SENSORS_NO_READ_CMD,
+					      delay, &adc);
+	mutex_unlock(&dev_data->lock);
+	if (ret)
+		return ret;
+
+	if (!ms_sensors_crc_valid(adc)) {
+		dev_err(&dev_data->client->dev,
+			"Humidity read crc check error\n");
+		return -ENODEV;
+	}
+
+	/* Humidity algorithm */
+	*humidity = (((s32)(adc >> 8) * 12500) >> 16) * 10 - 6000;
+	if (*humidity >= 100000)
+		*humidity = 100000;
+
+	return 0;
+}
+EXPORT_SYMBOL(ms_sensors_i2c_ht_read_humidity);
+
+/**
+ * ms_sensors_tp_crc_valid() - CRC check function for
+ *     Temperature and pressure devices.
+ *     This function is only used when reading PROM coefficients
+ *
+ * @prom:	pointer to PROM coefficients array
+ * @len:	length of PROM coefficients array
+ *
+ * Return: True if CRC is ok.
+ */
+static bool ms_sensors_tp_crc_valid(u16 *prom, u8 len)
+{
+	unsigned int cnt, n_bit;
+	u16 n_rem = 0x0000, crc_read = prom[0], crc = (*prom & 0xF000) >> 12;
+
+	prom[len - 1] = 0;
+	prom[0] &= 0x0FFF;      /* Clear the CRC computation part */
+
+	for (cnt = 0; cnt < len * 2; cnt++) {
+		if (cnt % 2 == 1)
+			n_rem ^= prom[cnt >> 1] & 0x00FF;
+		else
+			n_rem ^= prom[cnt >> 1] >> 8;
+
+		for (n_bit = 8; n_bit > 0; n_bit--) {
+			if (n_rem & 0x8000)
+				n_rem = (n_rem << 1) ^ 0x3000;
+			else
+				n_rem <<= 1;
+		}
+	}
+	n_rem >>= 12;
+	prom[0] = crc_read;
+
+	return n_rem == crc;
+}
+
+/**
+ * ms_sensors_tp_read_prom() - prom coeff read function
+ * @dev_data:	pointer to temperature/pressure device data
+ *
+ * This function will read prom coefficients and check CRC.
+ * This function is used for MS5637 and MS8607 chipsets.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int ms_sensors_tp_read_prom(struct ms_tp_dev *dev_data)
+{
+	int i, ret;
+
+	for (i = 0; i < MS_SENSORS_TP_PROM_WORDS_NB; i++) {
+		ret = ms_sensors_i2c_read_prom_word(
+					dev_data->client,
+					MS_SENSORS_TP_PROM_READ + (i << 1),
+					&dev_data->prom[i]);
+
+		if (ret)
+			return ret;
+	}
+
+	if (!ms_sensors_tp_crc_valid(dev_data->prom,
+				     MS_SENSORS_TP_PROM_WORDS_NB + 1)) {
+		dev_err(&dev_data->client->dev,
+			"Calibration coefficients crc check error\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(ms_sensors_tp_read_prom);
+
+/**
+ * ms_sensors_read_temp_and_pressure() - read temp and pressure
+ * @dev_data:	pointer to temperature/pressure device data
+ * @temperature:pointer to temperature destination value
+ * @pressure:	pointer to pressure destination value
+ *
+ * This function will read ADC and compute pressure and temperature value.
+ * This function is used for MS5637 and MS8607 chipsets.
+ *
+ * Return: 0 on success, negative errno otherwise.
+ */
+int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
+				      int *temperature,
+				      unsigned int *pressure)
+{
+	int ret;
+	u32 t_adc, p_adc;
+	s32 dt, temp;
+	s64 off, sens, t2, off2, sens2;
+	u16 *prom = dev_data->prom, delay;
+	u8 i;
+
+	mutex_lock(&dev_data->lock);
+	i = dev_data->res_index * 2;
+	delay = ms_sensors_tp_conversion_time[dev_data->res_index];
+
+	ret = ms_sensors_i2c_convert_and_read(
+					dev_data->client,
+					MS_SENSORS_TP_T_CONVERSION_START + i,
+					MS_SENSORS_TP_ADC_READ,
+					delay, &t_adc);
+	if (ret) {
+		mutex_unlock(&dev_data->lock);
+		return ret;
+	}
+
+	ret = ms_sensors_i2c_convert_and_read(
+					dev_data->client,
+					MS_SENSORS_TP_P_CONVERSION_START + i,
+					MS_SENSORS_TP_ADC_READ,
+					delay, &p_adc);
+	mutex_unlock(&dev_data->lock);
+	if (ret)
+		return ret;
+
+	dt = (s32)t_adc - (prom[5] << 8);
+
+	/* Actual temperature = 2000 + dT * TEMPSENS */
+	temp = 2000 + (((s64)dt * prom[6]) >> 23);
+
+	/* Second order temperature compensation */
+	if (temp < 2000) {
+		s64 tmp = (s64)temp - 2000;
+
+		t2 = (3 * ((s64)dt * (s64)dt)) >> 33;
+		off2 = (61 * tmp * tmp) >> 4;
+		sens2 = (29 * tmp * tmp) >> 4;
+
+		if (temp < -1500) {
+			s64 tmp = (s64)temp + 1500;
+
+			off2 += 17 * tmp * tmp;
+			sens2 += 9 * tmp * tmp;
+		}
+	} else {
+		t2 = (5 * ((s64)dt * (s64)dt)) >> 38;
+		off2 = 0;
+		sens2 = 0;
+	}
+
+	/* OFF = OFF_T1 + TCO * dT */
+	off = (((s64)prom[2]) << 17) + ((((s64)prom[4]) * (s64)dt) >> 6);
+	off -= off2;
+
+	/* Sensitivity at actual temperature = SENS_T1 + TCS * dT */
+	sens = (((s64)prom[1]) << 16) + (((s64)prom[3] * dt) >> 7);
+	sens -= sens2;
+
+	/* Temperature compensated pressure = D1 * SENS - OFF */
+	*temperature = (temp - t2) * 10;
+	*pressure = (u32)(((((s64)p_adc * sens) >> 21) - off) >> 15);
+
+	return 0;
+}
+EXPORT_SYMBOL(ms_sensors_read_temp_and_pressure);
+
+MODULE_DESCRIPTION("Measurement-Specialties common i2c driver");
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
+MODULE_LICENSE("GPL v2");
+
diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.h b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
new file mode 100644
index 0000000..918b8af
--- /dev/null
+++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
@@ -0,0 +1,53 @@
+/*
+ * Measurements Specialties common sensor driver
+ *
+ * Copyright (c) 2015 Measurement-Specialties
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _MS_SENSORS_I2C_H
+#define _MS_SENSORS_I2C_H
+
+#include <linux/i2c.h>
+#include <linux/mutex.h>
+
+#define MS_SENSORS_TP_PROM_WORDS_NB		7
+
+struct ms_ht_dev {
+	struct i2c_client *client;
+	struct mutex lock; /* mutex protecting data structure */
+	u8 res_index;
+};
+
+struct ms_tp_dev {
+	struct i2c_client *client;
+	struct mutex lock; /* mutex protecting data structure */
+	/* Added element for CRC computation */
+	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB + 1];
+	u8 res_index;
+};
+
+int ms_sensors_i2c_reset(void *cli, u8 cmd, unsigned int delay);
+int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word);
+int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
+				    unsigned int delay, u32 *adc);
+int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn);
+ssize_t ms_sensors_i2c_show_serial(struct ms_ht_dev *dev_data, char *buf);
+ssize_t ms_sensors_i2c_write_resolution(struct ms_ht_dev *dev_data, u8 i);
+ssize_t ms_sensors_i2c_show_battery_low(struct ms_ht_dev *dev_data, char *buf);
+ssize_t ms_sensors_i2c_show_heater(struct ms_ht_dev *dev_data, char *buf);
+ssize_t ms_sensors_i2c_write_heater(struct ms_ht_dev *dev_data,
+				    const char *buf, size_t len);
+int ms_sensors_i2c_ht_read_temperature(struct ms_ht_dev *dev_data,
+				       s32 *temperature);
+int ms_sensors_i2c_ht_read_humidity(struct ms_ht_dev *dev_data,
+				    u32 *humidity);
+int ms_sensors_tp_read_prom(struct ms_tp_dev *dev_data);
+int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
+				      int *temperature,
+				      unsigned int *pressure);
+
+#endif /* _MS_SENSORS_I2C_H */
-- 
2.3.7


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 2/6] Add tsys01 meas-spec driver support
  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-25 13:56 ` Ludovic Tancerel
  2015-09-27 16:55   ` Jonathan Cameron
  2015-09-25 13:56 ` [PATCH v3 3/6] Add tsys02d " Ludovic Tancerel
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Ludovic Tancerel @ 2015-09-25 13:56 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, ludovic.tancerel,
	William.Markezana

Support for TSYS01 temperature sensor

Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
---
 drivers/iio/temperature/Kconfig  |  11 ++
 drivers/iio/temperature/Makefile |   1 +
 drivers/iio/temperature/tsys01.c | 231 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 243 insertions(+)
 create mode 100644 drivers/iio/temperature/tsys01.c

diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 21feaa4..35712032 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -23,4 +23,15 @@ config TMP006
 	  This driver can also be built as a module. If so, the module will
 	  be called tmp006.
 
+config TSYS01
+	tristate "Measurement Specialties TSYS01 temperature sensor using I2C bus connection"
+	depends on I2C
+	select IIO_MS_SENSORS_I2C
+	help
+	  If you say yes here you get support for the Measurement Specialties
+	  TSYS01 I2C temperature sensor.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called tsys01.
+
 endmenu
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 40710a8..368a2a2 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_MLX90614) += mlx90614.o
 obj-$(CONFIG_TMP006) += tmp006.o
+obj-$(CONFIG_TSYS01) += tsys01.o
diff --git a/drivers/iio/temperature/tsys01.c b/drivers/iio/temperature/tsys01.c
new file mode 100644
index 0000000..5c4127f
--- /dev/null
+++ b/drivers/iio/temperature/tsys01.c
@@ -0,0 +1,231 @@
+/*
+ * tsys01.c - Support for Measurement-Specialties tsys01 temperature sensor
+ *
+ * Copyright (c) 2015 Measurement-Specialties
+ *
+ * Licensed under the GPL-2.
+ *
+ * Datasheet:
+ *  http://www.meas-spec.com/downloads/TSYS01_Digital_Temperature_Sensor.pdf
+ *
+ */
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/stat.h>
+#include "../common/ms_sensors/ms_sensors_i2c.h"
+
+/* TSYS01 Commands */
+#define TSYS01_RESET				0x1E
+#define TSYS01_CONVERSION_START			0x48
+#define TSYS01_ADC_READ				0x00
+#define TSYS01_PROM_READ			0xA0
+
+#define TSYS01_PROM_WORDS_NB			8
+
+struct tsys01_dev {
+	void *client;
+	struct mutex lock; /* lock during conversion */
+
+	int (*reset)(void *cli, u8 cmd, unsigned int delay);
+	int (*convert_and_read)(void *cli, u8 conv, u8 rd,
+				unsigned int delay, u32 *adc);
+	int (*read_prom_word)(void *cli, int cmd, u16 *word);
+
+	u16 prom[TSYS01_PROM_WORDS_NB];
+};
+
+/* Multiplication coefficients for temperature computation */
+static const int coeff_mul[] = { -1500000, 1000000, -2000000,
+				 4000000, -2000000 };
+
+static int tsys01_read_temperature(struct iio_dev *indio_dev,
+				   s32 *temperature)
+{
+	int ret, i;
+	u32 adc;
+	s64 temp = 0;
+	struct tsys01_dev *dev_data = iio_priv(indio_dev);
+
+	mutex_lock(&dev_data->lock);
+	ret = dev_data->convert_and_read(dev_data->client,
+					 TSYS01_CONVERSION_START,
+					 TSYS01_ADC_READ, 9000, &adc);
+	mutex_unlock(&dev_data->lock);
+	if (ret)
+		return ret;
+
+	adc >>= 8;
+
+	/* Temperature algorithm */
+	for (i = 4; i > 0; i--) {
+		temp += coeff_mul[i] *
+			(s64)dev_data->prom[5 - i];
+		temp *= (s64)adc;
+		temp = div64_s64(temp, 100000);
+	}
+	temp *= 10;
+	temp += coeff_mul[0] * (s64)dev_data->prom[5];
+	temp = div64_s64(temp, 100000);
+
+	*temperature = temp;
+
+	return 0;
+}
+
+static int tsys01_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *channel, int *val,
+			   int *val2, long mask)
+{
+	int ret;
+	s32 temperature;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
+		switch (channel->type) {
+		case IIO_TEMP:	/* in milli °C */
+			ret = tsys01_read_temperature(indio_dev, &temperature);
+			if (ret)
+				return ret;
+			*val = temperature;
+
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_chan_spec tsys01_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
+	}
+};
+
+static const struct iio_info tsys01_info = {
+	.read_raw = tsys01_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static bool tsys01_crc_valid(u16 *n_prom)
+{
+	u8 cnt;
+	u8 sum = 0;
+
+	for (cnt = 0; cnt < TSYS01_PROM_WORDS_NB; cnt++)
+		sum += ((n_prom[0] >> 8) + (n_prom[0] & 0xFF));
+
+	return (sum == 0);
+}
+
+static int tsys01_read_prom(struct iio_dev *indio_dev)
+{
+	int i, ret;
+	struct tsys01_dev *dev_data = iio_priv(indio_dev);
+	char buf[7 * TSYS01_PROM_WORDS_NB + 1];
+	char *ptr = buf;
+
+	for (i = 0; i < TSYS01_PROM_WORDS_NB; i++) {
+		ret = dev_data->read_prom_word(dev_data->client,
+					       TSYS01_PROM_READ + (i << 1),
+					       &dev_data->prom[i]);
+		if (ret)
+			return ret;
+
+		ret = sprintf(ptr, "0x%04x ", dev_data->prom[i]);
+		ptr += ret;
+	}
+
+	if (!tsys01_crc_valid(dev_data->prom)) {
+		dev_err(&indio_dev->dev, "prom crc check error\n");
+		return -ENODEV;
+	}
+	*ptr = 0;
+	dev_info(&indio_dev->dev, "PROM coefficients : %s\n", buf);
+
+	return 0;
+}
+
+static int tsys01_probe(struct iio_dev *indio_dev, struct device *dev)
+{
+	int ret;
+	struct tsys01_dev *dev_data = iio_priv(indio_dev);
+
+	mutex_init(&dev_data->lock);
+
+	indio_dev->info = &tsys01_info;
+	indio_dev->name = dev->driver->name;
+	indio_dev->dev.parent = dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = tsys01_channels;
+	indio_dev->num_channels = ARRAY_SIZE(tsys01_channels);
+
+	ret = dev_data->reset(dev_data->client, TSYS01_RESET, 3000);
+	if (ret)
+		return ret;
+
+	ret = tsys01_read_prom(indio_dev);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static int tsys01_i2c_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	struct tsys01_dev *dev_data;
+	struct iio_dev *indio_dev;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_WORD_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->reset = ms_sensors_i2c_reset;
+	dev_data->read_prom_word = ms_sensors_i2c_read_prom_word;
+	dev_data->convert_and_read = ms_sensors_i2c_convert_and_read;
+
+	i2c_set_clientdata(client, indio_dev);
+
+	return tsys01_probe(indio_dev, &client->dev);
+}
+
+static const struct i2c_device_id tsys01_id[] = {
+	{"tsys01", 0},
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, tsys01_id);
+
+static struct i2c_driver tsys01_driver = {
+	.probe = tsys01_i2c_probe,
+	.id_table = tsys01_id,
+	.driver = {
+		   .name = "tsys01",
+		   },
+};
+
+module_i2c_driver(tsys01_driver);
+
+MODULE_DESCRIPTION("Measurement-Specialties tsys01 temperature driver");
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.3.7


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 3/6] Add tsys02d meas-spec driver support
  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-25 13:56 ` [PATCH v3 2/6] Add tsys01 meas-spec driver support Ludovic Tancerel
@ 2015-09-25 13:56 ` Ludovic Tancerel
  2015-09-27 17:51   ` Jonathan Cameron
  2015-09-25 13:56 ` [PATCH v3 4/6] Add htu21 " Ludovic Tancerel
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Ludovic Tancerel @ 2015-09-25 13:56 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, ludovic.tancerel,
	William.Markezana

Support for TSYS02D temperature sensor

Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
---
 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
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");
-- 
2.3.7


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 4/6] Add htu21 meas-spec driver support
  2015-09-25 13:56 [PATCH v3 0/6] iio: TSYS01, TSYS02D, HTU21, MS5637, MS8607, Measurement Specialties driver developments Ludovic Tancerel
                   ` (2 preceding siblings ...)
  2015-09-25 13:56 ` [PATCH v3 3/6] Add tsys02d " Ludovic Tancerel
@ 2015-09-25 13:56 ` Ludovic Tancerel
  2015-09-27 17:54   ` Jonathan Cameron
  2015-09-25 13:56 ` [PATCH v3 5/6] Add ms5637 " Ludovic Tancerel
  2015-09-25 13:56 ` [PATCH v3 6/6] Add ms8607 " Ludovic Tancerel
  5 siblings, 1 reply; 22+ messages in thread
From: Ludovic Tancerel @ 2015-09-25 13:56 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, ludovic.tancerel,
	William.Markezana

Support for HTU21 temperature & humidity sensor

Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
---
 Documentation/ABI/testing/sysfs-bus-iio |  10 ++
 drivers/iio/humidity/Kconfig            |  11 ++
 drivers/iio/humidity/Makefile           |   1 +
 drivers/iio/humidity/htu21.c            | 227 ++++++++++++++++++++++++++++++++
 4 files changed, 249 insertions(+)
 create mode 100644 drivers/iio/humidity/htu21.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 70c9b1a..aac134a 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -1461,3 +1461,13 @@ Description:
 		measurements and return the average value as output data. Each
 		value resulted from <type>[_name]_oversampling_ratio measurements
 		is considered as one sample for <type>[_name]_sampling_frequency.
+
+What:           /sys/bus/iio/devices/iio:deviceX/heater_enable
+KernelVersion:  4.1.0
+Contact:        linux-iio@vger.kernel.org
+Description:
+                A '1' (enable) or '0' (disable) specifying the enable
+		of heater function. Same reading values apply
+		This ABI is especially applicable for humidity sensors
+		to heatup the device and get rid of any condensation
+		in some humidity environment
diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
index 688c0d1..3fab60c 100644
--- a/drivers/iio/humidity/Kconfig
+++ b/drivers/iio/humidity/Kconfig
@@ -12,6 +12,17 @@ config DHT11
 	  Other sensors should work as well as long as they speak the
 	  same protocol.
 
+config HTU21
+	tristate "Measurement Specialties HTU21 humidity & temperature sensor"
+	depends on I2C
+        select IIO_MS_SENSORS_I2C
+	help
+	  If you say yes here you get support for the Measurement Specialties
+	  HTU21 humidity and temperature sensor.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called htu21.
+
 config SI7005
 	tristate "SI7005 relative humidity and temperature sensor"
 	depends on I2C
diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
index 86e2d26..826b1d5 100644
--- a/drivers/iio/humidity/Makefile
+++ b/drivers/iio/humidity/Makefile
@@ -3,5 +3,6 @@
 #
 
 obj-$(CONFIG_DHT11) += dht11.o
+obj-$(CONFIG_HTU21) += htu21.o
 obj-$(CONFIG_SI7005) += si7005.o
 obj-$(CONFIG_SI7020) += si7020.o
diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
new file mode 100644
index 0000000..706faff
--- /dev/null
+++ b/drivers/iio/humidity/htu21.c
@@ -0,0 +1,227 @@
+/*
+ * htu21.c - Support for Measurement-Specialties
+ *           htu21 temperature & humidity sensor
+ *
+ * Copyright (c) 2014 Measurement-Specialties
+ *
+ * Licensed under the GPL-2.
+ *
+ * (7-bit I2C slave address 0x40)
+ *
+ * Datasheet:
+ *  http://www.meas-spec.com/downloads/HTU21D.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 HTU21_RESET				0xFE
+
+static const int htu21_samp_freq[4] = { 20, 40, 70, 120 };
+/* String copy of the above const for readability purpose */
+static const char htu21_show_samp_freq[] = "20 40 70 120";
+
+static int htu21_read_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *channel, int *val,
+			  int *val2, long mask)
+{
+	int ret, temperature;
+	unsigned int humidity;
+	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;
+		case IIO_HUMIDITYRELATIVE:	/* in milli %RH */
+			ret = ms_sensors_i2c_ht_read_humidity(dev_data,
+							      &humidity);
+			if (ret)
+				return ret;
+			*val = humidity;
+
+			return IIO_VAL_INT;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = htu21_samp_freq[dev_data->res_index];
+
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int htu21_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(htu21_samp_freq);
+		while (i-- > 0)
+			if (val == htu21_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 htu21_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),
+	 },
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	 }
+};
+
+static ssize_t htu21_show_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 ssize_t htu21_show_heater(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_heater(dev_data, buf);
+}
+
+static ssize_t htu21_write_heater(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct ms_ht_dev *dev_data = iio_priv(indio_dev);
+
+	return ms_sensors_i2c_write_heater(dev_data, buf, len);
+}
+
+static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(htu21_show_samp_freq);
+static IIO_DEVICE_ATTR(battery_low, S_IRUGO,
+		       htu21_show_battery_low, NULL, 0);
+static IIO_DEVICE_ATTR(heater_enable, S_IRUGO | S_IWUSR,
+		       htu21_show_heater, htu21_write_heater, 0);
+
+static struct attribute *htu21_attributes[] = {
+	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_dev_attr_battery_low.dev_attr.attr,
+	&iio_dev_attr_heater_enable.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group htu21_attribute_group = {
+	.attrs = htu21_attributes,
+};
+
+static const struct iio_info htu21_info = {
+	.read_raw = htu21_read_raw,
+	.write_raw = htu21_write_raw,
+	.attrs = &htu21_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+int htu21_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 = &htu21_info;
+	indio_dev->name = id->name;
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = htu21_channels;
+	indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
+
+	i2c_set_clientdata(client, indio_dev);
+
+	ret = ms_sensors_i2c_reset(client, HTU21_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 htu21_id[] = {
+	{"htu21", 0},
+	{}
+};
+
+static struct i2c_driver htu21_driver = {
+	.probe = htu21_probe,
+	.id_table = htu21_id,
+	.driver = {
+		   .name = "htu21",
+		   },
+};
+
+module_i2c_driver(htu21_driver);
+
+MODULE_DESCRIPTION("Measurement-Specialties htu21 temperature and humidity driver");
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.3.7


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 5/6] Add ms5637 meas-spec driver support
  2015-09-25 13:56 [PATCH v3 0/6] iio: TSYS01, TSYS02D, HTU21, MS5637, MS8607, Measurement Specialties driver developments Ludovic Tancerel
                   ` (3 preceding siblings ...)
  2015-09-25 13:56 ` [PATCH v3 4/6] Add htu21 " Ludovic Tancerel
@ 2015-09-25 13:56 ` Ludovic Tancerel
  2015-09-27 17:57   ` Jonathan Cameron
  2015-09-25 13:56 ` [PATCH v3 6/6] Add ms8607 " Ludovic Tancerel
  5 siblings, 1 reply; 22+ messages in thread
From: Ludovic Tancerel @ 2015-09-25 13:56 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, ludovic.tancerel,
	William.Markezana

Support for MS5637 temperature & pressure sensor

Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
---
 drivers/iio/pressure/Kconfig  |  15 +++-
 drivers/iio/pressure/Makefile |   1 +
 drivers/iio/pressure/ms5637.c | 187 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iio/pressure/ms5637.c

diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index fa62950..8142cfe 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -53,9 +53,9 @@ config MPL3115
           will be called mpl3115.
 
 config MS5611
-	tristate "Measurement Specialities MS5611 pressure sensor driver"
+	tristate "Measurement Specialties MS5611 pressure sensor driver"
 	help
-	  Say Y here to build support for the Measurement Specialities
+	  Say Y here to build support for the Measurement Specialties
 	  MS5611 pressure and temperature sensor.
 
 	  To compile this driver as a module, choose M here: the module will
@@ -79,6 +79,17 @@ config MS5611_SPI
 	  To compile this driver as a module, choose M here: the module will
 	  be called ms5611_spi.
 
+config MS5637
+	tristate "Measurement Specialties MS5637 pressure & temperature sensor"
+	depends on I2C
+        select IIO_MS_SENSORS_I2C
+	help
+	  If you say yes here you get support for the Measurement Specialties
+	  MS5637 pressure and temperature sensor.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called ms5637.
+
 config IIO_ST_PRESS
 	tristate "STMicroelectronics pressure sensor Driver"
 	depends on (I2C || SPI_MASTER) && SYSFS
diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
index a4f98f8..46571c96 100644
--- a/drivers/iio/pressure/Makefile
+++ b/drivers/iio/pressure/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_MPL3115) += mpl3115.o
 obj-$(CONFIG_MS5611) += ms5611_core.o
 obj-$(CONFIG_MS5611_I2C) += ms5611_i2c.o
 obj-$(CONFIG_MS5611_SPI) += ms5611_spi.o
+obj-$(CONFIG_MS5637) += ms5637.o
 obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o
 st_pressure-y := st_pressure_core.o
 st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o
diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
new file mode 100644
index 0000000..0dbbd4e
--- /dev/null
+++ b/drivers/iio/pressure/ms5637.c
@@ -0,0 +1,187 @@
+/*
+ * ms5637.c - Support for Measurement-Specialties ms5637
+ *            pressure & temperature sensor
+ *
+ * Copyright (c) 2015 Measurement-Specialties
+ *
+ * Licensed under the GPL-2.
+ *
+ * (7-bit I2C slave address 0x76)
+ *
+ * Datasheet:
+ *  http://www.meas-spec.com/downloads/MS5637-02BA03.pdf
+ *
+ */
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/stat.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/mutex.h>
+
+#include "../common/ms_sensors/ms_sensors_i2c.h"
+
+static const int ms5637_samp_freq[6] = { 960, 480, 240, 120, 60, 30 };
+/* String copy of the above const for readability purpose */
+static const char ms5637_show_samp_freq[] = "960 480 240 120 60 30";
+
+static int ms5637_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *channel, int *val,
+			   int *val2, long mask)
+{
+	int ret;
+	int temperature;
+	unsigned int pressure;
+	struct ms_tp_dev *dev_data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
+		ret = ms_sensors_read_temp_and_pressure(dev_data,
+							&temperature,
+							&pressure);
+		if (ret)
+			return ret;
+
+		switch (channel->type) {
+		case IIO_TEMP:	/* in milli °C */
+			*val = temperature;
+
+			return IIO_VAL_INT;
+		case IIO_PRESSURE:	/* in kPa */
+			*val = pressure / 1000;
+			*val2 = (pressure % 1000) * 1000;
+
+			return IIO_VAL_INT_PLUS_MICRO;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = ms5637_samp_freq[dev_data->res_index];
+
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int ms5637_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct ms_tp_dev *dev_data = iio_priv(indio_dev);
+	int i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		i = ARRAY_SIZE(ms5637_samp_freq);
+		while (i-- > 0)
+			if (val == ms5637_samp_freq[i])
+				break;
+		if (i < 0)
+			return -EINVAL;
+		dev_data->res_index = i;
+
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_chan_spec ms5637_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	},
+	{
+		.type = IIO_PRESSURE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	}
+};
+
+static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(ms5637_show_samp_freq);
+
+static struct attribute *ms5637_attributes[] = {
+	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	NULL,
+};
+
+static const struct attribute_group ms5637_attribute_group = {
+	.attrs = ms5637_attributes,
+};
+
+static const struct iio_info ms5637_info = {
+	.read_raw = ms5637_read_raw,
+	.write_raw = ms5637_write_raw,
+	.attrs = &ms5637_attribute_group,
+	.driver_module = THIS_MODULE,
+};
+
+static int ms5637_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct ms_tp_dev *dev_data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_WORD_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 = 5;
+	mutex_init(&dev_data->lock);
+
+	indio_dev->info = &ms5637_info;
+	indio_dev->name = id->name;
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = ms5637_channels;
+	indio_dev->num_channels = ARRAY_SIZE(ms5637_channels);
+
+	i2c_set_clientdata(client, indio_dev);
+
+	ret = ms_sensors_i2c_reset(client, 0x1E, 3000);
+	if (ret)
+		return ret;
+
+	ret = ms_sensors_tp_read_prom(dev_data);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id ms5637_id[] = {
+	{"ms5637", 0},
+	{}
+};
+
+static struct i2c_driver ms5637_driver = {
+	.probe = ms5637_probe,
+	.id_table = ms5637_id,
+	.driver = {
+		   .name = "ms5637"
+		   },
+};
+
+module_i2c_driver(ms5637_driver);
+
+MODULE_DESCRIPTION("Measurement-Specialties ms5637 temperature & pressure driver");
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.3.7


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 6/6] Add ms8607 meas-spec driver support
  2015-09-25 13:56 [PATCH v3 0/6] iio: TSYS01, TSYS02D, HTU21, MS5637, MS8607, Measurement Specialties driver developments Ludovic Tancerel
                   ` (4 preceding siblings ...)
  2015-09-25 13:56 ` [PATCH v3 5/6] Add ms5637 " Ludovic Tancerel
@ 2015-09-25 13:56 ` Ludovic Tancerel
  2015-09-27 18:00   ` Jonathan Cameron
  5 siblings, 1 reply; 22+ messages in thread
From: Ludovic Tancerel @ 2015-09-25 13:56 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, linux-iio, ludovic.tancerel,
	William.Markezana

Support for MS8607 temperature, pressure & humidity sensor.
This part is using functions from MS5637 for temperature and pressure
and HTU21 for humidity

Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
---
 Documentation/ABI/testing/sysfs-bus-iio-meas-spec |  1 +
 drivers/iio/humidity/Kconfig                      |  2 ++
 drivers/iio/humidity/htu21.c                      | 33 ++++++++++++++++++++---
 drivers/iio/pressure/Kconfig                      |  2 ++
 drivers/iio/pressure/ms5637.c                     |  6 ++++-
 5 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
index 6d47e54..1a6265e 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
+++ b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
@@ -5,3 +5,4 @@ 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
+		This ABI is available for htu21, ms8607
diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
index 3fab60c..75ca043 100644
--- a/drivers/iio/humidity/Kconfig
+++ b/drivers/iio/humidity/Kconfig
@@ -19,6 +19,8 @@ config HTU21
 	help
 	  If you say yes here you get support for the Measurement Specialties
 	  HTU21 humidity and temperature sensor.
+	  This driver is also used for MS8607 temperature, pressure & humidity
+	  sensor
 
 	  This driver can also be built as a module. If so, the module will
 	  be called htu21.
diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
index 706faff..0530545 100644
--- a/drivers/iio/humidity/htu21.c
+++ b/drivers/iio/humidity/htu21.c
@@ -1,6 +1,7 @@
 /*
  * htu21.c - Support for Measurement-Specialties
  *           htu21 temperature & humidity sensor
+ *	     and humidity part of MS8607 sensor
  *
  * Copyright (c) 2014 Measurement-Specialties
  *
@@ -10,6 +11,8 @@
  *
  * Datasheet:
  *  http://www.meas-spec.com/downloads/HTU21D.pdf
+ * Datasheet:
+ *  http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
  *
  */
 
@@ -25,6 +28,11 @@
 
 #define HTU21_RESET				0xFE
 
+enum {
+	HTU21,
+	MS8607
+};
+
 static const int htu21_samp_freq[4] = { 20, 40, 70, 120 };
 /* String copy of the above const for readability purpose */
 static const char htu21_show_samp_freq[] = "20 40 70 120";
@@ -107,6 +115,18 @@ static const struct iio_chan_spec htu21_channels[] = {
 	 }
 };
 
+/*
+ * Meas Spec recommendation is to not read temperature
+ * on this driver part for MS8607
+ */
+static const struct iio_chan_spec ms8607_channels[] = {
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	 }
+};
+
 static ssize_t htu21_show_battery_low(struct device *dev,
 				      struct device_attribute *attr, char *buf)
 {
@@ -189,8 +209,14 @@ int htu21_probe(struct i2c_client *client,
 	indio_dev->name = id->name;
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = htu21_channels;
-	indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
+
+	if (id->driver_data == MS8607) {
+		indio_dev->channels = ms8607_channels;
+		indio_dev->num_channels = ARRAY_SIZE(ms8607_channels);
+	} else {
+		indio_dev->channels = htu21_channels;
+		indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
+	}
 
 	i2c_set_clientdata(client, indio_dev);
 
@@ -207,7 +233,8 @@ int htu21_probe(struct i2c_client *client,
 }
 
 static const struct i2c_device_id htu21_id[] = {
-	{"htu21", 0},
+	{"htu21", HTU21},
+	{"ms8607-h", MS8607},
 	{}
 };
 
diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index 8142cfe..53f9a44 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -86,6 +86,8 @@ config MS5637
 	help
 	  If you say yes here you get support for the Measurement Specialties
 	  MS5637 pressure and temperature sensor.
+	  This driver is also used for MS8607 temperature, pressure & humidity
+	  sensor
 
 	  This driver can also be built as a module. If so, the module will
 	  be called ms5637.
diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
index 0dbbd4e..f6b5544 100644
--- a/drivers/iio/pressure/ms5637.c
+++ b/drivers/iio/pressure/ms5637.c
@@ -1,5 +1,5 @@
 /*
- * ms5637.c - Support for Measurement-Specialties ms5637
+ * ms5637.c - Support for Measurement-Specialties ms5637 and ms8607
  *            pressure & temperature sensor
  *
  * Copyright (c) 2015 Measurement-Specialties
@@ -10,8 +10,11 @@
  *
  * Datasheet:
  *  http://www.meas-spec.com/downloads/MS5637-02BA03.pdf
+ * Datasheet:
+ *  http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
  *
  */
+
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
@@ -168,6 +171,7 @@ static int ms5637_probe(struct i2c_client *client,
 
 static const struct i2c_device_id ms5637_id[] = {
 	{"ms5637", 0},
+	{"ms8607-tp", 1},
 	{}
 };
 
-- 
2.3.7


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/6] Add meas-spec sensors common part
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-27 16:23 UTC (permalink / raw)
  To: Ludovic Tancerel, knaack.h, lars, pmeerw, linux-iio, William.Markezana

On 25/09/15 14:56, Ludovic Tancerel wrote:
> Measurement specialties drivers common part.
> These functions are used by further drivers in the patchset: TSYS01, TSYS02D, HTU21, MS5637, MS8607
> 
> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
A few more bits and bobs inline.
> ---
>  drivers/iio/common/Kconfig                     |   1 +
>  drivers/iio/common/Makefile                    |   1 +
>  drivers/iio/common/ms_sensors/Kconfig          |   6 +
>  drivers/iio/common/ms_sensors/Makefile         |   5 +
>  drivers/iio/common/ms_sensors/ms_sensors_i2c.c | 645 +++++++++++++++++++++++++
>  drivers/iio/common/ms_sensors/ms_sensors_i2c.h |  53 ++
>  6 files changed, 711 insertions(+)
>  create mode 100644 drivers/iio/common/ms_sensors/Kconfig
>  create mode 100644 drivers/iio/common/ms_sensors/Makefile
>  create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>  create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.h
> 
> diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> index 790f106..26a6026 100644
> --- a/drivers/iio/common/Kconfig
> +++ b/drivers/iio/common/Kconfig
> @@ -3,5 +3,6 @@
>  #
>  
>  source "drivers/iio/common/hid-sensors/Kconfig"
> +source "drivers/iio/common/ms_sensors/Kconfig"
>  source "drivers/iio/common/ssp_sensors/Kconfig"
>  source "drivers/iio/common/st_sensors/Kconfig"
> diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
> index b1e4d9c..585da6a 100644
> --- a/drivers/iio/common/Makefile
> +++ b/drivers/iio/common/Makefile
> @@ -8,5 +8,6 @@
>  
>  # When adding new entries keep the list in alphabetical order
>  obj-y += hid-sensors/
> +obj-y += ms_sensors/
>  obj-y += ssp_sensors/
>  obj-y += st_sensors/
> diff --git a/drivers/iio/common/ms_sensors/Kconfig b/drivers/iio/common/ms_sensors/Kconfig
> new file mode 100644
> index 0000000..b28a92b
> --- /dev/null
> +++ b/drivers/iio/common/ms_sensors/Kconfig
> @@ -0,0 +1,6 @@
> +#
> +# Measurements Specialties sensors common library
> +#
> +
> +config IIO_MS_SENSORS_I2C
> +        tristate
> diff --git a/drivers/iio/common/ms_sensors/Makefile b/drivers/iio/common/ms_sensors/Makefile
> new file mode 100644
> index 0000000..7846428
> --- /dev/null
> +++ b/drivers/iio/common/ms_sensors/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for the Measurement Specialties sensor common modules.
> +#
> +
> +obj-$(CONFIG_IIO_MS_SENSORS_I2C) += ms_sensors_i2c.o
> diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
> new file mode 100644
> index 0000000..4b1bc31
> --- /dev/null
> +++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
> @@ -0,0 +1,645 @@
> +/*
> + * Measurements Specialties driver common i2c functions
> + *
> + * Copyright (c) 2015 Measurement-Specialties
> + *
> + * Licensed under the GPL-2.
Drop this blank line.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/iio/iio.h>
> +#include <linux/device.h>
> +#include <linux/delay.h>
> +
> +#include "ms_sensors_i2c.h"
> +
> +/* Conversion times in us */
> +static const u16 ms_sensors_ht_t_conversion_time[] = { 50000, 25000,
> +						       13000, 7000 };
> +static const u16 ms_sensors_ht_h_conversion_time[] = { 16000, 3000,
> +						       5000, 8000 };
> +static const u16 ms_sensors_tp_conversion_time[] = { 500, 1100, 2100,
> +						     4100, 8220, 16440 };
> +
> +#define MS_SENSORS_SERIAL_READ_MSB		0xFA0F
> +#define MS_SENSORS_SERIAL_READ_LSB		0xFCC9
> +#define MS_SENSORS_CONFIG_REG_WRITE		0xE6
> +#define MS_SENSORS_CONFIG_REG_READ		0xE7
> +#define MS_SENSORS_HT_T_CONVERSION_START	0xF3
> +#define MS_SENSORS_HT_H_CONVERSION_START	0xF5
> +
> +#define MS_SENSORS_TP_PROM_READ			0xA0
> +#define MS_SENSORS_TP_T_CONVERSION_START	0x50
> +#define MS_SENSORS_TP_P_CONVERSION_START	0x40
> +#define MS_SENSORS_TP_ADC_READ			0x00
> +
> +#define MS_SENSORS_NO_READ_CMD			0xFF
> +
> +/**
> + * ms_sensors_i2c_reset() - i2c reset function
> + * @cli:	pointer to i2c client
> + * @cmd:	reset cmd. Depends on device in use
> + * @delay:	usleep minimal delay after reset command is issued
> + *
> + * Generic I2C reset function for Measurement Specialties devices.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int ms_sensors_i2c_reset(void *cli, u8 cmd, unsigned int delay)
> +{
> +	int ret;
> +	struct i2c_client *client = cli;
> +
> +	ret = i2c_smbus_write_byte(client, cmd);
> +	if (ret) {
> +		dev_err(&client->dev, "Failed to reset device\n");
> +		return ret;
> +	}
> +	usleep_range(delay, delay + 1000);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_reset);
> +
> +/**
> + * ms_sensors_i2c_read_prom_word() - i2c prom word read function
> + * @cli:	pointer to i2c client
> + * @cmd:	PROM read cmd. Depends on device and prom id
> + * @word:	pointer to word destination value
> + *
> + * Generic i2c prom word read function for Measurement Specialties devices.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word)
> +{
> +	int ret;
> +	struct i2c_client *client = (struct i2c_client *)cli;
Why pass an void * given the function name says this will always be an i2c_client?

Once that's removed this wrapper does very little and I'd be tempted to
drop it in favour of direct calls to the smbus read.
> +
> +	ret = i2c_smbus_read_word_swapped(client, cmd);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Failed to read prom word\n");
> +		return ret;
> +	}
> +	*word = ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_read_prom_word);
> +
> +/**
> + * ms_sensors_i2c_convert_and_read() - i2c ADC conversion & read function
> + * @cli:	pointer to i2c client
> + * @conv:	ADC conversion command. Depends on device in use
> + * @rd:		ADC read command. Depends on device in use
> + * @delay:	usleep minimal delay after conversion command is issued
> + * @adc:	pointer to ADC destination value
> + *
> + * Generic i2c ADC conversion & read function for Measurement Specialties
> + * devices.
> + * The function will issue conversion command, sleep appopriate delay, and
> + * issue command to read ADC.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
> +				    unsigned int delay, u32 *adc)
> +{
> +	int ret;
> +	u32 buf = 0;
> +	struct i2c_client *client = (struct i2c_client *)cli;
> +
> +	/* Trigger conversion */
> +	ret = i2c_smbus_write_byte(client, conv);
> +	if (ret)
> +		goto err;
> +	usleep_range(delay, delay + 1000);
> +
> +	/* Retrieve ADC value */
> +	if (rd != MS_SENSORS_NO_READ_CMD)
> +		ret = i2c_smbus_read_i2c_block_data(client, rd, 3, (u8 *)&buf);
> +	else
> +		ret = i2c_master_recv(client, (u8 *)&buf, 3);
> +	if (ret < 0)
> +		goto err;
> +
> +	dev_dbg(&client->dev, "ADC raw value : %x\n", be32_to_cpu(buf) >> 8);
> +	*adc = be32_to_cpu(buf) >> 8;
> +
> +	return 0;
> +err:
> +	dev_err(&client->dev, "Unable to make sensor adc conversion\n");
> +	return ret;
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_convert_and_read);
> +
> +/**
> + * ms_sensors_crc_valid() - CRC check function
> + * @value:	input and CRC compare value
> + *
> + * Cyclic Redundancy Check function used in TSYS02D, HTU21, MS8607.
> + * This function performs a x^8 + x^5 + x^4 + 1 polynomial CRC.
> + * The argument contains CRC value in LSB byte while the bytes 1 and 2
> + * are used for CRC computation.
> + *
> + * Return: 1 if CRC is valid, 0 otherwise.
Can this be done with the standard kernel crc32 functions? (not dug into
it enough to be sure!)
> + */
> +static bool ms_sensors_crc_valid(u32 value)
> +{
> +	u32 polynom = 0x988000;	/* x^8 + x^5 + x^4 + 1 */
> +	u32 msb = 0x800000;
> +	u32 mask = 0xFF8000;
> +	u32 result = value & 0xFFFF00;
> +	u8 crc = value & 0xFF;
> +
> +	while (msb != 0x80) {
> +		if (result & msb)
> +			result = ((result ^ polynom) & mask)
> +				| (result & ~mask);
> +		msb >>= 1;
> +		mask >>= 1;
> +		polynom >>= 1;
> +	}
> +
> +	return result == crc;
> +}
> +
> +/**
> + * ms_sensors_i2c_read_serial() - i2c serial number read function
> + * @cli:	pointer to i2c client
> + * @sn:		pointer to 64-bits destination value
> + *
> + * Generic i2c serial number read function for Measurement Specialties devices.
> + * This function is used for TSYS02d, HTU21, MS8607 chipset.
> + * Refer to datasheet:
> + *	http://www.meas-spec.com/downloads/HTU2X_Serial_Number_Reading.pdf
THat's a first.  A whole datasheet on how the heck the serial number works!

Got to wonder how anyone ever came up with that.  I'm going to conclude
you got it right and not read any more ;)
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn)
> +{
> +	u8 i;
> +	u64 rcv_buf = 0;
> +	u16 send_buf;
> +	int ret;
> +
> +	struct i2c_msg msg[2] = {
> +		{
> +		 .addr = client->addr,
> +		 .flags = client->flags,
> +		 .len = 2,
> +		 .buf = (__u8 *)&send_buf,
> +		 },
> +		{
> +		 .addr = client->addr,
> +		 .flags = client->flags | I2C_M_RD,
> +		 .buf = (__u8 *)&rcv_buf,
> +		 },
> +	};
> +
> +	/* Read MSB part of serial number */
> +	send_buf = cpu_to_be16(MS_SENSORS_SERIAL_READ_MSB);
> +	msg[1].len = 8;
> +	ret = i2c_transfer(client->adapter, msg, 2);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Unable to read device serial number");
> +		return ret;
> +	}
> +
> +	rcv_buf = be64_to_cpu(rcv_buf);
> +	dev_dbg(&client->dev, "Serial MSB raw : %llx\n", rcv_buf);
> +
> +	for (i = 0; i < 64; i += 16) {
> +		if (!ms_sensors_crc_valid((rcv_buf >> i) & 0xFFFF))
> +			return -ENODEV;
> +	}
> +
Umm. I'm really unclear what you are doing here.  You read into
a 64 bit buffer, then do a hand endian swap and shift?  Should be possible
to at least use standard functions to swap the byte order.

> +	*sn = (((rcv_buf >> 32) & 0xFF000000) |
> +	       ((rcv_buf >> 24) & 0x00FF0000) |
> +	       ((rcv_buf >> 16) & 0x0000FF00) |
> +	       ((rcv_buf >> 8) & 0x000000FF)) << 16;
> +
> +	/* Read LSB part of serial number */
> +	send_buf = cpu_to_be16(MS_SENSORS_SERIAL_READ_LSB);
> +	msg[1].len = 6;
> +	rcv_buf = 0;
> +	ret = i2c_transfer(client->adapter, msg, 2);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Unable to read device serial number");
> +		return ret;
> +	}
> +
> +	rcv_buf = be64_to_cpu(rcv_buf) >> 16;
> +	dev_dbg(&client->dev, "Serial MSB raw : %llx\n", rcv_buf);
> +
> +	for (i = 0; i < 48; i += 24) {
> +		if (!ms_sensors_crc_valid((rcv_buf >> i) & 0xFFFFFF))
> +			return -ENODEV;
> +	}
> +
> +	*sn |= (rcv_buf & 0xFFFF00) << 40 | (rcv_buf >> 32);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_read_serial);
> +
> +static int ms_sensors_i2c_read_config_reg(struct i2c_client *client,
> +					  u8 *config_reg)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte(client, MS_SENSORS_CONFIG_REG_READ);
> +	if (ret) {
> +		dev_err(&client->dev, "Unable to read config register");
> +		return ret;
> +	}
> +
> +	ret = i2c_master_recv(client, config_reg, 1);
> +	if (ret < 0) {
> +		dev_err(&client->dev, "Unable to read config register");
> +		return ret;
> +	}
> +	dev_dbg(&client->dev, "Config register :%x\n", *config_reg);
> +
> +	return 0;
> +}
> +
> +/**
> + * ms_sensors_i2c_write_resolution() - set resolution i2c function
> + * @dev_data:	pointer to temperature/humidity device data
> + * @i:		resolution index to set
> + *
> + * This function will program the appropriate resolution based on the index
> + * provided when user space will set samp_freq channel.
> + * This function is used for TSYS02D, HTU21 and MS8607 chipsets.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +ssize_t ms_sensors_i2c_write_resolution(struct ms_ht_dev *dev_data,
> +					u8 i)
> +{
> +	u8 config_reg;
> +	int ret;
> +
> +	ret = ms_sensors_i2c_read_config_reg(dev_data->client, &config_reg);
> +	if (ret)
> +		return ret;
> +
> +	config_reg &= 0x7E;
> +	config_reg |= ((i & 1) << 7) + ((i & 2) >> 1);
> +
> +	return i2c_smbus_write_byte_data(dev_data->client,
> +					 MS_SENSORS_CONFIG_REG_WRITE,
> +					 config_reg);
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_write_resolution);
> +
> +/**
> + * ms_sensors_i2c_show_battery_low() - show device battery low indicator
> + * @dev_data:	pointer to temperature/humidity device data
> + * @buf:	pointer to char buffer to write result
> + *
> + * This function will read battery indicator value in the device and
> + * return 1 if the device voltage is below 2.25V.
> + * This function is used for TSYS02D, HTU21 and MS8607 chipsets.
> + *
> + * Return: length of sprintf on success, negative errno otherwise.
> + */
> +ssize_t ms_sensors_i2c_show_battery_low(struct ms_ht_dev *dev_data,
> +					char *buf)
> +{
> +	int ret;
> +	u8 config_reg;
> +
> +	mutex_lock(&dev_data->lock);
> +	ret = ms_sensors_i2c_read_config_reg(dev_data->client, &config_reg);
> +	mutex_unlock(&dev_data->lock);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", (config_reg & 0x40) >> 6);
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_show_battery_low);
> +
> +/**
> + * ms_sensors_i2c_show_heater() - show device heater
> + * @dev_data:	pointer to temperature/humidity device data
> + * @buf:	pointer to char buffer to write result
> + *
> + * This function will read heater enable value in the device and
> + * return 1 if the heater is enabled.
> + * This function is used for HTU21 and MS8607 chipsets.
> + *
> + * Return: length of sprintf on success, negative errno otherwise.
> + */
> +ssize_t ms_sensors_i2c_show_heater(struct ms_ht_dev *dev_data,
> +				   char *buf)
> +{
> +	u8 config_reg;
> +	int ret;
> +
> +	mutex_lock(&dev_data->lock);
> +	ret = ms_sensors_i2c_read_config_reg(dev_data->client, &config_reg);
> +	mutex_unlock(&dev_data->lock);
> +	if (ret)
> +		return ret;
> +
> +	return sprintf(buf, "%d\n", (config_reg & 0x4) >> 2);
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_show_heater);
> +
> +/**
> + * ms_sensors_i2c_write_heater() - write device heater
> + * @dev_data:	pointer to temperature/humidity device data
> + * @buf:	pointer to char buffer from user space
> + * @len:	length of buf
> + *
> + * This function will write 1 or 0 value in the device
> + * to enable or disable heater.
> + * This function is used for HTU21 and MS8607 chipsets.
> + *
> + * Return: length of buffer, negative errno otherwise.
> + */
> +ssize_t ms_sensors_i2c_write_heater(struct ms_ht_dev *dev_data,
> +				    const char *buf, size_t len)
> +{
> +	u8 val, config_reg;
> +	int ret;
> +
> +	ret = kstrtou8(buf, 10, &val);
> +	if (ret)
> +		return ret;
> +
> +	if (val > 1)
> +		return -EINVAL;
> +
> +	mutex_lock(&dev_data->lock);
> +	ret = ms_sensors_i2c_read_config_reg(dev_data->client, &config_reg);
> +	if (ret) {
> +		mutex_unlock(&dev_data->lock);
> +		return ret;
> +	}
> +
> +	config_reg &= 0xFB;
> +	config_reg |= val << 2;
> +
> +	ret = i2c_smbus_write_byte_data(dev_data->client,
> +					MS_SENSORS_CONFIG_REG_WRITE,
> +					config_reg);
> +	mutex_unlock(&dev_data->lock);
> +	if (ret) {
> +		dev_err(&dev_data->client->dev, "Unable to write config register\n");
> +		return ret;
> +	}
> +
> +	return len;
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_write_heater);
> +
> +/**
> + * ms_sensors_i2c_ht_read_temperature() - i2c read temperature
> + * @dev_data:	pointer to temperature/humidity device data
> + * @temperature:pointer to temperature destination value
> + *
> + * This function will get temperature ADC value from the device,
> + * check the CRC and compute the temperature value.
> + * This function is used for TSYS02D, HTU21 and MS8607 chipsets.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int ms_sensors_i2c_ht_read_temperature(struct ms_ht_dev *dev_data,
> +				       s32 *temperature)
> +{
> +	int ret;
> +	u32 adc;
> +	u16 delay;
> +
> +	mutex_lock(&dev_data->lock);
> +	delay = ms_sensors_ht_t_conversion_time[dev_data->res_index];
> +	ret = ms_sensors_i2c_convert_and_read(dev_data->client,
> +					      MS_SENSORS_HT_T_CONVERSION_START,
> +					      MS_SENSORS_NO_READ_CMD,
> +					      delay, &adc);
> +	mutex_unlock(&dev_data->lock);
> +	if (ret)
> +		return ret;
> +
> +	if (!ms_sensors_crc_valid(adc)) {
> +		dev_err(&dev_data->client->dev,
> +			"Temperature read crc check error\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Temperature algorithm */
> +	*temperature = (((s64)(adc >> 8) * 175720) >> 16) - 46850;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_ht_read_temperature);
> +
> +/**
> + * ms_sensors_i2c_ht_read_humidity() - i2c read humidity
> + * @dev_data:	pointer to temperature/humidity device data
> + * @humidity:	pointer to humidity destination value
> + *
> + * This function will get humidity ADC value from the device,
> + * check the CRC and compute the temperature value.
> + * This function is used for HTU21 and MS8607 chipsets.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int ms_sensors_i2c_ht_read_humidity(struct ms_ht_dev *dev_data,
> +				    u32 *humidity)
> +{
> +	int ret;
> +	u32 adc;
> +	u16 delay;
> +
> +	mutex_lock(&dev_data->lock);
> +	delay = ms_sensors_ht_h_conversion_time[dev_data->res_index];
> +	ret = ms_sensors_i2c_convert_and_read(dev_data->client,
> +					      MS_SENSORS_HT_H_CONVERSION_START,
> +					      MS_SENSORS_NO_READ_CMD,
> +					      delay, &adc);
> +	mutex_unlock(&dev_data->lock);
> +	if (ret)
> +		return ret;
> +
> +	if (!ms_sensors_crc_valid(adc)) {
> +		dev_err(&dev_data->client->dev,
> +			"Humidity read crc check error\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Humidity algorithm */
> +	*humidity = (((s32)(adc >> 8) * 12500) >> 16) * 10 - 6000;
> +	if (*humidity >= 100000)
> +		*humidity = 100000;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ms_sensors_i2c_ht_read_humidity);
> +
> +/**
> + * ms_sensors_tp_crc_valid() - CRC check function for
> + *     Temperature and pressure devices.
> + *     This function is only used when reading PROM coefficients
> + *
> + * @prom:	pointer to PROM coefficients array
> + * @len:	length of PROM coefficients array
> + *
> + * Return: True if CRC is ok.
> + */
> +static bool ms_sensors_tp_crc_valid(u16 *prom, u8 len)
> +{
> +	unsigned int cnt, n_bit;
> +	u16 n_rem = 0x0000, crc_read = prom[0], crc = (*prom & 0xF000) >> 12;
> +
> +	prom[len - 1] = 0;
> +	prom[0] &= 0x0FFF;      /* Clear the CRC computation part */
> +
> +	for (cnt = 0; cnt < len * 2; cnt++) {
> +		if (cnt % 2 == 1)
> +			n_rem ^= prom[cnt >> 1] & 0x00FF;
> +		else
> +			n_rem ^= prom[cnt >> 1] >> 8;
> +
> +		for (n_bit = 8; n_bit > 0; n_bit--) {
> +			if (n_rem & 0x8000)
> +				n_rem = (n_rem << 1) ^ 0x3000;
> +			else
> +				n_rem <<= 1;
> +		}
> +	}
> +	n_rem >>= 12;
> +	prom[0] = crc_read;
> +
> +	return n_rem == crc;
> +}
> +
> +/**
> + * ms_sensors_tp_read_prom() - prom coeff read function
> + * @dev_data:	pointer to temperature/pressure device data
> + *
> + * This function will read prom coefficients and check CRC.
> + * This function is used for MS5637 and MS8607 chipsets.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int ms_sensors_tp_read_prom(struct ms_tp_dev *dev_data)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < MS_SENSORS_TP_PROM_WORDS_NB; i++) {
> +		ret = ms_sensors_i2c_read_prom_word(
> +					dev_data->client,
> +					MS_SENSORS_TP_PROM_READ + (i << 1),
> +					&dev_data->prom[i]);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (!ms_sensors_tp_crc_valid(dev_data->prom,
> +				     MS_SENSORS_TP_PROM_WORDS_NB + 1)) {
> +		dev_err(&dev_data->client->dev,
> +			"Calibration coefficients crc check error\n");
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ms_sensors_tp_read_prom);
> +
> +/**
> + * ms_sensors_read_temp_and_pressure() - read temp and pressure
> + * @dev_data:	pointer to temperature/pressure device data
> + * @temperature:pointer to temperature destination value
> + * @pressure:	pointer to pressure destination value
> + *
> + * This function will read ADC and compute pressure and temperature value.
> + * This function is used for MS5637 and MS8607 chipsets.
> + *
> + * Return: 0 on success, negative errno otherwise.
> + */
> +int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
> +				      int *temperature,
> +				      unsigned int *pressure)
> +{
> +	int ret;
> +	u32 t_adc, p_adc;
> +	s32 dt, temp;
> +	s64 off, sens, t2, off2, sens2;
> +	u16 *prom = dev_data->prom, delay;
> +	u8 i;
> +
> +	mutex_lock(&dev_data->lock);
> +	i = dev_data->res_index * 2;
This local variable seens rather unnecessary. Just put it inline in the
calls.
> +	delay = ms_sensors_tp_conversion_time[dev_data->res_index];
> +
> +	ret = ms_sensors_i2c_convert_and_read(
> +					dev_data->client,
> +					MS_SENSORS_TP_T_CONVERSION_START + i,
> +					MS_SENSORS_TP_ADC_READ,
> +					delay, &t_adc);
> +	if (ret) {
> +		mutex_unlock(&dev_data->lock);
> +		return ret;
> +	}
> +
> +	ret = ms_sensors_i2c_convert_and_read(
> +					dev_data->client,
> +					MS_SENSORS_TP_P_CONVERSION_START + i,
> +					MS_SENSORS_TP_ADC_READ,
> +					delay, &p_adc);
> +	mutex_unlock(&dev_data->lock);
> +	if (ret)
> +		return ret;
> +
> +	dt = (s32)t_adc - (prom[5] << 8);
> +
> +	/* Actual temperature = 2000 + dT * TEMPSENS */
> +	temp = 2000 + (((s64)dt * prom[6]) >> 23);
> +
> +	/* Second order temperature compensation */
> +	if (temp < 2000) {
> +		s64 tmp = (s64)temp - 2000;
> +
> +		t2 = (3 * ((s64)dt * (s64)dt)) >> 33;
> +		off2 = (61 * tmp * tmp) >> 4;
> +		sens2 = (29 * tmp * tmp) >> 4;
> +
> +		if (temp < -1500) {
> +			s64 tmp = (s64)temp + 1500;
> +
> +			off2 += 17 * tmp * tmp;
> +			sens2 += 9 * tmp * tmp;
> +		}
> +	} else {
> +		t2 = (5 * ((s64)dt * (s64)dt)) >> 38;
> +		off2 = 0;
> +		sens2 = 0;
> +	}
> +
> +	/* OFF = OFF_T1 + TCO * dT */
> +	off = (((s64)prom[2]) << 17) + ((((s64)prom[4]) * (s64)dt) >> 6);
> +	off -= off2;
> +
> +	/* Sensitivity at actual temperature = SENS_T1 + TCS * dT */
> +	sens = (((s64)prom[1]) << 16) + (((s64)prom[3] * dt) >> 7);
> +	sens -= sens2;
> +
> +	/* Temperature compensated pressure = D1 * SENS - OFF */
> +	*temperature = (temp - t2) * 10;
> +	*pressure = (u32)(((((s64)p_adc * sens) >> 21) - off) >> 15);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(ms_sensors_read_temp_and_pressure);
> +
> +MODULE_DESCRIPTION("Measurement-Specialties common i2c driver");
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
> +MODULE_LICENSE("GPL v2");
> +
> diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.h b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
> new file mode 100644
> index 0000000..918b8af
> --- /dev/null
> +++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
> @@ -0,0 +1,53 @@
> +/*
> + * Measurements Specialties common sensor driver
> + *
> + * Copyright (c) 2015 Measurement-Specialties
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _MS_SENSORS_I2C_H
> +#define _MS_SENSORS_I2C_H
> +
> +#include <linux/i2c.h>
> +#include <linux/mutex.h>
> +
> +#define MS_SENSORS_TP_PROM_WORDS_NB		7
> +
> +struct ms_ht_dev {
> +	struct i2c_client *client;
> +	struct mutex lock; /* mutex protecting data structure */
> +	u8 res_index;
> +};
> +
kernel doc comments preferred.
> +struct ms_tp_dev {
> +	struct i2c_client *client;
> +	struct mutex lock; /* mutex protecting data structure */
> +	/* Added element for CRC computation */
> +	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB + 1];
> +	u8 res_index;
> +};
> +
> +int ms_sensors_i2c_reset(void *cli, u8 cmd, unsigned int delay);
> +int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word);
> +int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
> +				    unsigned int delay, u32 *adc);
> +int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn);
> +ssize_t ms_sensors_i2c_show_serial(struct ms_ht_dev *dev_data, char *buf);
> +ssize_t ms_sensors_i2c_write_resolution(struct ms_ht_dev *dev_data, u8 i);
> +ssize_t ms_sensors_i2c_show_battery_low(struct ms_ht_dev *dev_data, char *buf);
> +ssize_t ms_sensors_i2c_show_heater(struct ms_ht_dev *dev_data, char *buf);
> +ssize_t ms_sensors_i2c_write_heater(struct ms_ht_dev *dev_data,
> +				    const char *buf, size_t len);
> +int ms_sensors_i2c_ht_read_temperature(struct ms_ht_dev *dev_data,
> +				       s32 *temperature);
> +int ms_sensors_i2c_ht_read_humidity(struct ms_ht_dev *dev_data,
> +				    u32 *humidity);
> +int ms_sensors_tp_read_prom(struct ms_tp_dev *dev_data);
> +int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
> +				      int *temperature,
> +				      unsigned int *pressure);
The presence or absense of _i2c in the function naming does feel rather
random.  All of these functions ultimately make i2c calls so perhaps you
can explain your reasoning behind having it in some and not others.

> +
> +#endif /* _MS_SENSORS_I2C_H */
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/6] Add tsys01 meas-spec driver support
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-27 16:55 UTC (permalink / raw)
  To: Ludovic Tancerel, knaack.h, lars, pmeerw, linux-iio, William.Markezana

On 25/09/15 14:56, Ludovic Tancerel wrote:
> Support for TSYS01 temperature sensor
> 
> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
This is fine as far as i am concerned, though I would like to leave it
on the list for a little while for others to have a chance to comment.
> ---
>  drivers/iio/temperature/Kconfig  |  11 ++
>  drivers/iio/temperature/Makefile |   1 +
>  drivers/iio/temperature/tsys01.c | 231 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 243 insertions(+)
>  create mode 100644 drivers/iio/temperature/tsys01.c
> 
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 21feaa4..35712032 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -23,4 +23,15 @@ config TMP006
>  	  This driver can also be built as a module. If so, the module will
>  	  be called tmp006.
>  
> +config TSYS01
> +	tristate "Measurement Specialties TSYS01 temperature sensor using I2C bus connection"
> +	depends on I2C
> +	select IIO_MS_SENSORS_I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  TSYS01 I2C temperature sensor.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called tsys01.
> +
>  endmenu
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 40710a8..368a2a2 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -4,3 +4,4 @@
>  
>  obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
> +obj-$(CONFIG_TSYS01) += tsys01.o
> diff --git a/drivers/iio/temperature/tsys01.c b/drivers/iio/temperature/tsys01.c
> new file mode 100644
> index 0000000..5c4127f
> --- /dev/null
> +++ b/drivers/iio/temperature/tsys01.c
> @@ -0,0 +1,231 @@
> +/*
> + * tsys01.c - Support for Measurement-Specialties tsys01 temperature sensor
> + *
> + * Copyright (c) 2015 Measurement-Specialties
> + *
> + * Licensed under the GPL-2.
> + *
> + * Datasheet:
> + *  http://www.meas-spec.com/downloads/TSYS01_Digital_Temperature_Sensor.pdf
> + *
> + */
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/stat.h>
> +#include "../common/ms_sensors/ms_sensors_i2c.h"
> +
> +/* TSYS01 Commands */
> +#define TSYS01_RESET				0x1E
> +#define TSYS01_CONVERSION_START			0x48
> +#define TSYS01_ADC_READ				0x00
> +#define TSYS01_PROM_READ			0xA0
> +
> +#define TSYS01_PROM_WORDS_NB			8
> +
> +struct tsys01_dev {
> +	void *client;
> +	struct mutex lock; /* lock during conversion */
> +
> +	int (*reset)(void *cli, u8 cmd, unsigned int delay);
> +	int (*convert_and_read)(void *cli, u8 conv, u8 rd,
> +				unsigned int delay, u32 *adc);
> +	int (*read_prom_word)(void *cli, int cmd, u16 *word);
> +
> +	u16 prom[TSYS01_PROM_WORDS_NB];
> +};
> +
> +/* Multiplication coefficients for temperature computation */
> +static const int coeff_mul[] = { -1500000, 1000000, -2000000,
> +				 4000000, -2000000 };
> +
> +static int tsys01_read_temperature(struct iio_dev *indio_dev,
> +				   s32 *temperature)
> +{
> +	int ret, i;
> +	u32 adc;
> +	s64 temp = 0;
> +	struct tsys01_dev *dev_data = iio_priv(indio_dev);
> +
> +	mutex_lock(&dev_data->lock);
> +	ret = dev_data->convert_and_read(dev_data->client,
> +					 TSYS01_CONVERSION_START,
> +					 TSYS01_ADC_READ, 9000, &adc);
> +	mutex_unlock(&dev_data->lock);
> +	if (ret)
> +		return ret;
> +
> +	adc >>= 8;
> +
> +	/* Temperature algorithm */
> +	for (i = 4; i > 0; i--) {
> +		temp += coeff_mul[i] *
> +			(s64)dev_data->prom[5 - i];
> +		temp *= (s64)adc;
> +		temp = div64_s64(temp, 100000);
> +	}
> +	temp *= 10;
> +	temp += coeff_mul[0] * (s64)dev_data->prom[5];
> +	temp = div64_s64(temp, 100000);
> +
> +	*temperature = temp;
> +
> +	return 0;
> +}
> +
> +static int tsys01_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *channel, int *val,
> +			   int *val2, long mask)
> +{
> +	int ret;
> +	s32 temperature;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED:
> +		switch (channel->type) {
> +		case IIO_TEMP:	/* in milli °C */
> +			ret = tsys01_read_temperature(indio_dev, &temperature);
> +			if (ret)
> +				return ret;
> +			*val = temperature;
> +
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_chan_spec tsys01_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
> +	}
> +};
> +
> +static const struct iio_info tsys01_info = {
> +	.read_raw = tsys01_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static bool tsys01_crc_valid(u16 *n_prom)
> +{
> +	u8 cnt;
> +	u8 sum = 0;
> +
> +	for (cnt = 0; cnt < TSYS01_PROM_WORDS_NB; cnt++)
> +		sum += ((n_prom[0] >> 8) + (n_prom[0] & 0xFF));
> +
> +	return (sum == 0);
> +}
> +
> +static int tsys01_read_prom(struct iio_dev *indio_dev)
> +{
> +	int i, ret;
> +	struct tsys01_dev *dev_data = iio_priv(indio_dev);
> +	char buf[7 * TSYS01_PROM_WORDS_NB + 1];
> +	char *ptr = buf;
> +
> +	for (i = 0; i < TSYS01_PROM_WORDS_NB; i++) {
> +		ret = dev_data->read_prom_word(dev_data->client,
> +					       TSYS01_PROM_READ + (i << 1),
> +					       &dev_data->prom[i]);
> +		if (ret)
> +			return ret;
> +
> +		ret = sprintf(ptr, "0x%04x ", dev_data->prom[i]);
> +		ptr += ret;
> +	}
> +
> +	if (!tsys01_crc_valid(dev_data->prom)) {
> +		dev_err(&indio_dev->dev, "prom crc check error\n");
> +		return -ENODEV;
> +	}
> +	*ptr = 0;
> +	dev_info(&indio_dev->dev, "PROM coefficients : %s\n", buf);
> +
> +	return 0;
> +}
> +
> +static int tsys01_probe(struct iio_dev *indio_dev, struct device *dev)
> +{
> +	int ret;
> +	struct tsys01_dev *dev_data = iio_priv(indio_dev);
> +
> +	mutex_init(&dev_data->lock);
> +
> +	indio_dev->info = &tsys01_info;
> +	indio_dev->name = dev->driver->name;
> +	indio_dev->dev.parent = dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = tsys01_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(tsys01_channels);
> +
> +	ret = dev_data->reset(dev_data->client, TSYS01_RESET, 3000);
> +	if (ret)
> +		return ret;
> +
> +	ret = tsys01_read_prom(indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static int tsys01_i2c_probe(struct i2c_client *client,
> +			    const struct i2c_device_id *id)
> +{
> +	struct tsys01_dev *dev_data;
> +	struct iio_dev *indio_dev;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WORD_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->reset = ms_sensors_i2c_reset;
> +	dev_data->read_prom_word = ms_sensors_i2c_read_prom_word;
> +	dev_data->convert_and_read = ms_sensors_i2c_convert_and_read;
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
This separation into i2c probe and main probe is something one would
generally only introduce at the point of adding support for another bus.
It just adds complexity here for little gain.  If there is intent
to add spi support shortly then fair enough, leave it as it is.
> +	return tsys01_probe(indio_dev, &client->dev);
> +}
> +
> +static const struct i2c_device_id tsys01_id[] = {
> +	{"tsys01", 0},
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, tsys01_id);
> +
> +static struct i2c_driver tsys01_driver = {
> +	.probe = tsys01_i2c_probe,
> +	.id_table = tsys01_id,
> +	.driver = {
> +		   .name = "tsys01",
> +		   },
> +};
> +
> +module_i2c_driver(tsys01_driver);
> +
> +MODULE_DESCRIPTION("Measurement-Specialties tsys01 temperature driver");
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
> +MODULE_LICENSE("GPL v2");
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 3/6] Add tsys02d meas-spec driver support
  2015-09-25 13:56 ` [PATCH v3 3/6] Add tsys02d " Ludovic Tancerel
@ 2015-09-27 17:51   ` Jonathan Cameron
  2015-09-29  9:40     ` ludovic.tancerel
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-27 17:51 UTC (permalink / raw)
  To: Ludovic Tancerel, knaack.h, lars, pmeerw, linux-iio, William.Markezana

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");
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 4/6] Add htu21 meas-spec driver support
  2015-09-25 13:56 ` [PATCH v3 4/6] Add htu21 " Ludovic Tancerel
@ 2015-09-27 17:54   ` Jonathan Cameron
  0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-27 17:54 UTC (permalink / raw)
  To: Ludovic Tancerel, knaack.h, lars, pmeerw, linux-iio, William.Markezana

On 25/09/15 14:56, Ludovic Tancerel wrote:
> Support for HTU21 temperature & humidity sensor
> 
> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
My only comment on this one is that its so similar (presumably because the
silicon is the same) to the tsy02 driver that I suspect someone else
may come along and merge the two drivers sometime in the future.  That
would save perhaps 100 lines of code.

I know you weren't keen to do this and I'm not that bothered but thought
I'd just mention it again :)

Anyhow, will let this sit for a little longer.

Jonathan
> ---
>  Documentation/ABI/testing/sysfs-bus-iio |  10 ++
>  drivers/iio/humidity/Kconfig            |  11 ++
>  drivers/iio/humidity/Makefile           |   1 +
>  drivers/iio/humidity/htu21.c            | 227 ++++++++++++++++++++++++++++++++
>  4 files changed, 249 insertions(+)
>  create mode 100644 drivers/iio/humidity/htu21.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 70c9b1a..aac134a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1461,3 +1461,13 @@ Description:
>  		measurements and return the average value as output data. Each
>  		value resulted from <type>[_name]_oversampling_ratio measurements
>  		is considered as one sample for <type>[_name]_sampling_frequency.
> +
> +What:           /sys/bus/iio/devices/iio:deviceX/heater_enable
> +KernelVersion:  4.1.0
> +Contact:        linux-iio@vger.kernel.org
> +Description:
> +                A '1' (enable) or '0' (disable) specifying the enable
> +		of heater function. Same reading values apply
> +		This ABI is especially applicable for humidity sensors
> +		to heatup the device and get rid of any condensation
> +		in some humidity environment
> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
> index 688c0d1..3fab60c 100644
> --- a/drivers/iio/humidity/Kconfig
> +++ b/drivers/iio/humidity/Kconfig
> @@ -12,6 +12,17 @@ config DHT11
>  	  Other sensors should work as well as long as they speak the
>  	  same protocol.
>  
> +config HTU21
> +	tristate "Measurement Specialties HTU21 humidity & temperature sensor"
> +	depends on I2C
> +        select IIO_MS_SENSORS_I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  HTU21 humidity and temperature sensor.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called htu21.
> +
>  config SI7005
>  	tristate "SI7005 relative humidity and temperature sensor"
>  	depends on I2C
> diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
> index 86e2d26..826b1d5 100644
> --- a/drivers/iio/humidity/Makefile
> +++ b/drivers/iio/humidity/Makefile
> @@ -3,5 +3,6 @@
>  #
>  
>  obj-$(CONFIG_DHT11) += dht11.o
> +obj-$(CONFIG_HTU21) += htu21.o
>  obj-$(CONFIG_SI7005) += si7005.o
>  obj-$(CONFIG_SI7020) += si7020.o
> diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
> new file mode 100644
> index 0000000..706faff
> --- /dev/null
> +++ b/drivers/iio/humidity/htu21.c
> @@ -0,0 +1,227 @@
> +/*
> + * htu21.c - Support for Measurement-Specialties
> + *           htu21 temperature & humidity sensor
> + *
> + * Copyright (c) 2014 Measurement-Specialties
> + *
> + * Licensed under the GPL-2.
> + *
> + * (7-bit I2C slave address 0x40)
> + *
> + * Datasheet:
> + *  http://www.meas-spec.com/downloads/HTU21D.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 HTU21_RESET				0xFE
> +
> +static const int htu21_samp_freq[4] = { 20, 40, 70, 120 };
> +/* String copy of the above const for readability purpose */
> +static const char htu21_show_samp_freq[] = "20 40 70 120";
> +
> +static int htu21_read_raw(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *channel, int *val,
> +			  int *val2, long mask)
> +{
> +	int ret, temperature;
> +	unsigned int humidity;
> +	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;
> +		case IIO_HUMIDITYRELATIVE:	/* in milli %RH */
> +			ret = ms_sensors_i2c_ht_read_humidity(dev_data,
> +							      &humidity);
> +			if (ret)
> +				return ret;
> +			*val = humidity;
> +
> +			return IIO_VAL_INT;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = htu21_samp_freq[dev_data->res_index];
> +
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int htu21_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(htu21_samp_freq);
> +		while (i-- > 0)
> +			if (val == htu21_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 htu21_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),
> +	 },
> +	{
> +		.type = IIO_HUMIDITYRELATIVE,
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	 }
> +};
> +
> +static ssize_t htu21_show_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 ssize_t htu21_show_heater(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_heater(dev_data, buf);
> +}
> +
> +static ssize_t htu21_write_heater(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct ms_ht_dev *dev_data = iio_priv(indio_dev);
> +
> +	return ms_sensors_i2c_write_heater(dev_data, buf, len);
> +}
> +
> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(htu21_show_samp_freq);
> +static IIO_DEVICE_ATTR(battery_low, S_IRUGO,
> +		       htu21_show_battery_low, NULL, 0);
> +static IIO_DEVICE_ATTR(heater_enable, S_IRUGO | S_IWUSR,
> +		       htu21_show_heater, htu21_write_heater, 0);
> +
> +static struct attribute *htu21_attributes[] = {
> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_battery_low.dev_attr.attr,
> +	&iio_dev_attr_heater_enable.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group htu21_attribute_group = {
> +	.attrs = htu21_attributes,
> +};
> +
> +static const struct iio_info htu21_info = {
> +	.read_raw = htu21_read_raw,
> +	.write_raw = htu21_write_raw,
> +	.attrs = &htu21_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +int htu21_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 = &htu21_info;
> +	indio_dev->name = id->name;
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = htu21_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	ret = ms_sensors_i2c_reset(client, HTU21_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 htu21_id[] = {
> +	{"htu21", 0},
> +	{}
> +};
> +
> +static struct i2c_driver htu21_driver = {
> +	.probe = htu21_probe,
> +	.id_table = htu21_id,
> +	.driver = {
> +		   .name = "htu21",
> +		   },
> +};
> +
> +module_i2c_driver(htu21_driver);
> +
> +MODULE_DESCRIPTION("Measurement-Specialties htu21 temperature and humidity driver");
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
> +MODULE_LICENSE("GPL v2");
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 5/6] Add ms5637 meas-spec driver support
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-27 17:57 UTC (permalink / raw)
  To: Ludovic Tancerel, knaack.h, lars, pmeerw, linux-iio, William.Markezana

On 25/09/15 14:56, Ludovic Tancerel wrote:
> Support for MS5637 temperature & pressure sensor
> 
> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
Only issue here a separate typo fix seems to have snuck in here.

All good, but should be a separate patch so please break that out for the
next version.

Thanks,

Jonathan
> ---
>  drivers/iio/pressure/Kconfig  |  15 +++-
>  drivers/iio/pressure/Makefile |   1 +
>  drivers/iio/pressure/ms5637.c | 187 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 201 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/iio/pressure/ms5637.c
> 
> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> index fa62950..8142cfe 100644
> --- a/drivers/iio/pressure/Kconfig
> +++ b/drivers/iio/pressure/Kconfig
> @@ -53,9 +53,9 @@ config MPL3115
>            will be called mpl3115.
>  
>  config MS5611
> -	tristate "Measurement Specialities MS5611 pressure sensor driver"
> +	tristate "Measurement Specialties MS5611 pressure sensor driver"
>  	help
> -	  Say Y here to build support for the Measurement Specialities
> +	  Say Y here to build support for the Measurement Specialties
>  	  MS5611 pressure and temperature sensor.
THis looks to have snuck into the wrong patch to me...
>  
>  	  To compile this driver as a module, choose M here: the module will
> @@ -79,6 +79,17 @@ config MS5611_SPI
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called ms5611_spi.
>  
> +config MS5637
> +	tristate "Measurement Specialties MS5637 pressure & temperature sensor"
> +	depends on I2C
> +        select IIO_MS_SENSORS_I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  MS5637 pressure and temperature sensor.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called ms5637.
> +
>  config IIO_ST_PRESS
>  	tristate "STMicroelectronics pressure sensor Driver"
>  	depends on (I2C || SPI_MASTER) && SYSFS
> diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
> index a4f98f8..46571c96 100644
> --- a/drivers/iio/pressure/Makefile
> +++ b/drivers/iio/pressure/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_MPL3115) += mpl3115.o
>  obj-$(CONFIG_MS5611) += ms5611_core.o
>  obj-$(CONFIG_MS5611_I2C) += ms5611_i2c.o
>  obj-$(CONFIG_MS5611_SPI) += ms5611_spi.o
> +obj-$(CONFIG_MS5637) += ms5637.o
>  obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o
>  st_pressure-y := st_pressure_core.o
>  st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o
> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
> new file mode 100644
> index 0000000..0dbbd4e
> --- /dev/null
> +++ b/drivers/iio/pressure/ms5637.c
> @@ -0,0 +1,187 @@
> +/*
> + * ms5637.c - Support for Measurement-Specialties ms5637
> + *            pressure & temperature sensor
> + *
> + * Copyright (c) 2015 Measurement-Specialties
> + *
> + * Licensed under the GPL-2.
> + *
> + * (7-bit I2C slave address 0x76)
> + *
> + * Datasheet:
> + *  http://www.meas-spec.com/downloads/MS5637-02BA03.pdf
> + *
> + */
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/stat.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/mutex.h>
> +
> +#include "../common/ms_sensors/ms_sensors_i2c.h"
> +
> +static const int ms5637_samp_freq[6] = { 960, 480, 240, 120, 60, 30 };
> +/* String copy of the above const for readability purpose */
> +static const char ms5637_show_samp_freq[] = "960 480 240 120 60 30";
> +
> +static int ms5637_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *channel, int *val,
> +			   int *val2, long mask)
> +{
> +	int ret;
> +	int temperature;
> +	unsigned int pressure;
> +	struct ms_tp_dev *dev_data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED:
> +		ret = ms_sensors_read_temp_and_pressure(dev_data,
> +							&temperature,
> +							&pressure);
> +		if (ret)
> +			return ret;
> +
> +		switch (channel->type) {
> +		case IIO_TEMP:	/* in milli °C */
> +			*val = temperature;
> +
> +			return IIO_VAL_INT;
> +		case IIO_PRESSURE:	/* in kPa */
> +			*val = pressure / 1000;
> +			*val2 = (pressure % 1000) * 1000;
> +
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = ms5637_samp_freq[dev_data->res_index];
> +
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int ms5637_write_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int val, int val2, long mask)
> +{
> +	struct ms_tp_dev *dev_data = iio_priv(indio_dev);
> +	int i;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		i = ARRAY_SIZE(ms5637_samp_freq);
> +		while (i-- > 0)
> +			if (val == ms5637_samp_freq[i])
> +				break;
> +		if (i < 0)
> +			return -EINVAL;
> +		dev_data->res_index = i;
> +
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_chan_spec ms5637_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	},
> +	{
> +		.type = IIO_PRESSURE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	}
> +};
> +
> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(ms5637_show_samp_freq);
> +
> +static struct attribute *ms5637_attributes[] = {
> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group ms5637_attribute_group = {
> +	.attrs = ms5637_attributes,
> +};
> +
> +static const struct iio_info ms5637_info = {
> +	.read_raw = ms5637_read_raw,
> +	.write_raw = ms5637_write_raw,
> +	.attrs = &ms5637_attribute_group,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int ms5637_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct ms_tp_dev *dev_data;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_WORD_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 = 5;
> +	mutex_init(&dev_data->lock);
> +
> +	indio_dev->info = &ms5637_info;
> +	indio_dev->name = id->name;
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = ms5637_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(ms5637_channels);
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	ret = ms_sensors_i2c_reset(client, 0x1E, 3000);
> +	if (ret)
> +		return ret;
> +
> +	ret = ms_sensors_tp_read_prom(dev_data);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id ms5637_id[] = {
> +	{"ms5637", 0},
> +	{}
> +};
> +
> +static struct i2c_driver ms5637_driver = {
> +	.probe = ms5637_probe,
> +	.id_table = ms5637_id,
> +	.driver = {
> +		   .name = "ms5637"
> +		   },
> +};
> +
> +module_i2c_driver(ms5637_driver);
> +
> +MODULE_DESCRIPTION("Measurement-Specialties ms5637 temperature & pressure driver");
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
> +MODULE_LICENSE("GPL v2");
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 6/6] Add ms8607 meas-spec driver support
  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
  0 siblings, 1 reply; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-27 18:00 UTC (permalink / raw)
  To: Ludovic Tancerel, knaack.h, lars, pmeerw, linux-iio, William.Markezana

On 25/09/15 14:56, Ludovic Tancerel wrote:
> Support for MS8607 temperature, pressure & humidity sensor.
> This part is using functions from MS5637 for temperature and pressure
> and HTU21 for humidity
> 
> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
As I commented (early today) in response to the previous thread, I'd prefer
a little more than -h and -tp in the naming but otherwise this is good.

btw. Having added the tiny amount of infrastructure needed to do the different
device support in htu21 you could easily add the temperature part as well as
another option :)  Just saying...

Anyhow, the only real issues other than a bit of reorganization to split
the typo fix out of the previous patch are little things in patch 1.

Get those sorted and we can get this lot merged in plenty of time for
the next merge window.

Thanks,

Jonathan
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-meas-spec |  1 +
>  drivers/iio/humidity/Kconfig                      |  2 ++
>  drivers/iio/humidity/htu21.c                      | 33 ++++++++++++++++++++---
>  drivers/iio/pressure/Kconfig                      |  2 ++
>  drivers/iio/pressure/ms5637.c                     |  6 ++++-
>  5 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
> index 6d47e54..1a6265e 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
> @@ -5,3 +5,4 @@ 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
> +		This ABI is available for htu21, ms8607
> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
> index 3fab60c..75ca043 100644
> --- a/drivers/iio/humidity/Kconfig
> +++ b/drivers/iio/humidity/Kconfig
> @@ -19,6 +19,8 @@ config HTU21
>  	help
>  	  If you say yes here you get support for the Measurement Specialties
>  	  HTU21 humidity and temperature sensor.
> +	  This driver is also used for MS8607 temperature, pressure & humidity
> +	  sensor
>  
>  	  This driver can also be built as a module. If so, the module will
>  	  be called htu21.
> diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
> index 706faff..0530545 100644
> --- a/drivers/iio/humidity/htu21.c
> +++ b/drivers/iio/humidity/htu21.c
> @@ -1,6 +1,7 @@
>  /*
>   * htu21.c - Support for Measurement-Specialties
>   *           htu21 temperature & humidity sensor
> + *	     and humidity part of MS8607 sensor
>   *
>   * Copyright (c) 2014 Measurement-Specialties
>   *
> @@ -10,6 +11,8 @@
>   *
>   * Datasheet:
>   *  http://www.meas-spec.com/downloads/HTU21D.pdf
> + * Datasheet:
> + *  http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
>   *
>   */
>  
> @@ -25,6 +28,11 @@
>  
>  #define HTU21_RESET				0xFE
>  
> +enum {
> +	HTU21,
> +	MS8607
> +};
> +
>  static const int htu21_samp_freq[4] = { 20, 40, 70, 120 };
>  /* String copy of the above const for readability purpose */
>  static const char htu21_show_samp_freq[] = "20 40 70 120";
> @@ -107,6 +115,18 @@ static const struct iio_chan_spec htu21_channels[] = {
>  	 }
>  };
>  
> +/*
> + * Meas Spec recommendation is to not read temperature
> + * on this driver part for MS8607
> + */
> +static const struct iio_chan_spec ms8607_channels[] = {
> +	{
> +		.type = IIO_HUMIDITYRELATIVE,
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	 }
> +};
> +
>  static ssize_t htu21_show_battery_low(struct device *dev,
>  				      struct device_attribute *attr, char *buf)
>  {
> @@ -189,8 +209,14 @@ int htu21_probe(struct i2c_client *client,
>  	indio_dev->name = id->name;
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> -	indio_dev->channels = htu21_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
> +
> +	if (id->driver_data == MS8607) {
> +		indio_dev->channels = ms8607_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(ms8607_channels);
> +	} else {
> +		indio_dev->channels = htu21_channels;
> +		indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
> +	}
>  
>  	i2c_set_clientdata(client, indio_dev);
>  
> @@ -207,7 +233,8 @@ int htu21_probe(struct i2c_client *client,
>  }
>  
>  static const struct i2c_device_id htu21_id[] = {
> -	{"htu21", 0},
> +	{"htu21", HTU21},
> +	{"ms8607-h", MS8607},
>  	{}
>  };
>  
> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> index 8142cfe..53f9a44 100644
> --- a/drivers/iio/pressure/Kconfig
> +++ b/drivers/iio/pressure/Kconfig
> @@ -86,6 +86,8 @@ config MS5637
>  	help
>  	  If you say yes here you get support for the Measurement Specialties
>  	  MS5637 pressure and temperature sensor.
> +	  This driver is also used for MS8607 temperature, pressure & humidity
> +	  sensor
>  
>  	  This driver can also be built as a module. If so, the module will
>  	  be called ms5637.
> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
> index 0dbbd4e..f6b5544 100644
> --- a/drivers/iio/pressure/ms5637.c
> +++ b/drivers/iio/pressure/ms5637.c
> @@ -1,5 +1,5 @@
>  /*
> - * ms5637.c - Support for Measurement-Specialties ms5637
> + * ms5637.c - Support for Measurement-Specialties ms5637 and ms8607
>   *            pressure & temperature sensor
>   *
>   * Copyright (c) 2015 Measurement-Specialties
> @@ -10,8 +10,11 @@
>   *
>   * Datasheet:
>   *  http://www.meas-spec.com/downloads/MS5637-02BA03.pdf
> + * Datasheet:
> + *  http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
>   *
>   */
> +
>  #include <linux/init.h>
>  #include <linux/device.h>
>  #include <linux/kernel.h>
> @@ -168,6 +171,7 @@ static int ms5637_probe(struct i2c_client *client,
>  
>  static const struct i2c_device_id ms5637_id[] = {
>  	{"ms5637", 0},
> +	{"ms8607-tp", 1},
>  	{}
>  };
>  
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/6] Add meas-spec sensors common part
  2015-09-27 16:23   ` Jonathan Cameron
@ 2015-09-29  7:59     ` ludovic.tancerel
  2015-09-29  8:03       ` ludovic.tancerel
  0 siblings, 1 reply; 22+ messages in thread
From: ludovic.tancerel @ 2015-09-29  7:59 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: knaack.h, lars, pmeerw, linux-iio, William.Markezana

[-- Attachment #1: Type: text/plain, Size: 13390 bytes --]

Hello Jonathan, all,
thank you for reviewing

for once, I will reply shortly.
Hopefully, the message will go through the mail server.

Please have a look at comments below,
Regards,
Ludovic


Le 27 sept. 2015 à 18:23, Jonathan Cameron <jic23@kernel.org> a écrit :

> On 25/09/15 14:56, Ludovic Tancerel wrote:
>> Measurement specialties drivers common part.
>> These functions are used by further drivers in the patchset: TSYS01, TSYS02D, HTU21, MS5637, MS8607
>> 
>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
> A few more bits and bobs inline.
>> ---
>> drivers/iio/common/Kconfig                     |   1 +
>> drivers/iio/common/Makefile                    |   1 +
>> drivers/iio/common/ms_sensors/Kconfig          |   6 +
>> drivers/iio/common/ms_sensors/Makefile         |   5 +
>> drivers/iio/common/ms_sensors/ms_sensors_i2c.c | 645 +++++++++++++++++++++++++
>> drivers/iio/common/ms_sensors/ms_sensors_i2c.h |  53 ++
>> 6 files changed, 711 insertions(+)
>> create mode 100644 drivers/iio/common/ms_sensors/Kconfig
>> create mode 100644 drivers/iio/common/ms_sensors/Makefile
>> create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>> create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>> 
>> diff —git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
…
>> 
>> diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>> new file mode 100644
>> index 0000000..4b1bc31
>> --- /dev/null
>> +++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>> @@ -0,0 +1,645 @@
>> +/*
>> + * Measurements Specialties driver common i2c functions
>> + *
>> + * Copyright (c) 2015 Measurement-Specialties
>> + *
>> + * Licensed under the GPL-2.
> Drop this blank line.
OK
>> + *
>> + */
>> +
…
>> +
>> +/**
>> + * ms_sensors_i2c_read_prom_word() - i2c prom word read function
>> + * @cli:	pointer to i2c client
>> + * @cmd:	PROM read cmd. Depends on device and prom id
>> + * @word:	pointer to word destination value
>> + *
>> + * Generic i2c prom word read function for Measurement Specialties devices.
>> + *
>> + * Return: 0 on success, negative errno otherwise.
>> + */
>> +int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word)
>> +{
>> +	int ret;
>> +	struct i2c_client *client = (struct i2c_client *)cli;
> Why pass an void * given the function name says this will always be an i2c_client?
> 
> Once that's removed this wrapper does very little and I'd be tempted to
> drop it in favour of direct calls to the smbus read.

The void * is because this function might be used for the same chipset using SPI access (that is not supported yet).
I am passing the void *, to have common parameters for the future spi function.
This is the same for previous function ms_sensors_i2c_reset.

That references functions written for tsys01 written in patch v2/6 :
"
	dev_data->client = client;
	dev_data->reset = ms_sensors_i2c_reset;
	dev_data->read_prom_word = ms_sensors_i2c_read_prom_word;
	dev_data->convert_and_read = ms_sensors_i2c_convert_and_read;
"
The same could be done but using a spi function. Isn’t that the appropriate approach ?
This is what I understood when looking at other drivers.

These functions are also used in different drivers, so if I drop t he wrapper, I cannot reuse the function for TSYS01.

>> +
>> +	ret = i2c_smbus_read_word_swapped(client, cmd);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "Failed to read prom word\n");
>> +		return ret;
>> +	}
>> +	*word = ret;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(ms_sensors_i2c_read_prom_word);
>> +
>> +/**
>> + * ms_sensors_i2c_convert_and_read() - i2c ADC conversion & read function
>> + * @cli:	pointer to i2c client
>> + * @conv:	ADC conversion command. Depends on device in use
>> + * @rd:		ADC read command. Depends on device in use
>> + * @delay:	usleep minimal delay after conversion command is issued
>> + * @adc:	pointer to ADC destination value
>> + *
>> + * Generic i2c ADC conversion & read function for Measurement Specialties
>> + * devices.
>> + * The function will issue conversion command, sleep appopriate delay, and
>> + * issue command to read ADC.
>> + *
>> + * Return: 0 on success, negative errno otherwise.
>> + */
>> +int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
>> +				    unsigned int delay, u32 *adc)
>> +{
>> +	int ret;
>> +	u32 buf = 0;
>> +	struct i2c_client *client = (struct i2c_client *)cli;
>> +
>> +	/* Trigger conversion */
>> +	ret = i2c_smbus_write_byte(client, conv);
>> +	if (ret)
>> +		goto err;
>> +	usleep_range(delay, delay + 1000);
>> +
>> +	/* Retrieve ADC value */
>> +	if (rd != MS_SENSORS_NO_READ_CMD)
>> +		ret = i2c_smbus_read_i2c_block_data(client, rd, 3, (u8 *)&buf);
>> +	else
>> +		ret = i2c_master_recv(client, (u8 *)&buf, 3);
>> +	if (ret < 0)
>> +		goto err;
>> +
>> +	dev_dbg(&client->dev, "ADC raw value : %x\n", be32_to_cpu(buf) >> 8);
>> +	*adc = be32_to_cpu(buf) >> 8;
>> +
>> +	return 0;
>> +err:
>> +	dev_err(&client->dev, "Unable to make sensor adc conversion\n");
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(ms_sensors_i2c_convert_and_read);
>> +
>> +/**
>> + * ms_sensors_crc_valid() - CRC check function
>> + * @value:	input and CRC compare value
>> + *
>> + * Cyclic Redundancy Check function used in TSYS02D, HTU21, MS8607.
>> + * This function performs a x^8 + x^5 + x^4 + 1 polynomial CRC.
>> + * The argument contains CRC value in LSB byte while the bytes 1 and 2
>> + * are used for CRC computation.
>> + *
>> + * Return: 1 if CRC is valid, 0 otherwise.
> Can this be done with the standard kernel crc32 functions? (not dug into
> it enough to be sure!)

I don’t think so.
CRC computation here is truly a 16-bits CRC check.
I have looked at crc16 lib but this a different polynomial used.

>> + */
>> +static bool ms_sensors_crc_valid(u32 value)
>> +{
>> +	u32 polynom = 0x988000;	/* x^8 + x^5 + x^4 + 1 */
>> +	u32 msb = 0x800000;
>> +	u32 mask = 0xFF8000;
>> +	u32 result = value & 0xFFFF00;
>> +	u8 crc = value & 0xFF;
>> +
>> +	while (msb != 0x80) {
>> +		if (result & msb)
>> +			result = ((result ^ polynom) & mask)
>> +				| (result & ~mask);
>> +		msb >>= 1;
>> +		mask >>= 1;
>> +		polynom >>= 1;
>> +	}
>> +
>> +	return result == crc;
>> +}
>> +
>> +/**
>> + * ms_sensors_i2c_read_serial() - i2c serial number read function
>> + * @cli:	pointer to i2c client
>> + * @sn:		pointer to 64-bits destination value
>> + *
>> + * Generic i2c serial number read function for Measurement Specialties devices.
>> + * This function is used for TSYS02d, HTU21, MS8607 chipset.
>> + * Refer to datasheet:
>> + *	http://www.meas-spec.com/downloads/HTU2X_Serial_Number_Reading.pdf
> THat's a first.  A whole datasheet on how the heck the serial number works!
> 
> Got to wonder how anyone ever came up with that.  I'm going to conclude
> you got it right and not read any more ;)

Please read below.
If you read the spec, you will understand why there is one,
the Serial Number shape is really far fetched … :)

>> + *
>> + * Return: 0 on success, negative errno otherwise.
>> + */
>> +int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn)
>> +{
>> +	u8 i;
>> +	u64 rcv_buf = 0;
>> +	u16 send_buf;
>> +	int ret;
>> +
>> +	struct i2c_msg msg[2] = {
>> +		{
>> +		 .addr = client->addr,
>> +		 .flags = client->flags,
>> +		 .len = 2,
>> +		 .buf = (__u8 *)&send_buf,
>> +		 },
>> +		{
>> +		 .addr = client->addr,
>> +		 .flags = client->flags | I2C_M_RD,
>> +		 .buf = (__u8 *)&rcv_buf,
>> +		 },
>> +	};
>> +
>> +	/* Read MSB part of serial number */
>> +	send_buf = cpu_to_be16(MS_SENSORS_SERIAL_READ_MSB);
>> +	msg[1].len = 8;
>> +	ret = i2c_transfer(client->adapter, msg, 2);
>> +	if (ret < 0) {
>> +		dev_err(&client->dev, "Unable to read device serial number");
>> +		return ret;
>> +	}
>> +
>> +	rcv_buf = be64_to_cpu(rcv_buf);
>> +	dev_dbg(&client->dev, "Serial MSB raw : %llx\n", rcv_buf);
>> +
>> +	for (i = 0; i < 64; i += 16) {
>> +		if (!ms_sensors_crc_valid((rcv_buf >> i) & 0xFFFF))
>> +			return -ENODEV;
>> +	}
>> +
> Umm. I'm really unclear what you are doing here.  You read into
> a 64 bit buffer, then do a hand endian swap and shift?  Should be possible
> to at least use standard functions to swap the byte order.

There are several 8bits CRC interlaced in the bytes read containing serial number.
So the "for (i = 0; i < 64; i += 16)" is to check the different CRCs

The « *sn = … » is meant to regroup the bytes in between CRC words and retrieve the 32bits first portion of the serial number.
The shape is [ SNB 3 , CRC, SNB 2, CRC, SNB 1, CRC, SNB 0, CRC ]
I considered unifying both in the loop but this is not simpler

The second portion of serial number is read afterwards.

I agree this is overly complex for getting a serial number,
but this is how it is implemented in the sensor, and it is a wish from measurement specialties to read it at sensor init.

If you feel I should extract the SN bytes in a different way on the code below, let me know.
I can put it into a macro to improve readability ?

> 
>> +	*sn = (((rcv_buf >> 32) & 0xFF000000) |
>> +	       ((rcv_buf >> 24) & 0x00FF0000) |
>> +	       ((rcv_buf >> 16) & 0x0000FF00) |
>> +	       ((rcv_buf >> 8) & 0x000000FF)) << 16;
>> +
>> 
…

>> +int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
>> +				      int *temperature,
>> +				      unsigned int *pressure)
>> +{
>> +	int ret;
>> +	u32 t_adc, p_adc;
>> +	s32 dt, temp;
>> +	s64 off, sens, t2, off2, sens2;
>> +	u16 *prom = dev_data->prom, delay;
>> +	u8 i;
>> +
>> +	mutex_lock(&dev_data->lock);
>> +	i = dev_data->res_index * 2;
> This local variable seens rather unnecessary. Just put it inline in the
> calls.

OK

>> +	delay = ms_sensors_tp_conversion_time[dev_data->res_index];
>> +
>> +	ret = ms_sensors_i2c_convert_and_read(
>> +					dev_data->client,
>> +					MS_SENSORS_TP_T_CONVERSION_START + i,
>> +					MS_SENSORS_TP_ADC_READ,
>> +					delay, &t_adc);
>> +	if (ret) {
>> +		mutex_unlock(&dev_data->lock);
>> +		return ret;
>> +	}
>> +
>> +	

…

>> +
>> diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.h b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>> new file mode 100644
>> index 0000000..918b8af
>> --- /dev/null
>> +++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>> @@ -0,0 +1,53 @@
>> +/*
>> + * Measurements Specialties common sensor driver
>> + *
>> + * Copyright (c) 2015 Measurement-Specialties
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef _MS_SENSORS_I2C_H
>> +#define _MS_SENSORS_I2C_H
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/mutex.h>
>> +
>> +#define MS_SENSORS_TP_PROM_WORDS_NB		7
>> +
>> +struct ms_ht_dev {
>> +	struct i2c_client *client;
>> +	struct mutex lock; /* mutex protecting data structure */
>> +	u8 res_index;
>> +};
>> +
> kernel doc comments preferred.

OK
>> +struct ms_tp_dev {
>> +	struct i2c_client *client;
>> +	struct mutex lock; /* mutex protecting data structure */
>> +	/* Added element for CRC computation */
>> +	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB + 1];
>> +	u8 res_index;
>> +};
>> +
>> +int ms_sensors_i2c_reset(void *cli, u8 cmd, unsigned int delay);
>> +int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word);
>> +int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
>> +				    unsigned int delay, u32 *adc);
>> +int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn);
>> +ssize_t ms_sensors_i2c_show_serial(struct ms_ht_dev *dev_data, char *buf);
>> +ssize_t ms_sensors_i2c_write_resolution(struct ms_ht_dev *dev_data, u8 i);
>> +ssize_t ms_sensors_i2c_show_battery_low(struct ms_ht_dev *dev_data, char *buf);
>> +ssize_t ms_sensors_i2c_show_heater(struct ms_ht_dev *dev_data, char *buf);
>> +ssize_t ms_sensors_i2c_write_heater(struct ms_ht_dev *dev_data,
>> +				    const char *buf, size_t len);
>> +int ms_sensors_i2c_ht_read_temperature(struct ms_ht_dev *dev_data,
>> +				       s32 *temperature);
>> +int ms_sensors_i2c_ht_read_humidity(struct ms_ht_dev *dev_data,
>> +				    u32 *humidity);
>> +int ms_sensors_tp_read_prom(struct ms_tp_dev *dev_data);
>> +int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
>> +				      int *temperature,
>> +				      unsigned int *pressure);
> The presence or absense of _i2c in the function naming does feel rather
> random.  All of these functions ultimately make i2c calls so perhaps you
> can explain your reasoning behind having it in some and not others.

The functions used were meant to have _i2c when there is no direct calls to  smbus functions.
I agree that is rather awkward, I will change and put i2c everywhere.

> 
>> +
>> +#endif /* _MS_SENSORS_I2C_H */
>> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[-- Attachment #2: Type: text/html, Size: 34360 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/6] Add meas-spec sensors common part
  2015-09-29  7:59     ` ludovic.tancerel
@ 2015-09-29  8:03       ` ludovic.tancerel
  2015-09-29 17:20         ` Jonathan Cameron
  0 siblings, 1 reply; 22+ messages in thread
From: ludovic.tancerel @ 2015-09-29  8:03 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio,
	William.Markezana

Resent with formatting changes in my mail application,
as it seems some html makes the server throw it away.

Ludovic

> Hello Jonathan, all,
> thank you for reviewing
> 
> for once, I will reply shortly.
> Hopefully, the message will go through the mail server.
> 
> Please have a look at comments below,
> Regards,
> Ludovic
> 
> 
> Le 27 sept. 2015 à 18:23, Jonathan Cameron <jic23@kernel.org> a écrit :
> 
>> On 25/09/15 14:56, Ludovic Tancerel wrote:
>>> Measurement specialties drivers common part.
>>> These functions are used by further drivers in the patchset: TSYS01, TSYS02D, HTU21, MS5637, MS8607
>>> 
>>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
>> A few more bits and bobs inline.
>>> ---
>>> drivers/iio/common/Kconfig                     |   1 +
>>> drivers/iio/common/Makefile                    |   1 +
>>> drivers/iio/common/ms_sensors/Kconfig          |   6 +
>>> drivers/iio/common/ms_sensors/Makefile         |   5 +
>>> drivers/iio/common/ms_sensors/ms_sensors_i2c.c | 645 +++++++++++++++++++++++++
>>> drivers/iio/common/ms_sensors/ms_sensors_i2c.h |  53 ++
>>> 6 files changed, 711 insertions(+)
>>> create mode 100644 drivers/iio/common/ms_sensors/Kconfig
>>> create mode 100644 drivers/iio/common/ms_sensors/Makefile
>>> create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>>> create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>>> 
>>> diff —git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> …
>>> 
>>> diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>>> new file mode 100644
>>> index 0000000..4b1bc31
>>> --- /dev/null
>>> +++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>>> @@ -0,0 +1,645 @@
>>> +/*
>>> + * Measurements Specialties driver common i2c functions
>>> + *
>>> + * Copyright (c) 2015 Measurement-Specialties
>>> + *
>>> + * Licensed under the GPL-2.
>> Drop this blank line.
> OK
>>> + *
>>> + */
>>> +
> …
>>> +
>>> +/**
>>> + * ms_sensors_i2c_read_prom_word() - i2c prom word read function
>>> + * @cli:	pointer to i2c client
>>> + * @cmd:	PROM read cmd. Depends on device and prom id
>>> + * @word:	pointer to word destination value
>>> + *
>>> + * Generic i2c prom word read function for Measurement Specialties devices.
>>> + *
>>> + * Return: 0 on success, negative errno otherwise.
>>> + */
>>> +int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word)
>>> +{
>>> +	int ret;
>>> +	struct i2c_client *client = (struct i2c_client *)cli;
>> Why pass an void * given the function name says this will always be an i2c_client?
>> 
>> Once that's removed this wrapper does very little and I'd be tempted to
>> drop it in favour of direct calls to the smbus read.
> 
> The void * is because this function might be used for the same chipset using SPI access (that is not supported yet).
> I am passing the void *, to have common parameters for the future spi function.
> This is the same for previous function ms_sensors_i2c_reset.
> 
> That references functions written for tsys01 written in patch v2/6 :
> "
> 	dev_data->client = client;
> 	dev_data->reset = ms_sensors_i2c_reset;
> 	dev_data->read_prom_word = ms_sensors_i2c_read_prom_word;
> 	dev_data->convert_and_read = ms_sensors_i2c_convert_and_read;
> "
> The same could be done but using a spi function. Isn’t that the appropriate approach ?
> This is what I understood when looking at other drivers.
> 
> These functions are also used in different drivers, so if I drop t he wrapper, I cannot reuse the function for TSYS01.
> 
>>> +
>>> +	ret = i2c_smbus_read_word_swapped(client, cmd);
>>> +	if (ret < 0) {
>>> +		dev_err(&client->dev, "Failed to read prom word\n");
>>> +		return ret;
>>> +	}
>>> +	*word = ret;
>>> +
>>> +	return 0;
>>> +}
>>> +EXPORT_SYMBOL(ms_sensors_i2c_read_prom_word);
>>> +
>>> +/**
>>> + * ms_sensors_i2c_convert_and_read() - i2c ADC conversion & read function
>>> + * @cli:	pointer to i2c client
>>> + * @conv:	ADC conversion command. Depends on device in use
>>> + * @rd:		ADC read command. Depends on device in use
>>> + * @delay:	usleep minimal delay after conversion command is issued
>>> + * @adc:	pointer to ADC destination value
>>> + *
>>> + * Generic i2c ADC conversion & read function for Measurement Specialties
>>> + * devices.
>>> + * The function will issue conversion command, sleep appopriate delay, and
>>> + * issue command to read ADC.
>>> + *
>>> + * Return: 0 on success, negative errno otherwise.
>>> + */
>>> +int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
>>> +				    unsigned int delay, u32 *adc)
>>> +{
>>> +	int ret;
>>> +	u32 buf = 0;
>>> +	struct i2c_client *client = (struct i2c_client *)cli;
>>> +
>>> +	/* Trigger conversion */
>>> +	ret = i2c_smbus_write_byte(client, conv);
>>> +	if (ret)
>>> +		goto err;
>>> +	usleep_range(delay, delay + 1000);
>>> +
>>> +	/* Retrieve ADC value */
>>> +	if (rd != MS_SENSORS_NO_READ_CMD)
>>> +		ret = i2c_smbus_read_i2c_block_data(client, rd, 3, (u8 *)&buf);
>>> +	else
>>> +		ret = i2c_master_recv(client, (u8 *)&buf, 3);
>>> +	if (ret < 0)
>>> +		goto err;
>>> +
>>> +	dev_dbg(&client->dev, "ADC raw value : %x\n", be32_to_cpu(buf) >> 8);
>>> +	*adc = be32_to_cpu(buf) >> 8;
>>> +
>>> +	return 0;
>>> +err:
>>> +	dev_err(&client->dev, "Unable to make sensor adc conversion\n");
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL(ms_sensors_i2c_convert_and_read);
>>> +
>>> +/**
>>> + * ms_sensors_crc_valid() - CRC check function
>>> + * @value:	input and CRC compare value
>>> + *
>>> + * Cyclic Redundancy Check function used in TSYS02D, HTU21, MS8607.
>>> + * This function performs a x^8 + x^5 + x^4 + 1 polynomial CRC.
>>> + * The argument contains CRC value in LSB byte while the bytes 1 and 2
>>> + * are used for CRC computation.
>>> + *
>>> + * Return: 1 if CRC is valid, 0 otherwise.
>> Can this be done with the standard kernel crc32 functions? (not dug into
>> it enough to be sure!)
> 
> I don’t think so.
> CRC computation here is truly a 16-bits CRC check.
> I have looked at crc16 lib but this a different polynomial used.
> 
>>> + */
>>> +static bool ms_sensors_crc_valid(u32 value)
>>> +{
>>> +	u32 polynom = 0x988000;	/* x^8 + x^5 + x^4 + 1 */
>>> +	u32 msb = 0x800000;
>>> +	u32 mask = 0xFF8000;
>>> +	u32 result = value & 0xFFFF00;
>>> +	u8 crc = value & 0xFF;
>>> +
>>> +	while (msb != 0x80) {
>>> +		if (result & msb)
>>> +			result = ((result ^ polynom) & mask)
>>> +				| (result & ~mask);
>>> +		msb >>= 1;
>>> +		mask >>= 1;
>>> +		polynom >>= 1;
>>> +	}
>>> +
>>> +	return result == crc;
>>> +}
>>> +
>>> +/**
>>> + * ms_sensors_i2c_read_serial() - i2c serial number read function
>>> + * @cli:	pointer to i2c client
>>> + * @sn:		pointer to 64-bits destination value
>>> + *
>>> + * Generic i2c serial number read function for Measurement Specialties devices.
>>> + * This function is used for TSYS02d, HTU21, MS8607 chipset.
>>> + * Refer to datasheet:
>>> + *	http://www.meas-spec.com/downloads/HTU2X_Serial_Number_Reading.pdf
>> THat's a first.  A whole datasheet on how the heck the serial number works!
>> 
>> Got to wonder how anyone ever came up with that.  I'm going to conclude
>> you got it right and not read any more ;)
> 
> Please read below.
> If you read the spec, you will understand why there is one,
> the Serial Number shape is really far fetched … :)
> 
>>> + *
>>> + * Return: 0 on success, negative errno otherwise.
>>> + */
>>> +int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn)
>>> +{
>>> +	u8 i;
>>> +	u64 rcv_buf = 0;
>>> +	u16 send_buf;
>>> +	int ret;
>>> +
>>> +	struct i2c_msg msg[2] = {
>>> +		{
>>> +		 .addr = client->addr,
>>> +		 .flags = client->flags,
>>> +		 .len = 2,
>>> +		 .buf = (__u8 *)&send_buf,
>>> +		 },
>>> +		{
>>> +		 .addr = client->addr,
>>> +		 .flags = client->flags | I2C_M_RD,
>>> +		 .buf = (__u8 *)&rcv_buf,
>>> +		 },
>>> +	};
>>> +
>>> +	/* Read MSB part of serial number */
>>> +	send_buf = cpu_to_be16(MS_SENSORS_SERIAL_READ_MSB);
>>> +	msg[1].len = 8;
>>> +	ret = i2c_transfer(client->adapter, msg, 2);
>>> +	if (ret < 0) {
>>> +		dev_err(&client->dev, "Unable to read device serial number");
>>> +		return ret;
>>> +	}
>>> +
>>> +	rcv_buf = be64_to_cpu(rcv_buf);
>>> +	dev_dbg(&client->dev, "Serial MSB raw : %llx\n", rcv_buf);
>>> +
>>> +	for (i = 0; i < 64; i += 16) {
>>> +		if (!ms_sensors_crc_valid((rcv_buf >> i) & 0xFFFF))
>>> +			return -ENODEV;
>>> +	}
>>> +
>> Umm. I'm really unclear what you are doing here.  You read into
>> a 64 bit buffer, then do a hand endian swap and shift?  Should be possible
>> to at least use standard functions to swap the byte order.
> 
> There are several 8bits CRC interlaced in the bytes read containing serial number.
> So the "for (i = 0; i < 64; i += 16)" is to check the different CRCs
> 
> The « *sn = … » is meant to regroup the bytes in between CRC words and retrieve the 32bits first portion of the serial number.
> The shape is [ SNB 3 , CRC, SNB 2, CRC, SNB 1, CRC, SNB 0, CRC ]
> I considered unifying both in the loop but this is not simpler
> 
> The second portion of serial number is read afterwards.
> 
> I agree this is overly complex for getting a serial number,
> but this is how it is implemented in the sensor, and it is a wish from measurement specialties to read it at sensor init.
> 
> If you feel I should extract the SN bytes in a different way on the code below, let me know.
> I can put it into a macro to improve readability ?
> 
>> 
>>> +	*sn = (((rcv_buf >> 32) & 0xFF000000) |
>>> +	       ((rcv_buf >> 24) & 0x00FF0000) |
>>> +	       ((rcv_buf >> 16) & 0x0000FF00) |
>>> +	       ((rcv_buf >> 8) & 0x000000FF)) << 16;
>>> +
>>> 
> …
> 
>>> +int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
>>> +				      int *temperature,
>>> +				      unsigned int *pressure)
>>> +{
>>> +	int ret;
>>> +	u32 t_adc, p_adc;
>>> +	s32 dt, temp;
>>> +	s64 off, sens, t2, off2, sens2;
>>> +	u16 *prom = dev_data->prom, delay;
>>> +	u8 i;
>>> +
>>> +	mutex_lock(&dev_data->lock);
>>> +	i = dev_data->res_index * 2;
>> This local variable seens rather unnecessary. Just put it inline in the
>> calls.
> 
> OK
> 
>>> +	delay = ms_sensors_tp_conversion_time[dev_data->res_index];
>>> +
>>> +	ret = ms_sensors_i2c_convert_and_read(
>>> +					dev_data->client,
>>> +					MS_SENSORS_TP_T_CONVERSION_START + i,
>>> +					MS_SENSORS_TP_ADC_READ,
>>> +					delay, &t_adc);
>>> +	if (ret) {
>>> +		mutex_unlock(&dev_data->lock);
>>> +		return ret;
>>> +	}
>>> +
>>> +	
> 
> …
> 
>>> +
>>> diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.h b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>>> new file mode 100644
>>> index 0000000..918b8af
>>> --- /dev/null
>>> +++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>>> @@ -0,0 +1,53 @@
>>> +/*
>>> + * Measurements Specialties common sensor driver
>>> + *
>>> + * Copyright (c) 2015 Measurement-Specialties
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#ifndef _MS_SENSORS_I2C_H
>>> +#define _MS_SENSORS_I2C_H
>>> +
>>> +#include <linux/i2c.h>
>>> +#include <linux/mutex.h>
>>> +
>>> +#define MS_SENSORS_TP_PROM_WORDS_NB		7
>>> +
>>> +struct ms_ht_dev {
>>> +	struct i2c_client *client;
>>> +	struct mutex lock; /* mutex protecting data structure */
>>> +	u8 res_index;
>>> +};
>>> +
>> kernel doc comments preferred.
> 
> OK
>>> +struct ms_tp_dev {
>>> +	struct i2c_client *client;
>>> +	struct mutex lock; /* mutex protecting data structure */
>>> +	/* Added element for CRC computation */
>>> +	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB + 1];
>>> +	u8 res_index;
>>> +};
>>> +
>>> +int ms_sensors_i2c_reset(void *cli, u8 cmd, unsigned int delay);
>>> +int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word);
>>> +int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
>>> +				    unsigned int delay, u32 *adc);
>>> +int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn);
>>> +ssize_t ms_sensors_i2c_show_serial(struct ms_ht_dev *dev_data, char *buf);
>>> +ssize_t ms_sensors_i2c_write_resolution(struct ms_ht_dev *dev_data, u8 i);
>>> +ssize_t ms_sensors_i2c_show_battery_low(struct ms_ht_dev *dev_data, char *buf);
>>> +ssize_t ms_sensors_i2c_show_heater(struct ms_ht_dev *dev_data, char *buf);
>>> +ssize_t ms_sensors_i2c_write_heater(struct ms_ht_dev *dev_data,
>>> +				    const char *buf, size_t len);
>>> +int ms_sensors_i2c_ht_read_temperature(struct ms_ht_dev *dev_data,
>>> +				       s32 *temperature);
>>> +int ms_sensors_i2c_ht_read_humidity(struct ms_ht_dev *dev_data,
>>> +				    u32 *humidity);
>>> +int ms_sensors_tp_read_prom(struct ms_tp_dev *dev_data);
>>> +int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
>>> +				      int *temperature,
>>> +				      unsigned int *pressure);
>> The presence or absense of _i2c in the function naming does feel rather
>> random.  All of these functions ultimately make i2c calls so perhaps you
>> can explain your reasoning behind having it in some and not others.
> 
> The functions used were meant to have _i2c when there is no direct calls to  smbus functions.
> I agree that is rather awkward, I will change and put i2c everywhere.
> 
>> 
>>> +
>>> +#endif /* _MS_SENSORS_I2C_H */
>>> 
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/6] Add tsys01 meas-spec driver support
  2015-09-27 16:55   ` Jonathan Cameron
@ 2015-09-29  9:36     ` ludovic.tancerel
  2015-09-29 17:21       ` Jonathan Cameron
  0 siblings, 1 reply; 22+ messages in thread
From: ludovic.tancerel @ 2015-09-29  9:36 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio,
	William.Markezana


Thank you for reviewing
Please have a look at comments below,

Regards,
Ludovic

Le 27 sept. 2015 à 18:55, Jonathan Cameron <jic23@kernel.org> a écrit :

> On 25/09/15 14:56, Ludovic Tancerel wrote:
>> Support for TSYS01 temperature sensor
>> 
>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
> This is fine as far as i am concerned, though I would like to leave it
> on the list for a little while for others to have a chance to comment.
>> ---
>> drivers/iio/temperature/Kconfig  |  11 ++
>> drivers/iio/temperature/Makefile |   1 +
>> drivers/iio/temperature/tsys01.c | 231 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 243 insertions(+)
>> create mode 100644 drivers/iio/temperature/tsys01.c
>> 
>> 
…

>> +static int tsys01_i2c_probe(struct i2c_client *client,
>> +			    const struct i2c_device_id *id)
>> +{
>> +	struct tsys01_dev *dev_data;
>> +	struct iio_dev *indio_dev;
>> +
>> +	if (!i2c_check_functionality(client->adapter,
>> +				     I2C_FUNC_SMBUS_WORD_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->reset = ms_sensors_i2c_reset;
>> +	dev_data->read_prom_word = ms_sensors_i2c_read_prom_word;
>> +	dev_data->convert_and_read = ms_sensors_i2c_convert_and_read;
>> +
>> +	i2c_set_clientdata(client, indio_dev);
>> +
> This separation into i2c probe and main probe is something one would
> generally only introduce at the point of adding support for another bus.
> It just adds complexity here for little gain.  If there is intent
> to add spi support shortly then fair enough, leave it as it is.

I don’t plan on doing the SPI stuff myself,
so if you feel I should simplify as there is no plan to do it, please let me know.

I prefer to keep it as I made this change especially to comply a request to have this similar to other existing meas-spec drivers (ms5611).


>> +	return tsys01_probe(indio_dev, &client->dev);
>> +}
>> +
>> +static const struct i2c_device_id tsys01_id[] = {
>> +	{"tsys01", 0},
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, tsys01_id);
>> +
>> +static struct i2c_driver tsys01_driver = {
>> +	.probe = tsys01_i2c_probe,
>> +	.id_table = tsys01_id,
>> +	.driver = {
>> +		   .name = "tsys01",
>> +		   },
>> +};
>> +
>> +module_i2c_driver(tsys01_driver);
>> +
>> +MODULE_DESCRIPTION("Measurement-Specialties tsys01 temperature driver");
>> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
>> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
>> +MODULE_LICENSE("GPL v2");
>> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 3/6] Add tsys02d meas-spec driver support
  2015-09-27 17:51   ` Jonathan Cameron
@ 2015-09-29  9:40     ` ludovic.tancerel
  0 siblings, 0 replies; 22+ messages in thread
From: ludovic.tancerel @ 2015-09-29  9:40 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio,
	William.Markezana


Thanks again …
There is no request from measurement specialties to handle this as poll threshold event.
This can definitely evolve if anyone needs it, or if an equivalent ABI parameter becomes available in IIO.
I will leave as it is.

Thanks,
Ludovic


Le 27 sept. 2015 à 19:51, Jonathan Cameron <jic23@kernel.org> a écrit :

> 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");
>> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 5/6] Add ms5637 meas-spec driver support
  2015-09-27 17:57   ` Jonathan Cameron
@ 2015-09-29  9:45     ` ludovic.tancerel
  0 siblings, 0 replies; 22+ messages in thread
From: ludovic.tancerel @ 2015-09-29  9:45 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio,
	William.Markezana


This was a typo on "Measurement Specialties" name in previous MS5611 drivers.
I will definitely separate this into tiny additionnal patch.

Thanks,
Ludovic

Le 27 sept. 2015 à 19:57, Jonathan Cameron <jic23@kernel.org> a écrit :

> On 25/09/15 14:56, Ludovic Tancerel wrote:
>> Support for MS5637 temperature & pressure sensor
>> 
>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
> Only issue here a separate typo fix seems to have snuck in here.
> 
> All good, but should be a separate patch so please break that out for the
> next version.
> 
> Thanks,
> 
> Jonathan
>> ---
>> drivers/iio/pressure/Kconfig  |  15 +++-
>> drivers/iio/pressure/Makefile |   1 +
>> drivers/iio/pressure/ms5637.c | 187 ++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 201 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/iio/pressure/ms5637.c
>> 
>> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
>> index fa62950..8142cfe 100644
>> --- a/drivers/iio/pressure/Kconfig
>> +++ b/drivers/iio/pressure/Kconfig
>> @@ -53,9 +53,9 @@ config MPL3115
>>           will be called mpl3115.
>> 
>> config MS5611
>> -	tristate "Measurement Specialities MS5611 pressure sensor driver"
>> +	tristate "Measurement Specialties MS5611 pressure sensor driver"
>> 	help
>> -	  Say Y here to build support for the Measurement Specialities
>> +	  Say Y here to build support for the Measurement Specialties
>> 	  MS5611 pressure and temperature sensor.
> THis looks to have snuck into the wrong patch to me...
>> 
>> 	  To compile this driver as a module, choose M here: the module will
>> @@ -79,6 +79,17 @@ config MS5611_SPI
>> 	  To compile this driver as a module, choose M here: the module will
>> 	  be called ms5611_spi.
>> 
>> +config MS5637
>> +	tristate "Measurement Specialties MS5637 pressure & temperature sensor"
>> +	depends on I2C
>> +        select IIO_MS_SENSORS_I2C
>> +	help
>> +	  If you say yes here you get support for the Measurement Specialties
>> +	  MS5637 pressure and temperature sensor.
>> +
>> +	  This driver can also be built as a module. If so, the module will
>> +	  be called ms5637.
>> +
>> config IIO_ST_PRESS
>> 	tristate "STMicroelectronics pressure sensor Driver"
>> 	depends on (I2C || SPI_MASTER) && SYSFS
>> diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
>> index a4f98f8..46571c96 100644
>> --- a/drivers/iio/pressure/Makefile
>> +++ b/drivers/iio/pressure/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_MPL3115) += mpl3115.o
>> obj-$(CONFIG_MS5611) += ms5611_core.o
>> obj-$(CONFIG_MS5611_I2C) += ms5611_i2c.o
>> obj-$(CONFIG_MS5611_SPI) += ms5611_spi.o
>> +obj-$(CONFIG_MS5637) += ms5637.o
>> obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o
>> st_pressure-y := st_pressure_core.o
>> st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o
>> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
>> new file mode 100644
>> index 0000000..0dbbd4e
>> --- /dev/null
>> +++ b/drivers/iio/pressure/ms5637.c
>> @@ -0,0 +1,187 @@
>> +/*
>> + * ms5637.c - Support for Measurement-Specialties ms5637
>> + *            pressure & temperature sensor
>> + *
>> + * Copyright (c) 2015 Measurement-Specialties
>> + *
>> + * Licensed under the GPL-2.
>> + *
>> + * (7-bit I2C slave address 0x76)
>> + *
>> + * Datasheet:
>> + *  http://www.meas-spec.com/downloads/MS5637-02BA03.pdf
>> + *
>> + */
>> +#include <linux/init.h>
>> +#include <linux/device.h>
>> +#include <linux/kernel.h>
>> +#include <linux/stat.h>
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +#include <linux/mutex.h>
>> +
>> +#include "../common/ms_sensors/ms_sensors_i2c.h"
>> +
>> +static const int ms5637_samp_freq[6] = { 960, 480, 240, 120, 60, 30 };
>> +/* String copy of the above const for readability purpose */
>> +static const char ms5637_show_samp_freq[] = "960 480 240 120 60 30";
>> +
>> +static int ms5637_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *channel, int *val,
>> +			   int *val2, long mask)
>> +{
>> +	int ret;
>> +	int temperature;
>> +	unsigned int pressure;
>> +	struct ms_tp_dev *dev_data = iio_priv(indio_dev);
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_PROCESSED:
>> +		ret = ms_sensors_read_temp_and_pressure(dev_data,
>> +							&temperature,
>> +							&pressure);
>> +		if (ret)
>> +			return ret;
>> +
>> +		switch (channel->type) {
>> +		case IIO_TEMP:	/* in milli °C */
>> +			*val = temperature;
>> +
>> +			return IIO_VAL_INT;
>> +		case IIO_PRESSURE:	/* in kPa */
>> +			*val = pressure / 1000;
>> +			*val2 = (pressure % 1000) * 1000;
>> +
>> +			return IIO_VAL_INT_PLUS_MICRO;
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>> +		*val = ms5637_samp_freq[dev_data->res_index];
>> +
>> +		return IIO_VAL_INT;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>> +static int ms5637_write_raw(struct iio_dev *indio_dev,
>> +			    struct iio_chan_spec const *chan,
>> +			    int val, int val2, long mask)
>> +{
>> +	struct ms_tp_dev *dev_data = iio_priv(indio_dev);
>> +	int i;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>> +		i = ARRAY_SIZE(ms5637_samp_freq);
>> +		while (i-- > 0)
>> +			if (val == ms5637_samp_freq[i])
>> +				break;
>> +		if (i < 0)
>> +			return -EINVAL;
>> +		dev_data->res_index = i;
>> +
>> +		return 0;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>> +static const struct iio_chan_spec ms5637_channels[] = {
>> +	{
>> +		.type = IIO_TEMP,
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>> +	},
>> +	{
>> +		.type = IIO_PRESSURE,
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
>> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>> +	}
>> +};
>> +
>> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(ms5637_show_samp_freq);
>> +
>> +static struct attribute *ms5637_attributes[] = {
>> +	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
>> +	NULL,
>> +};
>> +
>> +static const struct attribute_group ms5637_attribute_group = {
>> +	.attrs = ms5637_attributes,
>> +};
>> +
>> +static const struct iio_info ms5637_info = {
>> +	.read_raw = ms5637_read_raw,
>> +	.write_raw = ms5637_write_raw,
>> +	.attrs = &ms5637_attribute_group,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +static int ms5637_probe(struct i2c_client *client,
>> +			const struct i2c_device_id *id)
>> +{
>> +	struct ms_tp_dev *dev_data;
>> +	struct iio_dev *indio_dev;
>> +	int ret;
>> +
>> +	if (!i2c_check_functionality(client->adapter,
>> +				     I2C_FUNC_SMBUS_READ_WORD_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 = 5;
>> +	mutex_init(&dev_data->lock);
>> +
>> +	indio_dev->info = &ms5637_info;
>> +	indio_dev->name = id->name;
>> +	indio_dev->dev.parent = &client->dev;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	indio_dev->channels = ms5637_channels;
>> +	indio_dev->num_channels = ARRAY_SIZE(ms5637_channels);
>> +
>> +	i2c_set_clientdata(client, indio_dev);
>> +
>> +	ret = ms_sensors_i2c_reset(client, 0x1E, 3000);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = ms_sensors_tp_read_prom(dev_data);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return devm_iio_device_register(&client->dev, indio_dev);
>> +}
>> +
>> +static const struct i2c_device_id ms5637_id[] = {
>> +	{"ms5637", 0},
>> +	{}
>> +};
>> +
>> +static struct i2c_driver ms5637_driver = {
>> +	.probe = ms5637_probe,
>> +	.id_table = ms5637_id,
>> +	.driver = {
>> +		   .name = "ms5637"
>> +		   },
>> +};
>> +
>> +module_i2c_driver(ms5637_driver);
>> +
>> +MODULE_DESCRIPTION("Measurement-Specialties ms5637 temperature & pressure driver");
>> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
>> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
>> +MODULE_LICENSE("GPL v2");


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 6/6] Add ms8607 meas-spec driver support
  2015-09-27 18:00   ` Jonathan Cameron
@ 2015-09-29 10:00     ` ludovic.tancerel
  2015-09-29 17:27       ` Jonathan Cameron
  0 siblings, 1 reply; 22+ messages in thread
From: ludovic.tancerel @ 2015-09-29 10:00 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio,
	William.Markezana

Please read below


Le 27 sept. 2015 à 20:00, Jonathan Cameron <jic23@kernel.org> a écrit :

> On 25/09/15 14:56, Ludovic Tancerel wrote:
>> Support for MS8607 temperature, pressure & humidity sensor.
>> This part is using functions from MS5637 for temperature and pressure
>> and HTU21 for humidity
>> 
>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
> As I commented (early today) in response to the previous thread, I'd prefer
> a little more than -h and -tp in the naming but otherwise this is good.
> 
OK, I will change it

> btw. Having added the tiny amount of infrastructure needed to do the different
> device support in htu21 you could easily add the temperature part as well as
> another option :)  Just saying…

I somehow agree, but the MS8607 is really a MS5673 + HTU21 part,
it is a bit different for TSYS02D and MS5637.
Again, all this has been discussed with Meas-Spec and this is their wish to organize as it is.

> 
> Anyhow, the only real issues other than a bit of reorganization to split
> the typo fix out of the previous patch are little things in patch 1.
> 
> Get those sorted and we can get this lot merged in plenty of time for
> the next merge window.

This will come soon…
Thanks a lot for the feedback.

Shall I add somewhere « Reviewed by » tag in the patchset ?
Regards,
Ludovic

> 
> Thanks,
> 
> Jonathan
>> ---
>> Documentation/ABI/testing/sysfs-bus-iio-meas-spec |  1 +
>> drivers/iio/humidity/Kconfig                      |  2 ++
>> drivers/iio/humidity/htu21.c                      | 33 ++++++++++++++++++++---
>> drivers/iio/pressure/Kconfig                      |  2 ++
>> drivers/iio/pressure/ms5637.c                     |  6 ++++-
>> 5 files changed, 40 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>> index 6d47e54..1a6265e 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>> @@ -5,3 +5,4 @@ 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
>> +		This ABI is available for htu21, ms8607
>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>> index 3fab60c..75ca043 100644
>> --- a/drivers/iio/humidity/Kconfig
>> +++ b/drivers/iio/humidity/Kconfig
>> @@ -19,6 +19,8 @@ config HTU21
>> 	help
>> 	  If you say yes here you get support for the Measurement Specialties
>> 	  HTU21 humidity and temperature sensor.
>> +	  This driver is also used for MS8607 temperature, pressure & humidity
>> +	  sensor
>> 
>> 	  This driver can also be built as a module. If so, the module will
>> 	  be called htu21.
>> diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
>> index 706faff..0530545 100644
>> --- a/drivers/iio/humidity/htu21.c
>> +++ b/drivers/iio/humidity/htu21.c
>> @@ -1,6 +1,7 @@
>> /*
>>  * htu21.c - Support for Measurement-Specialties
>>  *           htu21 temperature & humidity sensor
>> + *	     and humidity part of MS8607 sensor
>>  *
>>  * Copyright (c) 2014 Measurement-Specialties
>>  *
>> @@ -10,6 +11,8 @@
>>  *
>>  * Datasheet:
>>  *  http://www.meas-spec.com/downloads/HTU21D.pdf
>> + * Datasheet:
>> + *  http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
>>  *
>>  */
>> 
>> @@ -25,6 +28,11 @@
>> 
>> #define HTU21_RESET				0xFE
>> 
>> +enum {
>> +	HTU21,
>> +	MS8607
>> +};
>> +
>> static const int htu21_samp_freq[4] = { 20, 40, 70, 120 };
>> /* String copy of the above const for readability purpose */
>> static const char htu21_show_samp_freq[] = "20 40 70 120";
>> @@ -107,6 +115,18 @@ static const struct iio_chan_spec htu21_channels[] = {
>> 	 }
>> };
>> 
>> +/*
>> + * Meas Spec recommendation is to not read temperature
>> + * on this driver part for MS8607
>> + */
>> +static const struct iio_chan_spec ms8607_channels[] = {
>> +	{
>> +		.type = IIO_HUMIDITYRELATIVE,
>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
>> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>> +	 }
>> +};
>> +
>> static ssize_t htu21_show_battery_low(struct device *dev,
>> 				      struct device_attribute *attr, char *buf)
>> {
>> @@ -189,8 +209,14 @@ int htu21_probe(struct i2c_client *client,
>> 	indio_dev->name = id->name;
>> 	indio_dev->dev.parent = &client->dev;
>> 	indio_dev->modes = INDIO_DIRECT_MODE;
>> -	indio_dev->channels = htu21_channels;
>> -	indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
>> +
>> +	if (id->driver_data == MS8607) {
>> +		indio_dev->channels = ms8607_channels;
>> +		indio_dev->num_channels = ARRAY_SIZE(ms8607_channels);
>> +	} else {
>> +		indio_dev->channels = htu21_channels;
>> +		indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
>> +	}
>> 
>> 	i2c_set_clientdata(client, indio_dev);
>> 
>> @@ -207,7 +233,8 @@ int htu21_probe(struct i2c_client *client,
>> }
>> 
>> static const struct i2c_device_id htu21_id[] = {
>> -	{"htu21", 0},
>> +	{"htu21", HTU21},
>> +	{"ms8607-h", MS8607},
>> 	{}
>> };
>> 
>> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
>> index 8142cfe..53f9a44 100644
>> --- a/drivers/iio/pressure/Kconfig
>> +++ b/drivers/iio/pressure/Kconfig
>> @@ -86,6 +86,8 @@ config MS5637
>> 	help
>> 	  If you say yes here you get support for the Measurement Specialties
>> 	  MS5637 pressure and temperature sensor.
>> +	  This driver is also used for MS8607 temperature, pressure & humidity
>> +	  sensor
>> 
>> 	  This driver can also be built as a module. If so, the module will
>> 	  be called ms5637.
>> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
>> index 0dbbd4e..f6b5544 100644
>> --- a/drivers/iio/pressure/ms5637.c
>> +++ b/drivers/iio/pressure/ms5637.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * ms5637.c - Support for Measurement-Specialties ms5637
>> + * ms5637.c - Support for Measurement-Specialties ms5637 and ms8607
>>  *            pressure & temperature sensor
>>  *
>>  * Copyright (c) 2015 Measurement-Specialties
>> @@ -10,8 +10,11 @@
>>  *
>>  * Datasheet:
>>  *  http://www.meas-spec.com/downloads/MS5637-02BA03.pdf
>> + * Datasheet:
>> + *  http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
>>  *
>>  */
>> +
>> #include <linux/init.h>
>> #include <linux/device.h>
>> #include <linux/kernel.h>
>> @@ -168,6 +171,7 @@ static int ms5637_probe(struct i2c_client *client,
>> 
>> static const struct i2c_device_id ms5637_id[] = {
>> 	{"ms5637", 0},
>> +	{"ms8607-tp", 1},
>> 	{}
>> };
>> 
>> 
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 1/6] Add meas-spec sensors common part
  2015-09-29  8:03       ` ludovic.tancerel
@ 2015-09-29 17:20         ` Jonathan Cameron
  0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-29 17:20 UTC (permalink / raw)
  To: ludovic.tancerel
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio,
	William.Markezana

On 29/09/15 09:03, ludovic.tancerel@maplehightech.com wrote:
> Resent with formatting changes in my mail application,
> as it seems some html makes the server throw it away.
> 
> Ludovic
> 
>> Hello Jonathan, all,
>> thank you for reviewing
>>
>> for once, I will reply shortly.
>> Hopefully, the message will go through the mail server.
>>
>> Please have a look at comments below,
responses inline.
>> Regards,
>> Ludovic
>>
>>
>> Le 27 sept. 2015 à 18:23, Jonathan Cameron <jic23@kernel.org> a écrit :
>>
>>> On 25/09/15 14:56, Ludovic Tancerel wrote:
>>>> Measurement specialties drivers common part.
>>>> These functions are used by further drivers in the patchset: TSYS01, TSYS02D, HTU21, MS5637, MS8607
>>>>
>>>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
>>> A few more bits and bobs inline.
>>>> ---
>>>> drivers/iio/common/Kconfig                     |   1 +
>>>> drivers/iio/common/Makefile                    |   1 +
>>>> drivers/iio/common/ms_sensors/Kconfig          |   6 +
>>>> drivers/iio/common/ms_sensors/Makefile         |   5 +
>>>> drivers/iio/common/ms_sensors/ms_sensors_i2c.c | 645 +++++++++++++++++++++++++
>>>> drivers/iio/common/ms_sensors/ms_sensors_i2c.h |  53 ++
>>>> 6 files changed, 711 insertions(+)
>>>> create mode 100644 drivers/iio/common/ms_sensors/Kconfig
>>>> create mode 100644 drivers/iio/common/ms_sensors/Makefile
>>>> create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>>>> create mode 100644 drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>>>>
>>>> diff —git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
>> …
>>>>
>>>> diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.c b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>>>> new file mode 100644
>>>> index 0000000..4b1bc31
>>>> --- /dev/null
>>>> +++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.c
>>>> @@ -0,0 +1,645 @@
>>>> +/*
>>>> + * Measurements Specialties driver common i2c functions
>>>> + *
>>>> + * Copyright (c) 2015 Measurement-Specialties
>>>> + *
>>>> + * Licensed under the GPL-2.
>>> Drop this blank line.
>> OK
>>>> + *
>>>> + */
>>>> +
>> …
>>>> +
>>>> +/**
>>>> + * ms_sensors_i2c_read_prom_word() - i2c prom word read function
>>>> + * @cli:	pointer to i2c client
>>>> + * @cmd:	PROM read cmd. Depends on device and prom id
>>>> + * @word:	pointer to word destination value
>>>> + *
>>>> + * Generic i2c prom word read function for Measurement Specialties devices.
>>>> + *
>>>> + * Return: 0 on success, negative errno otherwise.
>>>> + */
>>>> +int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word)
>>>> +{
>>>> +	int ret;
>>>> +	struct i2c_client *client = (struct i2c_client *)cli;
>>> Why pass an void * given the function name says this will always be an i2c_client?
>>>
>>> Once that's removed this wrapper does very little and I'd be tempted to
>>> drop it in favour of direct calls to the smbus read.
>>
>> The void * is because this function might be used for the same chipset using SPI access (that is not supported yet).
>> I am passing the void *, to have common parameters for the future spi function.
>> This is the same for previous function ms_sensors_i2c_reset.
>>
>> That references functions written for tsys01 written in patch v2/6 :
>> "
>> 	dev_data->client = client;
>> 	dev_data->reset = ms_sensors_i2c_reset;
>> 	dev_data->read_prom_word = ms_sensors_i2c_read_prom_word;
>> 	dev_data->convert_and_read = ms_sensors_i2c_convert_and_read;
>> "
>> The same could be done but using a spi function. Isn’t that the appropriate approach ?
>> This is what I understood when looking at other drivers.
>>
>> These functions are also used in different drivers, so if I drop t he wrapper, I cannot reuse the function for TSYS01.
>>
>>>> +
>>>> +	ret = i2c_smbus_read_word_swapped(client, cmd);
>>>> +	if (ret < 0) {
>>>> +		dev_err(&client->dev, "Failed to read prom word\n");
>>>> +		return ret;
>>>> +	}
>>>> +	*word = ret;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(ms_sensors_i2c_read_prom_word);
>>>> +
>>>> +/**
>>>> + * ms_sensors_i2c_convert_and_read() - i2c ADC conversion & read function
>>>> + * @cli:	pointer to i2c client
>>>> + * @conv:	ADC conversion command. Depends on device in use
>>>> + * @rd:		ADC read command. Depends on device in use
>>>> + * @delay:	usleep minimal delay after conversion command is issued
>>>> + * @adc:	pointer to ADC destination value
>>>> + *
>>>> + * Generic i2c ADC conversion & read function for Measurement Specialties
>>>> + * devices.
>>>> + * The function will issue conversion command, sleep appopriate delay, and
>>>> + * issue command to read ADC.
>>>> + *
>>>> + * Return: 0 on success, negative errno otherwise.
>>>> + */
>>>> +int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
>>>> +				    unsigned int delay, u32 *adc)
>>>> +{
>>>> +	int ret;
>>>> +	u32 buf = 0;
>>>> +	struct i2c_client *client = (struct i2c_client *)cli;
>>>> +
>>>> +	/* Trigger conversion */
>>>> +	ret = i2c_smbus_write_byte(client, conv);
>>>> +	if (ret)
>>>> +		goto err;
>>>> +	usleep_range(delay, delay + 1000);
>>>> +
>>>> +	/* Retrieve ADC value */
>>>> +	if (rd != MS_SENSORS_NO_READ_CMD)
>>>> +		ret = i2c_smbus_read_i2c_block_data(client, rd, 3, (u8 *)&buf);
>>>> +	else
>>>> +		ret = i2c_master_recv(client, (u8 *)&buf, 3);
>>>> +	if (ret < 0)
>>>> +		goto err;
>>>> +
>>>> +	dev_dbg(&client->dev, "ADC raw value : %x\n", be32_to_cpu(buf) >> 8);
>>>> +	*adc = be32_to_cpu(buf) >> 8;
>>>> +
>>>> +	return 0;
>>>> +err:
>>>> +	dev_err(&client->dev, "Unable to make sensor adc conversion\n");
>>>> +	return ret;
>>>> +}
>>>> +EXPORT_SYMBOL(ms_sensors_i2c_convert_and_read);
>>>> +
>>>> +/**
>>>> + * ms_sensors_crc_valid() - CRC check function
>>>> + * @value:	input and CRC compare value
>>>> + *
>>>> + * Cyclic Redundancy Check function used in TSYS02D, HTU21, MS8607.
>>>> + * This function performs a x^8 + x^5 + x^4 + 1 polynomial CRC.
>>>> + * The argument contains CRC value in LSB byte while the bytes 1 and 2
>>>> + * are used for CRC computation.
>>>> + *
>>>> + * Return: 1 if CRC is valid, 0 otherwise.
>>> Can this be done with the standard kernel crc32 functions? (not dug into
>>> it enough to be sure!)
>>
>> I don’t think so.
>> CRC computation here is truly a 16-bits CRC check.
>> I have looked at crc16 lib but this a different polynomial used.
Fair enough.
>>
>>>> + */
>>>> +static bool ms_sensors_crc_valid(u32 value)
>>>> +{
>>>> +	u32 polynom = 0x988000;	/* x^8 + x^5 + x^4 + 1 */
>>>> +	u32 msb = 0x800000;
>>>> +	u32 mask = 0xFF8000;
>>>> +	u32 result = value & 0xFFFF00;
>>>> +	u8 crc = value & 0xFF;
>>>> +
>>>> +	while (msb != 0x80) {
>>>> +		if (result & msb)
>>>> +			result = ((result ^ polynom) & mask)
>>>> +				| (result & ~mask);
>>>> +		msb >>= 1;
>>>> +		mask >>= 1;
>>>> +		polynom >>= 1;
>>>> +	}
>>>> +
>>>> +	return result == crc;
>>>> +}
>>>> +
>>>> +/**
>>>> + * ms_sensors_i2c_read_serial() - i2c serial number read function
>>>> + * @cli:	pointer to i2c client
>>>> + * @sn:		pointer to 64-bits destination value
>>>> + *
>>>> + * Generic i2c serial number read function for Measurement Specialties devices.
>>>> + * This function is used for TSYS02d, HTU21, MS8607 chipset.
>>>> + * Refer to datasheet:
>>>> + *	http://www.meas-spec.com/downloads/HTU2X_Serial_Number_Reading.pdf
>>> THat's a first.  A whole datasheet on how the heck the serial number works!
>>>
>>> Got to wonder how anyone ever came up with that.  I'm going to conclude
>>> you got it right and not read any more ;)
>>
>> Please read below.
>> If you read the spec, you will understand why there is one,
>> the Serial Number shape is really far fetched … :)
>>
>>>> + *
>>>> + * Return: 0 on success, negative errno otherwise.
>>>> + */
>>>> +int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn)
>>>> +{
>>>> +	u8 i;
>>>> +	u64 rcv_buf = 0;
>>>> +	u16 send_buf;
>>>> +	int ret;
>>>> +
>>>> +	struct i2c_msg msg[2] = {
>>>> +		{
>>>> +		 .addr = client->addr,
>>>> +		 .flags = client->flags,
>>>> +		 .len = 2,
>>>> +		 .buf = (__u8 *)&send_buf,
>>>> +		 },
>>>> +		{
>>>> +		 .addr = client->addr,
>>>> +		 .flags = client->flags | I2C_M_RD,
>>>> +		 .buf = (__u8 *)&rcv_buf,
>>>> +		 },
>>>> +	};
>>>> +
>>>> +	/* Read MSB part of serial number */
>>>> +	send_buf = cpu_to_be16(MS_SENSORS_SERIAL_READ_MSB);
>>>> +	msg[1].len = 8;
>>>> +	ret = i2c_transfer(client->adapter, msg, 2);
>>>> +	if (ret < 0) {
>>>> +		dev_err(&client->dev, "Unable to read device serial number");
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	rcv_buf = be64_to_cpu(rcv_buf);
>>>> +	dev_dbg(&client->dev, "Serial MSB raw : %llx\n", rcv_buf);
>>>> +
>>>> +	for (i = 0; i < 64; i += 16) {
>>>> +		if (!ms_sensors_crc_valid((rcv_buf >> i) & 0xFFFF))
>>>> +			return -ENODEV;
>>>> +	}
>>>> +
>>> Umm. I'm really unclear what you are doing here.  You read into
>>> a 64 bit buffer, then do a hand endian swap and shift?  Should be possible
>>> to at least use standard functions to swap the byte order.
>>
>> There are several 8bits CRC interlaced in the bytes read containing serial number.
>> So the "for (i = 0; i < 64; i += 16)" is to check the different CRCs
>>
>> The « *sn = … » is meant to regroup the bytes in between CRC words and retrieve the 32bits first portion of the serial number.
>> The shape is [ SNB 3 , CRC, SNB 2, CRC, SNB 1, CRC, SNB 0, CRC ]
>> I considered unifying both in the loop but this is not simpler
>>
>> The second portion of serial number is read afterwards.
>>
>> I agree this is overly complex for getting a serial number,
>> but this is how it is implemented in the sensor, and it is a wish from measurement specialties to read it at sensor init.
>>
>> If you feel I should extract the SN bytes in a different way on the code below, let me know.
>> I can put it into a macro to improve readability ?
>>
>>>
>>>> +	*sn = (((rcv_buf >> 32) & 0xFF000000) |
>>>> +	       ((rcv_buf >> 24) & 0x00FF0000) |
>>>> +	       ((rcv_buf >> 16) & 0x0000FF00) |
>>>> +	       ((rcv_buf >> 8) & 0x000000FF)) << 16;
>>>> +
>>>>
>> …
All makes sense.  Perhaps the best bet is a comment giving 
a brief description of what is going on is the best bet.  The explanation
you put here is nice and clear so perhaps start from that.
>>
>>>> +int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
>>>> +				      int *temperature,
>>>> +				      unsigned int *pressure)
>>>> +{
>>>> +	int ret;
>>>> +	u32 t_adc, p_adc;
>>>> +	s32 dt, temp;
>>>> +	s64 off, sens, t2, off2, sens2;
>>>> +	u16 *prom = dev_data->prom, delay;
>>>> +	u8 i;
>>>> +
>>>> +	mutex_lock(&dev_data->lock);
>>>> +	i = dev_data->res_index * 2;
>>> This local variable seens rather unnecessary. Just put it inline in the
>>> calls.
>>
>> OK
>>
>>>> +	delay = ms_sensors_tp_conversion_time[dev_data->res_index];
>>>> +
>>>> +	ret = ms_sensors_i2c_convert_and_read(
>>>> +					dev_data->client,
>>>> +					MS_SENSORS_TP_T_CONVERSION_START + i,
>>>> +					MS_SENSORS_TP_ADC_READ,
>>>> +					delay, &t_adc);
>>>> +	if (ret) {
>>>> +		mutex_unlock(&dev_data->lock);
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	
>>
>> …
>>
>>>> +
>>>> diff --git a/drivers/iio/common/ms_sensors/ms_sensors_i2c.h b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>>>> new file mode 100644
>>>> index 0000000..918b8af
>>>> --- /dev/null
>>>> +++ b/drivers/iio/common/ms_sensors/ms_sensors_i2c.h
>>>> @@ -0,0 +1,53 @@
>>>> +/*
>>>> + * Measurements Specialties common sensor driver
>>>> + *
>>>> + * Copyright (c) 2015 Measurement-Specialties
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + */
>>>> +
>>>> +#ifndef _MS_SENSORS_I2C_H
>>>> +#define _MS_SENSORS_I2C_H
>>>> +
>>>> +#include <linux/i2c.h>
>>>> +#include <linux/mutex.h>
>>>> +
>>>> +#define MS_SENSORS_TP_PROM_WORDS_NB		7
>>>> +
>>>> +struct ms_ht_dev {
>>>> +	struct i2c_client *client;
>>>> +	struct mutex lock; /* mutex protecting data structure */
>>>> +	u8 res_index;
>>>> +};
>>>> +
>>> kernel doc comments preferred.
>>
>> OK
>>>> +struct ms_tp_dev {
>>>> +	struct i2c_client *client;
>>>> +	struct mutex lock; /* mutex protecting data structure */
>>>> +	/* Added element for CRC computation */
>>>> +	u16 prom[MS_SENSORS_TP_PROM_WORDS_NB + 1];
>>>> +	u8 res_index;
>>>> +};
>>>> +
>>>> +int ms_sensors_i2c_reset(void *cli, u8 cmd, unsigned int delay);
>>>> +int ms_sensors_i2c_read_prom_word(void *cli, int cmd, u16 *word);
>>>> +int ms_sensors_i2c_convert_and_read(void *cli, u8 conv, u8 rd,
>>>> +				    unsigned int delay, u32 *adc);
>>>> +int ms_sensors_i2c_read_serial(struct i2c_client *client, u64 *sn);
>>>> +ssize_t ms_sensors_i2c_show_serial(struct ms_ht_dev *dev_data, char *buf);
>>>> +ssize_t ms_sensors_i2c_write_resolution(struct ms_ht_dev *dev_data, u8 i);
>>>> +ssize_t ms_sensors_i2c_show_battery_low(struct ms_ht_dev *dev_data, char *buf);
>>>> +ssize_t ms_sensors_i2c_show_heater(struct ms_ht_dev *dev_data, char *buf);
>>>> +ssize_t ms_sensors_i2c_write_heater(struct ms_ht_dev *dev_data,
>>>> +				    const char *buf, size_t len);
>>>> +int ms_sensors_i2c_ht_read_temperature(struct ms_ht_dev *dev_data,
>>>> +				       s32 *temperature);
>>>> +int ms_sensors_i2c_ht_read_humidity(struct ms_ht_dev *dev_data,
>>>> +				    u32 *humidity);
>>>> +int ms_sensors_tp_read_prom(struct ms_tp_dev *dev_data);
>>>> +int ms_sensors_read_temp_and_pressure(struct ms_tp_dev *dev_data,
>>>> +				      int *temperature,
>>>> +				      unsigned int *pressure);
>>> The presence or absense of _i2c in the function naming does feel rather
>>> random.  All of these functions ultimately make i2c calls so perhaps you
>>> can explain your reasoning behind having it in some and not others.
>>
>> The functions used were meant to have _i2c when there is no direct calls to  smbus functions.
>> I agree that is rather awkward, I will change and put i2c everywhere.
Fair enough. Though personally I'd drop it everywhere ;)
>>
>>>
>>>> +
>>>> +#endif /* _MS_SENSORS_I2C_H */
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 2/6] Add tsys01 meas-spec driver support
  2015-09-29  9:36     ` ludovic.tancerel
@ 2015-09-29 17:21       ` Jonathan Cameron
  0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-29 17:21 UTC (permalink / raw)
  To: ludovic.tancerel
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio,
	William.Markezana

On 29/09/15 10:36, ludovic.tancerel@maplehightech.com wrote:
> 
> Thank you for reviewing
> Please have a look at comments below,
> 
> Regards,
> Ludovic
> 
> Le 27 sept. 2015 à 18:55, Jonathan Cameron <jic23@kernel.org> a écrit :
> 
>> On 25/09/15 14:56, Ludovic Tancerel wrote:
>>> Support for TSYS01 temperature sensor
>>>
>>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
>> This is fine as far as i am concerned, though I would like to leave it
>> on the list for a little while for others to have a chance to comment.
>>> ---
>>> drivers/iio/temperature/Kconfig  |  11 ++
>>> drivers/iio/temperature/Makefile |   1 +
>>> drivers/iio/temperature/tsys01.c | 231 +++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 243 insertions(+)
>>> create mode 100644 drivers/iio/temperature/tsys01.c
>>>
>>>
> …
> 
>>> +static int tsys01_i2c_probe(struct i2c_client *client,
>>> +			    const struct i2c_device_id *id)
>>> +{
>>> +	struct tsys01_dev *dev_data;
>>> +	struct iio_dev *indio_dev;
>>> +
>>> +	if (!i2c_check_functionality(client->adapter,
>>> +				     I2C_FUNC_SMBUS_WORD_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->reset = ms_sensors_i2c_reset;
>>> +	dev_data->read_prom_word = ms_sensors_i2c_read_prom_word;
>>> +	dev_data->convert_and_read = ms_sensors_i2c_convert_and_read;
>>> +
>>> +	i2c_set_clientdata(client, indio_dev);
>>> +
>> This separation into i2c probe and main probe is something one would
>> generally only introduce at the point of adding support for another bus.
>> It just adds complexity here for little gain.  If there is intent
>> to add spi support shortly then fair enough, leave it as it is.
> 
> I don’t plan on doing the SPI stuff myself,
> so if you feel I should simplify as there is no plan to do it, please let me know.
> 
> I prefer to keep it as I made this change especially to comply a
> request to have this similar to other existing meas-spec drivers
> (ms5611).
It's not something I feel strongly about so leave it as it is.

> 
>>> +	return tsys01_probe(indio_dev, &client->dev);
>>> +}
>>> +
>>> +static const struct i2c_device_id tsys01_id[] = {
>>> +	{"tsys01", 0},
>>> +	{}
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, tsys01_id);
>>> +
>>> +static struct i2c_driver tsys01_driver = {
>>> +	.probe = tsys01_i2c_probe,
>>> +	.id_table = tsys01_id,
>>> +	.driver = {
>>> +		   .name = "tsys01",
>>> +		   },
>>> +};
>>> +
>>> +module_i2c_driver(tsys01_driver);
>>> +
>>> +MODULE_DESCRIPTION("Measurement-Specialties tsys01 temperature driver");
>>> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
>>> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@maplehightech.com>");
>>> +MODULE_LICENSE("GPL v2");
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 6/6] Add ms8607 meas-spec driver support
  2015-09-29 10:00     ` ludovic.tancerel
@ 2015-09-29 17:27       ` Jonathan Cameron
  0 siblings, 0 replies; 22+ messages in thread
From: Jonathan Cameron @ 2015-09-29 17:27 UTC (permalink / raw)
  To: ludovic.tancerel
  Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald, linux-iio,
	William.Markezana

On 29/09/15 11:00, ludovic.tancerel@maplehightech.com wrote:
> Please read below
> 
> 
> Le 27 sept. 2015 à 20:00, Jonathan Cameron <jic23@kernel.org> a écrit :
> 
>> On 25/09/15 14:56, Ludovic Tancerel wrote:
>>> Support for MS8607 temperature, pressure & humidity sensor.
>>> This part is using functions from MS5637 for temperature and pressure
>>> and HTU21 for humidity
>>>
>>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com>
>> As I commented (early today) in response to the previous thread, I'd prefer
>> a little more than -h and -tp in the naming but otherwise this is good.
>>
> OK, I will change it
> 
>> btw. Having added the tiny amount of infrastructure needed to do the different
>> device support in htu21 you could easily add the temperature part as well as
>> another option :)  Just saying…
> 
> I somehow agree, but the MS8607 is really a MS5673 + HTU21 part,
> it is a bit different for TSYS02D and MS5637.
> Again, all this has been discussed with Meas-Spec and this is their wish to organize as it is.
As state, I'm not fussed, but if I get a patch from someone else down
the line combining the drivers, I might well take it as long as they
are willing to then help maintain the resulting driver...
(probably never happen!)
> 
>>
>> Anyhow, the only real issues other than a bit of reorganization to split
>> the typo fix out of the previous patch are little things in patch 1.
>>
>> Get those sorted and we can get this lot merged in plenty of time for
>> the next merge window.
> 
> This will come soon…
> Thanks a lot for the feedback.
> 
> Shall I add somewhere « Reviewed by » tag in the patchset ?
Typically you only add those if a reviewer explicitly gives
you such a tag.  In my case, as maintainer I in theory reviewed
everything anyway (or sometimes accepted a review from someone I
trust) and it will have my Signed-off-by tag when
I apply it which 'trumps' any other sign of anyway ;)

The only tag I'd put in explicitly for myself or add without
an explicit one from a reviewer is tested-by for anyone who happened
to have the hardware to test and has said it worked fine (including
me).  Acts as a way of tracking those with hardware down if there is
a problem in future.

J

> Regards,
> Ludovic
> 
>>
>> Thanks,
>>
>> Jonathan
>>> ---
>>> Documentation/ABI/testing/sysfs-bus-iio-meas-spec |  1 +
>>> drivers/iio/humidity/Kconfig                      |  2 ++
>>> drivers/iio/humidity/htu21.c                      | 33 ++++++++++++++++++++---
>>> drivers/iio/pressure/Kconfig                      |  2 ++
>>> drivers/iio/pressure/ms5637.c                     |  6 ++++-
>>> 5 files changed, 40 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>>> index 6d47e54..1a6265e 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>>> @@ -5,3 +5,4 @@ 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
>>> +		This ABI is available for htu21, ms8607
>>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>>> index 3fab60c..75ca043 100644
>>> --- a/drivers/iio/humidity/Kconfig
>>> +++ b/drivers/iio/humidity/Kconfig
>>> @@ -19,6 +19,8 @@ config HTU21
>>> 	help
>>> 	  If you say yes here you get support for the Measurement Specialties
>>> 	  HTU21 humidity and temperature sensor.
>>> +	  This driver is also used for MS8607 temperature, pressure & humidity
>>> +	  sensor
>>>
>>> 	  This driver can also be built as a module. If so, the module will
>>> 	  be called htu21.
>>> diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
>>> index 706faff..0530545 100644
>>> --- a/drivers/iio/humidity/htu21.c
>>> +++ b/drivers/iio/humidity/htu21.c
>>> @@ -1,6 +1,7 @@
>>> /*
>>>  * htu21.c - Support for Measurement-Specialties
>>>  *           htu21 temperature & humidity sensor
>>> + *	     and humidity part of MS8607 sensor
>>>  *
>>>  * Copyright (c) 2014 Measurement-Specialties
>>>  *
>>> @@ -10,6 +11,8 @@
>>>  *
>>>  * Datasheet:
>>>  *  http://www.meas-spec.com/downloads/HTU21D.pdf
>>> + * Datasheet:
>>> + *  http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
>>>  *
>>>  */
>>>
>>> @@ -25,6 +28,11 @@
>>>
>>> #define HTU21_RESET				0xFE
>>>
>>> +enum {
>>> +	HTU21,
>>> +	MS8607
>>> +};
>>> +
>>> static const int htu21_samp_freq[4] = { 20, 40, 70, 120 };
>>> /* String copy of the above const for readability purpose */
>>> static const char htu21_show_samp_freq[] = "20 40 70 120";
>>> @@ -107,6 +115,18 @@ static const struct iio_chan_spec htu21_channels[] = {
>>> 	 }
>>> };
>>>
>>> +/*
>>> + * Meas Spec recommendation is to not read temperature
>>> + * on this driver part for MS8607
>>> + */
>>> +static const struct iio_chan_spec ms8607_channels[] = {
>>> +	{
>>> +		.type = IIO_HUMIDITYRELATIVE,
>>> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
>>> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>>> +	 }
>>> +};
>>> +
>>> static ssize_t htu21_show_battery_low(struct device *dev,
>>> 				      struct device_attribute *attr, char *buf)
>>> {
>>> @@ -189,8 +209,14 @@ int htu21_probe(struct i2c_client *client,
>>> 	indio_dev->name = id->name;
>>> 	indio_dev->dev.parent = &client->dev;
>>> 	indio_dev->modes = INDIO_DIRECT_MODE;
>>> -	indio_dev->channels = htu21_channels;
>>> -	indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
>>> +
>>> +	if (id->driver_data == MS8607) {
>>> +		indio_dev->channels = ms8607_channels;
>>> +		indio_dev->num_channels = ARRAY_SIZE(ms8607_channels);
>>> +	} else {
>>> +		indio_dev->channels = htu21_channels;
>>> +		indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
>>> +	}
>>>
>>> 	i2c_set_clientdata(client, indio_dev);
>>>
>>> @@ -207,7 +233,8 @@ int htu21_probe(struct i2c_client *client,
>>> }
>>>
>>> static const struct i2c_device_id htu21_id[] = {
>>> -	{"htu21", 0},
>>> +	{"htu21", HTU21},
>>> +	{"ms8607-h", MS8607},
>>> 	{}
>>> };
>>>
>>> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
>>> index 8142cfe..53f9a44 100644
>>> --- a/drivers/iio/pressure/Kconfig
>>> +++ b/drivers/iio/pressure/Kconfig
>>> @@ -86,6 +86,8 @@ config MS5637
>>> 	help
>>> 	  If you say yes here you get support for the Measurement Specialties
>>> 	  MS5637 pressure and temperature sensor.
>>> +	  This driver is also used for MS8607 temperature, pressure & humidity
>>> +	  sensor
>>>
>>> 	  This driver can also be built as a module. If so, the module will
>>> 	  be called ms5637.
>>> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
>>> index 0dbbd4e..f6b5544 100644
>>> --- a/drivers/iio/pressure/ms5637.c
>>> +++ b/drivers/iio/pressure/ms5637.c
>>> @@ -1,5 +1,5 @@
>>> /*
>>> - * ms5637.c - Support for Measurement-Specialties ms5637
>>> + * ms5637.c - Support for Measurement-Specialties ms5637 and ms8607
>>>  *            pressure & temperature sensor
>>>  *
>>>  * Copyright (c) 2015 Measurement-Specialties
>>> @@ -10,8 +10,11 @@
>>>  *
>>>  * Datasheet:
>>>  *  http://www.meas-spec.com/downloads/MS5637-02BA03.pdf
>>> + * Datasheet:
>>> + *  http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
>>>  *
>>>  */
>>> +
>>> #include <linux/init.h>
>>> #include <linux/device.h>
>>> #include <linux/kernel.h>
>>> @@ -168,6 +171,7 @@ static int ms5637_probe(struct i2c_client *client,
>>>
>>> static const struct i2c_device_id ms5637_id[] = {
>>> 	{"ms5637", 0},
>>> +	{"ms8607-tp", 1},
>>> 	{}
>>> };
>>>
>>>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-09-29 17:27 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.