Re: [PATCH v2 3/3] iio: dps310: Add pressure sensing capability
From: Eddie James
Date: Tue May 14 2019 - 16:27:45 EST
On 5/11/19 4:48 AM, Jonathan Cameron wrote:
On Wed, 8 May 2019 14:35:28 -0500
Eddie James <eajames@xxxxxxxxxxxxx> wrote:
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@xxxxxxxxxxxxx>
Hi Eddie,
A few comments inline. One is around how you might look at adding
fifo support (pushing to an IIO buffer) in the future... The IIO
data model isn't as flexible as this device can be, so we may need
to put some restrictions on combinations of options.
Jonathan
---
drivers/iio/pressure/dps310.c | 331 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 307 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/pressure/dps310.c b/drivers/iio/pressure/dps310.c
index c42808e..a7ee28c 100644
--- a/drivers/iio/pressure/dps310.c
+++ b/drivers/iio/pressure/dps310.c
@@ -11,11 +11,11 @@
* c0 * 0.5 + c1 * T_raw / kT ÂC
*
* TODO:
- * - Pressure sensor readings
* - Optionally support the FIFO
*/
#include <linux/i2c.h>
+#include <linux/math64.h>
#include <linux/module.h>
#include <linux/regmap.h>
@@ -29,6 +29,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)
@@ -82,6 +84,8 @@ struct dps310_data {
struct regmap *regmap;
+
static bool dps310_is_writeable_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
@@ -253,24 +387,141 @@ 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);
This may need a bit of thought if there is any chance we will later support
the fifo.
The IIO model is that of scans that read all channels at each 'trigger'. In
devices like this which allow for different sampling rates for different sensor
channels there are two options.
1) Don't support it.
2) Deal with registering two separate IIO devices and do the demux in the
driver to the relevant one.
All depends on whether there is a substantial usecase for different sampling
rates or not. Here I suspect the answer is not.
The complexity is that, you then need to work out how to 'upgrade' the
interface when buffered support is added. Obvious options are:
1) Refuse to move to buffered mode if the sampling frequencies are different.
2) Force the slower channel to be sampled faster if that is possible.
3) Change to only having one exposed sampling frequency at all - the problem
with this last one is that it changes the ABI for existing users.
It may be no one ever cares about the fifo mode though as high speed pressure
measurement is 'unusual' ;)
Thanks for the comments Jonathan. I will follow your suggestions
throughout the driver.
The sampling rates are a bit confusing for me, and I haven't looked into
the buffered mode at all. Are you saying that in the current form, it
won't work to use different sampling frequencies for the two sensors
(without buffered mode I mean)? I haven't noticed any problems in my
tests. I'm inclined force the slower channel to be sampled faster if
necessary when buffered mode is implemented.
Thanks,
Eddie
+
+ 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, r, t_ready;
+ 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;
+
+ /* Refresh temp if it's ready, otherwise just use the latest value */
+ r = regmap_read(data->regmap, DPS310_MEAS_CFG, &t_ready);
+ if (r >= 0 && t_ready & DPS310_TMP_RDY)
+ dps310_read_temp_ready(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);
This is rather far from raw :) It might be better at this point to just
go for PROCESSED and apply the scale in here.
+ 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;
}
}
-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) {
@@ -279,7 +530,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;
@@ -312,6 +563,24 @@ static int dps310_read_raw(struct iio_dev *iio,
}
}
+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 = {
.reg_bits = 8,
.val_bits = 8,
@@ -393,6 +662,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
*/
@@ -402,16 +678,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;
@@ -424,7 +707,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;