* [PATCH v2 1/4] iio: adis16480: fix pps mode sampling frequency math
2021-02-18 11:40 [PATCH v2 0/4] Fix/Improve sync clock mode handling Nuno Sa
@ 2021-02-18 11:40 ` Nuno Sa
2021-02-18 11:40 ` [PATCH v2 2/4] iio: adis16475: improve sync scale mode handling Nuno Sa
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nuno Sa @ 2021-02-18 11:40 UTC (permalink / raw)
To: linux-iio, devicetree
Cc: Jonathan Cameron, Rob Herring, Lars-Peter Clausen,
Michael Hennerich
When using PPS mode, the input clock needs to be scaled so that we have
an IMU sample rate between (optimally) 4000 and 4250. After this, we can
use the decimation filter to lower the sampling rate in order to get what
the user wants. Optimally, the user sample rate is a multiple of both the
IMU sample rate and the input clock. Hence, calculating the sync_scale
dynamically gives us better chances of achieving a perfect/integer value
for DEC_RATE. The math here is:
1. lcm of the input clock and the desired output rate.
2. get the highest multiple of the previous result lower than the adis
max rate.
3. The last result becomes the IMU sample rate. Use that to calculate
SYNC_SCALE and DEC_RATE (to get the user output rate).
Fixes: 326e2357553d3 ("iio: imu: adis16480: Add support for external clock")
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/iio/imu/adis16480.c | 128 ++++++++++++++++++++++++++----------
1 file changed, 94 insertions(+), 34 deletions(-)
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index dfe86c589325..c41b8ef1e250 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -10,6 +10,7 @@
#include <linux/of_irq.h>
#include <linux/interrupt.h>
#include <linux/delay.h>
+#include <linux/math.h>
#include <linux/mutex.h>
#include <linux/device.h>
#include <linux/kernel.h>
@@ -17,6 +18,7 @@
#include <linux/slab.h>
#include <linux/sysfs.h>
#include <linux/module.h>
+#include <linux/lcm.h>
#include <linux/iio/iio.h>
#include <linux/iio/sysfs.h>
@@ -170,6 +172,11 @@ static const char * const adis16480_int_pin_names[4] = {
[ADIS16480_PIN_DIO4] = "DIO4",
};
+static bool low_rate_allow;
+module_param(low_rate_allow, bool, 0444);
+MODULE_PARM_DESC(low_rate_allow,
+ "Allow IMU rates below the minimum advisable when external clk is used in PPS mode (default: N)");
+
#ifdef CONFIG_DEBUG_FS
static ssize_t adis16480_show_firmware_revision(struct file *file,
@@ -312,7 +319,8 @@ static int adis16480_debugfs_init(struct iio_dev *indio_dev)
static int adis16480_set_freq(struct iio_dev *indio_dev, int val, int val2)
{
struct adis16480 *st = iio_priv(indio_dev);
- unsigned int t, reg;
+ unsigned int t, sample_rate = st->clk_freq;
+ int ret;
if (val < 0 || val2 < 0)
return -EINVAL;
@@ -321,28 +329,65 @@ static int adis16480_set_freq(struct iio_dev *indio_dev, int val, int val2)
if (t == 0)
return -EINVAL;
+ mutex_lock(&st->adis.state_lock);
/*
- * When using PPS mode, the rate of data collection is equal to the
- * product of the external clock frequency and the scale factor in the
- * SYNC_SCALE register.
- * When using sync mode, or internal clock, the output data rate is
- * equal with the clock frequency divided by DEC_RATE + 1.
+ * When using PPS mode, the input clock needs to be scaled so that we have an IMU
+ * sample rate between (optimally) 4000 and 4250. After this, we can use the
+ * decimation filter to lower the sampling rate in order to get what the user wants.
+ * Optimally, the user sample rate is a multiple of both the IMU sample rate and
+ * the input clock. Hence, calculating the sync_scale dynamically gives us better
+ * chances of achieving a perfect/integer value for DEC_RATE. The math here is:
+ * 1. lcm of the input clock and the desired output rate.
+ * 2. get the highest multiple of the previous result lower than the adis max rate.
+ * 3. The last result becomes the IMU sample rate. Use that to calculate SYNC_SCALE
+ * and DEC_RATE (to get the user output rate)
*/
if (st->clk_mode == ADIS16480_CLK_PPS) {
- t = t / st->clk_freq;
- reg = ADIS16495_REG_SYNC_SCALE;
- } else {
- t = st->clk_freq / t;
- reg = ADIS16480_REG_DEC_RATE;
+ unsigned long scaled_rate = lcm(st->clk_freq, t);
+ int sync_scale;
+
+ /*
+ * If lcm is bigger than the IMU maximum sampling rate there's no perfect
+ * solution. In this case, we get the highest multiple of the input clock
+ * lower than the IMU max sample rate.
+ */
+ if (scaled_rate > st->chip_info->int_clk)
+ scaled_rate = st->chip_info->int_clk / st->clk_freq * st->clk_freq;
+ else
+ scaled_rate = st->chip_info->int_clk / scaled_rate * scaled_rate;
+
+ /*
+ * This is not an hard requirement but it's not advised to run the IMU
+ * with a sample rate lower than 4000Hz due to possible undersampling
+ * issues. However, there are users that might really want to take the risk.
+ * Hence, we provide a module parameter for them. If set, we allow sample
+ * rates lower than 4KHz. By default, we won't allow this and we just roundup
+ * the rate to the next multiple of the input clock bigger than 4KHz. This
+ * is done like this as in some cases (when DEC_RATE is 0) might give
+ * us the closest value to the one desired by the user...
+ */
+ if (scaled_rate < 4000000 && !low_rate_allow)
+ scaled_rate = roundup(4000000, st->clk_freq);
+
+ sync_scale = scaled_rate / st->clk_freq;
+ ret = __adis_write_reg_16(&st->adis, ADIS16495_REG_SYNC_SCALE, sync_scale);
+ if (ret)
+ goto error;
+
+ sample_rate = scaled_rate;
}
+ t = DIV_ROUND_CLOSEST(sample_rate, t);
+ if (t)
+ t--;
+
if (t > st->chip_info->max_dec_rate)
t = st->chip_info->max_dec_rate;
- if ((t != 0) && (st->clk_mode != ADIS16480_CLK_PPS))
- t--;
-
- return adis_write_reg_16(&st->adis, reg, t);
+ ret = __adis_write_reg_16(&st->adis, ADIS16480_REG_DEC_RATE, t);
+error:
+ mutex_unlock(&st->adis.state_lock);
+ return ret;
}
static int adis16480_get_freq(struct iio_dev *indio_dev, int *val, int *val2)
@@ -350,34 +395,35 @@ static int adis16480_get_freq(struct iio_dev *indio_dev, int *val, int *val2)
struct adis16480 *st = iio_priv(indio_dev);
uint16_t t;
int ret;
- unsigned int freq;
- unsigned int reg;
+ unsigned int freq, sample_rate = st->clk_freq;
- if (st->clk_mode == ADIS16480_CLK_PPS)
- reg = ADIS16495_REG_SYNC_SCALE;
- else
- reg = ADIS16480_REG_DEC_RATE;
+ mutex_lock(&st->adis.state_lock);
+
+ if (st->clk_mode == ADIS16480_CLK_PPS) {
+ u16 sync_scale;
+
+ ret = __adis_read_reg_16(&st->adis, ADIS16495_REG_SYNC_SCALE, &sync_scale);
+ if (ret)
+ goto error;
- ret = adis_read_reg_16(&st->adis, reg, &t);
+ sample_rate = st->clk_freq * sync_scale;
+ }
+
+ ret = __adis_read_reg_16(&st->adis, ADIS16480_REG_DEC_RATE, &t);
if (ret)
- return ret;
+ goto error;
- /*
- * When using PPS mode, the rate of data collection is equal to the
- * product of the external clock frequency and the scale factor in the
- * SYNC_SCALE register.
- * When using sync mode, or internal clock, the output data rate is
- * equal with the clock frequency divided by DEC_RATE + 1.
- */
- if (st->clk_mode == ADIS16480_CLK_PPS)
- freq = st->clk_freq * t;
- else
- freq = st->clk_freq / (t + 1);
+ mutex_unlock(&st->adis.state_lock);
+
+ freq = DIV_ROUND_CLOSEST(sample_rate, (t + 1));
*val = freq / 1000;
*val2 = (freq % 1000) * 1000;
return IIO_VAL_INT_PLUS_MICRO;
+error:
+ mutex_unlock(&st->adis.state_lock);
+ return ret;
}
enum {
@@ -1278,6 +1324,20 @@ static int adis16480_probe(struct spi_device *spi)
st->clk_freq = clk_get_rate(st->ext_clk);
st->clk_freq *= 1000; /* micro */
+ if (st->clk_mode == ADIS16480_CLK_PPS) {
+ u16 sync_scale;
+
+ /*
+ * In PPS mode, the IMU sample rate is the clk_freq * sync_scale. Hence,
+ * default the IMU sample rate to the highest multiple of the input clock
+ * lower than the IMU max sample rate. The internal sample rate is the
+ * max...
+ */
+ sync_scale = st->chip_info->int_clk / st->clk_freq;
+ ret = __adis_write_reg_16(&st->adis, ADIS16495_REG_SYNC_SCALE, sync_scale);
+ if (ret)
+ return ret;
+ }
} else {
st->clk_freq = st->chip_info->int_clk;
}
--
2.30.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/4] iio: adis16475: improve sync scale mode handling
2021-02-18 11:40 [PATCH v2 0/4] Fix/Improve sync clock mode handling Nuno Sa
2021-02-18 11:40 ` [PATCH v2 1/4] iio: adis16480: fix pps mode sampling frequency math Nuno Sa
@ 2021-02-18 11:40 ` Nuno Sa
2021-02-18 11:40 ` [PATCH v2 3/4] dt-bindings: adis16475: remove property Nuno Sa
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Nuno Sa @ 2021-02-18 11:40 UTC (permalink / raw)
To: linux-iio, devicetree
Cc: Jonathan Cameron, Rob Herring, Lars-Peter Clausen,
Michael Hennerich
With this patch, we don't force users to define the IMU scaled internal
sampling rate once in the devicetree. Now it's dynamically calculated
at runtime depending on the desired output rate given by users.
Calculating the sync_scale dynamically gives us better chances of
achieving a perfect/integer value for DEC_RATE (thus giving more
flexibility). The math is:
1. lcm of the input clock and the desired output rate.
2. get the highest multiple of the previous result lower than the adis
max rate.
3. The last result becomes the IMU sample rate. Use that to calculate
SYNC_SCALE and DEC_RATE (to get the user output rate).
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/iio/imu/adis16475.c | 118 +++++++++++++++++++++++++++---------
1 file changed, 90 insertions(+), 28 deletions(-)
diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 197d48240991..5759cd8ffbef 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -17,6 +17,8 @@
#include <linux/iio/sysfs.h>
#include <linux/iio/trigger_consumer.h>
#include <linux/irq.h>
+#include <linux/lcm.h>
+#include <linux/math.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/property.h>
@@ -101,6 +103,7 @@ struct adis16475 {
u32 clk_freq;
bool burst32;
unsigned long lsb_flag;
+ u16 sync_mode;
/* Alignment needed for the timestamp */
__be16 data[ADIS16475_MAX_SCAN_DATA] __aligned(8);
};
@@ -117,6 +120,11 @@ enum {
ADIS16475_SCAN_CRC_FAILURE,
};
+static bool low_rate_allow;
+module_param(low_rate_allow, bool, 0444);
+MODULE_PARM_DESC(low_rate_allow,
+ "Allow IMU rates below the minimum advisable when external clk is used in SCALED mode (default: N)");
+
#ifdef CONFIG_DEBUG_FS
static ssize_t adis16475_show_firmware_revision(struct file *file,
char __user *userbuf,
@@ -253,25 +261,92 @@ static int adis16475_get_freq(struct adis16475 *st, u32 *freq)
{
int ret;
u16 dec;
+ u32 sample_rate = st->clk_freq;
+
+ mutex_lock(&st->adis.state_lock);
+
+ if (st->sync_mode == ADIS16475_SYNC_SCALED) {
+ u16 sync_scale;
+
+ ret = __adis_read_reg_16(&st->adis, ADIS16475_REG_UP_SCALE, &sync_scale);
+ if (ret)
+ goto error;
+
+ sample_rate = st->clk_freq * sync_scale;
+ }
- ret = adis_read_reg_16(&st->adis, ADIS16475_REG_DEC_RATE, &dec);
+ ret = __adis_read_reg_16(&st->adis, ADIS16475_REG_DEC_RATE, &dec);
if (ret)
- return -EINVAL;
+ goto error;
- *freq = DIV_ROUND_CLOSEST(st->clk_freq, dec + 1);
+ mutex_unlock(&st->adis.state_lock);
+
+ *freq = DIV_ROUND_CLOSEST(sample_rate, dec + 1);
return 0;
+error:
+ mutex_unlock(&st->adis.state_lock);
+ return ret;
}
static int adis16475_set_freq(struct adis16475 *st, const u32 freq)
{
u16 dec;
int ret;
+ u32 sample_rate = st->clk_freq;
if (!freq)
return -EINVAL;
- dec = DIV_ROUND_CLOSEST(st->clk_freq, freq);
+ mutex_lock(&st->adis.state_lock);
+ /*
+ * When using sync scaled mode, the input clock needs to be scaled so that we have
+ * an IMU sample rate between (optimally) 1900 and 2100. After this, we can use the
+ * decimation filter to lower the sampling rate in order to get what the user wants.
+ * Optimally, the user sample rate is a multiple of both the IMU sample rate and
+ * the input clock. Hence, calculating the sync_scale dynamically gives us better
+ * chances of achieving a perfect/integer value for DEC_RATE. The math here is:
+ * 1. lcm of the input clock and the desired output rate.
+ * 2. get the highest multiple of the previous result lower than the adis max rate.
+ * 3. The last result becomes the IMU sample rate. Use that to calculate SYNC_SCALE
+ * and DEC_RATE (to get the user output rate)
+ */
+ if (st->sync_mode == ADIS16475_SYNC_SCALED) {
+ unsigned long scaled_rate = lcm(st->clk_freq, freq);
+ int sync_scale;
+
+ /*
+ * If lcm is bigger than the IMU maximum sampling rate there's no perfect
+ * solution. In this case, we get the highest multiple of the input clock
+ * lower than the IMU max sample rate.
+ */
+ if (scaled_rate > 2100000)
+ scaled_rate = 2100000 / st->clk_freq * st->clk_freq;
+ else
+ scaled_rate = 2100000 / scaled_rate * scaled_rate;
+
+ /*
+ * This is not an hard requirement but it's not advised to run the IMU
+ * with a sample rate lower than 4000Hz due to possible undersampling
+ * issues. However, there are users that might really want to take the risk.
+ * Hence, we provide a module parameter for them. If set, we allow sample
+ * rates lower than 4KHz. By default, we won't allow this and we just roundup
+ * the rate to the next multiple of the input clock bigger than 4KHz. This
+ * is done like this as in some cases (when DEC_RATE is 0) might give
+ * us the closest value to the one desired by the user...
+ */
+ if (scaled_rate < 1900000 && !low_rate_allow)
+ scaled_rate = roundup(1900000, st->clk_freq);
+
+ sync_scale = scaled_rate / st->clk_freq;
+ ret = __adis_write_reg_16(&st->adis, ADIS16475_REG_UP_SCALE, sync_scale);
+ if (ret)
+ goto error;
+
+ sample_rate = scaled_rate;
+ }
+
+ dec = DIV_ROUND_CLOSEST(sample_rate, freq);
if (dec)
dec--;
@@ -281,7 +356,7 @@ static int adis16475_set_freq(struct adis16475 *st, const u32 freq)
ret = adis_write_reg_16(&st->adis, ADIS16475_REG_DEC_RATE, dec);
if (ret)
- return ret;
+ goto error;
/*
* If decimation is used, then gyro and accel data will have meaningful
@@ -290,6 +365,9 @@ static int adis16475_set_freq(struct adis16475 *st, const u32 freq)
assign_bit(ADIS16475_LSB_DEC_MASK, &st->lsb_flag, dec);
return 0;
+error:
+ mutex_unlock(&st->adis.state_lock);
+ return ret;
}
/* The values are approximated. */
@@ -1085,6 +1163,7 @@ static int adis16475_config_sync_mode(struct adis16475 *st)
}
sync = &st->info->sync[sync_mode];
+ st->sync_mode = sync->sync_mode;
/* All the other modes require external input signal */
if (sync->sync_mode != ADIS16475_SYNC_OUTPUT) {
@@ -1112,37 +1191,20 @@ static int adis16475_config_sync_mode(struct adis16475 *st)
if (sync->sync_mode == ADIS16475_SYNC_SCALED) {
u16 up_scale;
- u32 scaled_out_freq = 0;
+
/*
- * If we are in scaled mode, we must have an up_scale.
- * In scaled mode the allowable input clock range is
- * 1 Hz to 128 Hz, and the allowable output range is
- * 1900 to 2100 Hz. Hence, a scale must be given to
- * get the allowable output.
+ * In sync scaled mode, the IMU sample rate is the clk_freq * sync_scale.
+ * Hence, default the IMU sample rate to the highest multiple of the input
+ * clock lower than the IMU max sample rate. The optimal range is
+ * 1900-2100 sps...
*/
- ret = device_property_read_u32(dev,
- "adi,scaled-output-hz",
- &scaled_out_freq);
- if (ret) {
- dev_err(dev, "adi,scaled-output-hz must be given when in scaled sync mode");
- return -EINVAL;
- } else if (scaled_out_freq < 1900 ||
- scaled_out_freq > 2100) {
- dev_err(dev, "Invalid value: %u for adi,scaled-output-hz",
- scaled_out_freq);
- return -EINVAL;
- }
-
- up_scale = DIV_ROUND_CLOSEST(scaled_out_freq,
- st->clk_freq);
+ up_scale = 2100 / st->clk_freq;
ret = __adis_write_reg_16(&st->adis,
ADIS16475_REG_UP_SCALE,
up_scale);
if (ret)
return ret;
-
- st->clk_freq = scaled_out_freq;
}
st->clk_freq *= 1000;
--
2.30.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/4] dt-bindings: adis16475: remove property
2021-02-18 11:40 [PATCH v2 0/4] Fix/Improve sync clock mode handling Nuno Sa
2021-02-18 11:40 ` [PATCH v2 1/4] iio: adis16480: fix pps mode sampling frequency math Nuno Sa
2021-02-18 11:40 ` [PATCH v2 2/4] iio: adis16475: improve sync scale mode handling Nuno Sa
@ 2021-02-18 11:40 ` Nuno Sa
2021-02-18 11:40 ` [PATCH v2 4/4] iio: adis: add helpers for locking Nuno Sa
2021-02-21 13:59 ` [PATCH v2 0/4] Fix/Improve sync clock mode handling Jonathan Cameron
4 siblings, 0 replies; 6+ messages in thread
From: Nuno Sa @ 2021-02-18 11:40 UTC (permalink / raw)
To: linux-iio, devicetree
Cc: Jonathan Cameron, Rob Herring, Lars-Peter Clausen,
Michael Hennerich, Nuno Sá, Rob Herring
From: Nuno Sá <nuno.sa@analog.com>
`adi,scaled-output-hz` is no longer used by the driver.
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Nuno Sá <nuno.sa@analog.com>
---
.../devicetree/bindings/iio/imu/adi,adis16475.yaml | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
index 79fba1508e89..a7574210175a 100644
--- a/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16475.yaml
@@ -71,15 +71,6 @@ properties:
minimum: 0
maximum: 3
- adi,scaled-output-hz:
- description:
- This property must be present if the clock mode is scaled-sync through
- clock-names property. In this mode, the input clock can have a range
- of 1Hz to 128HZ which must be scaled to originate an allowable sample
- rate. This property specifies that rate.
- minimum: 1900
- maximum: 2100
-
required:
- compatible
- reg
--
2.30.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 4/4] iio: adis: add helpers for locking
2021-02-18 11:40 [PATCH v2 0/4] Fix/Improve sync clock mode handling Nuno Sa
` (2 preceding siblings ...)
2021-02-18 11:40 ` [PATCH v2 3/4] dt-bindings: adis16475: remove property Nuno Sa
@ 2021-02-18 11:40 ` Nuno Sa
2021-02-21 13:59 ` [PATCH v2 0/4] Fix/Improve sync clock mode handling Jonathan Cameron
4 siblings, 0 replies; 6+ messages in thread
From: Nuno Sa @ 2021-02-18 11:40 UTC (permalink / raw)
To: linux-iio, devicetree
Cc: Jonathan Cameron, Rob Herring, Lars-Peter Clausen,
Michael Hennerich
Add some helpers to lock and unlock the device. As this is such a simple
change, we update all the users that were using the lock already in this
patch.
Signed-off-by: Nuno Sa <nuno.sa@analog.com>
---
drivers/iio/imu/adis16400.c | 22 ++++++++++------------
drivers/iio/imu/adis16475.c | 10 +++++-----
drivers/iio/imu/adis16480.c | 15 +++++++--------
include/linux/iio/imu/adis.h | 10 ++++++++++
4 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
index 54af2ed664f6..b1ec14bf439f 100644
--- a/drivers/iio/imu/adis16400.c
+++ b/drivers/iio/imu/adis16400.c
@@ -505,7 +505,6 @@ static int adis16400_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int val, int val2, long info)
{
struct adis16400_state *st = iio_priv(indio_dev);
- struct mutex *slock = &st->adis.state_lock;
int ret, sps;
switch (info) {
@@ -518,18 +517,18 @@ static int adis16400_write_raw(struct iio_dev *indio_dev,
* Need to cache values so we can update if the frequency
* changes.
*/
- mutex_lock(slock);
+ adis_dev_lock(&st->adis);
st->filt_int = val;
/* Work out update to current value */
sps = st->variant->get_freq(st);
if (sps < 0) {
- mutex_unlock(slock);
+ adis_dev_unlock(&st->adis);
return sps;
}
ret = __adis16400_set_filter(indio_dev, sps,
val * 1000 + val2 / 1000);
- mutex_unlock(slock);
+ adis_dev_unlock(&st->adis);
return ret;
case IIO_CHAN_INFO_SAMP_FREQ:
sps = val * 1000 + val2 / 1000;
@@ -537,9 +536,9 @@ static int adis16400_write_raw(struct iio_dev *indio_dev,
if (sps <= 0)
return -EINVAL;
- mutex_lock(slock);
+ adis_dev_lock(&st->adis);
ret = st->variant->set_freq(st, sps);
- mutex_unlock(slock);
+ adis_dev_unlock(&st->adis);
return ret;
default:
return -EINVAL;
@@ -550,7 +549,6 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan, int *val, int *val2, long info)
{
struct adis16400_state *st = iio_priv(indio_dev);
- struct mutex *slock = &st->adis.state_lock;
int16_t val16;
int ret;
@@ -606,17 +604,17 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
*val = st->variant->temp_offset;
return IIO_VAL_INT;
case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
- mutex_lock(slock);
+ adis_dev_lock(&st->adis);
/* Need both the number of taps and the sampling frequency */
ret = __adis_read_reg_16(&st->adis,
ADIS16400_SENS_AVG,
&val16);
if (ret) {
- mutex_unlock(slock);
+ adis_dev_unlock(&st->adis);
return ret;
}
ret = st->variant->get_freq(st);
- mutex_unlock(slock);
+ adis_dev_unlock(&st->adis);
if (ret)
return ret;
ret /= adis16400_3db_divisors[val16 & 0x07];
@@ -624,9 +622,9 @@ static int adis16400_read_raw(struct iio_dev *indio_dev,
*val2 = (ret % 1000) * 1000;
return IIO_VAL_INT_PLUS_MICRO;
case IIO_CHAN_INFO_SAMP_FREQ:
- mutex_lock(slock);
+ adis_dev_lock(&st->adis);
ret = st->variant->get_freq(st);
- mutex_unlock(slock);
+ adis_dev_unlock(&st->adis);
if (ret)
return ret;
*val = ret / 1000;
diff --git a/drivers/iio/imu/adis16475.c b/drivers/iio/imu/adis16475.c
index 5759cd8ffbef..8f6bea4b6608 100644
--- a/drivers/iio/imu/adis16475.c
+++ b/drivers/iio/imu/adis16475.c
@@ -263,7 +263,7 @@ static int adis16475_get_freq(struct adis16475 *st, u32 *freq)
u16 dec;
u32 sample_rate = st->clk_freq;
- mutex_lock(&st->adis.state_lock);
+ adis_dev_lock(&st->adis);
if (st->sync_mode == ADIS16475_SYNC_SCALED) {
u16 sync_scale;
@@ -279,13 +279,13 @@ static int adis16475_get_freq(struct adis16475 *st, u32 *freq)
if (ret)
goto error;
- mutex_unlock(&st->adis.state_lock);
+ adis_dev_unlock(&st->adis);
*freq = DIV_ROUND_CLOSEST(sample_rate, dec + 1);
return 0;
error:
- mutex_unlock(&st->adis.state_lock);
+ adis_dev_unlock(&st->adis);
return ret;
}
@@ -298,7 +298,7 @@ static int adis16475_set_freq(struct adis16475 *st, const u32 freq)
if (!freq)
return -EINVAL;
- mutex_lock(&st->adis.state_lock);
+ adis_dev_lock(&st->adis);
/*
* When using sync scaled mode, the input clock needs to be scaled so that we have
* an IMU sample rate between (optimally) 1900 and 2100. After this, we can use the
@@ -366,7 +366,7 @@ static int adis16475_set_freq(struct adis16475 *st, const u32 freq)
return 0;
error:
- mutex_unlock(&st->adis.state_lock);
+ adis_dev_unlock(&st->adis);
return ret;
}
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index c41b8ef1e250..f81b86690b76 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -329,7 +329,7 @@ static int adis16480_set_freq(struct iio_dev *indio_dev, int val, int val2)
if (t == 0)
return -EINVAL;
- mutex_lock(&st->adis.state_lock);
+ adis_dev_lock(&st->adis);
/*
* When using PPS mode, the input clock needs to be scaled so that we have an IMU
* sample rate between (optimally) 4000 and 4250. After this, we can use the
@@ -386,7 +386,7 @@ static int adis16480_set_freq(struct iio_dev *indio_dev, int val, int val2)
ret = __adis_write_reg_16(&st->adis, ADIS16480_REG_DEC_RATE, t);
error:
- mutex_unlock(&st->adis.state_lock);
+ adis_dev_unlock(&st->adis);
return ret;
}
@@ -397,7 +397,7 @@ static int adis16480_get_freq(struct iio_dev *indio_dev, int *val, int *val2)
int ret;
unsigned int freq, sample_rate = st->clk_freq;
- mutex_lock(&st->adis.state_lock);
+ adis_dev_lock(&st->adis);
if (st->clk_mode == ADIS16480_CLK_PPS) {
u16 sync_scale;
@@ -413,7 +413,7 @@ static int adis16480_get_freq(struct iio_dev *indio_dev, int *val, int *val2)
if (ret)
goto error;
- mutex_unlock(&st->adis.state_lock);
+ adis_dev_unlock(&st->adis);
freq = DIV_ROUND_CLOSEST(sample_rate, (t + 1));
@@ -422,7 +422,7 @@ static int adis16480_get_freq(struct iio_dev *indio_dev, int *val, int *val2)
return IIO_VAL_INT_PLUS_MICRO;
error:
- mutex_unlock(&st->adis.state_lock);
+ adis_dev_unlock(&st->adis);
return ret;
}
@@ -598,7 +598,6 @@ static int adis16480_set_filter_freq(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan, unsigned int freq)
{
struct adis16480 *st = iio_priv(indio_dev);
- struct mutex *slock = &st->adis.state_lock;
unsigned int enable_mask, offset, reg;
unsigned int diff, best_diff;
unsigned int i, best_freq;
@@ -609,7 +608,7 @@ static int adis16480_set_filter_freq(struct iio_dev *indio_dev,
offset = ad16480_filter_data[chan->scan_index][1];
enable_mask = BIT(offset + 2);
- mutex_lock(slock);
+ adis_dev_lock(&st->adis);
ret = __adis_read_reg_16(&st->adis, reg, &val);
if (ret)
@@ -637,7 +636,7 @@ static int adis16480_set_filter_freq(struct iio_dev *indio_dev,
ret = __adis_write_reg_16(&st->adis, reg, val);
out_unlock:
- mutex_unlock(slock);
+ adis_dev_unlock(&st->adis);
return ret;
}
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index 04e96d688ba9..f9b728d490b1 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -428,6 +428,16 @@ static inline int adis_initial_startup(struct adis *adis)
return ret;
}
+static inline void adis_dev_lock(struct adis *adis)
+{
+ mutex_lock(&adis->state_lock);
+}
+
+static inline void adis_dev_unlock(struct adis *adis)
+{
+ mutex_unlock(&adis->state_lock);
+}
+
int adis_single_conversion(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan, unsigned int error_mask,
int *val);
--
2.30.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/4] Fix/Improve sync clock mode handling
2021-02-18 11:40 [PATCH v2 0/4] Fix/Improve sync clock mode handling Nuno Sa
` (3 preceding siblings ...)
2021-02-18 11:40 ` [PATCH v2 4/4] iio: adis: add helpers for locking Nuno Sa
@ 2021-02-21 13:59 ` Jonathan Cameron
4 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-02-21 13:59 UTC (permalink / raw)
To: Nuno Sa
Cc: linux-iio, devicetree, Rob Herring, Lars-Peter Clausen,
Michael Hennerich
On Thu, 18 Feb 2021 12:40:35 +0100
Nuno Sa <nuno.sa@analog.com> wrote:
> The first patch in this series is just a simple helper to lock/unlock
> the device. Having these helpers make the code slightly neater (IMHO).
>
> The following patches introduces changes in the sampling frequency
> calculation when sync scale/pps modes are used. First, it's important
> to understand the purpose of this mode and how it should be used. Let's
> say our part has an internal rate of 4250 (e.g adis1649x family) and the
> user wants an output rate of 200SPS. Obviously, we can't use this
> sampling rate and divide back down to get 200 SPS with decimation on.
> Hence, we can use this mode to give an input clock of 1HZ, scale it to
> something like 4200 or 4000 SPS and then use the decimation filter to get
> the exact desired 200SPS. There are also some limits that should be
> taken into account when scaling:
>
> * For the devices in the adis16475 driver:
> - Input sync frequency range is 1 to 128 Hz
> - Native sample rate: 2 kSPS. Optimal range: 1900-2100 sps
>
> * For the adis1649x family (adis16480 driver):
> - Input sync frequency range is 1 to 128 Hz
> - Native sample rate: 4.25 kSPS. Optimal range: 4000-4250 sps
>
> I'm not 100% convinced on how to handle the optimal minimum. For now,
> I'm just throwing a warning saying we might get into trouble if we get a
> value lower than that. I was also tempted to just return -EINVAL or
> clamp the value. However, I know that there are ADI customers that
> (for some reason) are using a sampling rate lower than the minimum
> advised.
>
> That said, the patch for the adis16480 driver is a fix as this mode was
> being wrongly handled. There should not be a "separation" between using
> the sync_scale and the dec_rate registers. The way things were being done,
> we could easily get into a situation where the part could be running with
> an internal rate way lower than the optimal minimum.
>
> For the adis16475 drivers, things were not really wrong. They were just
> not optimal as we were forcing users to specify the IMU scaled internal
> rate once in the devicetree. Calculating things at runtime gives much
> more flexibility to choose the output rate.
Series applied. We may want to revisit this sometime in the future
but for now, lets get things working reasonably well.
Jonathan
>
> Changes in v2:
> * Moved the lock helper patch to the end of the series.
> * Changed all the users of the lock to use the helper functions.
> * Added a module parameter to allow users to run the IMUs at lower
> rates than the advisable.
>
> Nuno Sa (3):
> iio: adis16480: fix pps mode sampling frequency math
> iio: adis16475: improve sync scale mode handling
> iio: adis: add helpers for locking
>
> Nuno Sá (1):
> dt-bindings: adis16475: remove property
>
> .../bindings/iio/imu/adi,adis16475.yaml | 9 --
> drivers/iio/imu/adis16400.c | 22 ++-
> drivers/iio/imu/adis16475.c | 118 ++++++++++++----
> drivers/iio/imu/adis16480.c | 133 +++++++++++++-----
> include/linux/iio/imu/adis.h | 10 ++
> 5 files changed, 206 insertions(+), 86 deletions(-)
>
^ permalink raw reply [flat|nested] 6+ messages in thread