linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] iio: Add driver for Infineon DPS310
@ 2019-05-06 20:58 Eddie James
  2019-05-06 20:58 ` [PATCH 1/3] " Eddie James
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Eddie James @ 2019-05-06 20:58 UTC (permalink / raw)
  To: linux-iio; +Cc: linux-kernel, joel, pmeerw, lars, knaack.h, jic23, Eddie James

The DPS310 is a temperature and pressure sensor. It can be accessed over i2c
and SPI.

The driver supports polled measurement of temperature and pressure over i2c
only.

Christopher Bostic (1):
  iio: dps310: Temperature measurement errata

Eddie James (1):
  iio: dps310: Add pressure sensing capability

Joel Stanley (1):
  iio: Add driver for Infineon DPS310

 MAINTAINERS                   |   6 +
 drivers/iio/pressure/Kconfig  |  10 +
 drivers/iio/pressure/Makefile |   1 +
 drivers/iio/pressure/dps310.c | 753 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 770 insertions(+)
 create mode 100644 drivers/iio/pressure/dps310.c

-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH 1/3] iio: Add driver for Infineon DPS310
  2019-05-06 20:58 [PATCH 0/3] iio: Add driver for Infineon DPS310 Eddie James
@ 2019-05-06 20:58 ` Eddie James
  2019-05-06 21:18   ` Peter Meerwald-Stadler
  2019-05-06 20:58 ` [PATCH 2/3] iio: dps310: Temperature measurement errata Eddie James
  2019-05-06 20:58 ` [PATCH 3/3] iio: dps310: Add pressure sensing capability Eddie James
  2 siblings, 1 reply; 8+ messages in thread
From: Eddie James @ 2019-05-06 20:58 UTC (permalink / raw)
  To: linux-iio; +Cc: linux-kernel, joel, pmeerw, lars, knaack.h, jic23, Eddie James

From: Joel Stanley <joel@jms.id.au>

The DPS310 is a temperature and pressure sensor. It can be accessed over
i2c and SPI.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 MAINTAINERS                   |   6 +
 drivers/iio/pressure/Kconfig  |  10 +
 drivers/iio/pressure/Makefile |   1 +
 drivers/iio/pressure/dps310.c | 429 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 446 insertions(+)
 create mode 100644 drivers/iio/pressure/dps310.c

diff --git a/MAINTAINERS b/MAINTAINERS
index c61c49d..a91dfa9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7743,6 +7743,12 @@ W:	http://industrypack.sourceforge.net
 S:	Maintained
 F:	drivers/ipack/
 
+INFINEON DPS310 Driver
+M:	Eddie James <eajames@linux.ibm.com>
+L:	linux-iio@vger.kernel.org
+F:	drivers/iio/pressure/dps310.c
+S:	Maintained
+
 INFINIBAND SUBSYSTEM
 M:	Doug Ledford <dledford@redhat.com>
 M:	Jason Gunthorpe <jgg@mellanox.com>
diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index efeb89f..681a1cc 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -52,6 +52,16 @@ config IIO_CROS_EC_BARO
 	  To compile this driver as a module, choose M here: the module
 	  will be called cros_ec_baro.
 
+config DPS310
+       tristate "Infineon DPS310 pressure and temperature sensor"
+       depends on I2C
+       select REGMAP_I2C
+       help
+	 Support for the Infineon DPS310 digital barometric pressure sensor.
+
+	 This driver can also be built as a module.  If so, the module will be
+	 called dps310.
+
 config HID_SENSOR_PRESS
 	depends on HID_SENSOR_HUB
 	select IIO_BUFFER
diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
index c2058d7..d8f5ace 100644
--- a/drivers/iio/pressure/Makefile
+++ b/drivers/iio/pressure/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o
 bmp280-objs := bmp280-core.o bmp280-regmap.o
 obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o
 obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o
+obj-$(CONFIG_DPS310) += dps310.o
 obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o
 obj-$(CONFIG_HID_SENSOR_PRESS)   += hid-sensor-press.o
 obj-$(CONFIG_HP03) += hp03.o
diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
new file mode 100644
index 0000000..21d5dcb
--- /dev/null
+++ b/drivers/iio/pressure/dps310.c
@@ -0,0 +1,429 @@
+// SPDX-License-Identifier: GPL-2.0+
+// Copyright IBM Corp 2019
+/*
+ * The DPS310 is a barometric pressure and temperature sensor.
+ * Currently only reading a single temperature is supported by
+ * this driver.
+ *
+ * https://www.infineon.com/dgdl/?fileId=5546d462576f34750157750826c42242
+ *
+ * Temperature calculation:
+ *   c0 * 0.5 + c1 * T_raw / kT °C
+ *
+ * TODO:
+ *  - Pressure sensor readings
+ *  - Optionally support the FIFO
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define DPS310_PRS_B0		0x00
+#define DPS310_PRS_B1		0x01
+#define DPS310_PRS_B2		0x02
+#define DPS310_TMP_B0		0x03
+#define DPS310_TMP_B1		0x04
+#define DPS310_TMP_B2		0x05
+#define DPS310_PRS_CFG		0x06
+#define DPS310_TMP_CFG		0x07
+#define  DPS310_TMP_RATE_BITS	GENMASK(6, 4)
+#define  DPS310_TMP_PRC_BITS	GENMASK(3, 0)
+#define  DPS310_TMP_EXT		BIT(7)
+#define DPS310_MEAS_CFG		0x08
+#define  DPS310_MEAS_CTRL_BITS	GENMASK(2, 0)
+#define   DPS310_PRS_EN		BIT(0)
+#define   DPS310_TEMP_EN	BIT(1)
+#define   DPS310_BACKGROUND	BIT(2)
+#define  DPS310_PRS_RDY		BIT(4)
+#define  DPS310_TMP_RDY		BIT(5)
+#define  DPS310_SENSOR_RDY	BIT(6)
+#define  DPS310_COEF_RDY	BIT(7)
+#define DPS310_CFG_REG		0x09
+#define  DPS310_INT_HL		BIT(7)
+#define  DPS310_TMP_SHIFT_EN	BIT(3)
+#define  DPS310_PRS_SHIFT_EN	BIT(4)
+#define  DPS310_FIFO_EN		BIT(5)
+#define  DPS310_SPI_EN		BIT(6)
+#define DPS310_RESET		0x0c
+#define  DPS310_RESET_MAGIC	(BIT(0) | BIT(3))
+#define DPS310_COEF_BASE	0x10
+
+#define DPS310_PRS_BASE		DPS310_PRS_B0
+#define DPS310_TMP_BASE		DPS310_TMP_B0
+
+#define DPS310_CALC_RATE(_n)	ilog2(_n)
+#define DPS310_CALC_PRC(_n)	ilog2(_n)
+
+const int scale_factor[] = {
+	 524288,
+	1572864,
+	3670016,
+	7864320,
+	 253952,
+	 516096,
+	1040384,
+	2088960,
+};
+
+struct dps310_data {
+	struct i2c_client *client;
+	struct regmap *regmap;
+
+	s32 c0, c1;
+	s32 temp_raw;
+};
+
+static const struct iio_chan_spec dps310_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
+			BIT(IIO_CHAN_INFO_SCALE) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_RAW),
+	},
+};
+
+/* To be called after checking the TMP_RDY bit in MEAS_CFG */
+static int dps310_get_temp_coef(struct dps310_data *data)
+{
+	struct regmap *regmap = data->regmap;
+	u8 coef[3] = {0};
+	int r;
+	u32 c0, c1;
+
+	/*
+	 * Read temperature calibration coefficients c0 and c1 from the
+	 * COEF register. The numbers are 12-bit 2's compliment numbers
+	 */
+	r = regmap_bulk_read(regmap, DPS310_COEF_BASE, coef, 3);
+	if (r < 0)
+		return r;
+
+	c0 = (coef[0] << 4) | (coef[1] >> 4);
+	data->c0 = sign_extend32(c0, 11);
+
+	c1 = ((coef[1] & GENMASK(3, 0)) << 8) | coef[2];
+	data->c1 = sign_extend32(c1, 11);
+
+	return 0;
+}
+
+static int dps310_get_temp_precision(struct dps310_data *data)
+{
+	int val, r;
+
+	r = regmap_read(data->regmap, DPS310_TMP_CFG, &val);
+	if (r < 0)
+		return r;
+
+	/*
+	 * Scale factor is bottom 4 bits of the register, but 1111 is
+	 * reserved so just grab bottom three
+	 */
+	return BIT(val & GENMASK(2, 0));
+}
+
+static int dps310_set_temp_precision(struct dps310_data *data, int val)
+{
+	int ret;
+	u8 shift_en;
+
+	if (val < 0 || val > 128)
+		return -EINVAL;
+
+	shift_en = val >= 16 ? DPS310_TMP_SHIFT_EN : 0;
+	ret = regmap_write_bits(data->regmap, DPS310_CFG_REG,
+				DPS310_TMP_SHIFT_EN, shift_en);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(data->regmap, DPS310_TMP_CFG,
+				  DPS310_TMP_PRC_BITS, DPS310_CALC_PRC(val));
+}
+
+static int dps310_set_temp_samp_freq(struct dps310_data *data, int freq)
+{
+	u8 val;
+
+	if (freq < 0 || freq > 128)
+		return -EINVAL;
+
+	val = DPS310_CALC_RATE(freq) << 4;
+
+	return regmap_update_bits(data->regmap, DPS310_TMP_CFG,
+				  DPS310_TMP_RATE_BITS, val);
+}
+
+static int dps310_get_temp_samp_freq(struct dps310_data *data)
+{
+	int val, r;
+
+	r = regmap_read(data->regmap, DPS310_TMP_CFG, &val);
+	if (r < 0)
+		return r;
+
+	return BIT((val & DPS310_TMP_RATE_BITS) >> 4);
+}
+
+static int dps310_get_temp_k(struct dps310_data *data)
+{
+	int r = dps310_get_temp_precision(data);
+
+	if (r < 0)
+		return r;
+
+	return scale_factor[DPS310_CALC_PRC(r)];
+}
+
+static int dps310_read_temp(struct dps310_data *data)
+{
+	struct device *dev = &data->client->dev;
+	struct regmap *regmap = data->regmap;
+	u8 val[3] = {0};
+	int r, ready;
+	int T_raw;
+
+	r = regmap_read(regmap, DPS310_MEAS_CFG, &ready);
+	if (r < 0)
+		return r;
+	if (!(ready & DPS310_TMP_RDY)) {
+		dev_dbg(dev, "temperature not ready\n");
+		return -EAGAIN;
+	}
+
+	r = regmap_bulk_read(regmap, DPS310_TMP_BASE, val, 3);
+	if (r < 0)
+		return r;
+
+	T_raw = (val[0] << 16) | (val[1] << 8) | val[2];
+	data->temp_raw = sign_extend32(T_raw, 23);
+
+	return 0;
+}
+
+static bool dps310_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case DPS310_PRS_CFG:
+	case DPS310_TMP_CFG:
+	case DPS310_MEAS_CFG:
+	case DPS310_CFG_REG:
+	case DPS310_RESET:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool dps310_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case DPS310_PRS_B0:
+	case DPS310_PRS_B1:
+	case DPS310_PRS_B2:
+	case DPS310_TMP_B0:
+	case DPS310_TMP_B1:
+	case DPS310_TMP_B2:
+	case DPS310_MEAS_CFG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static int dps310_write_raw(struct iio_dev *iio,
+			    struct iio_chan_spec const *chan, int val,
+			    int val2, long mask)
+{
+	struct dps310_data *data = iio_priv(iio);
+
+	if (chan->type != IIO_TEMP)
+		return -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return dps310_set_temp_samp_freq(data, val);
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return dps310_set_temp_precision(data, val);
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static int dps310_read_raw(struct iio_dev *iio,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct dps310_data *data = iio_priv(iio);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = dps310_get_temp_samp_freq(data);
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_RAW:
+		ret = dps310_read_temp(data);
+		if (ret)
+			return ret;
+
+		*val = data->temp_raw * data->c1;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_OFFSET:
+		ret = dps310_get_temp_k(data);
+		if (ret < 0)
+			return ret;
+
+		*val = (data->c0 >> 1) * ret;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = dps310_get_temp_k(data);
+		if (ret < 0)
+			return ret;
+
+		*val = 1000; /* milliCelsius per Celsius */
+		*val2 = ret;
+		return IIO_VAL_FRACTIONAL;
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*val = dps310_get_temp_precision(data);
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}
+
+static const struct regmap_config dps310_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.writeable_reg = dps310_is_writeable_reg,
+	.volatile_reg = dps310_is_volatile_reg,
+	.cache_type = REGCACHE_RBTREE,
+	.max_register = 0x29,
+};
+
+static const struct iio_info dps310_info = {
+	.read_raw = dps310_read_raw,
+	.write_raw = dps310_write_raw,
+};
+
+static int dps310_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct dps310_data *data;
+	struct iio_dev *iio;
+	int r, ready;
+
+	iio = devm_iio_device_alloc(&client->dev,  sizeof(*data));
+	if (!iio)
+		return -ENOMEM;
+
+	data = iio_priv(iio);
+	data->client = client;
+
+	iio->dev.parent = &client->dev;
+	iio->name = id->name;
+	iio->channels = dps310_channels;
+	iio->num_channels = ARRAY_SIZE(dps310_channels);
+	iio->info = &dps310_info;
+	iio->modes = INDIO_DIRECT_MODE;
+
+	data->regmap = devm_regmap_init_i2c(client, &dps310_regmap_config);
+	if (IS_ERR(data->regmap))
+		return PTR_ERR(data->regmap);
+
+	/*
+	 * Set up external (MEMS) temperature sensor in single sample, one
+	 * measurement per second mode
+	 */
+	r = regmap_write(data->regmap, DPS310_TMP_CFG,
+			 DPS310_TMP_EXT | DPS310_CALC_RATE(1) |
+			 DPS310_CALC_PRC(1));
+	if (r < 0)
+		goto err;
+
+	/* Temp shift is disabled when PRC <= 8 */
+	r = regmap_write_bits(data->regmap, DPS310_CFG_REG,
+			      DPS310_TMP_SHIFT_EN, 0);
+	if (r < 0)
+		goto err;
+
+	/* Turn on temperature measurement in the background */
+	r = regmap_write_bits(data->regmap, DPS310_MEAS_CFG,
+			      DPS310_MEAS_CTRL_BITS,
+			      DPS310_TEMP_EN | DPS310_BACKGROUND);
+	if (r < 0)
+		goto err;
+
+	/*
+	 * Calibration coefficients required for reporting temperature.
+	 * They are available 40ms after the device has started
+	 */
+	r = regmap_read_poll_timeout(data->regmap, DPS310_MEAS_CFG, ready,
+				     ready & DPS310_COEF_RDY, 10000, 40000);
+	if (r < 0)
+		goto err;
+
+	r = dps310_get_temp_coef(data);
+	if (r < 0)
+		goto err;
+
+	r = devm_iio_device_register(&client->dev, iio);
+	if (r)
+		goto err;
+
+	i2c_set_clientdata(client, iio);
+
+	dev_info(&client->dev, "%s: sensor '%s'\n", dev_name(&iio->dev),
+		 client->name);
+
+	return 0;
+
+err:
+	regmap_write(data->regmap, DPS310_RESET, DPS310_RESET_MAGIC);
+	return r;
+}
+
+static int dps310_remove(struct i2c_client *client)
+{
+	struct dps310_data *data = i2c_get_clientdata(client);
+
+	return regmap_write(data->regmap, DPS310_RESET, DPS310_RESET_MAGIC);
+}
+
+static const struct i2c_device_id dps310_id[] = {
+	{ "dps310", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, dps310_id);
+
+static const unsigned short normal_i2c[] = {
+	0x77, 0x76, I2C_CLIENT_END
+};
+
+static struct i2c_driver dps310_driver = {
+	.driver = {
+		.name = "dps310",
+	},
+	.probe = dps310_probe,
+	.remove = dps310_remove,
+	.address_list = normal_i2c,
+	.id_table = dps310_id,
+};
+module_i2c_driver(dps310_driver);
+
+MODULE_AUTHOR("Joel Stanley <joel@jms.id.au>");
+MODULE_DESCRIPTION("Infineon DPS310 pressure and temperature sensor");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 2/3] iio: dps310: Temperature measurement errata
  2019-05-06 20:58 [PATCH 0/3] iio: Add driver for Infineon DPS310 Eddie James
  2019-05-06 20:58 ` [PATCH 1/3] " Eddie James
@ 2019-05-06 20:58 ` Eddie James
  2019-05-06 20:58 ` [PATCH 3/3] iio: dps310: Add pressure sensing capability Eddie James
  2 siblings, 0 replies; 8+ messages in thread
From: Eddie James @ 2019-05-06 20:58 UTC (permalink / raw)
  To: linux-iio
  Cc: linux-kernel, joel, pmeerw, lars, knaack.h, jic23,
	Christopher Bostic, Eddie James

From: Christopher Bostic <cbostic@linux.vnet.ibm.com>

Add a manufacturer's suggested workaround to deal with early revisions
of chip that don't indicate correct temperature. Readings can be in the
~60C range when they should be in the ~20's.

Signed-off-by: Christopher Bostic <cbostic@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/iio/pressure/dps310.c | 51 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index 21d5dcb..3e03685 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -214,6 +214,9 @@ static bool dps310_is_writeable_reg(struct device *dev, unsigned int reg)
 	case DPS310_MEAS_CFG:
 	case DPS310_CFG_REG:
 	case DPS310_RESET:
+	case 0x0e:
+	case 0x0f:
+	case 0x62:
 		return true;
 	default:
 		return false;
@@ -230,6 +233,7 @@ static bool dps310_is_volatile_reg(struct device *dev, unsigned int reg)
 	case DPS310_TMP_B1:
 	case DPS310_TMP_B2:
 	case DPS310_MEAS_CFG:
+	case 0x32:
 		return true;
 	default:
 		return false;
@@ -311,7 +315,7 @@ static const struct regmap_config dps310_regmap_config = {
 	.writeable_reg = dps310_is_writeable_reg,
 	.volatile_reg = dps310_is_volatile_reg,
 	.cache_type = REGCACHE_RBTREE,
-	.max_register = 0x29,
+	.max_register = 0x62,
 };
 
 static const struct iio_info dps310_info = {
@@ -319,6 +323,47 @@ static const struct iio_info dps310_info = {
 	.write_raw = dps310_write_raw,
 };
 
+/*
+ * Some verions of chip will read temperatures in the ~60C range when
+ * its actually ~20C. This is the manufacturer recommended workaround
+ * to correct the issue.
+ */
+static int dps310_temp_workaround(struct dps310_data *data)
+{
+	int r, reg;
+
+	r = regmap_read(data->regmap, 0x32, &reg);
+	if (r < 0)
+		return r;
+
+	/*
+	 * If bit 1 is set then the device is okay, and the workaround does not
+	 * need to be applied
+	 */
+	if (reg & BIT(1))
+		return 0;
+
+	r = regmap_write(data->regmap, 0x0e, 0xA5);
+	if (r < 0)
+		return r;
+
+	r = regmap_write(data->regmap, 0x0f, 0x96);
+	if (r < 0)
+		return r;
+
+	r = regmap_write(data->regmap, 0x62, 0x02);
+	if (r < 0)
+		return r;
+
+	r = regmap_write(data->regmap, 0x0e, 0x00);
+	if (r < 0)
+		return r;
+
+	r = regmap_write(data->regmap, 0x0f, 0x00);
+
+	return r;
+}
+
 static int dps310_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
@@ -380,6 +425,10 @@ static int dps310_probe(struct i2c_client *client,
 	if (r < 0)
 		goto err;
 
+	r = dps310_temp_workaround(data);
+	if (r < 0)
+		return r;
+
 	r = devm_iio_device_register(&client->dev, iio);
 	if (r)
 		goto err;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 3/3] iio: dps310: Add pressure sensing capability
  2019-05-06 20:58 [PATCH 0/3] iio: Add driver for Infineon DPS310 Eddie James
  2019-05-06 20:58 ` [PATCH 1/3] " Eddie James
  2019-05-06 20:58 ` [PATCH 2/3] iio: dps310: Temperature measurement errata Eddie James
@ 2019-05-06 20:58 ` Eddie James
  2019-05-06 21:25   ` Peter Meerwald-Stadler
  2 siblings, 1 reply; 8+ messages in thread
From: Eddie James @ 2019-05-06 20:58 UTC (permalink / raw)
  To: linux-iio; +Cc: linux-kernel, joel, pmeerw, lars, knaack.h, jic23, Eddie James

The DPS310 supports measurement of pressure, so support that in the
driver. Use background measurement like the temperature sensing and
default to lowest precision and lowest measurement rate.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/iio/pressure/dps310.c | 327 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 301 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index 3e03685..fe88480 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/i2c.h>
+#include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 
@@ -29,6 +30,8 @@
 #define DPS310_TMP_B1		0x04
 #define DPS310_TMP_B2		0x05
 #define DPS310_PRS_CFG		0x06
+#define  DPS310_PRS_RATE_BITS	GENMASK(6, 4)
+#define  DPS310_PRS_PRC_BITS	GENMASK(3, 0)
 #define DPS310_TMP_CFG		0x07
 #define  DPS310_TMP_RATE_BITS	GENMASK(6, 4)
 #define  DPS310_TMP_PRC_BITS	GENMASK(3, 0)
@@ -51,6 +54,7 @@
 #define DPS310_RESET		0x0c
 #define  DPS310_RESET_MAGIC	(BIT(0) | BIT(3))
 #define DPS310_COEF_BASE	0x10
+#define DPS310_NUM_COEF_REGS	0x12
 
 #define DPS310_PRS_BASE		DPS310_PRS_B0
 #define DPS310_TMP_BASE		DPS310_TMP_B0
@@ -58,7 +62,7 @@
 #define DPS310_CALC_RATE(_n)	ilog2(_n)
 #define DPS310_CALC_PRC(_n)	ilog2(_n)
 
-const int scale_factor[] = {
+static const int scale_factor[] = {
 	 524288,
 	1572864,
 	3670016,
@@ -74,6 +78,8 @@ struct dps310_data {
 	struct regmap *regmap;
 
 	s32 c0, c1;
+	s32 c00, c10, c20, c30, c01, c11, c21;
+	s32 pressure_raw;
 	s32 temp_raw;
 };
 
@@ -86,33 +92,81 @@ static const struct iio_chan_spec dps310_channels[] = {
 			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
 			BIT(IIO_CHAN_INFO_RAW),
 	},
+	{
+		.type = IIO_PRESSURE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_RAW),
+	},
 };
 
-/* To be called after checking the TMP_RDY bit in MEAS_CFG */
-static int dps310_get_temp_coef(struct dps310_data *data)
+/* To be called after checking the COEF_RDY bit in MEAS_CFG */
+static int dps310_get_coefs(struct dps310_data *data)
 {
 	struct regmap *regmap = data->regmap;
-	u8 coef[3] = {0};
+
 	int r;
+	u8 coef[DPS310_NUM_COEF_REGS] = {0};
 	u32 c0, c1;
+	u32 c00, c10, c20, c30, c01, c11, c21;
 
-	/*
-	 * Read temperature calibration coefficients c0 and c1 from the
-	 * COEF register. The numbers are 12-bit 2's compliment numbers
-	 */
-	r = regmap_bulk_read(regmap, DPS310_COEF_BASE, coef, 3);
+	/* Read all sensor calibration coefficients from the COEF registers. */
+	r = regmap_bulk_read(regmap, DPS310_COEF_BASE, coef,
+			     DPS310_NUM_COEF_REGS);
 	if (r < 0)
 		return r;
 
+	/*
+	 * Calculate temperature calibration coefficients c0 and c1. The numbers
+	 * are 12-bit 2's complement numbers.
+	 */
 	c0 = (coef[0] << 4) | (coef[1] >> 4);
 	data->c0 = sign_extend32(c0, 11);
 
 	c1 = ((coef[1] & GENMASK(3, 0)) << 8) | coef[2];
 	data->c1 = sign_extend32(c1, 11);
 
+	/*
+	 * Calculate pressure calibration coefficients. c00 and c10 are 20 bit
+	 * 2's complement numbers, while the rest are 16 bit 2's complement
+	 * numbers.
+	 */
+	c00 = (coef[3] << 12) | (coef[4] << 4) | (coef[5] >> 4);
+	data->c00 = sign_extend32(c00, 19);
+
+	c10 = ((coef[5] & GENMASK(3, 0)) << 16) | (coef[6] << 8) | coef[7];
+	data->c10 = sign_extend32(c10, 19);
+
+	c01 = (coef[8] << 8) | coef[9];
+	data->c01 = sign_extend32(c01, 15);
+
+	c11 = (coef[10] << 8) | coef[11];
+	data->c11 = sign_extend32(c11, 15);
+
+	c20 = (coef[12] << 8) | coef[13];
+	data->c20 = sign_extend32(c20, 15);
+
+	c21 = (coef[14] << 8) | coef[15];
+	data->c21 = sign_extend32(c21, 15);
+
+	c30 = (coef[16] << 8) | coef[17];
+	data->c30 = sign_extend32(c30, 15);
+
 	return 0;
 }
 
+static int dps310_get_pres_precision(struct dps310_data *data)
+{
+	int val, r;
+
+	r = regmap_read(data->regmap, DPS310_PRS_CFG, &val);
+	if (r < 0)
+		return r;
+
+	return BIT(val & GENMASK(2, 0));
+}
+
 static int dps310_get_temp_precision(struct dps310_data *data)
 {
 	int val, r;
@@ -128,6 +182,24 @@ static int dps310_get_temp_precision(struct dps310_data *data)
 	return BIT(val & GENMASK(2, 0));
 }
 
+static int dps310_set_pres_precision(struct dps310_data *data, int val)
+{
+	int ret;
+	u8 shift_en;
+
+	if (val < 0 || val > 128)
+		return -EINVAL;
+
+	shift_en = val >= 16 ? DPS310_PRS_SHIFT_EN : 0;
+	ret = regmap_write_bits(data->regmap, DPS310_CFG_REG,
+				DPS310_PRS_SHIFT_EN, shift_en);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(data->regmap, DPS310_PRS_CFG,
+				  DPS310_PRS_PRC_BITS, DPS310_CALC_PRC(val));
+}
+
 static int dps310_set_temp_precision(struct dps310_data *data, int val)
 {
 	int ret;
@@ -146,6 +218,19 @@ static int dps310_set_temp_precision(struct dps310_data *data, int val)
 				  DPS310_TMP_PRC_BITS, DPS310_CALC_PRC(val));
 }
 
+static int dps310_set_pres_samp_freq(struct dps310_data *data, int freq)
+{
+	u8 val;
+
+	if (freq < 0 || freq > 128)
+		return -EINVAL;
+
+	val = DPS310_CALC_RATE(freq) << 4;
+
+	return regmap_update_bits(data->regmap, DPS310_PRS_CFG,
+				  DPS310_PRS_RATE_BITS, val);
+}
+
 static int dps310_set_temp_samp_freq(struct dps310_data *data, int freq)
 {
 	u8 val;
@@ -159,6 +244,17 @@ static int dps310_set_temp_samp_freq(struct dps310_data *data, int freq)
 				  DPS310_TMP_RATE_BITS, val);
 }
 
+static int dps310_get_pres_samp_freq(struct dps310_data *data)
+{
+	int val, r;
+
+	r = regmap_read(data->regmap, DPS310_PRS_CFG, &val);
+	if (r < 0)
+		return r;
+
+	return BIT((val & DPS310_PRS_RATE_BITS) >> 4);
+}
+
 static int dps310_get_temp_samp_freq(struct dps310_data *data)
 {
 	int val, r;
@@ -170,6 +266,16 @@ static int dps310_get_temp_samp_freq(struct dps310_data *data)
 	return BIT((val & DPS310_TMP_RATE_BITS) >> 4);
 }
 
+static int dps310_get_pres_k(struct dps310_data *data)
+{
+	int r = dps310_get_pres_precision(data);
+
+	if (r < 0)
+		return r;
+
+	return scale_factor[DPS310_CALC_PRC(r)];
+}
+
 static int dps310_get_temp_k(struct dps310_data *data)
 {
 	int r = dps310_get_temp_precision(data);
@@ -180,7 +286,33 @@ static int dps310_get_temp_k(struct dps310_data *data)
 	return scale_factor[DPS310_CALC_PRC(r)];
 }
 
-static int dps310_read_temp(struct dps310_data *data)
+static int dps310_read_pres_raw(struct dps310_data *data)
+{
+	struct device *dev = &data->client->dev;
+	int r, ready;
+	u8 val[3];
+	s32 raw;
+
+	r = regmap_read(data->regmap, DPS310_MEAS_CFG, &ready);
+	if (r < 0)
+		return r;
+
+	if (!(ready & DPS310_PRS_RDY)) {
+		dev_dbg(dev, "pressure not ready\n");
+		return -EAGAIN;
+	}
+
+	r = regmap_bulk_read(data->regmap, DPS310_PRS_BASE, val, 3);
+	if (r < 0)
+		return r;
+
+	raw = (val[0] << 16) | (val[1] << 8) | val[2];
+	data->pressure_raw = sign_extend32(raw, 23);
+
+	return 0;
+}
+
+static int dps310_read_temp_raw(struct dps310_data *data)
 {
 	struct device *dev = &data->client->dev;
 	struct regmap *regmap = data->regmap;
@@ -246,26 +378,139 @@ static int dps310_write_raw(struct iio_dev *iio,
 {
 	struct dps310_data *data = iio_priv(iio);
 
-	if (chan->type != IIO_TEMP)
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		switch (chan->type) {
+		case IIO_PRESSURE:
+			return dps310_set_pres_samp_freq(data, val);
+
+		case IIO_TEMP:
+			return dps310_set_temp_samp_freq(data, val);
+
+		default:
+			return -EINVAL;
+		}
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		switch (chan->type) {
+		case IIO_PRESSURE:
+			return dps310_set_pres_precision(data, val);
+
+		case IIO_TEMP:
+			return dps310_set_temp_precision(data, val);
+
+		default:
+			return -EINVAL;
+		}
+
+	default:
 		return -EINVAL;
+	}
+}
+
+static int dps310_calculate_pressure(struct dps310_data *data)
+{
+	int i;
+	int kpi = dps310_get_pres_k(data);
+	int kti = dps310_get_temp_k(data);
+	s64 rem = 0ULL;
+	s64 pressure = 0ULL;
+	s64 p;
+	s64 t;
+	s64 denoms[7];
+	s64 nums[7];
+	s64 rems[7];
+	s64 kp;
+	s64 kt;
+
+	if (kpi < 0)
+		return kpi;
+
+	if (kti < 0)
+		return kti;
+
+	kp = (s64)kpi;
+	kt = (s64)kti;
+
+	/* Ignore errors and use the latest temperature */
+	dps310_read_temp_raw(data);
+
+	p = (s64)data->pressure_raw;
+	t = (s64)data->temp_raw;
+
+	/* Section 4.9.1 of the DPS310 spec; algebra'd to avoid underflow */
+	nums[0] = (s64)data->c00;
+	denoms[0] = 1LL;
+	nums[1] = p * (s64)data->c10;
+	denoms[1] = kp;
+	nums[2] = p * p * (s64)data->c20;
+	denoms[2] = kp * kp;
+	nums[3] = p * p * p * (s64)data->c30;
+	denoms[3] = kp * kp * kp;
+	nums[4] = t * (s64)data->c01;
+	denoms[4] = kt;
+	nums[5] = t * p * (s64)data->c11;
+	denoms[5] = kp * kt;
+	nums[6] = t * p * p * (s64)data->c21;
+	denoms[6] = kp * kp * kt;
+
+	/* Kernel lacks a div64_s64_rem function; denoms are all positive */
+	for (i = 0; i < 7; ++i) {
+		u64 rem;
+
+		if (nums[i] < 0LL) {
+			pressure -= div64_u64_rem(-nums[i], denoms[i], &rem);
+			rems[i] = -rem;
+		} else {
+			pressure += div64_u64_rem(nums[i], denoms[i], &rem);
+			rems[i] = (s64)rem;
+		}
+	}
+
+	/* Increase precision and calculate the remainder sum */
+	for (i = 0; i < 7; ++i)
+		rem += div64_s64((s64)rems[i] * 1000000000LL, denoms[i]);
+
+	pressure += div_s64(rem, 1000000000LL);
+
+	return (int)pressure;
+}
+
+static int dps310_read_pressure(struct dps310_data *data, int *val, int *val2,
+				long mask)
+{
+	int ret;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		return dps310_set_temp_samp_freq(data, val);
+		*val = dps310_get_pres_samp_freq(data);
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_RAW:
+		ret = dps310_read_pres_raw(data);
+		if (ret)
+			return ret;
+
+		*val = dps310_calculate_pressure(data);
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		*val = 1;
+		*val2 = 1000; /* Convert Pa to KPa per IIO ABI */
+		return IIO_VAL_FRACTIONAL;
+
 	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
-		return dps310_set_temp_precision(data, val);
+		*val = dps310_get_pres_precision(data);
+		return IIO_VAL_INT;
+
 	default:
 		return -EINVAL;
 	}
-
-	return -EINVAL;
 }
 
-static int dps310_read_raw(struct iio_dev *iio,
-			   struct iio_chan_spec const *chan,
-			   int *val, int *val2, long mask)
+static int dps310_read_temp(struct dps310_data *data, int *val, int *val2,
+			    long mask)
 {
-	struct dps310_data *data = iio_priv(iio);
 	int ret;
 
 	switch (mask) {
@@ -274,7 +519,7 @@ static int dps310_read_raw(struct iio_dev *iio,
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_RAW:
-		ret = dps310_read_temp(data);
+		ret = dps310_read_temp_raw(data);
 		if (ret)
 			return ret;
 
@@ -305,8 +550,24 @@ static int dps310_read_raw(struct iio_dev *iio,
 	default:
 		return -EINVAL;
 	}
+}
 
-	return -EINVAL;
+static int dps310_read_raw(struct iio_dev *iio,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct dps310_data *data = iio_priv(iio);
+
+	switch (chan->type) {
+	case IIO_PRESSURE:
+		return dps310_read_pressure(data, val, val2, mask);
+
+	case IIO_TEMP:
+		return dps310_read_temp(data, val, val2, mask);
+
+	default:
+		return -EINVAL;
+	}
 }
 
 static const struct regmap_config dps310_regmap_config = {
@@ -390,6 +651,13 @@ static int dps310_probe(struct i2c_client *client,
 		return PTR_ERR(data->regmap);
 
 	/*
+	 * Set up pressure sensor in single sample, one measurement per second
+	 * mode
+	 */
+	r = regmap_write(data->regmap, DPS310_PRS_CFG,
+			 DPS310_CALC_RATE(1) | DPS310_CALC_PRC(1));
+
+	/*
 	 * Set up external (MEMS) temperature sensor in single sample, one
 	 * measurement per second mode
 	 */
@@ -399,16 +667,23 @@ static int dps310_probe(struct i2c_client *client,
 	if (r < 0)
 		goto err;
 
-	/* Temp shift is disabled when PRC <= 8 */
+	/* Temp and pressure shifts are disabled when PRC <= 8 */
 	r = regmap_write_bits(data->regmap, DPS310_CFG_REG,
-			      DPS310_TMP_SHIFT_EN, 0);
+			      DPS310_TMP_SHIFT_EN | DPS310_PRS_SHIFT_EN, 0);
+	if (r < 0)
+		goto err;
+
+	/* MEAS_CFG doesn't seem to update unless first written with 0 */
+	r = regmap_write_bits(data->regmap, DPS310_MEAS_CFG,
+			      DPS310_MEAS_CTRL_BITS, 0);
 	if (r < 0)
 		goto err;
 
-	/* Turn on temperature measurement in the background */
+	/* Turn on temperature and pressure measurement in the background */
 	r = regmap_write_bits(data->regmap, DPS310_MEAS_CFG,
 			      DPS310_MEAS_CTRL_BITS,
-			      DPS310_TEMP_EN | DPS310_BACKGROUND);
+			      DPS310_PRS_EN | DPS310_TEMP_EN |
+			      DPS310_BACKGROUND);
 	if (r < 0)
 		goto err;
 
@@ -421,7 +696,7 @@ static int dps310_probe(struct i2c_client *client,
 	if (r < 0)
 		goto err;
 
-	r = dps310_get_temp_coef(data);
+	r = dps310_get_coefs(data);
 	if (r < 0)
 		goto err;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] iio: Add driver for Infineon DPS310
  2019-05-06 20:58 ` [PATCH 1/3] " Eddie James
@ 2019-05-06 21:18   ` Peter Meerwald-Stadler
  2019-05-07 16:01     ` Eddie James
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Meerwald-Stadler @ 2019-05-06 21:18 UTC (permalink / raw)
  To: Eddie James; +Cc: linux-iio, linux-kernel, joel, jic23

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

On Mon, 6 May 2019, Eddie James wrote:

> From: Joel Stanley <joel@jms.id.au>

some comments below
 
> The DPS310 is a temperature and pressure sensor. It can be accessed over
> i2c and SPI.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  MAINTAINERS                   |   6 +
>  drivers/iio/pressure/Kconfig  |  10 +
>  drivers/iio/pressure/Makefile |   1 +
>  drivers/iio/pressure/dps310.c | 429 ++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 446 insertions(+)
>  create mode 100644 drivers/iio/pressure/dps310.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c61c49d..a91dfa9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7743,6 +7743,12 @@ W:	http://industrypack.sourceforge.net
>  S:	Maintained
>  F:	drivers/ipack/
>  
> +INFINEON DPS310 Driver
> +M:	Eddie James <eajames@linux.ibm.com>
> +L:	linux-iio@vger.kernel.org
> +F:	drivers/iio/pressure/dps310.c
> +S:	Maintained
> +
>  INFINIBAND SUBSYSTEM
>  M:	Doug Ledford <dledford@redhat.com>
>  M:	Jason Gunthorpe <jgg@mellanox.com>
> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> index efeb89f..681a1cc 100644
> --- a/drivers/iio/pressure/Kconfig
> +++ b/drivers/iio/pressure/Kconfig
> @@ -52,6 +52,16 @@ config IIO_CROS_EC_BARO
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called cros_ec_baro.
>  
> +config DPS310
> +       tristate "Infineon DPS310 pressure and temperature sensor"
> +       depends on I2C
> +       select REGMAP_I2C
> +       help
> +	 Support for the Infineon DPS310 digital barometric pressure sensor.
> +
> +	 This driver can also be built as a module.  If so, the module will be
> +	 called dps310.
> +
>  config HID_SENSOR_PRESS
>  	depends on HID_SENSOR_HUB
>  	select IIO_BUFFER
> diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
> index c2058d7..d8f5ace 100644
> --- a/drivers/iio/pressure/Makefile
> +++ b/drivers/iio/pressure/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o
>  bmp280-objs := bmp280-core.o bmp280-regmap.o
>  obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o
>  obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o
> +obj-$(CONFIG_DPS310) += dps310.o
>  obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o
>  obj-$(CONFIG_HID_SENSOR_PRESS)   += hid-sensor-press.o
>  obj-$(CONFIG_HP03) += hp03.o
> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> new file mode 100644
> index 0000000..21d5dcb
> --- /dev/null
> +++ b/drivers/iio/pressure/dps310.c
> @@ -0,0 +1,429 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright IBM Corp 2019
> +/*
> + * The DPS310 is a barometric pressure and temperature sensor.
> + * Currently only reading a single temperature is supported by
> + * this driver.
> + *
> + * https://www.infineon.com/dgdl/?fileId=5546d462576f34750157750826c42242
> + *
> + * Temperature calculation:
> + *   c0 * 0.5 + c1 * T_raw / kT °C
> + *
> + * TODO:
> + *  - Pressure sensor readings
> + *  - Optionally support the FIFO
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define DPS310_PRS_B0		0x00
> +#define DPS310_PRS_B1		0x01
> +#define DPS310_PRS_B2		0x02
> +#define DPS310_TMP_B0		0x03
> +#define DPS310_TMP_B1		0x04
> +#define DPS310_TMP_B2		0x05
> +#define DPS310_PRS_CFG		0x06
> +#define DPS310_TMP_CFG		0x07
> +#define  DPS310_TMP_RATE_BITS	GENMASK(6, 4)
> +#define  DPS310_TMP_PRC_BITS	GENMASK(3, 0)
> +#define  DPS310_TMP_EXT		BIT(7)
> +#define DPS310_MEAS_CFG		0x08
> +#define  DPS310_MEAS_CTRL_BITS	GENMASK(2, 0)
> +#define   DPS310_PRS_EN		BIT(0)
> +#define   DPS310_TEMP_EN	BIT(1)
> +#define   DPS310_BACKGROUND	BIT(2)
> +#define  DPS310_PRS_RDY		BIT(4)
> +#define  DPS310_TMP_RDY		BIT(5)
> +#define  DPS310_SENSOR_RDY	BIT(6)
> +#define  DPS310_COEF_RDY	BIT(7)
> +#define DPS310_CFG_REG		0x09
> +#define  DPS310_INT_HL		BIT(7)
> +#define  DPS310_TMP_SHIFT_EN	BIT(3)
> +#define  DPS310_PRS_SHIFT_EN	BIT(4)
> +#define  DPS310_FIFO_EN		BIT(5)
> +#define  DPS310_SPI_EN		BIT(6)
> +#define DPS310_RESET		0x0c
> +#define  DPS310_RESET_MAGIC	(BIT(0) | BIT(3))
> +#define DPS310_COEF_BASE	0x10
> +
> +#define DPS310_PRS_BASE		DPS310_PRS_B0
> +#define DPS310_TMP_BASE		DPS310_TMP_B0
> +
> +#define DPS310_CALC_RATE(_n)	ilog2(_n)
> +#define DPS310_CALC_PRC(_n)	ilog2(_n)
> +
> +const int scale_factor[] = {

static
maybe plural, scale_factors?
maybe some comment would be good what these values relate to

> +	 524288,
> +	1572864,
> +	3670016,
> +	7864320,
> +	 253952,
> +	 516096,
> +	1040384,
> +	2088960,
> +};
> +
> +struct dps310_data {
> +	struct i2c_client *client;
> +	struct regmap *regmap;
> +
> +	s32 c0, c1;
> +	s32 temp_raw;
> +};
> +
> +static const struct iio_chan_spec dps310_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
> +			BIT(IIO_CHAN_INFO_SCALE) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_RAW),
> +	},
> +};
> +
> +/* To be called after checking the TMP_RDY bit in MEAS_CFG */
> +static int dps310_get_temp_coef(struct dps310_data *data)
> +{
> +	struct regmap *regmap = data->regmap;
> +	u8 coef[3] = {0};

initialization needed?

> +	int r;
> +	u32 c0, c1;
> +
> +	/*
> +	 * Read temperature calibration coefficients c0 and c1 from the
> +	 * COEF register. The numbers are 12-bit 2's compliment numbers
> +	 */
> +	r = regmap_bulk_read(regmap, DPS310_COEF_BASE, coef, 3);

sizeof(coef) instead of 3

> +	if (r < 0)
> +		return r;
> +
> +	c0 = (coef[0] << 4) | (coef[1] >> 4);
> +	data->c0 = sign_extend32(c0, 11);
> +
> +	c1 = ((coef[1] & GENMASK(3, 0)) << 8) | coef[2];
> +	data->c1 = sign_extend32(c1, 11);
> +
> +	return 0;
> +}
> +
> +static int dps310_get_temp_precision(struct dps310_data *data)
> +{
> +	int val, r;
> +
> +	r = regmap_read(data->regmap, DPS310_TMP_CFG, &val);
> +	if (r < 0)
> +		return r;
> +
> +	/*
> +	 * Scale factor is bottom 4 bits of the register, but 1111 is
> +	 * reserved so just grab bottom three
> +	 */
> +	return BIT(val & GENMASK(2, 0));
> +}
> +
> +static int dps310_set_temp_precision(struct dps310_data *data, int val)
> +{
> +	int ret;
> +	u8 shift_en;
> +
> +	if (val < 0 || val > 128)
> +		return -EINVAL;
> +
> +	shift_en = val >= 16 ? DPS310_TMP_SHIFT_EN : 0;
> +	ret = regmap_write_bits(data->regmap, DPS310_CFG_REG,
> +				DPS310_TMP_SHIFT_EN, shift_en);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(data->regmap, DPS310_TMP_CFG,
> +				  DPS310_TMP_PRC_BITS, DPS310_CALC_PRC(val));
> +}
> +
> +static int dps310_set_temp_samp_freq(struct dps310_data *data, int freq)
> +{
> +	u8 val;
> +
> +	if (freq < 0 || freq > 128)
> +		return -EINVAL;
> +
> +	val = DPS310_CALC_RATE(freq) << 4;
> +
> +	return regmap_update_bits(data->regmap, DPS310_TMP_CFG,
> +				  DPS310_TMP_RATE_BITS, val);
> +}
> +
> +static int dps310_get_temp_samp_freq(struct dps310_data *data)
> +{
> +	int val, r;
> +
> +	r = regmap_read(data->regmap, DPS310_TMP_CFG, &val);
> +	if (r < 0)
> +		return r;
> +
> +	return BIT((val & DPS310_TMP_RATE_BITS) >> 4);
> +}
> +
> +static int dps310_get_temp_k(struct dps310_data *data)
> +{
> +	int r = dps310_get_temp_precision(data);
> +
> +	if (r < 0)
> +		return r;
> +
> +	return scale_factor[DPS310_CALC_PRC(r)];
> +}
> +
> +static int dps310_read_temp(struct dps310_data *data)
> +{
> +	struct device *dev = &data->client->dev;
> +	struct regmap *regmap = data->regmap;
> +	u8 val[3] = {0};

initialization needed?

> +	int r, ready;
> +	int T_raw;
> +
> +	r = regmap_read(regmap, DPS310_MEAS_CFG, &ready);
> +	if (r < 0)
> +		return r;
> +	if (!(ready & DPS310_TMP_RDY)) {
> +		dev_dbg(dev, "temperature not ready\n");

most drivers wait for the results to become ready

> +		return -EAGAIN;
> +	}
> +
> +	r = regmap_bulk_read(regmap, DPS310_TMP_BASE, val, 3);

sizeof(val)

> +	if (r < 0)
> +		return r;
> +
> +	T_raw = (val[0] << 16) | (val[1] << 8) | val[2];
> +	data->temp_raw = sign_extend32(T_raw, 23);
> +
> +	return 0;
> +}
> +
> +static bool dps310_is_writeable_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case DPS310_PRS_CFG:
> +	case DPS310_TMP_CFG:
> +	case DPS310_MEAS_CFG:
> +	case DPS310_CFG_REG:
> +	case DPS310_RESET:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static bool dps310_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case DPS310_PRS_B0:
> +	case DPS310_PRS_B1:
> +	case DPS310_PRS_B2:
> +	case DPS310_TMP_B0:
> +	case DPS310_TMP_B1:
> +	case DPS310_TMP_B2:
> +	case DPS310_MEAS_CFG:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static int dps310_write_raw(struct iio_dev *iio,
> +			    struct iio_chan_spec const *chan, int val,
> +			    int val2, long mask)
> +{
> +	struct dps310_data *data = iio_priv(iio);
> +
> +	if (chan->type != IIO_TEMP)
> +		return -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return dps310_set_temp_samp_freq(data, val);
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		return dps310_set_temp_precision(data, val);
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;

not needed, dead code

> +}
> +
> +static int dps310_read_raw(struct iio_dev *iio,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct dps310_data *data = iio_priv(iio);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = dps310_get_temp_samp_freq(data);
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_RAW:
> +		ret = dps310_read_temp(data);
> +		if (ret)
> +			return ret;
> +
> +		*val = data->temp_raw * data->c1;

one could try to avoid the multipliation here and somehow mangle it into 
_SCALE/_OFFSET

> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_OFFSET:
> +		ret = dps310_get_temp_k(data);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = (data->c0 >> 1) * ret;
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = dps310_get_temp_k(data);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = 1000; /* milliCelsius per Celsius */
> +		*val2 = ret;
> +		return IIO_VAL_FRACTIONAL;
> +
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		*val = dps310_get_temp_precision(data);
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return -EINVAL;

not needed, dead code

> +}
> +
> +static const struct regmap_config dps310_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.writeable_reg = dps310_is_writeable_reg,
> +	.volatile_reg = dps310_is_volatile_reg,
> +	.cache_type = REGCACHE_RBTREE,
> +	.max_register = 0x29,
> +};
> +
> +static const struct iio_info dps310_info = {
> +	.read_raw = dps310_read_raw,
> +	.write_raw = dps310_write_raw,
> +};
> +
> +static int dps310_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct dps310_data *data;
> +	struct iio_dev *iio;
> +	int r, ready;
> +
> +	iio = devm_iio_device_alloc(&client->dev,  sizeof(*data));
> +	if (!iio)
> +		return -ENOMEM;
> +
> +	data = iio_priv(iio);
> +	data->client = client;
> +
> +	iio->dev.parent = &client->dev;
> +	iio->name = id->name;
> +	iio->channels = dps310_channels;
> +	iio->num_channels = ARRAY_SIZE(dps310_channels);
> +	iio->info = &dps310_info;
> +	iio->modes = INDIO_DIRECT_MODE;
> +
> +	data->regmap = devm_regmap_init_i2c(client, &dps310_regmap_config);
> +	if (IS_ERR(data->regmap))
> +		return PTR_ERR(data->regmap);
> +
> +	/*
> +	 * Set up external (MEMS) temperature sensor in single sample, one
> +	 * measurement per second mode
> +	 */
> +	r = regmap_write(data->regmap, DPS310_TMP_CFG,
> +			 DPS310_TMP_EXT | DPS310_CALC_RATE(1) |
> +			 DPS310_CALC_PRC(1));
> +	if (r < 0)
> +		goto err;
> +
> +	/* Temp shift is disabled when PRC <= 8 */
> +	r = regmap_write_bits(data->regmap, DPS310_CFG_REG,
> +			      DPS310_TMP_SHIFT_EN, 0);
> +	if (r < 0)
> +		goto err;
> +
> +	/* Turn on temperature measurement in the background */
> +	r = regmap_write_bits(data->regmap, DPS310_MEAS_CFG,
> +			      DPS310_MEAS_CTRL_BITS,
> +			      DPS310_TEMP_EN | DPS310_BACKGROUND);
> +	if (r < 0)
> +		goto err;
> +
> +	/*
> +	 * Calibration coefficients required for reporting temperature.
> +	 * They are available 40ms after the device has started
> +	 */
> +	r = regmap_read_poll_timeout(data->regmap, DPS310_MEAS_CFG, ready,
> +				     ready & DPS310_COEF_RDY, 10000, 40000);
> +	if (r < 0)
> +		goto err;
> +
> +	r = dps310_get_temp_coef(data);
> +	if (r < 0)
> +		goto err;
> +
> +	r = devm_iio_device_register(&client->dev, iio);
> +	if (r)
> +		goto err;
> +
> +	i2c_set_clientdata(client, iio);
> +
> +	dev_info(&client->dev, "%s: sensor '%s'\n", dev_name(&iio->dev),
> +		 client->name);

don't clutter the log

> +
> +	return 0;
> +
> +err:
> +	regmap_write(data->regmap, DPS310_RESET, DPS310_RESET_MAGIC);
> +	return r;
> +}
> +
> +static int dps310_remove(struct i2c_client *client)
> +{
> +	struct dps310_data *data = i2c_get_clientdata(client);
> +
> +	return regmap_write(data->regmap, DPS310_RESET, DPS310_RESET_MAGIC);
> +}
> +
> +static const struct i2c_device_id dps310_id[] = {
> +	{ "dps310", 0 },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, dps310_id);
> +
> +static const unsigned short normal_i2c[] = {
> +	0x77, 0x76, I2C_CLIENT_END
> +};
> +
> +static struct i2c_driver dps310_driver = {
> +	.driver = {
> +		.name = "dps310",
> +	},
> +	.probe = dps310_probe,
> +	.remove = dps310_remove,
> +	.address_list = normal_i2c,
> +	.id_table = dps310_id,
> +};
> +module_i2c_driver(dps310_driver);
> +
> +MODULE_AUTHOR("Joel Stanley <joel@jms.id.au>");
> +MODULE_DESCRIPTION("Infineon DPS310 pressure and temperature sensor");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald-Stadler
Mobile: +43 664 24 44 418

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] iio: dps310: Add pressure sensing capability
  2019-05-06 20:58 ` [PATCH 3/3] iio: dps310: Add pressure sensing capability Eddie James
@ 2019-05-06 21:25   ` Peter Meerwald-Stadler
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Meerwald-Stadler @ 2019-05-06 21:25 UTC (permalink / raw)
  To: Eddie James; +Cc: linux-iio, linux-kernel, joel, jic23

On Mon, 6 May 2019, Eddie James wrote:

some comments below

> The DPS310 supports measurement of pressure, so support that in the
> driver. Use background measurement like the temperature sensing and
> default to lowest precision and lowest measurement rate.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/iio/pressure/dps310.c | 327 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 301 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> index 3e03685..fe88480 100644
> --- a/drivers/iio/pressure/dps310.c
> +++ b/drivers/iio/pressure/dps310.c
> @@ -16,6 +16,7 @@
>   */

this should update the TODO, right? it implements pressure

>  
>  #include <linux/i2c.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
>  
> @@ -29,6 +30,8 @@
>  #define DPS310_TMP_B1		0x04
>  #define DPS310_TMP_B2		0x05
>  #define DPS310_PRS_CFG		0x06
> +#define  DPS310_PRS_RATE_BITS	GENMASK(6, 4)
> +#define  DPS310_PRS_PRC_BITS	GENMASK(3, 0)
>  #define DPS310_TMP_CFG		0x07
>  #define  DPS310_TMP_RATE_BITS	GENMASK(6, 4)
>  #define  DPS310_TMP_PRC_BITS	GENMASK(3, 0)
> @@ -51,6 +54,7 @@
>  #define DPS310_RESET		0x0c
>  #define  DPS310_RESET_MAGIC	(BIT(0) | BIT(3))
>  #define DPS310_COEF_BASE	0x10
> +#define DPS310_NUM_COEF_REGS	0x12
>  
>  #define DPS310_PRS_BASE		DPS310_PRS_B0
>  #define DPS310_TMP_BASE		DPS310_TMP_B0
> @@ -58,7 +62,7 @@
>  #define DPS310_CALC_RATE(_n)	ilog2(_n)
>  #define DPS310_CALC_PRC(_n)	ilog2(_n)
>  
> -const int scale_factor[] = {
> +static const int scale_factor[] = {

this change should be in the previous/initial patch

>  	 524288,
>  	1572864,
>  	3670016,
> @@ -74,6 +78,8 @@ struct dps310_data {
>  	struct regmap *regmap;
>  
>  	s32 c0, c1;
> +	s32 c00, c10, c20, c30, c01, c11, c21;
> +	s32 pressure_raw;
>  	s32 temp_raw;
>  };
>  
> @@ -86,33 +92,81 @@ static const struct iio_chan_spec dps310_channels[] = {
>  			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>  			BIT(IIO_CHAN_INFO_RAW),
>  	},
> +	{
> +		.type = IIO_PRESSURE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_SCALE) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_RAW),
> +	},
>  };
>  
> -/* To be called after checking the TMP_RDY bit in MEAS_CFG */
> -static int dps310_get_temp_coef(struct dps310_data *data)
> +/* To be called after checking the COEF_RDY bit in MEAS_CFG */
> +static int dps310_get_coefs(struct dps310_data *data)
>  {
>  	struct regmap *regmap = data->regmap;
> -	u8 coef[3] = {0};
> +
>  	int r;
> +	u8 coef[DPS310_NUM_COEF_REGS] = {0};
>  	u32 c0, c1;
> +	u32 c00, c10, c20, c30, c01, c11, c21;
>  
> -	/*
> -	 * Read temperature calibration coefficients c0 and c1 from the
> -	 * COEF register. The numbers are 12-bit 2's compliment numbers
> -	 */
> -	r = regmap_bulk_read(regmap, DPS310_COEF_BASE, coef, 3);
> +	/* Read all sensor calibration coefficients from the COEF registers. */
> +	r = regmap_bulk_read(regmap, DPS310_COEF_BASE, coef,
> +			     DPS310_NUM_COEF_REGS);
>  	if (r < 0)
>  		return r;
>  
> +	/*
> +	 * Calculate temperature calibration coefficients c0 and c1. The numbers
> +	 * are 12-bit 2's complement numbers.
> +	 */
>  	c0 = (coef[0] << 4) | (coef[1] >> 4);
>  	data->c0 = sign_extend32(c0, 11);
>  
>  	c1 = ((coef[1] & GENMASK(3, 0)) << 8) | coef[2];
>  	data->c1 = sign_extend32(c1, 11);
>  
> +	/*
> +	 * Calculate pressure calibration coefficients. c00 and c10 are 20 bit
> +	 * 2's complement numbers, while the rest are 16 bit 2's complement
> +	 * numbers.
> +	 */
> +	c00 = (coef[3] << 12) | (coef[4] << 4) | (coef[5] >> 4);
> +	data->c00 = sign_extend32(c00, 19);
> +
> +	c10 = ((coef[5] & GENMASK(3, 0)) << 16) | (coef[6] << 8) | coef[7];
> +	data->c10 = sign_extend32(c10, 19);
> +
> +	c01 = (coef[8] << 8) | coef[9];
> +	data->c01 = sign_extend32(c01, 15);
> +
> +	c11 = (coef[10] << 8) | coef[11];
> +	data->c11 = sign_extend32(c11, 15);
> +
> +	c20 = (coef[12] << 8) | coef[13];
> +	data->c20 = sign_extend32(c20, 15);
> +
> +	c21 = (coef[14] << 8) | coef[15];
> +	data->c21 = sign_extend32(c21, 15);
> +
> +	c30 = (coef[16] << 8) | coef[17];
> +	data->c30 = sign_extend32(c30, 15);
> +
>  	return 0;
>  }
>  
> +static int dps310_get_pres_precision(struct dps310_data *data)
> +{
> +	int val, r;
> +
> +	r = regmap_read(data->regmap, DPS310_PRS_CFG, &val);
> +	if (r < 0)
> +		return r;
> +
> +	return BIT(val & GENMASK(2, 0));
> +}
> +
>  static int dps310_get_temp_precision(struct dps310_data *data)
>  {
>  	int val, r;
> @@ -128,6 +182,24 @@ static int dps310_get_temp_precision(struct dps310_data *data)
>  	return BIT(val & GENMASK(2, 0));
>  }
>  
> +static int dps310_set_pres_precision(struct dps310_data *data, int val)
> +{
> +	int ret;
> +	u8 shift_en;
> +
> +	if (val < 0 || val > 128)
> +		return -EINVAL;
> +
> +	shift_en = val >= 16 ? DPS310_PRS_SHIFT_EN : 0;
> +	ret = regmap_write_bits(data->regmap, DPS310_CFG_REG,
> +				DPS310_PRS_SHIFT_EN, shift_en);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_update_bits(data->regmap, DPS310_PRS_CFG,
> +				  DPS310_PRS_PRC_BITS, DPS310_CALC_PRC(val));
> +}
> +
>  static int dps310_set_temp_precision(struct dps310_data *data, int val)
>  {
>  	int ret;
> @@ -146,6 +218,19 @@ static int dps310_set_temp_precision(struct dps310_data *data, int val)
>  				  DPS310_TMP_PRC_BITS, DPS310_CALC_PRC(val));
>  }
>  
> +static int dps310_set_pres_samp_freq(struct dps310_data *data, int freq)
> +{
> +	u8 val;
> +
> +	if (freq < 0 || freq > 128)
> +		return -EINVAL;
> +
> +	val = DPS310_CALC_RATE(freq) << 4;
> +
> +	return regmap_update_bits(data->regmap, DPS310_PRS_CFG,
> +				  DPS310_PRS_RATE_BITS, val);
> +}
> +
>  static int dps310_set_temp_samp_freq(struct dps310_data *data, int freq)
>  {
>  	u8 val;
> @@ -159,6 +244,17 @@ static int dps310_set_temp_samp_freq(struct dps310_data *data, int freq)
>  				  DPS310_TMP_RATE_BITS, val);
>  }
>  
> +static int dps310_get_pres_samp_freq(struct dps310_data *data)
> +{
> +	int val, r;
> +
> +	r = regmap_read(data->regmap, DPS310_PRS_CFG, &val);
> +	if (r < 0)
> +		return r;
> +
> +	return BIT((val & DPS310_PRS_RATE_BITS) >> 4);
> +}
> +
>  static int dps310_get_temp_samp_freq(struct dps310_data *data)
>  {
>  	int val, r;
> @@ -170,6 +266,16 @@ static int dps310_get_temp_samp_freq(struct dps310_data *data)
>  	return BIT((val & DPS310_TMP_RATE_BITS) >> 4);
>  }
>  
> +static int dps310_get_pres_k(struct dps310_data *data)
> +{
> +	int r = dps310_get_pres_precision(data);
> +
> +	if (r < 0)
> +		return r;
> +
> +	return scale_factor[DPS310_CALC_PRC(r)];
> +}
> +
>  static int dps310_get_temp_k(struct dps310_data *data)
>  {
>  	int r = dps310_get_temp_precision(data);
> @@ -180,7 +286,33 @@ static int dps310_get_temp_k(struct dps310_data *data)
>  	return scale_factor[DPS310_CALC_PRC(r)];
>  }
>  
> -static int dps310_read_temp(struct dps310_data *data)
> +static int dps310_read_pres_raw(struct dps310_data *data)
> +{
> +	struct device *dev = &data->client->dev;
> +	int r, ready;
> +	u8 val[3];
> +	s32 raw;
> +
> +	r = regmap_read(data->regmap, DPS310_MEAS_CFG, &ready);
> +	if (r < 0)
> +		return r;
> +
> +	if (!(ready & DPS310_PRS_RDY)) {
> +		dev_dbg(dev, "pressure not ready\n");
> +		return -EAGAIN;
> +	}
> +
> +	r = regmap_bulk_read(data->regmap, DPS310_PRS_BASE, val, 3);
> +	if (r < 0)
> +		return r;
> +
> +	raw = (val[0] << 16) | (val[1] << 8) | val[2];
> +	data->pressure_raw = sign_extend32(raw, 23);
> +
> +	return 0;
> +}
> +
> +static int dps310_read_temp_raw(struct dps310_data *data)
>  {
>  	struct device *dev = &data->client->dev;
>  	struct regmap *regmap = data->regmap;
> @@ -246,26 +378,139 @@ static int dps310_write_raw(struct iio_dev *iio,
>  {
>  	struct dps310_data *data = iio_priv(iio);
>  
> -	if (chan->type != IIO_TEMP)
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		switch (chan->type) {
> +		case IIO_PRESSURE:
> +			return dps310_set_pres_samp_freq(data, val);
> +
> +		case IIO_TEMP:
> +			return dps310_set_temp_samp_freq(data, val);
> +
> +		default:
> +			return -EINVAL;
> +		}
> +
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		switch (chan->type) {
> +		case IIO_PRESSURE:
> +			return dps310_set_pres_precision(data, val);
> +
> +		case IIO_TEMP:
> +			return dps310_set_temp_precision(data, val);
> +
> +		default:
> +			return -EINVAL;
> +		}
> +
> +	default:
>  		return -EINVAL;
> +	}
> +}
> +
> +static int dps310_calculate_pressure(struct dps310_data *data)
> +{
> +	int i;
> +	int kpi = dps310_get_pres_k(data);
> +	int kti = dps310_get_temp_k(data);
> +	s64 rem = 0ULL;
> +	s64 pressure = 0ULL;
> +	s64 p;
> +	s64 t;
> +	s64 denoms[7];
> +	s64 nums[7];
> +	s64 rems[7];
> +	s64 kp;
> +	s64 kt;
> +
> +	if (kpi < 0)
> +		return kpi;
> +
> +	if (kti < 0)
> +		return kti;
> +
> +	kp = (s64)kpi;
> +	kt = (s64)kti;
> +
> +	/* Ignore errors and use the latest temperature */
> +	dps310_read_temp_raw(data);
> +
> +	p = (s64)data->pressure_raw;
> +	t = (s64)data->temp_raw;
> +
> +	/* Section 4.9.1 of the DPS310 spec; algebra'd to avoid underflow */
> +	nums[0] = (s64)data->c00;
> +	denoms[0] = 1LL;
> +	nums[1] = p * (s64)data->c10;
> +	denoms[1] = kp;
> +	nums[2] = p * p * (s64)data->c20;
> +	denoms[2] = kp * kp;
> +	nums[3] = p * p * p * (s64)data->c30;
> +	denoms[3] = kp * kp * kp;
> +	nums[4] = t * (s64)data->c01;
> +	denoms[4] = kt;
> +	nums[5] = t * p * (s64)data->c11;
> +	denoms[5] = kp * kt;
> +	nums[6] = t * p * p * (s64)data->c21;
> +	denoms[6] = kp * kp * kt;
> +
> +	/* Kernel lacks a div64_s64_rem function; denoms are all positive */
> +	for (i = 0; i < 7; ++i) {
> +		u64 rem;
> +
> +		if (nums[i] < 0LL) {
> +			pressure -= div64_u64_rem(-nums[i], denoms[i], &rem);
> +			rems[i] = -rem;
> +		} else {
> +			pressure += div64_u64_rem(nums[i], denoms[i], &rem);
> +			rems[i] = (s64)rem;
> +		}
> +	}
> +
> +	/* Increase precision and calculate the remainder sum */
> +	for (i = 0; i < 7; ++i)
> +		rem += div64_s64((s64)rems[i] * 1000000000LL, denoms[i]);
> +
> +	pressure += div_s64(rem, 1000000000LL);
> +
> +	return (int)pressure;
> +}
> +
> +static int dps310_read_pressure(struct dps310_data *data, int *val, int *val2,
> +				long mask)
> +{
> +	int ret;
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		return dps310_set_temp_samp_freq(data, val);
> +		*val = dps310_get_pres_samp_freq(data);
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_RAW:
> +		ret = dps310_read_pres_raw(data);
> +		if (ret)
> +			return ret;
> +
> +		*val = dps310_calculate_pressure(data);
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 1;
> +		*val2 = 1000; /* Convert Pa to KPa per IIO ABI */
> +		return IIO_VAL_FRACTIONAL;
> +
>  	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> -		return dps310_set_temp_precision(data, val);
> +		*val = dps310_get_pres_precision(data);
> +		return IIO_VAL_INT;
> +
>  	default:
>  		return -EINVAL;
>  	}
> -
> -	return -EINVAL;
>  }
>  
> -static int dps310_read_raw(struct iio_dev *iio,
> -			   struct iio_chan_spec const *chan,
> -			   int *val, int *val2, long mask)
> +static int dps310_read_temp(struct dps310_data *data, int *val, int *val2,
> +			    long mask)
>  {
> -	struct dps310_data *data = iio_priv(iio);
>  	int ret;
>  
>  	switch (mask) {
> @@ -274,7 +519,7 @@ static int dps310_read_raw(struct iio_dev *iio,
>  		return IIO_VAL_INT;
>  
>  	case IIO_CHAN_INFO_RAW:
> -		ret = dps310_read_temp(data);
> +		ret = dps310_read_temp_raw(data);
>  		if (ret)
>  			return ret;
>  
> @@ -305,8 +550,24 @@ static int dps310_read_raw(struct iio_dev *iio,
>  	default:
>  		return -EINVAL;
>  	}
> +}
>  
> -	return -EINVAL;
> +static int dps310_read_raw(struct iio_dev *iio,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	struct dps310_data *data = iio_priv(iio);
> +
> +	switch (chan->type) {
> +	case IIO_PRESSURE:
> +		return dps310_read_pressure(data, val, val2, mask);
> +
> +	case IIO_TEMP:
> +		return dps310_read_temp(data, val, val2, mask);
> +
> +	default:
> +		return -EINVAL;
> +	}
>  }
>  
>  static const struct regmap_config dps310_regmap_config = {
> @@ -390,6 +651,13 @@ static int dps310_probe(struct i2c_client *client,
>  		return PTR_ERR(data->regmap);
>  
>  	/*
> +	 * Set up pressure sensor in single sample, one measurement per second
> +	 * mode
> +	 */
> +	r = regmap_write(data->regmap, DPS310_PRS_CFG,
> +			 DPS310_CALC_RATE(1) | DPS310_CALC_PRC(1));
> +
> +	/*
>  	 * Set up external (MEMS) temperature sensor in single sample, one
>  	 * measurement per second mode
>  	 */
> @@ -399,16 +667,23 @@ static int dps310_probe(struct i2c_client *client,
>  	if (r < 0)
>  		goto err;
>  
> -	/* Temp shift is disabled when PRC <= 8 */
> +	/* Temp and pressure shifts are disabled when PRC <= 8 */
>  	r = regmap_write_bits(data->regmap, DPS310_CFG_REG,
> -			      DPS310_TMP_SHIFT_EN, 0);
> +			      DPS310_TMP_SHIFT_EN | DPS310_PRS_SHIFT_EN, 0);
> +	if (r < 0)
> +		goto err;
> +
> +	/* MEAS_CFG doesn't seem to update unless first written with 0 */
> +	r = regmap_write_bits(data->regmap, DPS310_MEAS_CFG,
> +			      DPS310_MEAS_CTRL_BITS, 0);
>  	if (r < 0)
>  		goto err;
>  
> -	/* Turn on temperature measurement in the background */
> +	/* Turn on temperature and pressure measurement in the background */
>  	r = regmap_write_bits(data->regmap, DPS310_MEAS_CFG,
>  			      DPS310_MEAS_CTRL_BITS,
> -			      DPS310_TEMP_EN | DPS310_BACKGROUND);
> +			      DPS310_PRS_EN | DPS310_TEMP_EN |
> +			      DPS310_BACKGROUND);
>  	if (r < 0)
>  		goto err;
>  
> @@ -421,7 +696,7 @@ static int dps310_probe(struct i2c_client *client,
>  	if (r < 0)
>  		goto err;
>  
> -	r = dps310_get_temp_coef(data);
> +	r = dps310_get_coefs(data);
>  	if (r < 0)
>  		goto err;
>  
> 

-- 

Peter Meerwald-Stadler
Mobile: +43 664 24 44 418

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] iio: Add driver for Infineon DPS310
  2019-05-06 21:18   ` Peter Meerwald-Stadler
@ 2019-05-07 16:01     ` Eddie James
  2019-05-11  9:52       ` Jonathan Cameron
  0 siblings, 1 reply; 8+ messages in thread
From: Eddie James @ 2019-05-07 16:01 UTC (permalink / raw)
  To: Peter Meerwald-Stadler, Eddie James; +Cc: linux-iio, linux-kernel, joel, jic23


On 5/6/19 4:18 PM, Peter Meerwald-Stadler wrote:
> On Mon, 6 May 2019, Eddie James wrote:
>
>> From: Joel Stanley <joel@jms.id.au>
> some comments below


Thanks for the quick review! Will get these in for v2, I did have a 
couple of questions below though.

Eddie


>   
>> The DPS310 is a temperature and pressure sensor. It can be accessed over
>> i2c and SPI.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   MAINTAINERS                   |   6 +
>>   drivers/iio/pressure/Kconfig  |  10 +
>>   drivers/iio/pressure/Makefile |   1 +
>>   drivers/iio/pressure/dps310.c | 429 ++++++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 446 insertions(+)
>>   create mode 100644 drivers/iio/pressure/dps310.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c61c49d..a91dfa9 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -7743,6 +7743,12 @@ W:	http://industrypack.sourceforge.net
>>   S:	Maintained
>>   F:	drivers/ipack/
>>   
>> +INFINEON DPS310 Driver
>> +M:	Eddie James <eajames@linux.ibm.com>
>> +L:	linux-iio@vger.kernel.org
>> +F:	drivers/iio/pressure/dps310.c
>> +S:	Maintained
>> +
>>   INFINIBAND SUBSYSTEM
>>   M:	Doug Ledford <dledford@redhat.com>
>>   M:	Jason Gunthorpe <jgg@mellanox.com>
>> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
>> index efeb89f..681a1cc 100644
>> --- a/drivers/iio/pressure/Kconfig
>> +++ b/drivers/iio/pressure/Kconfig
>> @@ -52,6 +52,16 @@ config IIO_CROS_EC_BARO
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called cros_ec_baro.
>>   
>> +config DPS310
>> +       tristate "Infineon DPS310 pressure and temperature sensor"
>> +       depends on I2C
>> +       select REGMAP_I2C
>> +       help
>> +	 Support for the Infineon DPS310 digital barometric pressure sensor.
>> +
>> +	 This driver can also be built as a module.  If so, the module will be
>> +	 called dps310.
>> +
>>   config HID_SENSOR_PRESS
>>   	depends on HID_SENSOR_HUB
>>   	select IIO_BUFFER
>> diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
>> index c2058d7..d8f5ace 100644
>> --- a/drivers/iio/pressure/Makefile
>> +++ b/drivers/iio/pressure/Makefile
>> @@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o
>>   bmp280-objs := bmp280-core.o bmp280-regmap.o
>>   obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o
>>   obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o
>> +obj-$(CONFIG_DPS310) += dps310.o
>>   obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o
>>   obj-$(CONFIG_HID_SENSOR_PRESS)   += hid-sensor-press.o
>>   obj-$(CONFIG_HP03) += hp03.o
>> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
>> new file mode 100644
>> index 0000000..21d5dcb
>> --- /dev/null
>> +++ b/drivers/iio/pressure/dps310.c
>> @@ -0,0 +1,429 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +// Copyright IBM Corp 2019
>> +/*
>> + * The DPS310 is a barometric pressure and temperature sensor.
>> + * Currently only reading a single temperature is supported by
>> + * this driver.
>> + *
>> + * https://www.infineon.com/dgdl/?fileId=5546d462576f34750157750826c42242
>> + *
>> + * Temperature calculation:
>> + *   c0 * 0.5 + c1 * T_raw / kT °C
>> + *
>> + * TODO:
>> + *  - Pressure sensor readings
>> + *  - Optionally support the FIFO
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +#define DPS310_PRS_B0		0x00
>> +#define DPS310_PRS_B1		0x01
>> +#define DPS310_PRS_B2		0x02
>> +#define DPS310_TMP_B0		0x03
>> +#define DPS310_TMP_B1		0x04
>> +#define DPS310_TMP_B2		0x05
>> +#define DPS310_PRS_CFG		0x06
>> +#define DPS310_TMP_CFG		0x07
>> +#define  DPS310_TMP_RATE_BITS	GENMASK(6, 4)
>> +#define  DPS310_TMP_PRC_BITS	GENMASK(3, 0)
>> +#define  DPS310_TMP_EXT		BIT(7)
>> +#define DPS310_MEAS_CFG		0x08
>> +#define  DPS310_MEAS_CTRL_BITS	GENMASK(2, 0)
>> +#define   DPS310_PRS_EN		BIT(0)
>> +#define   DPS310_TEMP_EN	BIT(1)
>> +#define   DPS310_BACKGROUND	BIT(2)
>> +#define  DPS310_PRS_RDY		BIT(4)
>> +#define  DPS310_TMP_RDY		BIT(5)
>> +#define  DPS310_SENSOR_RDY	BIT(6)
>> +#define  DPS310_COEF_RDY	BIT(7)
>> +#define DPS310_CFG_REG		0x09
>> +#define  DPS310_INT_HL		BIT(7)
>> +#define  DPS310_TMP_SHIFT_EN	BIT(3)
>> +#define  DPS310_PRS_SHIFT_EN	BIT(4)
>> +#define  DPS310_FIFO_EN		BIT(5)
>> +#define  DPS310_SPI_EN		BIT(6)
>> +#define DPS310_RESET		0x0c
>> +#define  DPS310_RESET_MAGIC	(BIT(0) | BIT(3))
>> +#define DPS310_COEF_BASE	0x10
>> +
>> +#define DPS310_PRS_BASE		DPS310_PRS_B0
>> +#define DPS310_TMP_BASE		DPS310_TMP_B0
>> +
>> +#define DPS310_CALC_RATE(_n)	ilog2(_n)
>> +#define DPS310_CALC_PRC(_n)	ilog2(_n)
>> +
>> +const int scale_factor[] = {
> static
> maybe plural, scale_factors?
> maybe some comment would be good what these values relate to
>
>> +	 524288,
>> +	1572864,
>> +	3670016,
>> +	7864320,
>> +	 253952,
>> +	 516096,
>> +	1040384,
>> +	2088960,
>> +};
>> +
>> +struct dps310_data {
>> +	struct i2c_client *client;
>> +	struct regmap *regmap;
>> +
>> +	s32 c0, c1;
>> +	s32 temp_raw;
>> +};
>> +
>> +static const struct iio_chan_spec dps310_channels[] = {
>> +	{
>> +		.type = IIO_TEMP,
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
>> +			BIT(IIO_CHAN_INFO_SCALE) |
>> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
>> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
>> +			BIT(IIO_CHAN_INFO_RAW),
>> +	},
>> +};
>> +
>> +/* To be called after checking the TMP_RDY bit in MEAS_CFG */
>> +static int dps310_get_temp_coef(struct dps310_data *data)
>> +{
>> +	struct regmap *regmap = data->regmap;
>> +	u8 coef[3] = {0};
> initialization needed?
>
>> +	int r;
>> +	u32 c0, c1;
>> +
>> +	/*
>> +	 * Read temperature calibration coefficients c0 and c1 from the
>> +	 * COEF register. The numbers are 12-bit 2's compliment numbers
>> +	 */
>> +	r = regmap_bulk_read(regmap, DPS310_COEF_BASE, coef, 3);
> sizeof(coef) instead of 3
>
>> +	if (r < 0)
>> +		return r;
>> +
>> +	c0 = (coef[0] << 4) | (coef[1] >> 4);
>> +	data->c0 = sign_extend32(c0, 11);
>> +
>> +	c1 = ((coef[1] & GENMASK(3, 0)) << 8) | coef[2];
>> +	data->c1 = sign_extend32(c1, 11);
>> +
>> +	return 0;
>> +}
>> +
>> +static int dps310_get_temp_precision(struct dps310_data *data)
>> +{
>> +	int val, r;
>> +
>> +	r = regmap_read(data->regmap, DPS310_TMP_CFG, &val);
>> +	if (r < 0)
>> +		return r;
>> +
>> +	/*
>> +	 * Scale factor is bottom 4 bits of the register, but 1111 is
>> +	 * reserved so just grab bottom three
>> +	 */
>> +	return BIT(val & GENMASK(2, 0));
>> +}
>> +
>> +static int dps310_set_temp_precision(struct dps310_data *data, int val)
>> +{
>> +	int ret;
>> +	u8 shift_en;
>> +
>> +	if (val < 0 || val > 128)
>> +		return -EINVAL;
>> +
>> +	shift_en = val >= 16 ? DPS310_TMP_SHIFT_EN : 0;
>> +	ret = regmap_write_bits(data->regmap, DPS310_CFG_REG,
>> +				DPS310_TMP_SHIFT_EN, shift_en);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return regmap_update_bits(data->regmap, DPS310_TMP_CFG,
>> +				  DPS310_TMP_PRC_BITS, DPS310_CALC_PRC(val));
>> +}
>> +
>> +static int dps310_set_temp_samp_freq(struct dps310_data *data, int freq)
>> +{
>> +	u8 val;
>> +
>> +	if (freq < 0 || freq > 128)
>> +		return -EINVAL;
>> +
>> +	val = DPS310_CALC_RATE(freq) << 4;
>> +
>> +	return regmap_update_bits(data->regmap, DPS310_TMP_CFG,
>> +				  DPS310_TMP_RATE_BITS, val);
>> +}
>> +
>> +static int dps310_get_temp_samp_freq(struct dps310_data *data)
>> +{
>> +	int val, r;
>> +
>> +	r = regmap_read(data->regmap, DPS310_TMP_CFG, &val);
>> +	if (r < 0)
>> +		return r;
>> +
>> +	return BIT((val & DPS310_TMP_RATE_BITS) >> 4);
>> +}
>> +
>> +static int dps310_get_temp_k(struct dps310_data *data)
>> +{
>> +	int r = dps310_get_temp_precision(data);
>> +
>> +	if (r < 0)
>> +		return r;
>> +
>> +	return scale_factor[DPS310_CALC_PRC(r)];
>> +}
>> +
>> +static int dps310_read_temp(struct dps310_data *data)
>> +{
>> +	struct device *dev = &data->client->dev;
>> +	struct regmap *regmap = data->regmap;
>> +	u8 val[3] = {0};
> initialization needed?
>
>> +	int r, ready;
>> +	int T_raw;
>> +
>> +	r = regmap_read(regmap, DPS310_MEAS_CFG, &ready);
>> +	if (r < 0)
>> +		return r;
>> +	if (!(ready & DPS310_TMP_RDY)) {
>> +		dev_dbg(dev, "temperature not ready\n");
> most drivers wait for the results to become ready\


OK, this may have to wait up to a second then, since the background 
measurement rate can be as low as 1 HZ... I can base the timeout on the 
sample rate. Would that be OK?


>
>> +		return -EAGAIN;
>> +	}
>> +
>> +	r = regmap_bulk_read(regmap, DPS310_TMP_BASE, val, 3);
> sizeof(val)
>
>> +	if (r < 0)
>> +		return r;
>> +
>> +	T_raw = (val[0] << 16) | (val[1] << 8) | val[2];
>> +	data->temp_raw = sign_extend32(T_raw, 23);
>> +
>> +	return 0;
>> +}
>> +
>> +static bool dps310_is_writeable_reg(struct device *dev, unsigned int reg)
>> +{
>> +	switch (reg) {
>> +	case DPS310_PRS_CFG:
>> +	case DPS310_TMP_CFG:
>> +	case DPS310_MEAS_CFG:
>> +	case DPS310_CFG_REG:
>> +	case DPS310_RESET:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +static bool dps310_is_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> +	switch (reg) {
>> +	case DPS310_PRS_B0:
>> +	case DPS310_PRS_B1:
>> +	case DPS310_PRS_B2:
>> +	case DPS310_TMP_B0:
>> +	case DPS310_TMP_B1:
>> +	case DPS310_TMP_B2:
>> +	case DPS310_MEAS_CFG:
>> +		return true;
>> +	default:
>> +		return false;
>> +	}
>> +}
>> +
>> +static int dps310_write_raw(struct iio_dev *iio,
>> +			    struct iio_chan_spec const *chan, int val,
>> +			    int val2, long mask)
>> +{
>> +	struct dps310_data *data = iio_priv(iio);
>> +
>> +	if (chan->type != IIO_TEMP)
>> +		return -EINVAL;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>> +		return dps310_set_temp_samp_freq(data, val);
>> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>> +		return dps310_set_temp_precision(data, val);
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return -EINVAL;
> not needed, dead code
>
>> +}
>> +
>> +static int dps310_read_raw(struct iio_dev *iio,
>> +			   struct iio_chan_spec const *chan,
>> +			   int *val, int *val2, long mask)
>> +{
>> +	struct dps310_data *data = iio_priv(iio);
>> +	int ret;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>> +		*val = dps310_get_temp_samp_freq(data);
>> +		return IIO_VAL_INT;
>> +
>> +	case IIO_CHAN_INFO_RAW:
>> +		ret = dps310_read_temp(data);
>> +		if (ret)
>> +			return ret;
>> +
>> +		*val = data->temp_raw * data->c1;
> one could try to avoid the multipliation here and somehow mangle it into
> _SCALE/_OFFSET


Don't think it's worth it? Would have to divide the offset by c1 and 
then multiply by c1 again in the scale...


>
>> +		return IIO_VAL_INT;
>> +
>> +	case IIO_CHAN_INFO_OFFSET:
>> +		ret = dps310_get_temp_k(data);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		*val = (data->c0 >> 1) * ret;
>> +		return IIO_VAL_INT;
>> +
>> +	case IIO_CHAN_INFO_SCALE:
>> +		ret = dps310_get_temp_k(data);
>> +		if (ret < 0)
>> +			return ret;
>> +
>> +		*val = 1000; /* milliCelsius per Celsius */
>> +		*val2 = ret;
>> +		return IIO_VAL_FRACTIONAL;
>> +
>> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
>> +		*val = dps310_get_temp_precision(data);
>> +		return IIO_VAL_INT;
>> +
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	return -EINVAL;
> not needed, dead code
>
>> +}
>> +
>> +static const struct regmap_config dps310_regmap_config = {
>> +	.reg_bits = 8,
>> +	.val_bits = 8,
>> +	.writeable_reg = dps310_is_writeable_reg,
>> +	.volatile_reg = dps310_is_volatile_reg,
>> +	.cache_type = REGCACHE_RBTREE,
>> +	.max_register = 0x29,
>> +};
>> +
>> +static const struct iio_info dps310_info = {
>> +	.read_raw = dps310_read_raw,
>> +	.write_raw = dps310_write_raw,
>> +};
>> +
>> +static int dps310_probe(struct i2c_client *client,
>> +			const struct i2c_device_id *id)
>> +{
>> +	struct dps310_data *data;
>> +	struct iio_dev *iio;
>> +	int r, ready;
>> +
>> +	iio = devm_iio_device_alloc(&client->dev,  sizeof(*data));
>> +	if (!iio)
>> +		return -ENOMEM;
>> +
>> +	data = iio_priv(iio);
>> +	data->client = client;
>> +
>> +	iio->dev.parent = &client->dev;
>> +	iio->name = id->name;
>> +	iio->channels = dps310_channels;
>> +	iio->num_channels = ARRAY_SIZE(dps310_channels);
>> +	iio->info = &dps310_info;
>> +	iio->modes = INDIO_DIRECT_MODE;
>> +
>> +	data->regmap = devm_regmap_init_i2c(client, &dps310_regmap_config);
>> +	if (IS_ERR(data->regmap))
>> +		return PTR_ERR(data->regmap);
>> +
>> +	/*
>> +	 * Set up external (MEMS) temperature sensor in single sample, one
>> +	 * measurement per second mode
>> +	 */
>> +	r = regmap_write(data->regmap, DPS310_TMP_CFG,
>> +			 DPS310_TMP_EXT | DPS310_CALC_RATE(1) |
>> +			 DPS310_CALC_PRC(1));
>> +	if (r < 0)
>> +		goto err;
>> +
>> +	/* Temp shift is disabled when PRC <= 8 */
>> +	r = regmap_write_bits(data->regmap, DPS310_CFG_REG,
>> +			      DPS310_TMP_SHIFT_EN, 0);
>> +	if (r < 0)
>> +		goto err;
>> +
>> +	/* Turn on temperature measurement in the background */
>> +	r = regmap_write_bits(data->regmap, DPS310_MEAS_CFG,
>> +			      DPS310_MEAS_CTRL_BITS,
>> +			      DPS310_TEMP_EN | DPS310_BACKGROUND);
>> +	if (r < 0)
>> +		goto err;
>> +
>> +	/*
>> +	 * Calibration coefficients required for reporting temperature.
>> +	 * They are available 40ms after the device has started
>> +	 */
>> +	r = regmap_read_poll_timeout(data->regmap, DPS310_MEAS_CFG, ready,
>> +				     ready & DPS310_COEF_RDY, 10000, 40000);
>> +	if (r < 0)
>> +		goto err;
>> +
>> +	r = dps310_get_temp_coef(data);
>> +	if (r < 0)
>> +		goto err;
>> +
>> +	r = devm_iio_device_register(&client->dev, iio);
>> +	if (r)
>> +		goto err;
>> +
>> +	i2c_set_clientdata(client, iio);
>> +
>> +	dev_info(&client->dev, "%s: sensor '%s'\n", dev_name(&iio->dev),
>> +		 client->name);
> don't clutter the log
>
>> +
>> +	return 0;
>> +
>> +err:
>> +	regmap_write(data->regmap, DPS310_RESET, DPS310_RESET_MAGIC);
>> +	return r;
>> +}
>> +
>> +static int dps310_remove(struct i2c_client *client)
>> +{
>> +	struct dps310_data *data = i2c_get_clientdata(client);
>> +
>> +	return regmap_write(data->regmap, DPS310_RESET, DPS310_RESET_MAGIC);
>> +}
>> +
>> +static const struct i2c_device_id dps310_id[] = {
>> +	{ "dps310", 0 },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, dps310_id);
>> +
>> +static const unsigned short normal_i2c[] = {
>> +	0x77, 0x76, I2C_CLIENT_END
>> +};
>> +
>> +static struct i2c_driver dps310_driver = {
>> +	.driver = {
>> +		.name = "dps310",
>> +	},
>> +	.probe = dps310_probe,
>> +	.remove = dps310_remove,
>> +	.address_list = normal_i2c,
>> +	.id_table = dps310_id,
>> +};
>> +module_i2c_driver(dps310_driver);
>> +
>> +MODULE_AUTHOR("Joel Stanley <joel@jms.id.au>");
>> +MODULE_DESCRIPTION("Infineon DPS310 pressure and temperature sensor");
>> +MODULE_LICENSE("GPL v2");
>>


^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] iio: Add driver for Infineon DPS310
  2019-05-07 16:01     ` Eddie James
@ 2019-05-11  9:52       ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2019-05-11  9:52 UTC (permalink / raw)
  To: Eddie James
  Cc: Peter Meerwald-Stadler, Eddie James, linux-iio, linux-kernel, joel

On Tue, 7 May 2019 11:01:36 -0500
Eddie James <eajames@linux.vnet.ibm.com> wrote:

> On 5/6/19 4:18 PM, Peter Meerwald-Stadler wrote:
> > On Mon, 6 May 2019, Eddie James wrote:
> >  
> >> From: Joel Stanley <joel@jms.id.au>  
> > some comments below  
> 
> 
> Thanks for the quick review! Will get these in for v2, I did have a 
> couple of questions below though.
> 
> Eddie
> 
> 
> >     
> >> The DPS310 is a temperature and pressure sensor. It can be accessed over
> >> i2c and SPI.
> >>
> >> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> >> ---
> >>   MAINTAINERS                   |   6 +
> >>   drivers/iio/pressure/Kconfig  |  10 +
> >>   drivers/iio/pressure/Makefile |   1 +
> >>   drivers/iio/pressure/dps310.c | 429 ++++++++++++++++++++++++++++++++++++++++++
> >>   4 files changed, 446 insertions(+)
> >>   create mode 100644 drivers/iio/pressure/dps310.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index c61c49d..a91dfa9 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -7743,6 +7743,12 @@ W:	http://industrypack.sourceforge.net
> >>   S:	Maintained
> >>   F:	drivers/ipack/
> >>   
> >> +INFINEON DPS310 Driver
> >> +M:	Eddie James <eajames@linux.ibm.com>
> >> +L:	linux-iio@vger.kernel.org
> >> +F:	drivers/iio/pressure/dps310.c
> >> +S:	Maintained
> >> +
> >>   INFINIBAND SUBSYSTEM
> >>   M:	Doug Ledford <dledford@redhat.com>
> >>   M:	Jason Gunthorpe <jgg@mellanox.com>
> >> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> >> index efeb89f..681a1cc 100644
> >> --- a/drivers/iio/pressure/Kconfig
> >> +++ b/drivers/iio/pressure/Kconfig
> >> @@ -52,6 +52,16 @@ config IIO_CROS_EC_BARO
> >>   	  To compile this driver as a module, choose M here: the module
> >>   	  will be called cros_ec_baro.
> >>   
> >> +config DPS310
> >> +       tristate "Infineon DPS310 pressure and temperature sensor"
> >> +       depends on I2C
> >> +       select REGMAP_I2C
> >> +       help
> >> +	 Support for the Infineon DPS310 digital barometric pressure sensor.
> >> +
> >> +	 This driver can also be built as a module.  If so, the module will be
> >> +	 called dps310.
> >> +
> >>   config HID_SENSOR_PRESS
> >>   	depends on HID_SENSOR_HUB
> >>   	select IIO_BUFFER
> >> diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
> >> index c2058d7..d8f5ace 100644
> >> --- a/drivers/iio/pressure/Makefile
> >> +++ b/drivers/iio/pressure/Makefile
> >> @@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o
> >>   bmp280-objs := bmp280-core.o bmp280-regmap.o
> >>   obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o
> >>   obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o
> >> +obj-$(CONFIG_DPS310) += dps310.o
> >>   obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o
> >>   obj-$(CONFIG_HID_SENSOR_PRESS)   += hid-sensor-press.o
> >>   obj-$(CONFIG_HP03) += hp03.o
> >> diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
> >> new file mode 100644
> >> index 0000000..21d5dcb
> >> --- /dev/null
> >> +++ b/drivers/iio/pressure/dps310.c
> >> @@ -0,0 +1,429 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +// Copyright IBM Corp 2019
> >> +/*
> >> + * The DPS310 is a barometric pressure and temperature sensor.
> >> + * Currently only reading a single temperature is supported by
> >> + * this driver.
> >> + *
> >> + * https://www.infineon.com/dgdl/?fileId=5546d462576f34750157750826c42242
> >> + *
> >> + * Temperature calculation:
> >> + *   c0 * 0.5 + c1 * T_raw / kT °C
> >> + *
> >> + * TODO:
> >> + *  - Pressure sensor readings
> >> + *  - Optionally support the FIFO
> >> + */
> >> +
> >> +#include <linux/i2c.h>
> >> +#include <linux/module.h>
> >> +#include <linux/regmap.h>
> >> +
> >> +#include <linux/iio/iio.h>
> >> +#include <linux/iio/sysfs.h>
> >> +
> >> +#define DPS310_PRS_B0		0x00
> >> +#define DPS310_PRS_B1		0x01
> >> +#define DPS310_PRS_B2		0x02
> >> +#define DPS310_TMP_B0		0x03
> >> +#define DPS310_TMP_B1		0x04
> >> +#define DPS310_TMP_B2		0x05
> >> +#define DPS310_PRS_CFG		0x06
> >> +#define DPS310_TMP_CFG		0x07
> >> +#define  DPS310_TMP_RATE_BITS	GENMASK(6, 4)
> >> +#define  DPS310_TMP_PRC_BITS	GENMASK(3, 0)
> >> +#define  DPS310_TMP_EXT		BIT(7)
> >> +#define DPS310_MEAS_CFG		0x08
> >> +#define  DPS310_MEAS_CTRL_BITS	GENMASK(2, 0)
> >> +#define   DPS310_PRS_EN		BIT(0)
> >> +#define   DPS310_TEMP_EN	BIT(1)
> >> +#define   DPS310_BACKGROUND	BIT(2)
> >> +#define  DPS310_PRS_RDY		BIT(4)
> >> +#define  DPS310_TMP_RDY		BIT(5)
> >> +#define  DPS310_SENSOR_RDY	BIT(6)
> >> +#define  DPS310_COEF_RDY	BIT(7)
> >> +#define DPS310_CFG_REG		0x09
> >> +#define  DPS310_INT_HL		BIT(7)
> >> +#define  DPS310_TMP_SHIFT_EN	BIT(3)
> >> +#define  DPS310_PRS_SHIFT_EN	BIT(4)
> >> +#define  DPS310_FIFO_EN		BIT(5)
> >> +#define  DPS310_SPI_EN		BIT(6)
> >> +#define DPS310_RESET		0x0c
> >> +#define  DPS310_RESET_MAGIC	(BIT(0) | BIT(3))
> >> +#define DPS310_COEF_BASE	0x10
> >> +
> >> +#define DPS310_PRS_BASE		DPS310_PRS_B0
> >> +#define DPS310_TMP_BASE		DPS310_TMP_B0
> >> +
> >> +#define DPS310_CALC_RATE(_n)	ilog2(_n)
> >> +#define DPS310_CALC_PRC(_n)	ilog2(_n)
> >> +
> >> +const int scale_factor[] = {  
> > static
> > maybe plural, scale_factors?
> > maybe some comment would be good what these values relate to
> >  
> >> +	 524288,
> >> +	1572864,
> >> +	3670016,
> >> +	7864320,
> >> +	 253952,
> >> +	 516096,
> >> +	1040384,
> >> +	2088960,
> >> +};
> >> +
> >> +struct dps310_data {
> >> +	struct i2c_client *client;
> >> +	struct regmap *regmap;
> >> +
> >> +	s32 c0, c1;
> >> +	s32 temp_raw;
> >> +};
> >> +
> >> +static const struct iio_chan_spec dps310_channels[] = {
> >> +	{
> >> +		.type = IIO_TEMP,
> >> +		.info_mask_separate = BIT(IIO_CHAN_INFO_OFFSET) |
> >> +			BIT(IIO_CHAN_INFO_SCALE) |
> >> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
> >> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> >> +			BIT(IIO_CHAN_INFO_RAW),
> >> +	},
> >> +};
> >> +
> >> +/* To be called after checking the TMP_RDY bit in MEAS_CFG */
> >> +static int dps310_get_temp_coef(struct dps310_data *data)
> >> +{
> >> +	struct regmap *regmap = data->regmap;
> >> +	u8 coef[3] = {0};  
> > initialization needed?
> >  
> >> +	int r;
> >> +	u32 c0, c1;
> >> +
> >> +	/*
> >> +	 * Read temperature calibration coefficients c0 and c1 from the
> >> +	 * COEF register. The numbers are 12-bit 2's compliment numbers
> >> +	 */
> >> +	r = regmap_bulk_read(regmap, DPS310_COEF_BASE, coef, 3);  
> > sizeof(coef) instead of 3
> >  
> >> +	if (r < 0)
> >> +		return r;
> >> +
> >> +	c0 = (coef[0] << 4) | (coef[1] >> 4);
> >> +	data->c0 = sign_extend32(c0, 11);
> >> +
> >> +	c1 = ((coef[1] & GENMASK(3, 0)) << 8) | coef[2];
> >> +	data->c1 = sign_extend32(c1, 11);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static int dps310_get_temp_precision(struct dps310_data *data)
> >> +{
> >> +	int val, r;
> >> +
> >> +	r = regmap_read(data->regmap, DPS310_TMP_CFG, &val);
> >> +	if (r < 0)
> >> +		return r;
> >> +
> >> +	/*
> >> +	 * Scale factor is bottom 4 bits of the register, but 1111 is
> >> +	 * reserved so just grab bottom three
> >> +	 */
> >> +	return BIT(val & GENMASK(2, 0));
> >> +}
> >> +
> >> +static int dps310_set_temp_precision(struct dps310_data *data, int val)
> >> +{
> >> +	int ret;
> >> +	u8 shift_en;
> >> +
> >> +	if (val < 0 || val > 128)
> >> +		return -EINVAL;
> >> +
> >> +	shift_en = val >= 16 ? DPS310_TMP_SHIFT_EN : 0;
> >> +	ret = regmap_write_bits(data->regmap, DPS310_CFG_REG,
> >> +				DPS310_TMP_SHIFT_EN, shift_en);
> >> +	if (ret)
> >> +		return ret;
> >> +
> >> +	return regmap_update_bits(data->regmap, DPS310_TMP_CFG,
> >> +				  DPS310_TMP_PRC_BITS, DPS310_CALC_PRC(val));
> >> +}
> >> +
> >> +static int dps310_set_temp_samp_freq(struct dps310_data *data, int freq)
> >> +{
> >> +	u8 val;
> >> +
> >> +	if (freq < 0 || freq > 128)
> >> +		return -EINVAL;
> >> +
> >> +	val = DPS310_CALC_RATE(freq) << 4;
> >> +
> >> +	return regmap_update_bits(data->regmap, DPS310_TMP_CFG,
> >> +				  DPS310_TMP_RATE_BITS, val);
> >> +}
> >> +
> >> +static int dps310_get_temp_samp_freq(struct dps310_data *data)
> >> +{
> >> +	int val, r;
> >> +
> >> +	r = regmap_read(data->regmap, DPS310_TMP_CFG, &val);
> >> +	if (r < 0)
> >> +		return r;
> >> +
> >> +	return BIT((val & DPS310_TMP_RATE_BITS) >> 4);
> >> +}
> >> +
> >> +static int dps310_get_temp_k(struct dps310_data *data)
> >> +{
> >> +	int r = dps310_get_temp_precision(data);
> >> +
> >> +	if (r < 0)
> >> +		return r;
> >> +
> >> +	return scale_factor[DPS310_CALC_PRC(r)];
> >> +}
> >> +
> >> +static int dps310_read_temp(struct dps310_data *data)
> >> +{
> >> +	struct device *dev = &data->client->dev;
> >> +	struct regmap *regmap = data->regmap;
> >> +	u8 val[3] = {0};  
> > initialization needed?
> >  
> >> +	int r, ready;
> >> +	int T_raw;
> >> +
> >> +	r = regmap_read(regmap, DPS310_MEAS_CFG, &ready);
> >> +	if (r < 0)
> >> +		return r;
> >> +	if (!(ready & DPS310_TMP_RDY)) {
> >> +		dev_dbg(dev, "temperature not ready\n");  
> > most drivers wait for the results to become ready\  
> 
> 
> OK, this may have to wait up to a second then, since the background 
> measurement rate can be as low as 1 HZ... I can base the timeout on the 
> sample rate. Would that be OK?
Yes. I may be that we actually want to pick a my power hungry and faster
default to avoid any stupid userspace programs wondering why they
don't get an answer for 1 second.

> 
> 
> >  
> >> +		return -EAGAIN;
> >> +	}
> >> +
> >> +	r = regmap_bulk_read(regmap, DPS310_TMP_BASE, val, 3);  
> > sizeof(val)
> >  
> >> +	if (r < 0)
> >> +		return r;
> >> +
> >> +	T_raw = (val[0] << 16) | (val[1] << 8) | val[2];
> >> +	data->temp_raw = sign_extend32(T_raw, 23);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static bool dps310_is_writeable_reg(struct device *dev, unsigned int reg)
> >> +{
> >> +	switch (reg) {
> >> +	case DPS310_PRS_CFG:
> >> +	case DPS310_TMP_CFG:
> >> +	case DPS310_MEAS_CFG:
> >> +	case DPS310_CFG_REG:
> >> +	case DPS310_RESET:
> >> +		return true;
> >> +	default:
> >> +		return false;
> >> +	}
> >> +}
> >> +
> >> +static bool dps310_is_volatile_reg(struct device *dev, unsigned int reg)
> >> +{
> >> +	switch (reg) {
> >> +	case DPS310_PRS_B0:
> >> +	case DPS310_PRS_B1:
> >> +	case DPS310_PRS_B2:
> >> +	case DPS310_TMP_B0:
> >> +	case DPS310_TMP_B1:
> >> +	case DPS310_TMP_B2:
> >> +	case DPS310_MEAS_CFG:
> >> +		return true;
> >> +	default:
> >> +		return false;
> >> +	}
> >> +}
> >> +
> >> +static int dps310_write_raw(struct iio_dev *iio,
> >> +			    struct iio_chan_spec const *chan, int val,
> >> +			    int val2, long mask)
> >> +{
> >> +	struct dps310_data *data = iio_priv(iio);
> >> +
> >> +	if (chan->type != IIO_TEMP)
> >> +		return -EINVAL;
> >> +
> >> +	switch (mask) {
> >> +	case IIO_CHAN_INFO_SAMP_FREQ:
> >> +		return dps310_set_temp_samp_freq(data, val);
> >> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> >> +		return dps310_set_temp_precision(data, val);
> >> +	default:
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	return -EINVAL;  
> > not needed, dead code
> >  
> >> +}
> >> +
> >> +static int dps310_read_raw(struct iio_dev *iio,
> >> +			   struct iio_chan_spec const *chan,
> >> +			   int *val, int *val2, long mask)
> >> +{
> >> +	struct dps310_data *data = iio_priv(iio);
> >> +	int ret;
> >> +
> >> +	switch (mask) {
> >> +	case IIO_CHAN_INFO_SAMP_FREQ:
> >> +		*val = dps310_get_temp_samp_freq(data);
> >> +		return IIO_VAL_INT;
> >> +
> >> +	case IIO_CHAN_INFO_RAW:
> >> +		ret = dps310_read_temp(data);
> >> +		if (ret)
> >> +			return ret;
> >> +
> >> +		*val = data->temp_raw * data->c1;  
> > one could try to avoid the multipliation here and somehow mangle it into
> > _SCALE/_OFFSET  
> 
> 
> Don't think it's worth it? Would have to divide the offset by c1 and 
> then multiply by c1 again in the scale...

I think you are actually better just reporting as PROCESSED.

> 
> 
> >  
> >> +		return IIO_VAL_INT;
> >> +
> >> +	case IIO_CHAN_INFO_OFFSET:
> >> +		ret = dps310_get_temp_k(data);
> >> +		if (ret < 0)
> >> +			return ret;
> >> +
> >> +		*val = (data->c0 >> 1) * ret;
> >> +		return IIO_VAL_INT;
> >> +
> >> +	case IIO_CHAN_INFO_SCALE:
> >> +		ret = dps310_get_temp_k(data);
> >> +		if (ret < 0)
> >> +			return ret;
> >> +
> >> +		*val = 1000; /* milliCelsius per Celsius */
> >> +		*val2 = ret;
> >> +		return IIO_VAL_FRACTIONAL;
> >> +
> >> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> >> +		*val = dps310_get_temp_precision(data);
> >> +		return IIO_VAL_INT;
> >> +
> >> +	default:
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	return -EINVAL;  
> > not needed, dead code
> >  
> >> +}
> >> +
> >> +static const struct regmap_config dps310_regmap_config = {
> >> +	.reg_bits = 8,
> >> +	.val_bits = 8,
> >> +	.writeable_reg = dps310_is_writeable_reg,
> >> +	.volatile_reg = dps310_is_volatile_reg,
> >> +	.cache_type = REGCACHE_RBTREE,
> >> +	.max_register = 0x29,
> >> +};
> >> +
> >> +static const struct iio_info dps310_info = {
> >> +	.read_raw = dps310_read_raw,
> >> +	.write_raw = dps310_write_raw,
> >> +};
> >> +
> >> +static int dps310_probe(struct i2c_client *client,
> >> +			const struct i2c_device_id *id)
> >> +{
> >> +	struct dps310_data *data;
> >> +	struct iio_dev *iio;
> >> +	int r, ready;
> >> +
> >> +	iio = devm_iio_device_alloc(&client->dev,  sizeof(*data));
> >> +	if (!iio)
> >> +		return -ENOMEM;
> >> +
> >> +	data = iio_priv(iio);
> >> +	data->client = client;
> >> +
> >> +	iio->dev.parent = &client->dev;
> >> +	iio->name = id->name;
> >> +	iio->channels = dps310_channels;
> >> +	iio->num_channels = ARRAY_SIZE(dps310_channels);
> >> +	iio->info = &dps310_info;
> >> +	iio->modes = INDIO_DIRECT_MODE;
> >> +
> >> +	data->regmap = devm_regmap_init_i2c(client, &dps310_regmap_config);
> >> +	if (IS_ERR(data->regmap))
> >> +		return PTR_ERR(data->regmap);
> >> +
> >> +	/*
> >> +	 * Set up external (MEMS) temperature sensor in single sample, one
> >> +	 * measurement per second mode
> >> +	 */
> >> +	r = regmap_write(data->regmap, DPS310_TMP_CFG,
> >> +			 DPS310_TMP_EXT | DPS310_CALC_RATE(1) |
> >> +			 DPS310_CALC_PRC(1));
> >> +	if (r < 0)
> >> +		goto err;
> >> +
> >> +	/* Temp shift is disabled when PRC <= 8 */
> >> +	r = regmap_write_bits(data->regmap, DPS310_CFG_REG,
> >> +			      DPS310_TMP_SHIFT_EN, 0);
> >> +	if (r < 0)
> >> +		goto err;
> >> +
> >> +	/* Turn on temperature measurement in the background */
> >> +	r = regmap_write_bits(data->regmap, DPS310_MEAS_CFG,
> >> +			      DPS310_MEAS_CTRL_BITS,
> >> +			      DPS310_TEMP_EN | DPS310_BACKGROUND);
> >> +	if (r < 0)
> >> +		goto err;
> >> +
> >> +	/*
> >> +	 * Calibration coefficients required for reporting temperature.
> >> +	 * They are available 40ms after the device has started
> >> +	 */
> >> +	r = regmap_read_poll_timeout(data->regmap, DPS310_MEAS_CFG, ready,
> >> +				     ready & DPS310_COEF_RDY, 10000, 40000);
> >> +	if (r < 0)
> >> +		goto err;
> >> +
> >> +	r = dps310_get_temp_coef(data);
> >> +	if (r < 0)
> >> +		goto err;
> >> +
> >> +	r = devm_iio_device_register(&client->dev, iio);
> >> +	if (r)
> >> +		goto err;
> >> +
> >> +	i2c_set_clientdata(client, iio);
> >> +
> >> +	dev_info(&client->dev, "%s: sensor '%s'\n", dev_name(&iio->dev),
> >> +		 client->name);  
> > don't clutter the log
> >  
> >> +
> >> +	return 0;
> >> +
> >> +err:
> >> +	regmap_write(data->regmap, DPS310_RESET, DPS310_RESET_MAGIC);
> >> +	return r;
> >> +}
> >> +
> >> +static int dps310_remove(struct i2c_client *client)
> >> +{
> >> +	struct dps310_data *data = i2c_get_clientdata(client);
> >> +
> >> +	return regmap_write(data->regmap, DPS310_RESET, DPS310_RESET_MAGIC);
> >> +}
> >> +
> >> +static const struct i2c_device_id dps310_id[] = {
> >> +	{ "dps310", 0 },
> >> +	{}
> >> +};
> >> +MODULE_DEVICE_TABLE(i2c, dps310_id);
> >> +
> >> +static const unsigned short normal_i2c[] = {
> >> +	0x77, 0x76, I2C_CLIENT_END
> >> +};
> >> +
> >> +static struct i2c_driver dps310_driver = {
> >> +	.driver = {
> >> +		.name = "dps310",
> >> +	},
> >> +	.probe = dps310_probe,
> >> +	.remove = dps310_remove,
> >> +	.address_list = normal_i2c,
> >> +	.id_table = dps310_id,
> >> +};
> >> +module_i2c_driver(dps310_driver);
> >> +
> >> +MODULE_AUTHOR("Joel Stanley <joel@jms.id.au>");
> >> +MODULE_DESCRIPTION("Infineon DPS310 pressure and temperature sensor");
> >> +MODULE_LICENSE("GPL v2");
> >>  
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-05-11  9:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-06 20:58 [PATCH 0/3] iio: Add driver for Infineon DPS310 Eddie James
2019-05-06 20:58 ` [PATCH 1/3] " Eddie James
2019-05-06 21:18   ` Peter Meerwald-Stadler
2019-05-07 16:01     ` Eddie James
2019-05-11  9:52       ` Jonathan Cameron
2019-05-06 20:58 ` [PATCH 2/3] iio: dps310: Temperature measurement errata Eddie James
2019-05-06 20:58 ` [PATCH 3/3] iio: dps310: Add pressure sensing capability Eddie James
2019-05-06 21:25   ` Peter Meerwald-Stadler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).