linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] Factorize timestamp module
@ 2023-05-31 14:25 inv.git-commit
  2023-05-31 14:25 ` [PATCH 1/4] iio: imu: inv_icm42600: make timestamp module chip independant inv.git-commit
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: inv.git-commit @ 2023-05-31 14:25 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: lars, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

The purpose if this series is to make timestamping from
inv_icm42600 driver an independent module and use it for both
inv_icm42600 and inv_mpu6050 drivers.

Create a new inv_sensors_timestamp common module based on
inv_icm42600 driver and use it in the 2 drivers.

WARNING: this patch requires following commit in fixes-togreg
bbaae0c79ebd ("iio: imu: inv_icm42600: fix timestamp reset")

Jean-Baptiste Maneyrol (4):
  iio: imu: inv_icm42600: make timestamp module chip independent
  iio: move inv_icm42600 timestamp module in common
  iio: make invensense timestamp module generic
  iio: imu: inv_mpu6050: use the common inv_sensors timestamp module

 drivers/iio/common/Kconfig                    |   1 +
 drivers/iio/common/Makefile                   |   1 +
 drivers/iio/common/inv_sensors/Kconfig        |   7 ++
 drivers/iio/common/inv_sensors/Makefile       |   6 +
 .../inv_sensors/inv_sensors_timestamp.c}      | 105 +++++++++---------
 drivers/iio/imu/inv_icm42600/Kconfig          |   1 +
 drivers/iio/imu/inv_icm42600/Makefile         |   1 -
 .../iio/imu/inv_icm42600/inv_icm42600_accel.c |  32 ++++--
 .../imu/inv_icm42600/inv_icm42600_buffer.c    |  30 ++---
 .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  13 ++-
 .../iio/imu/inv_icm42600/inv_icm42600_gyro.c  |  32 ++++--
 .../imu/inv_icm42600/inv_icm42600_timestamp.h |  85 --------------
 drivers/iio/imu/inv_mpu6050/Kconfig           |   1 +
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    |  26 ++++-
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |   9 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    |  83 ++------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |   3 +-
 .../linux/iio/common/inv_sensors_timestamp.h  |  89 +++++++++++++++
 18 files changed, 255 insertions(+), 270 deletions(-)
 create mode 100644 drivers/iio/common/inv_sensors/Kconfig
 create mode 100644 drivers/iio/common/inv_sensors/Makefile
 rename drivers/iio/{imu/inv_icm42600/inv_icm42600_timestamp.c => common/inv_sensors/inv_sensors_timestamp.c} (55%)
 delete mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
 create mode 100644 include/linux/iio/common/inv_sensors_timestamp.h

-- 
2.34.1


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

* [PATCH 1/4] iio: imu: inv_icm42600: make timestamp module chip independant
  2023-05-31 14:25 [PATCH 0/4] Factorize timestamp module inv.git-commit
@ 2023-05-31 14:25 ` inv.git-commit
  2023-06-03 11:10   ` andy.shevchenko
  2023-05-31 14:25 ` [PATCH 2/4] iio: move inv_icm42600 timestamp module in common inv.git-commit
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: inv.git-commit @ 2023-05-31 14:25 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: lars, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Move icm42600 dependent function inside the core module.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c   | 11 +++++++++++
 .../iio/imu/inv_icm42600/inv_icm42600_timestamp.c  | 14 +-------------
 .../iio/imu/inv_icm42600/inv_icm42600_timestamp.h  |  4 ----
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index 7b3a2a0dc2cb..c34735b05830 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -516,6 +516,17 @@ static int inv_icm42600_irq_init(struct inv_icm42600_state *st, int irq,
 					 "inv_icm42600", st);
 }
 
+static int inv_icm42600_timestamp_setup(struct inv_icm42600_state *st)
+{
+	unsigned int val;
+
+	/* enable timestamp register */
+	val = INV_ICM42600_TMST_CONFIG_TMST_TO_REGS_EN |
+	      INV_ICM42600_TMST_CONFIG_TMST_EN;
+	return regmap_update_bits(st->map, INV_ICM42600_REG_TMST_CONFIG,
+				  INV_ICM42600_TMST_CONFIG_MASK, val);
+}
+
 static int inv_icm42600_enable_regulator_vddio(struct inv_icm42600_state *st)
 {
 	int ret;
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
index 37cbf08acb3a..ceae8ccb1747 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
@@ -4,10 +4,9 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/regmap.h>
 #include <linux/math64.h>
+#include <linux/errno.h>
 
-#include "inv_icm42600.h"
 #include "inv_icm42600_timestamp.h"
 
 /* internal chip period is 32kHz, 31250ns */
@@ -56,17 +55,6 @@ void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts,
 	inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD);
 }
 
-int inv_icm42600_timestamp_setup(struct inv_icm42600_state *st)
-{
-	unsigned int val;
-
-	/* enable timestamp register */
-	val = INV_ICM42600_TMST_CONFIG_TMST_TO_REGS_EN |
-	      INV_ICM42600_TMST_CONFIG_TMST_EN;
-	return regmap_update_bits(st->map, INV_ICM42600_REG_TMST_CONFIG,
-				  INV_ICM42600_TMST_CONFIG_MASK, val);
-}
-
 int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
 				      uint32_t period, bool fifo)
 {
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h b/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
index 4e4f331d4fe4..00fd452632a3 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
@@ -8,8 +8,6 @@
 
 #include <linux/kernel.h>
 
-struct inv_icm42600_state;
-
 /**
  * struct inv_icm42600_timestamp_interval - timestamps interval
  * @lo:	interval lower bound
@@ -53,8 +51,6 @@ struct inv_icm42600_timestamp {
 void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts,
 				 uint32_t period);
 
-int inv_icm42600_timestamp_setup(struct inv_icm42600_state *st);
-
 int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
 				      uint32_t period, bool fifo);
 
-- 
2.34.1


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

* [PATCH 2/4] iio: move inv_icm42600 timestamp module in common
  2023-05-31 14:25 [PATCH 0/4] Factorize timestamp module inv.git-commit
  2023-05-31 14:25 ` [PATCH 1/4] iio: imu: inv_icm42600: make timestamp module chip independant inv.git-commit
@ 2023-05-31 14:25 ` inv.git-commit
  2023-06-03 11:13   ` andy.shevchenko
  2023-06-04 10:58   ` Jonathan Cameron
  2023-05-31 14:25 ` [PATCH 3/4] iio: make invensense timestamp module generic inv.git-commit
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 17+ messages in thread
From: inv.git-commit @ 2023-05-31 14:25 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: lars, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Create new inv_sensors common modules and move inv_icm42600
timestamp module inside.
Modify inv_icm42600 driver to use timestamp module.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/common/Kconfig                           |  1 +
 drivers/iio/common/Makefile                          |  1 +
 drivers/iio/common/inv_sensors/Kconfig               |  7 +++++++
 drivers/iio/common/inv_sensors/Makefile              |  6 ++++++
 .../inv_sensors}/inv_icm42600_timestamp.c            | 12 ++++++++++--
 drivers/iio/imu/inv_icm42600/Kconfig                 |  1 +
 drivers/iio/imu/inv_icm42600/Makefile                |  1 -
 drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c    |  2 +-
 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c   |  2 +-
 drivers/iio/imu/inv_icm42600/inv_icm42600_core.c     |  3 ++-
 drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c     |  2 +-
 .../linux/iio/common}/inv_icm42600_timestamp.h       |  0
 12 files changed, 31 insertions(+), 7 deletions(-)
 create mode 100644 drivers/iio/common/inv_sensors/Kconfig
 create mode 100644 drivers/iio/common/inv_sensors/Makefile
 rename drivers/iio/{imu/inv_icm42600 => common/inv_sensors}/inv_icm42600_timestamp.c (91%)
 rename {drivers/iio/imu/inv_icm42600 => include/linux/iio/common}/inv_icm42600_timestamp.h (100%)

diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
index 0334b4954773..1ccb5ccf3706 100644
--- a/drivers/iio/common/Kconfig
+++ b/drivers/iio/common/Kconfig
@@ -5,6 +5,7 @@
 
 source "drivers/iio/common/cros_ec_sensors/Kconfig"
 source "drivers/iio/common/hid-sensors/Kconfig"
+source "drivers/iio/common/inv_sensors/Kconfig"
 source "drivers/iio/common/ms_sensors/Kconfig"
 source "drivers/iio/common/scmi_sensors/Kconfig"
 source "drivers/iio/common/ssp_sensors/Kconfig"
diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
index fad40e1e1718..d3e952239a62 100644
--- a/drivers/iio/common/Makefile
+++ b/drivers/iio/common/Makefile
@@ -10,6 +10,7 @@
 # When adding new entries keep the list in alphabetical order
 obj-y += cros_ec_sensors/
 obj-y += hid-sensors/
+obj-y += inv_sensors/
 obj-y += ms_sensors/
 obj-y += scmi_sensors/
 obj-y += ssp_sensors/
diff --git a/drivers/iio/common/inv_sensors/Kconfig b/drivers/iio/common/inv_sensors/Kconfig
new file mode 100644
index 000000000000..28815fb43157
--- /dev/null
+++ b/drivers/iio/common/inv_sensors/Kconfig
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# TDK-InvenSense sensors common library
+#
+
+config IIO_INV_SENSORS_TIMESTAMP
+	tristate
diff --git a/drivers/iio/common/inv_sensors/Makefile b/drivers/iio/common/inv_sensors/Makefile
new file mode 100644
index 000000000000..93bddb9356b8
--- /dev/null
+++ b/drivers/iio/common/inv_sensors/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for TDK-InvenSense sensors module.
+#
+
+obj-$(CONFIG_IIO_INV_SENSORS_TIMESTAMP) += inv_icm42600_timestamp.o
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
similarity index 91%
rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
rename to drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
index ceae8ccb1747..411f561e1a24 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
+++ b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
@@ -5,9 +5,9 @@
 
 #include <linux/kernel.h>
 #include <linux/math64.h>
+#include <linux/module.h>
 #include <linux/errno.h>
-
-#include "inv_icm42600_timestamp.h"
+#include <linux/iio/common/inv_icm42600_timestamp.h>
 
 /* internal chip period is 32kHz, 31250ns */
 #define INV_ICM42600_TIMESTAMP_PERIOD		31250
@@ -54,6 +54,7 @@ void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts,
 	/* use theoretical value for chip period */
 	inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD);
 }
+EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_init, IIO_INV_SENSORS_TIMESTAMP);
 
 int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
 				      uint32_t period, bool fifo)
@@ -66,6 +67,7 @@ int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
 
 	return 0;
 }
+EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP);
 
 static bool inv_validate_period(uint32_t period, uint32_t mult)
 {
@@ -153,6 +155,7 @@ void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
 	if (valid)
 		inv_align_timestamp_it(ts);
 }
+EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_interrupt, IIO_INV_SENSORS_TIMESTAMP);
 
 void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
 				      uint32_t fifo_period, size_t fifo_nb,
@@ -184,3 +187,8 @@ void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
 		ts->timestamp = ts->it.up - interval;
 	}
 }
+EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_apply_odr, IIO_INV_SENSORS_TIMESTAMP);
+
+MODULE_AUTHOR("InvenSense, Inc.");
+MODULE_DESCRIPTION("InvenSense sensors timestamp module");
+MODULE_LICENSE("GPL");
diff --git a/drivers/iio/imu/inv_icm42600/Kconfig b/drivers/iio/imu/inv_icm42600/Kconfig
index 50cbcfcb6cf1..f56b0816cc4d 100644
--- a/drivers/iio/imu/inv_icm42600/Kconfig
+++ b/drivers/iio/imu/inv_icm42600/Kconfig
@@ -3,6 +3,7 @@
 config INV_ICM42600
 	tristate
 	select IIO_BUFFER
+	select IIO_INV_SENSORS_TIMESTAMP
 
 config INV_ICM42600_I2C
 	tristate "InvenSense ICM-426xx I2C driver"
diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile
index 291714d9aa54..0f49f6df3647 100644
--- a/drivers/iio/imu/inv_icm42600/Makefile
+++ b/drivers/iio/imu/inv_icm42600/Makefile
@@ -6,7 +6,6 @@ inv-icm42600-y += inv_icm42600_gyro.o
 inv-icm42600-y += inv_icm42600_accel.o
 inv-icm42600-y += inv_icm42600_temp.o
 inv-icm42600-y += inv_icm42600_buffer.o
-inv-icm42600-y += inv_icm42600_timestamp.o
 
 obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
 inv-icm42600-i2c-y += inv_icm42600_i2c.o
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
index c3f433ad3af6..1015de636a94 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
@@ -12,12 +12,12 @@
 #include <linux/math64.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/common/inv_icm42600_timestamp.h>
 #include <linux/iio/kfifo_buf.h>
 
 #include "inv_icm42600.h"
 #include "inv_icm42600_temp.h"
 #include "inv_icm42600_buffer.h"
-#include "inv_icm42600_timestamp.h"
 
 #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info)		\
 	{								\
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
index 32d7f8364230..4a39d31e911f 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
@@ -11,9 +11,9 @@
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/common/inv_icm42600_timestamp.h>
 
 #include "inv_icm42600.h"
-#include "inv_icm42600_timestamp.h"
 #include "inv_icm42600_buffer.h"
 
 /* FIFO header: 1 byte */
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index c34735b05830..f3e379f9733d 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -16,10 +16,10 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/common/inv_icm42600_timestamp.h>
 
 #include "inv_icm42600.h"
 #include "inv_icm42600_buffer.h"
-#include "inv_icm42600_timestamp.h"
 
 static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
 	{
@@ -799,3 +799,4 @@ EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42600_pm_ops, IIO_ICM42600) = {
 MODULE_AUTHOR("InvenSense, Inc.");
 MODULE_DESCRIPTION("InvenSense ICM-426xx device driver");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_INV_SENSORS_TIMESTAMP);
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
index 9d94a8518e3c..6caea7b8a344 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
@@ -12,12 +12,12 @@
 #include <linux/math64.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/common/inv_icm42600_timestamp.h>
 #include <linux/iio/kfifo_buf.h>
 
 #include "inv_icm42600.h"
 #include "inv_icm42600_temp.h"
 #include "inv_icm42600_buffer.h"
-#include "inv_icm42600_timestamp.h"
 
 #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info)		\
 	{								\
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h b/include/linux/iio/common/inv_icm42600_timestamp.h
similarity index 100%
rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
rename to include/linux/iio/common/inv_icm42600_timestamp.h
-- 
2.34.1


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

* [PATCH 3/4] iio: make invensense timestamp module generic
  2023-05-31 14:25 [PATCH 0/4] Factorize timestamp module inv.git-commit
  2023-05-31 14:25 ` [PATCH 1/4] iio: imu: inv_icm42600: make timestamp module chip independant inv.git-commit
  2023-05-31 14:25 ` [PATCH 2/4] iio: move inv_icm42600 timestamp module in common inv.git-commit
@ 2023-05-31 14:25 ` inv.git-commit
  2023-06-03 11:19   ` andy.shevchenko
  2023-06-04 11:06   ` Jonathan Cameron
  2023-05-31 14:25 ` [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module inv.git-commit
  2023-06-03 11:24 ` [PATCH 0/4] Factorize " andy.shevchenko
  4 siblings, 2 replies; 17+ messages in thread
From: inv.git-commit @ 2023-05-31 14:25 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: lars, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Rename common module to inv_sensors_timestamp, add configuration
at init (chip internal clock, acceptable jitter, ...) and update
inv_icm42600 driver integration.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/common/inv_sensors/Makefile       |  2 +-
 ...00_timestamp.c => inv_sensors_timestamp.c} | 89 +++++++++----------
 .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 32 ++++---
 .../imu/inv_icm42600/inv_icm42600_buffer.c    | 30 +++----
 .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  1 -
 .../iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 32 ++++---
 .../linux/iio/common/inv_icm42600_timestamp.h | 81 -----------------
 .../linux/iio/common/inv_sensors_timestamp.h  | 89 +++++++++++++++++++
 8 files changed, 189 insertions(+), 167 deletions(-)
 rename drivers/iio/common/inv_sensors/{inv_icm42600_timestamp.c => inv_sensors_timestamp.c} (57%)
 delete mode 100644 include/linux/iio/common/inv_icm42600_timestamp.h
 create mode 100644 include/linux/iio/common/inv_sensors_timestamp.h

diff --git a/drivers/iio/common/inv_sensors/Makefile b/drivers/iio/common/inv_sensors/Makefile
index 93bddb9356b8..dcf39f249112 100644
--- a/drivers/iio/common/inv_sensors/Makefile
+++ b/drivers/iio/common/inv_sensors/Makefile
@@ -3,4 +3,4 @@
 # Makefile for TDK-InvenSense sensors module.
 #
 
-obj-$(CONFIG_IIO_INV_SENSORS_TIMESTAMP) += inv_icm42600_timestamp.o
+obj-$(CONFIG_IIO_INV_SENSORS_TIMESTAMP) += inv_sensors_timestamp.o
diff --git a/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c
similarity index 57%
rename from drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
rename to drivers/iio/common/inv_sensors/inv_sensors_timestamp.c
index 411f561e1a24..060191ef3125 100644
--- a/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
+++ b/drivers/iio/common/inv_sensors/inv_sensors_timestamp.c
@@ -7,20 +7,18 @@
 #include <linux/math64.h>
 #include <linux/module.h>
 #include <linux/errno.h>
-#include <linux/iio/common/inv_icm42600_timestamp.h>
-
-/* internal chip period is 32kHz, 31250ns */
-#define INV_ICM42600_TIMESTAMP_PERIOD		31250
-/* allow a jitter of +/- 2% */
-#define INV_ICM42600_TIMESTAMP_JITTER		2
-/* compute min and max periods accepted */
-#define INV_ICM42600_TIMESTAMP_MIN_PERIOD(_p)		\
-	(((_p) * (100 - INV_ICM42600_TIMESTAMP_JITTER)) / 100)
-#define INV_ICM42600_TIMESTAMP_MAX_PERIOD(_p)		\
-	(((_p) * (100 + INV_ICM42600_TIMESTAMP_JITTER)) / 100)
+#include <linux/iio/common/inv_sensors_timestamp.h>
+
+/* compute jitter, min and max following jitter in per mille */
+#define INV_SENSORS_TIMESTAMP_JITTER(_val, _jitter)		\
+	(div_s64((_val) * (_jitter), 1000))
+#define INV_SENSORS_TIMESTAMP_MIN(_val, _jitter)		\
+	(((_val) * (1000 - (_jitter))) / 1000)
+#define INV_SENSORS_TIMESTAMP_MAX(_val, _jitter)		\
+	(((_val) * (1000 + (_jitter))) / 1000)
 
 /* Add a new value inside an accumulator and update the estimate value */
-static void inv_update_acc(struct inv_icm42600_timestamp_acc *acc, uint32_t val)
+static void inv_update_acc(struct inv_sensors_timestamp_acc *acc, uint32_t val)
 {
 	uint64_t sum = 0;
 	size_t i;
@@ -39,56 +37,57 @@ static void inv_update_acc(struct inv_icm42600_timestamp_acc *acc, uint32_t val)
 	acc->val = div_u64(sum, i);
 }
 
-void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts,
-				 uint32_t period)
+void inv_sensors_timestamp_init(struct inv_sensors_timestamp *ts,
+				const struct inv_sensors_timestamp_chip *chip)
 {
-	/* initial odr for sensor after reset is 1kHz */
-	const uint32_t default_period = 1000000;
+	memset(ts, 0, sizeof(*ts));
+
+	/* save chip parameters and compute min and max clock period */
+	ts->chip = *chip;
+	ts->min_period = INV_SENSORS_TIMESTAMP_MIN(chip->clock_period, chip->jitter);
+	ts->max_period = INV_SENSORS_TIMESTAMP_MAX(chip->clock_period, chip->jitter);
 
 	/* current multiplier and period values after reset */
-	ts->mult = default_period / INV_ICM42600_TIMESTAMP_PERIOD;
-	ts->period = default_period;
-	/* new set multiplier is the one from chip initialization */
-	ts->new_mult = period / INV_ICM42600_TIMESTAMP_PERIOD;
+	ts->mult = chip->init_period / chip->clock_period;
+	ts->period = chip->init_period;
 
 	/* use theoretical value for chip period */
-	inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD);
+	inv_update_acc(&ts->chip_period, chip->clock_period);
 }
-EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_init, IIO_INV_SENSORS_TIMESTAMP);
+EXPORT_SYMBOL_NS_GPL(inv_sensors_timestamp_init, IIO_INV_SENSORS_TIMESTAMP);
 
-int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
-				      uint32_t period, bool fifo)
+int inv_sensors_timestamp_update_odr(struct inv_sensors_timestamp *ts,
+				     uint32_t period, bool fifo)
 {
 	/* when FIFO is on, prevent odr change if one is already pending */
 	if (fifo && ts->new_mult != 0)
 		return -EAGAIN;
 
-	ts->new_mult = period / INV_ICM42600_TIMESTAMP_PERIOD;
+	ts->new_mult = period / ts->chip.clock_period;
 
 	return 0;
 }
-EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP);
+EXPORT_SYMBOL_NS_GPL(inv_sensors_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP);
 
-static bool inv_validate_period(uint32_t period, uint32_t mult)
+static bool inv_validate_period(struct inv_sensors_timestamp *ts, uint32_t period, uint32_t mult)
 {
-	const uint32_t chip_period = INV_ICM42600_TIMESTAMP_PERIOD;
 	uint32_t period_min, period_max;
 
 	/* check that period is acceptable */
-	period_min = INV_ICM42600_TIMESTAMP_MIN_PERIOD(chip_period) * mult;
-	period_max = INV_ICM42600_TIMESTAMP_MAX_PERIOD(chip_period) * mult;
+	period_min = ts->min_period * mult;
+	period_max = ts->max_period * mult;
 	if (period > period_min && period < period_max)
 		return true;
 	else
 		return false;
 }
 
-static bool inv_update_chip_period(struct inv_icm42600_timestamp *ts,
-				   uint32_t mult, uint32_t period)
+static bool inv_update_chip_period(struct inv_sensors_timestamp *ts,
+				    uint32_t mult, uint32_t period)
 {
 	uint32_t new_chip_period;
 
-	if (!inv_validate_period(period, mult))
+	if (!inv_validate_period(ts, period, mult))
 		return false;
 
 	/* update chip internal period estimation */
@@ -99,7 +98,7 @@ static bool inv_update_chip_period(struct inv_icm42600_timestamp *ts,
 	return true;
 }
 
-static void inv_align_timestamp_it(struct inv_icm42600_timestamp *ts)
+static void inv_align_timestamp_it(struct inv_sensors_timestamp *ts)
 {
 	int64_t delta, jitter;
 	int64_t adjust;
@@ -108,7 +107,7 @@ static void inv_align_timestamp_it(struct inv_icm42600_timestamp *ts)
 	delta = ts->it.lo - ts->timestamp;
 
 	/* adjust timestamp while respecting jitter */
-	jitter = div_s64((int64_t)ts->period * INV_ICM42600_TIMESTAMP_JITTER, 100);
+	jitter = INV_SENSORS_TIMESTAMP_JITTER((int64_t)ts->period, ts->chip.jitter);
 	if (delta > jitter)
 		adjust = jitter;
 	else if (delta < -jitter)
@@ -119,13 +118,13 @@ static void inv_align_timestamp_it(struct inv_icm42600_timestamp *ts)
 	ts->timestamp += adjust;
 }
 
-void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
+void inv_sensors_timestamp_interrupt(struct inv_sensors_timestamp *ts,
 				      uint32_t fifo_period, size_t fifo_nb,
 				      size_t sensor_nb, int64_t timestamp)
 {
-	struct inv_icm42600_timestamp_interval *it;
+	struct inv_sensors_timestamp_interval *it;
 	int64_t delta, interval;
-	const uint32_t fifo_mult = fifo_period / INV_ICM42600_TIMESTAMP_PERIOD;
+	const uint32_t fifo_mult = fifo_period / ts->chip.clock_period;
 	uint32_t period = ts->period;
 	bool valid = false;
 
@@ -151,15 +150,15 @@ void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
 		return;
 	}
 
-	/* if interrupt interval is valid, sync with interrupt timestamp */
+	/* if interrupt interval is valid, align with interrupt timestamp */
 	if (valid)
 		inv_align_timestamp_it(ts);
 }
-EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_interrupt, IIO_INV_SENSORS_TIMESTAMP);
+EXPORT_SYMBOL_NS_GPL(inv_sensors_timestamp_interrupt, IIO_INV_SENSORS_TIMESTAMP);
 
-void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
-				      uint32_t fifo_period, size_t fifo_nb,
-				      unsigned int fifo_no)
+void inv_sensors_timestamp_apply_odr(struct inv_sensors_timestamp *ts,
+				     uint32_t fifo_period, size_t fifo_nb,
+				     unsigned int fifo_no)
 {
 	int64_t interval;
 	uint32_t fifo_mult;
@@ -180,14 +179,14 @@ void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
 	 */
 	if (ts->timestamp != 0) {
 		/* compute measured fifo period */
-		fifo_mult = fifo_period / INV_ICM42600_TIMESTAMP_PERIOD;
+		fifo_mult = fifo_period / ts->chip.clock_period;
 		fifo_period = fifo_mult * ts->chip_period.val;
 		/* computes time interval between interrupt and this sample */
 		interval = (int64_t)(fifo_nb - fifo_no) * (int64_t)fifo_period;
 		ts->timestamp = ts->it.up - interval;
 	}
 }
-EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_apply_odr, IIO_INV_SENSORS_TIMESTAMP);
+EXPORT_SYMBOL_NS_GPL(inv_sensors_timestamp_apply_odr, IIO_INV_SENSORS_TIMESTAMP);
 
 MODULE_AUTHOR("InvenSense, Inc.");
 MODULE_DESCRIPTION("InvenSense sensors timestamp module");
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
index 1015de636a94..795f31b47318 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
@@ -12,7 +12,7 @@
 #include <linux/math64.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
-#include <linux/iio/common/inv_icm42600_timestamp.h>
+#include <linux/iio/common/inv_sensors_timestamp.h>
 #include <linux/iio/kfifo_buf.h>
 
 #include "inv_icm42600.h"
@@ -98,7 +98,7 @@ static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
 					       const unsigned long *scan_mask)
 {
 	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
-	struct inv_icm42600_timestamp *ts = iio_priv(indio_dev);
+	struct inv_sensors_timestamp *ts = iio_priv(indio_dev);
 	struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
 	unsigned int fifo_en = 0;
 	unsigned int sleep_temp = 0;
@@ -126,7 +126,7 @@ static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
 	}
 
 	/* update data FIFO write */
-	inv_icm42600_timestamp_apply_odr(ts, 0, 0, 0);
+	inv_sensors_timestamp_apply_odr(ts, 0, 0, 0);
 	ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
 	if (ret)
 		goto out_unlock;
@@ -311,7 +311,7 @@ static int inv_icm42600_accel_write_odr(struct iio_dev *indio_dev,
 					int val, int val2)
 {
 	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
-	struct inv_icm42600_timestamp *ts = iio_priv(indio_dev);
+	struct inv_sensors_timestamp *ts = iio_priv(indio_dev);
 	struct device *dev = regmap_get_device(st->map);
 	unsigned int idx;
 	struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
@@ -330,8 +330,8 @@ static int inv_icm42600_accel_write_odr(struct iio_dev *indio_dev,
 	pm_runtime_get_sync(dev);
 	mutex_lock(&st->lock);
 
-	ret = inv_icm42600_timestamp_update_odr(ts, inv_icm42600_odr_to_period(conf.odr),
-						iio_buffer_enabled(indio_dev));
+	ret = inv_sensors_timestamp_update_odr(ts, inv_icm42600_odr_to_period(conf.odr),
+					       iio_buffer_enabled(indio_dev));
 	if (ret)
 		goto out_unlock;
 
@@ -707,7 +707,8 @@ struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st)
 {
 	struct device *dev = regmap_get_device(st->map);
 	const char *name;
-	struct inv_icm42600_timestamp *ts;
+	struct inv_sensors_timestamp_chip ts_chip;
+	struct inv_sensors_timestamp *ts;
 	struct iio_dev *indio_dev;
 	int ret;
 
@@ -719,8 +720,15 @@ struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st)
 	if (!indio_dev)
 		return ERR_PTR(-ENOMEM);
 
+	/*
+	 * clock period is 32kHz (31250ns)
+	 * jitter is +/- 2% (20 per mille)
+	 */
+	ts_chip.clock_period = 31250;
+	ts_chip.jitter = 20;
+	ts_chip.init_period = inv_icm42600_odr_to_period(st->conf.accel.odr);
 	ts = iio_priv(indio_dev);
-	inv_icm42600_timestamp_init(ts, inv_icm42600_odr_to_period(st->conf.accel.odr));
+	inv_sensors_timestamp_init(ts, &ts_chip);
 
 	iio_device_set_drvdata(indio_dev, st);
 	indio_dev->name = name;
@@ -745,7 +753,7 @@ struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st)
 int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev)
 {
 	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
-	struct inv_icm42600_timestamp *ts = iio_priv(indio_dev);
+	struct inv_sensors_timestamp *ts = iio_priv(indio_dev);
 	ssize_t i, size;
 	unsigned int no;
 	const void *accel, *gyro, *timestamp;
@@ -768,15 +776,15 @@ int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev)
 
 		/* update odr */
 		if (odr & INV_ICM42600_SENSOR_ACCEL)
-			inv_icm42600_timestamp_apply_odr(ts, st->fifo.period,
-							 st->fifo.nb.total, no);
+			inv_sensors_timestamp_apply_odr(ts, st->fifo.period,
+							st->fifo.nb.total, no);
 
 		/* buffer is copied to userspace, zeroing it to avoid any data leak */
 		memset(&buffer, 0, sizeof(buffer));
 		memcpy(&buffer.accel, accel, sizeof(buffer.accel));
 		/* convert 8 bits FIFO temperature in high resolution format */
 		buffer.temp = temp ? (*temp * 64) : 0;
-		ts_val = inv_icm42600_timestamp_pop(ts);
+		ts_val = inv_sensors_timestamp_pop(ts);
 		iio_push_to_buffers_with_timestamp(indio_dev, &buffer, ts_val);
 	}
 
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
index 4a39d31e911f..42a4a1a52d60 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
@@ -11,7 +11,7 @@
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
-#include <linux/iio/common/inv_icm42600_timestamp.h>
+#include <linux/iio/common/inv_sensors_timestamp.h>
 
 #include "inv_icm42600.h"
 #include "inv_icm42600_buffer.h"
@@ -275,12 +275,12 @@ static int inv_icm42600_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
 	struct device *dev = regmap_get_device(st->map);
-	struct inv_icm42600_timestamp *ts = iio_priv(indio_dev);
+	struct inv_sensors_timestamp *ts = iio_priv(indio_dev);
 
 	pm_runtime_get_sync(dev);
 
 	mutex_lock(&st->lock);
-	inv_icm42600_timestamp_reset(ts);
+	inv_sensors_timestamp_reset(ts);
 	mutex_unlock(&st->lock);
 
 	return 0;
@@ -504,7 +504,7 @@ int inv_icm42600_buffer_fifo_read(struct inv_icm42600_state *st,
 
 int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
 {
-	struct inv_icm42600_timestamp *ts;
+	struct inv_sensors_timestamp *ts;
 	int ret;
 
 	if (st->fifo.nb.total == 0)
@@ -512,8 +512,8 @@ int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
 
 	/* handle gyroscope timestamp and FIFO data parsing */
 	ts = iio_priv(st->indio_gyro);
-	inv_icm42600_timestamp_interrupt(ts, st->fifo.period, st->fifo.nb.total,
-					 st->fifo.nb.gyro, st->timestamp.gyro);
+	inv_sensors_timestamp_interrupt(ts, st->fifo.period, st->fifo.nb.total,
+					st->fifo.nb.gyro, st->timestamp.gyro);
 	if (st->fifo.nb.gyro > 0) {
 		ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
 		if (ret)
@@ -522,8 +522,8 @@ int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
 
 	/* handle accelerometer timestamp and FIFO data parsing */
 	ts = iio_priv(st->indio_accel);
-	inv_icm42600_timestamp_interrupt(ts, st->fifo.period, st->fifo.nb.total,
-					 st->fifo.nb.accel, st->timestamp.accel);
+	inv_sensors_timestamp_interrupt(ts, st->fifo.period, st->fifo.nb.total,
+					st->fifo.nb.accel, st->timestamp.accel);
 	if (st->fifo.nb.accel > 0) {
 		ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
 		if (ret)
@@ -536,7 +536,7 @@ int inv_icm42600_buffer_fifo_parse(struct inv_icm42600_state *st)
 int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
 				     unsigned int count)
 {
-	struct inv_icm42600_timestamp *ts;
+	struct inv_sensors_timestamp *ts;
 	int64_t gyro_ts, accel_ts;
 	int ret;
 
@@ -552,9 +552,9 @@ int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
 
 	if (st->fifo.nb.gyro > 0) {
 		ts = iio_priv(st->indio_gyro);
-		inv_icm42600_timestamp_interrupt(ts, st->fifo.period,
-						 st->fifo.nb.total, st->fifo.nb.gyro,
-						 gyro_ts);
+		inv_sensors_timestamp_interrupt(ts, st->fifo.period,
+						st->fifo.nb.total, st->fifo.nb.gyro,
+						gyro_ts);
 		ret = inv_icm42600_gyro_parse_fifo(st->indio_gyro);
 		if (ret)
 			return ret;
@@ -562,9 +562,9 @@ int inv_icm42600_buffer_hwfifo_flush(struct inv_icm42600_state *st,
 
 	if (st->fifo.nb.accel > 0) {
 		ts = iio_priv(st->indio_accel);
-		inv_icm42600_timestamp_interrupt(ts, st->fifo.period,
-						 st->fifo.nb.total, st->fifo.nb.accel,
-						 accel_ts);
+		inv_sensors_timestamp_interrupt(ts, st->fifo.period,
+						st->fifo.nb.total, st->fifo.nb.accel,
+						accel_ts);
 		ret = inv_icm42600_accel_parse_fifo(st->indio_accel);
 		if (ret)
 			return ret;
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index f3e379f9733d..0eb11911f786 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -16,7 +16,6 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/iio/iio.h>
-#include <linux/iio/common/inv_icm42600_timestamp.h>
 
 #include "inv_icm42600.h"
 #include "inv_icm42600_buffer.h"
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
index 6caea7b8a344..57dd7324e9e9 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
@@ -12,7 +12,7 @@
 #include <linux/math64.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
-#include <linux/iio/common/inv_icm42600_timestamp.h>
+#include <linux/iio/common/inv_sensors_timestamp.h>
 #include <linux/iio/kfifo_buf.h>
 
 #include "inv_icm42600.h"
@@ -98,7 +98,7 @@ static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev,
 					      const unsigned long *scan_mask)
 {
 	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
-	struct inv_icm42600_timestamp *ts = iio_priv(indio_dev);
+	struct inv_sensors_timestamp *ts = iio_priv(indio_dev);
 	struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
 	unsigned int fifo_en = 0;
 	unsigned int sleep_gyro = 0;
@@ -126,7 +126,7 @@ static int inv_icm42600_gyro_update_scan_mode(struct iio_dev *indio_dev,
 	}
 
 	/* update data FIFO write */
-	inv_icm42600_timestamp_apply_odr(ts, 0, 0, 0);
+	inv_sensors_timestamp_apply_odr(ts, 0, 0, 0);
 	ret = inv_icm42600_buffer_set_fifo_en(st, fifo_en | st->fifo.en);
 	if (ret)
 		goto out_unlock;
@@ -323,7 +323,7 @@ static int inv_icm42600_gyro_write_odr(struct iio_dev *indio_dev,
 				       int val, int val2)
 {
 	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
-	struct inv_icm42600_timestamp *ts = iio_priv(indio_dev);
+	struct inv_sensors_timestamp *ts = iio_priv(indio_dev);
 	struct device *dev = regmap_get_device(st->map);
 	unsigned int idx;
 	struct inv_icm42600_sensor_conf conf = INV_ICM42600_SENSOR_CONF_INIT;
@@ -342,8 +342,8 @@ static int inv_icm42600_gyro_write_odr(struct iio_dev *indio_dev,
 	pm_runtime_get_sync(dev);
 	mutex_lock(&st->lock);
 
-	ret = inv_icm42600_timestamp_update_odr(ts, inv_icm42600_odr_to_period(conf.odr),
-						iio_buffer_enabled(indio_dev));
+	ret = inv_sensors_timestamp_update_odr(ts, inv_icm42600_odr_to_period(conf.odr),
+					       iio_buffer_enabled(indio_dev));
 	if (ret)
 		goto out_unlock;
 
@@ -718,7 +718,8 @@ struct iio_dev *inv_icm42600_gyro_init(struct inv_icm42600_state *st)
 {
 	struct device *dev = regmap_get_device(st->map);
 	const char *name;
-	struct inv_icm42600_timestamp *ts;
+	struct inv_sensors_timestamp_chip ts_chip;
+	struct inv_sensors_timestamp *ts;
 	struct iio_dev *indio_dev;
 	int ret;
 
@@ -730,8 +731,15 @@ struct iio_dev *inv_icm42600_gyro_init(struct inv_icm42600_state *st)
 	if (!indio_dev)
 		return ERR_PTR(-ENOMEM);
 
+	/*
+	 * clock period is 32kHz (31250ns)
+	 * jitter is +/- 2% (20 per mille)
+	 */
+	ts_chip.clock_period = 31250;
+	ts_chip.jitter = 20;
+	ts_chip.init_period = inv_icm42600_odr_to_period(st->conf.accel.odr);
 	ts = iio_priv(indio_dev);
-	inv_icm42600_timestamp_init(ts, inv_icm42600_odr_to_period(st->conf.gyro.odr));
+	inv_sensors_timestamp_init(ts, &ts_chip);
 
 	iio_device_set_drvdata(indio_dev, st);
 	indio_dev->name = name;
@@ -757,7 +765,7 @@ struct iio_dev *inv_icm42600_gyro_init(struct inv_icm42600_state *st)
 int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev)
 {
 	struct inv_icm42600_state *st = iio_device_get_drvdata(indio_dev);
-	struct inv_icm42600_timestamp *ts = iio_priv(indio_dev);
+	struct inv_sensors_timestamp *ts = iio_priv(indio_dev);
 	ssize_t i, size;
 	unsigned int no;
 	const void *accel, *gyro, *timestamp;
@@ -780,15 +788,15 @@ int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev)
 
 		/* update odr */
 		if (odr & INV_ICM42600_SENSOR_GYRO)
-			inv_icm42600_timestamp_apply_odr(ts, st->fifo.period,
-							 st->fifo.nb.total, no);
+			inv_sensors_timestamp_apply_odr(ts, st->fifo.period,
+							st->fifo.nb.total, no);
 
 		/* buffer is copied to userspace, zeroing it to avoid any data leak */
 		memset(&buffer, 0, sizeof(buffer));
 		memcpy(&buffer.gyro, gyro, sizeof(buffer.gyro));
 		/* convert 8 bits FIFO temperature in high resolution format */
 		buffer.temp = temp ? (*temp * 64) : 0;
-		ts_val = inv_icm42600_timestamp_pop(ts);
+		ts_val = inv_sensors_timestamp_pop(ts);
 		iio_push_to_buffers_with_timestamp(indio_dev, &buffer, ts_val);
 	}
 
diff --git a/include/linux/iio/common/inv_icm42600_timestamp.h b/include/linux/iio/common/inv_icm42600_timestamp.h
deleted file mode 100644
index 00fd452632a3..000000000000
--- a/include/linux/iio/common/inv_icm42600_timestamp.h
+++ /dev/null
@@ -1,81 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2020 Invensense, Inc.
- */
-
-#ifndef INV_ICM42600_TIMESTAMP_H_
-#define INV_ICM42600_TIMESTAMP_H_
-
-#include <linux/kernel.h>
-
-/**
- * struct inv_icm42600_timestamp_interval - timestamps interval
- * @lo:	interval lower bound
- * @up:	interval upper bound
- */
-struct inv_icm42600_timestamp_interval {
-	int64_t lo;
-	int64_t up;
-};
-
-/**
- * struct inv_icm42600_timestamp_acc - accumulator for computing an estimation
- * @val:	current estimation of the value, the mean of all values
- * @idx:	current index of the next free place in values table
- * @values:	table of all measured values, use for computing the mean
- */
-struct inv_icm42600_timestamp_acc {
-	uint32_t val;
-	size_t idx;
-	uint32_t values[32];
-};
-
-/**
- * struct inv_icm42600_timestamp - timestamp management states
- * @it:			interrupts interval timestamps
- * @timestamp:		store last timestamp for computing next data timestamp
- * @mult:		current internal period multiplier
- * @new_mult:		new set internal period multiplier (not yet effective)
- * @period:		measured current period of the sensor
- * @chip_period:	accumulator for computing internal chip period
- */
-struct inv_icm42600_timestamp {
-	struct inv_icm42600_timestamp_interval it;
-	int64_t timestamp;
-	uint32_t mult;
-	uint32_t new_mult;
-	uint32_t period;
-	struct inv_icm42600_timestamp_acc chip_period;
-};
-
-void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts,
-				 uint32_t period);
-
-int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
-				      uint32_t period, bool fifo);
-
-void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
-				      uint32_t fifo_period, size_t fifo_nb,
-				      size_t sensor_nb, int64_t timestamp);
-
-static inline int64_t
-inv_icm42600_timestamp_pop(struct inv_icm42600_timestamp *ts)
-{
-	ts->timestamp += ts->period;
-	return ts->timestamp;
-}
-
-void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
-				      uint32_t fifo_period, size_t fifo_nb,
-				      unsigned int fifo_no);
-
-static inline void
-inv_icm42600_timestamp_reset(struct inv_icm42600_timestamp *ts)
-{
-	const struct inv_icm42600_timestamp_interval interval_init = {0LL, 0LL};
-
-	ts->it = interval_init;
-	ts->timestamp = 0;
-}
-
-#endif
diff --git a/include/linux/iio/common/inv_sensors_timestamp.h b/include/linux/iio/common/inv_sensors_timestamp.h
new file mode 100644
index 000000000000..23440bc7322c
--- /dev/null
+++ b/include/linux/iio/common/inv_sensors_timestamp.h
@@ -0,0 +1,89 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2020 Invensense, Inc.
+ */
+
+#ifndef INV_SENSORS_TIMESTAMP_H_
+#define INV_SENSORS_TIMESTAMP_H_
+
+#include <linux/kernel.h>
+
+struct inv_sensors_timestamp_chip {
+	uint32_t clock_period;
+	uint32_t jitter;
+	uint32_t init_period;
+};
+
+/**
+ * struct inv_sensors_timestamp_interval - timestamps interval
+ * @lo:	interval lower bound
+ * @up:	interval upper bound
+ */
+struct inv_sensors_timestamp_interval {
+	int64_t lo;
+	int64_t up;
+};
+
+/**
+ * struct inv_sensors_timestamp_acc - accumulator for computing an estimation
+ * @val:	current estimation of the value, the mean of all values
+ * @idx:	current index of the next free place in values table
+ * @values:	table of all measured values, use for computing the mean
+ */
+struct inv_sensors_timestamp_acc {
+	uint32_t val;
+	size_t idx;
+	uint32_t values[32];
+};
+
+/**
+ * struct inv_sensors_timestamp - timestamp management states
+ * @chip:		chip internal characteristics
+ * @it:			interrupts interval timestamps
+ * @timestamp:		store last timestamp for computing next data timestamp
+ * @mult:		current internal period multiplier
+ * @new_mult:		new set internal period multiplier (not yet effective)
+ * @period:		measured current period of the sensor
+ * @chip_period:	accumulator for computing internal chip period
+ */
+struct inv_sensors_timestamp {
+	struct inv_sensors_timestamp_chip chip;
+	uint32_t min_period;
+	uint32_t max_period;
+	struct inv_sensors_timestamp_interval it;
+	int64_t timestamp;
+	uint32_t mult;
+	uint32_t new_mult;
+	uint32_t period;
+	struct inv_sensors_timestamp_acc chip_period;
+};
+
+void inv_sensors_timestamp_init(struct inv_sensors_timestamp *ts,
+				const struct inv_sensors_timestamp_chip *chip);
+
+int inv_sensors_timestamp_update_odr(struct inv_sensors_timestamp *ts,
+				     uint32_t period, bool fifo);
+
+void inv_sensors_timestamp_interrupt(struct inv_sensors_timestamp *ts,
+				     uint32_t fifo_period, size_t fifo_nb,
+				     size_t sensor_nb, int64_t timestamp);
+
+static inline int64_t inv_sensors_timestamp_pop(struct inv_sensors_timestamp *ts)
+{
+	ts->timestamp += ts->period;
+	return ts->timestamp;
+}
+
+void inv_sensors_timestamp_apply_odr(struct inv_sensors_timestamp *ts,
+				     uint32_t fifo_period, size_t fifo_nb,
+				     unsigned int fifo_no);
+
+static inline void inv_sensors_timestamp_reset(struct inv_sensors_timestamp *ts)
+{
+	const struct inv_sensors_timestamp_interval interval_init = {0LL, 0LL};
+
+	ts->it = interval_init;
+	ts->timestamp = 0;
+}
+
+#endif
-- 
2.34.1


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

* [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module
  2023-05-31 14:25 [PATCH 0/4] Factorize timestamp module inv.git-commit
                   ` (2 preceding siblings ...)
  2023-05-31 14:25 ` [PATCH 3/4] iio: make invensense timestamp module generic inv.git-commit
@ 2023-05-31 14:25 ` inv.git-commit
  2023-06-03 11:23   ` andy.shevchenko
  2023-06-04 11:08   ` Jonathan Cameron
  2023-06-03 11:24 ` [PATCH 0/4] Factorize " andy.shevchenko
  4 siblings, 2 replies; 17+ messages in thread
From: inv.git-commit @ 2023-05-31 14:25 UTC (permalink / raw)
  To: jic23, linux-iio; +Cc: lars, Jean-Baptiste Maneyrol

From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

Replace timestamping by the new common inv_sensors timestamp
module.

Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
---
 drivers/iio/imu/inv_mpu6050/Kconfig           |  1 +
 drivers/iio/imu/inv_mpu6050/inv_mpu_core.c    | 26 ++++--
 drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h     |  9 +-
 drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c    | 83 ++-----------------
 drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c |  3 +-
 5 files changed, 33 insertions(+), 89 deletions(-)

diff --git a/drivers/iio/imu/inv_mpu6050/Kconfig b/drivers/iio/imu/inv_mpu6050/Kconfig
index 64dd73dcc4ba..5f62e4fd475d 100644
--- a/drivers/iio/imu/inv_mpu6050/Kconfig
+++ b/drivers/iio/imu/inv_mpu6050/Kconfig
@@ -7,6 +7,7 @@ config INV_MPU6050_IIO
 	tristate
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
+	select IIO_INV_SENSORS_TIMESTAMP
 
 config INV_MPU6050_I2C
 	tristate "Invensense MPU6050 devices (I2C)"
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
index 592a6e60b413..022bc314f4a8 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
@@ -13,6 +13,7 @@
 #include <linux/irq.h>
 #include <linux/interrupt.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/common/inv_sensors_timestamp.h>
 #include <linux/acpi.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
@@ -521,6 +522,7 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
 	int result;
 	u8 d;
 	struct inv_mpu6050_state *st = iio_priv(indio_dev);
+	struct inv_sensors_timestamp_chip timestamp;
 
 	result = inv_mpu6050_set_gyro_fsr(st, st->chip_config.fsr);
 	if (result)
@@ -544,12 +546,12 @@ static int inv_mpu6050_init_config(struct iio_dev *indio_dev)
 	if (result)
 		return result;
 
-	/*
-	 * Internal chip period is 1ms (1kHz).
-	 * Let's use at the beginning the theorical value before measuring
-	 * with interrupt timestamps.
-	 */
-	st->chip_period = NSEC_PER_MSEC;
+	/* clock jitter is +/- 2% */
+	timestamp.clock_period = NSEC_PER_SEC / INV_MPU6050_INTERNAL_FREQ_HZ;
+	timestamp.jitter = 20;
+	timestamp.init_period =
+			NSEC_PER_SEC / INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
+	inv_sensors_timestamp_init(&st->timestamp, &timestamp);
 
 	/* magn chip init, noop if not present in the chip */
 	result = inv_mpu_magn_probe(st);
@@ -936,6 +938,8 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t count)
 {
 	int fifo_rate;
+	u32 fifo_period;
+	bool fifo_on;
 	u8 d;
 	int result;
 	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
@@ -952,12 +956,21 @@ inv_mpu6050_fifo_rate_store(struct device *dev, struct device_attribute *attr,
 	d = INV_MPU6050_FIFO_RATE_TO_DIVIDER(fifo_rate);
 	/* compute back the fifo rate to handle truncation cases */
 	fifo_rate = INV_MPU6050_DIVIDER_TO_FIFO_RATE(d);
+	fifo_period = NSEC_PER_SEC / fifo_rate;
 
 	mutex_lock(&st->lock);
 	if (d == st->chip_config.divider) {
 		result = 0;
 		goto fifo_rate_fail_unlock;
 	}
+
+	fifo_on = st->chip_config.accl_fifo_enable ||
+		  st->chip_config.gyro_fifo_enable ||
+		  st->chip_config.magn_fifo_enable;
+	result = inv_sensors_timestamp_update_odr(&st->timestamp, fifo_period, fifo_on);
+	if (result)
+		goto fifo_rate_fail_unlock;
+
 	result = pm_runtime_resume_and_get(pdev);
 	if (result)
 		goto fifo_rate_fail_unlock;
@@ -1785,3 +1798,4 @@ EXPORT_NS_GPL_DEV_PM_OPS(inv_mpu_pmops, IIO_MPU6050) = {
 MODULE_AUTHOR("Invensense Corporation");
 MODULE_DESCRIPTION("Invensense device MPU6050 driver");
 MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(IIO_INV_SENSORS_TIMESTAMP);
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
index b4ab2c397d0f..1dc2d4e451a2 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h
@@ -11,6 +11,7 @@
 #include <linux/mutex.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
+#include <linux/iio/common/inv_sensors_timestamp.h>
 #include <linux/regmap.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/kfifo_buf.h>
@@ -170,9 +171,7 @@ struct inv_mpu6050_hw {
  *  @map		regmap pointer.
  *  @irq		interrupt number.
  *  @irq_mask		the int_pin_cfg mask to configure interrupt type.
- *  @chip_period:	chip internal period estimation (~1kHz).
- *  @it_timestamp:	timestamp from previous interrupt.
- *  @data_timestamp:	timestamp for next data sample.
+ *  @timestamp:		timestamping module
  *  @vdd_supply:	VDD voltage regulator for the chip.
  *  @vddio_supply	I/O voltage regulator for the chip.
  *  @magn_disabled:     magnetometer disabled for backward compatibility reason.
@@ -196,9 +195,7 @@ struct inv_mpu6050_state {
 	int irq;
 	u8 irq_mask;
 	unsigned skip_samples;
-	s64 chip_period;
-	s64 it_timestamp;
-	s64 data_timestamp;
+	struct inv_sensors_timestamp timestamp;
 	struct regulator *vdd_supply;
 	struct regulator *vddio_supply;
 	bool magn_disabled;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
index 45c37525c2f1..6a6cee0f535b 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
@@ -13,82 +13,9 @@
 #include <linux/interrupt.h>
 #include <linux/poll.h>
 #include <linux/math64.h>
+#include <linux/iio/common/inv_sensors_timestamp.h>
 #include "inv_mpu_iio.h"
 
-/**
- *  inv_mpu6050_update_period() - Update chip internal period estimation
- *
- *  @st:		driver state
- *  @timestamp:		the interrupt timestamp
- *  @nb:		number of data set in the fifo
- *
- *  This function uses interrupt timestamps to estimate the chip period and
- *  to choose the data timestamp to come.
- */
-static void inv_mpu6050_update_period(struct inv_mpu6050_state *st,
-				      s64 timestamp, size_t nb)
-{
-	/* Period boundaries for accepting timestamp */
-	const s64 period_min =
-		(NSEC_PER_MSEC * (100 - INV_MPU6050_TS_PERIOD_JITTER)) / 100;
-	const s64 period_max =
-		(NSEC_PER_MSEC * (100 + INV_MPU6050_TS_PERIOD_JITTER)) / 100;
-	const s32 divider = INV_MPU6050_FREQ_DIVIDER(st);
-	s64 delta, interval;
-	bool use_it_timestamp = false;
-
-	if (st->it_timestamp == 0) {
-		/* not initialized, forced to use it_timestamp */
-		use_it_timestamp = true;
-	} else if (nb == 1) {
-		/*
-		 * Validate the use of it timestamp by checking if interrupt
-		 * has been delayed.
-		 * nb > 1 means interrupt was delayed for more than 1 sample,
-		 * so it's obviously not good.
-		 * Compute the chip period between 2 interrupts for validating.
-		 */
-		delta = div_s64(timestamp - st->it_timestamp, divider);
-		if (delta > period_min && delta < period_max) {
-			/* update chip period and use it timestamp */
-			st->chip_period = (st->chip_period + delta) / 2;
-			use_it_timestamp = true;
-		}
-	}
-
-	if (use_it_timestamp) {
-		/*
-		 * Manage case of multiple samples in the fifo (nb > 1):
-		 * compute timestamp corresponding to the first sample using
-		 * estimated chip period.
-		 */
-		interval = (nb - 1) * st->chip_period * divider;
-		st->data_timestamp = timestamp - interval;
-	}
-
-	/* save it timestamp */
-	st->it_timestamp = timestamp;
-}
-
-/**
- *  inv_mpu6050_get_timestamp() - Return the current data timestamp
- *
- *  @st:		driver state
- *  @return:		current data timestamp
- *
- *  This function returns the current data timestamp and prepares for next one.
- */
-static s64 inv_mpu6050_get_timestamp(struct inv_mpu6050_state *st)
-{
-	s64 ts;
-
-	/* return current data timestamp and increment */
-	ts = st->data_timestamp;
-	st->data_timestamp += st->chip_period * INV_MPU6050_FREQ_DIVIDER(st);
-
-	return ts;
-}
-
 static int inv_reset_fifo(struct iio_dev *indio_dev)
 {
 	int result;
@@ -121,6 +48,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 	size_t bytes_per_datum;
 	int result;
 	u16 fifo_count;
+	u32 fifo_period;
 	s64 timestamp;
 	int int_status;
 	size_t i, nb;
@@ -177,7 +105,10 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 
 	/* compute and process all complete datum */
 	nb = fifo_count / bytes_per_datum;
-	inv_mpu6050_update_period(st, pf->timestamp, nb);
+	/* Each FIFO data contains all sensors, so same number for FIFO and sensor data */
+	fifo_period = NSEC_PER_SEC / INV_MPU6050_DIVIDER_TO_FIFO_RATE(st->chip_config.divider);
+	inv_sensors_timestamp_interrupt(&st->timestamp, fifo_period, nb, nb, pf->timestamp);
+	inv_sensors_timestamp_apply_odr(&st->timestamp, fifo_period, nb, 0);
 	for (i = 0; i < nb; ++i) {
 		result = regmap_noinc_read(st->map, st->reg->fifo_r_w,
 					   st->data, bytes_per_datum);
@@ -188,7 +119,7 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
 			st->skip_samples--;
 			continue;
 		}
-		timestamp = inv_mpu6050_get_timestamp(st);
+		timestamp = inv_sensors_timestamp_pop(&st->timestamp);
 		iio_push_to_buffers_with_timestamp(indio_dev, st->data, timestamp);
 	}
 
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
index 882546897255..c26bb9988d7a 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_trigger.c
@@ -106,7 +106,8 @@ int inv_mpu6050_prepare_fifo(struct inv_mpu6050_state *st, bool enable)
 	int ret;
 
 	if (enable) {
-		st->it_timestamp = 0;
+		/* reset timestamping */
+		inv_sensors_timestamp_reset(&st->timestamp);
 		/* reset FIFO */
 		d = st->chip_config.user_ctrl | INV_MPU6050_BIT_FIFO_RST;
 		ret = regmap_write(st->map, st->reg->user_ctrl, d);
-- 
2.34.1


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

* Re: [PATCH 1/4] iio: imu: inv_icm42600: make timestamp module chip independant
  2023-05-31 14:25 ` [PATCH 1/4] iio: imu: inv_icm42600: make timestamp module chip independant inv.git-commit
@ 2023-06-03 11:10   ` andy.shevchenko
  0 siblings, 0 replies; 17+ messages in thread
From: andy.shevchenko @ 2023-06-03 11:10 UTC (permalink / raw)
  To: inv.git-commit; +Cc: jic23, linux-iio, lars, Jean-Baptiste Maneyrol

Wed, May 31, 2023 at 02:25:10PM +0000, inv.git-commit@tdk.com kirjoitti:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Move icm42600 dependent function inside the core module.

...

>  #include <linux/kernel.h>
> -#include <linux/regmap.h>
>  #include <linux/math64.h>
> +#include <linux/errno.h>

Can this be more ordered?


-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common
  2023-05-31 14:25 ` [PATCH 2/4] iio: move inv_icm42600 timestamp module in common inv.git-commit
@ 2023-06-03 11:13   ` andy.shevchenko
  2023-06-04 10:58   ` Jonathan Cameron
  1 sibling, 0 replies; 17+ messages in thread
From: andy.shevchenko @ 2023-06-03 11:13 UTC (permalink / raw)
  To: inv.git-commit; +Cc: jic23, linux-iio, lars, Jean-Baptiste Maneyrol

Wed, May 31, 2023 at 02:25:11PM +0000, inv.git-commit@tdk.com kirjoitti:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Create new inv_sensors common modules and move inv_icm42600
> timestamp module inside.
> Modify inv_icm42600 driver to use timestamp module.

...

>  #include <linux/kernel.h>
>  #include <linux/math64.h>
> +#include <linux/module.h>
>  #include <linux/errno.h>

> -

Leave this blank line. It helps to divide generic headers and IIO (subsystem)
related.

> -#include "inv_icm42600_timestamp.h"
> +#include <linux/iio/common/inv_icm42600_timestamp.h>

...

>  #include <linux/delay.h>

+ Blank line?

>  #include <linux/iio/iio.h>

Keep it ordered? (But yeah, this one probably out of scope of the series.)

>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>

>  #include "inv_icm42600.h"
> -#include "inv_icm42600_timestamp.h"
>  #include "inv_icm42600_buffer.h"

...

>  #include <linux/property.h>
>  #include <linux/regmap.h>

+ Blank line?

>  #include <linux/iio/iio.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>

Keep it ordered?

>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_buffer.h"
> -#include "inv_icm42600_timestamp.h"

...

>  #include <linux/math64.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  #include <linux/iio/kfifo_buf.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_temp.h"
>  #include "inv_icm42600_buffer.h"
> -#include "inv_icm42600_timestamp.h"

As per above comments.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 3/4] iio: make invensense timestamp module generic
  2023-05-31 14:25 ` [PATCH 3/4] iio: make invensense timestamp module generic inv.git-commit
@ 2023-06-03 11:19   ` andy.shevchenko
  2023-06-04 11:06   ` Jonathan Cameron
  1 sibling, 0 replies; 17+ messages in thread
From: andy.shevchenko @ 2023-06-03 11:19 UTC (permalink / raw)
  To: inv.git-commit; +Cc: jic23, linux-iio, lars, Jean-Baptiste Maneyrol

Wed, May 31, 2023 at 02:25:12PM +0000, inv.git-commit@tdk.com kirjoitti:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Rename common module to inv_sensors_timestamp, add configuration
> at init (chip internal clock, acceptable jitter, ...) and update
> inv_icm42600 driver integration.

...

>  	/* check that period is acceptable */
> -	period_min = INV_ICM42600_TIMESTAMP_MIN_PERIOD(chip_period) * mult;
> -	period_max = INV_ICM42600_TIMESTAMP_MAX_PERIOD(chip_period) * mult;
> +	period_min = ts->min_period * mult;
> +	period_max = ts->max_period * mult;

Side note: wondering if linear_range.h APIs can somehow help with this.

>  	if (period > period_min && period < period_max)
>  		return true;
>  	else
>  		return false;

Another side note: this can be simplified in a follow up at some point.

...

> -	/* if interrupt interval is valid, sync with interrupt timestamp */
> +	/* if interrupt interval is valid, align with interrupt timestamp */

Not sure why this change, but probably it's inline with the rest of the
changes.

...

> --- /dev/null
> +++ b/include/linux/iio/common/inv_sensors_timestamp.h

Have you used -M -C for `git-format-patch`?

...

> +#ifndef INV_SENSORS_TIMESTAMP_H_
> +#define INV_SENSORS_TIMESTAMP_H_

> +#include <linux/kernel.h>

While I see this in the original code, I have found no evidence this header
is used here in any possible way.

Actually kernel.h in the _headers_ is quite discouraged.

> +#endif

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module
  2023-05-31 14:25 ` [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module inv.git-commit
@ 2023-06-03 11:23   ` andy.shevchenko
  2023-06-04 11:08   ` Jonathan Cameron
  1 sibling, 0 replies; 17+ messages in thread
From: andy.shevchenko @ 2023-06-03 11:23 UTC (permalink / raw)
  To: inv.git-commit; +Cc: jic23, linux-iio, lars, Jean-Baptiste Maneyrol

Wed, May 31, 2023 at 02:25:13PM +0000, inv.git-commit@tdk.com kirjoitti:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Replace timestamping by the new common inv_sensors timestamp
> module.

...

>  #include <linux/irq.h>
>  #include <linux/interrupt.h>

>  #include <linux/iio/iio.h>
> +#include <linux/iio/common/inv_sensors_timestamp.h>

While at it, can you split it to a separate group of headers that may be
located after generic linux/* ?

Also would be nice to keep them in order.

>  #include <linux/acpi.h>
>  #include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>

>  #include <linux/mutex.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_sensors_timestamp.h>
>  #include <linux/regmap.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/kfifo_buf.h>

Same.

This actually a mess, but it's another story.

...

>  #include <linux/interrupt.h>
>  #include <linux/poll.h>
>  #include <linux/math64.h>

+ Blank line

> +#include <linux/iio/common/inv_sensors_timestamp.h>

+ Blank line.

>  #include "inv_mpu_iio.h"

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 0/4] Factorize timestamp module
  2023-05-31 14:25 [PATCH 0/4] Factorize timestamp module inv.git-commit
                   ` (3 preceding siblings ...)
  2023-05-31 14:25 ` [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module inv.git-commit
@ 2023-06-03 11:24 ` andy.shevchenko
  4 siblings, 0 replies; 17+ messages in thread
From: andy.shevchenko @ 2023-06-03 11:24 UTC (permalink / raw)
  To: inv.git-commit; +Cc: jic23, linux-iio, lars, Jean-Baptiste Maneyrol

Wed, May 31, 2023 at 02:25:09PM +0000, inv.git-commit@tdk.com kirjoitti:
> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> The purpose if this series is to make timestamping from
> inv_icm42600 driver an independent module and use it for both
> inv_icm42600 and inv_mpu6050 drivers.
> 
> Create a new inv_sensors_timestamp common module based on
> inv_icm42600 driver and use it in the 2 drivers.
> 
> WARNING: this patch requires following commit in fixes-togreg
> bbaae0c79ebd ("iio: imu: inv_icm42600: fix timestamp reset")

This is so nicely prepared series, but I have a few style nit-picks
(individually commented) and one to address, i.e. kernel.h inclusion.

With that addressed,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Thank you!

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common
  2023-05-31 14:25 ` [PATCH 2/4] iio: move inv_icm42600 timestamp module in common inv.git-commit
  2023-06-03 11:13   ` andy.shevchenko
@ 2023-06-04 10:58   ` Jonathan Cameron
  2023-06-05 19:07     ` Jean-Baptiste Maneyrol
  1 sibling, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2023-06-04 10:58 UTC (permalink / raw)
  To: inv.git-commit; +Cc: linux-iio, lars, Jean-Baptiste Maneyrol

On Wed, 31 May 2023 14:25:11 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Create new inv_sensors common modules and move inv_icm42600
> timestamp module inside.
> Modify inv_icm42600 driver to use timestamp module.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
Hi Jean-Baptiste,

Any plan to use this outside of IMU drivers?  If not I'd be tempted
to keep it more local.

drivers/iio/imu/inv_common/ or similar and avoid the global
header by using a "../inv_common/" include path.

Changes themselves look fine to me.

Jonathan

> ---
>  drivers/iio/common/Kconfig                           |  1 +
>  drivers/iio/common/Makefile                          |  1 +
>  drivers/iio/common/inv_sensors/Kconfig               |  7 +++++++
>  drivers/iio/common/inv_sensors/Makefile              |  6 ++++++
>  .../inv_sensors}/inv_icm42600_timestamp.c            | 12 ++++++++++--
>  drivers/iio/imu/inv_icm42600/Kconfig                 |  1 +
>  drivers/iio/imu/inv_icm42600/Makefile                |  1 -
>  drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c    |  2 +-
>  drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c   |  2 +-
>  drivers/iio/imu/inv_icm42600/inv_icm42600_core.c     |  3 ++-
>  drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c     |  2 +-
>  .../linux/iio/common}/inv_icm42600_timestamp.h       |  0
>  12 files changed, 31 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/iio/common/inv_sensors/Kconfig
>  create mode 100644 drivers/iio/common/inv_sensors/Makefile
>  rename drivers/iio/{imu/inv_icm42600 => common/inv_sensors}/inv_icm42600_timestamp.c (91%)
>  rename {drivers/iio/imu/inv_icm42600 => include/linux/iio/common}/inv_icm42600_timestamp.h (100%)
> 
> diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> index 0334b4954773..1ccb5ccf3706 100644
> --- a/drivers/iio/common/Kconfig
> +++ b/drivers/iio/common/Kconfig
> @@ -5,6 +5,7 @@
>  
>  source "drivers/iio/common/cros_ec_sensors/Kconfig"
>  source "drivers/iio/common/hid-sensors/Kconfig"
> +source "drivers/iio/common/inv_sensors/Kconfig"
>  source "drivers/iio/common/ms_sensors/Kconfig"
>  source "drivers/iio/common/scmi_sensors/Kconfig"
>  source "drivers/iio/common/ssp_sensors/Kconfig"
> diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
> index fad40e1e1718..d3e952239a62 100644
> --- a/drivers/iio/common/Makefile
> +++ b/drivers/iio/common/Makefile
> @@ -10,6 +10,7 @@
>  # When adding new entries keep the list in alphabetical order
>  obj-y += cros_ec_sensors/
>  obj-y += hid-sensors/
> +obj-y += inv_sensors/
>  obj-y += ms_sensors/
>  obj-y += scmi_sensors/
>  obj-y += ssp_sensors/
> diff --git a/drivers/iio/common/inv_sensors/Kconfig b/drivers/iio/common/inv_sensors/Kconfig
> new file mode 100644
> index 000000000000..28815fb43157
> --- /dev/null
> +++ b/drivers/iio/common/inv_sensors/Kconfig
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# TDK-InvenSense sensors common library
> +#
> +
> +config IIO_INV_SENSORS_TIMESTAMP
> +	tristate
> diff --git a/drivers/iio/common/inv_sensors/Makefile b/drivers/iio/common/inv_sensors/Makefile
> new file mode 100644
> index 000000000000..93bddb9356b8
> --- /dev/null
> +++ b/drivers/iio/common/inv_sensors/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for TDK-InvenSense sensors module.
> +#
> +
> +obj-$(CONFIG_IIO_INV_SENSORS_TIMESTAMP) += inv_icm42600_timestamp.o
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> similarity index 91%
> rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> rename to drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> index ceae8ccb1747..411f561e1a24 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> +++ b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> @@ -5,9 +5,9 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/math64.h>
> +#include <linux/module.h>
>  #include <linux/errno.h>
> -
> -#include "inv_icm42600_timestamp.h"
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  
>  /* internal chip period is 32kHz, 31250ns */
>  #define INV_ICM42600_TIMESTAMP_PERIOD		31250
> @@ -54,6 +54,7 @@ void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts,
>  	/* use theoretical value for chip period */
>  	inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD);
>  }
> +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_init, IIO_INV_SENSORS_TIMESTAMP);
>  
>  int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
>  				      uint32_t period, bool fifo)
> @@ -66,6 +67,7 @@ int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP);
>  
>  static bool inv_validate_period(uint32_t period, uint32_t mult)
>  {
> @@ -153,6 +155,7 @@ void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
>  	if (valid)
>  		inv_align_timestamp_it(ts);
>  }
> +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_interrupt, IIO_INV_SENSORS_TIMESTAMP);
>  
>  void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
>  				      uint32_t fifo_period, size_t fifo_nb,
> @@ -184,3 +187,8 @@ void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
>  		ts->timestamp = ts->it.up - interval;
>  	}
>  }
> +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_apply_odr, IIO_INV_SENSORS_TIMESTAMP);
> +
> +MODULE_AUTHOR("InvenSense, Inc.");
> +MODULE_DESCRIPTION("InvenSense sensors timestamp module");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/iio/imu/inv_icm42600/Kconfig b/drivers/iio/imu/inv_icm42600/Kconfig
> index 50cbcfcb6cf1..f56b0816cc4d 100644
> --- a/drivers/iio/imu/inv_icm42600/Kconfig
> +++ b/drivers/iio/imu/inv_icm42600/Kconfig
> @@ -3,6 +3,7 @@
>  config INV_ICM42600
>  	tristate
>  	select IIO_BUFFER
> +	select IIO_INV_SENSORS_TIMESTAMP
>  
>  config INV_ICM42600_I2C
>  	tristate "InvenSense ICM-426xx I2C driver"
> diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile
> index 291714d9aa54..0f49f6df3647 100644
> --- a/drivers/iio/imu/inv_icm42600/Makefile
> +++ b/drivers/iio/imu/inv_icm42600/Makefile
> @@ -6,7 +6,6 @@ inv-icm42600-y += inv_icm42600_gyro.o
>  inv-icm42600-y += inv_icm42600_accel.o
>  inv-icm42600-y += inv_icm42600_temp.o
>  inv-icm42600-y += inv_icm42600_buffer.o
> -inv-icm42600-y += inv_icm42600_timestamp.o
>  
>  obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
>  inv-icm42600-i2c-y += inv_icm42600_i2c.o
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> index c3f433ad3af6..1015de636a94 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> @@ -12,12 +12,12 @@
>  #include <linux/math64.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  #include <linux/iio/kfifo_buf.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_temp.h"
>  #include "inv_icm42600_buffer.h"
> -#include "inv_icm42600_timestamp.h"
>  
>  #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info)		\
>  	{								\
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> index 32d7f8364230..4a39d31e911f 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> @@ -11,9 +11,9 @@
>  #include <linux/delay.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  
>  #include "inv_icm42600.h"
> -#include "inv_icm42600_timestamp.h"
>  #include "inv_icm42600_buffer.h"
>  
>  /* FIFO header: 1 byte */
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index c34735b05830..f3e379f9733d 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -16,10 +16,10 @@
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_buffer.h"
> -#include "inv_icm42600_timestamp.h"
>  
>  static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
>  	{
> @@ -799,3 +799,4 @@ EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42600_pm_ops, IIO_ICM42600) = {
>  MODULE_AUTHOR("InvenSense, Inc.");
>  MODULE_DESCRIPTION("InvenSense ICM-426xx device driver");
>  MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(IIO_INV_SENSORS_TIMESTAMP);
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> index 9d94a8518e3c..6caea7b8a344 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> @@ -12,12 +12,12 @@
>  #include <linux/math64.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  #include <linux/iio/kfifo_buf.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_temp.h"
>  #include "inv_icm42600_buffer.h"
> -#include "inv_icm42600_timestamp.h"
>  
>  #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info)		\
>  	{								\
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h b/include/linux/iio/common/inv_icm42600_timestamp.h
> similarity index 100%
> rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
> rename to include/linux/iio/common/inv_icm42600_timestamp.h


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

* Re: [PATCH 3/4] iio: make invensense timestamp module generic
  2023-05-31 14:25 ` [PATCH 3/4] iio: make invensense timestamp module generic inv.git-commit
  2023-06-03 11:19   ` andy.shevchenko
@ 2023-06-04 11:06   ` Jonathan Cameron
  1 sibling, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-06-04 11:06 UTC (permalink / raw)
  To: inv.git-commit; +Cc: linux-iio, lars, Jean-Baptiste Maneyrol

On Wed, 31 May 2023 14:25:12 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Rename common module to inv_sensors_timestamp, add configuration
> at init (chip internal clock, acceptable jitter, ...) and update
> inv_icm42600 driver integration.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
Docs update needed below.



> diff --git a/include/linux/iio/common/inv_sensors_timestamp.h b/include/linux/iio/common/inv_sensors_timestamp.h
> new file mode 100644
> index 000000000000..23440bc7322c
> --- /dev/null
> +++ b/include/linux/iio/common/inv_sensors_timestamp.h

> +
> +/**
> + * struct inv_sensors_timestamp - timestamp management states
> + * @chip:		chip internal characteristics
> + * @it:			interrupts interval timestamps
> + * @timestamp:		store last timestamp for computing next data timestamp
> + * @mult:		current internal period multiplier
> + * @new_mult:		new set internal period multiplier (not yet effective)
> + * @period:		measured current period of the sensor
> + * @chip_period:	accumulator for computing internal chip period

Docs need an update.

> + */
> +struct inv_sensors_timestamp {
> +	struct inv_sensors_timestamp_chip chip;
> +	uint32_t min_period;
> +	uint32_t max_period;
> +	struct inv_sensors_timestamp_interval it;
> +	int64_t timestamp;
> +	uint32_t mult;
> +	uint32_t new_mult;
> +	uint32_t period;
> +	struct inv_sensors_timestamp_acc chip_period;
> +};
> +



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

* Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module
  2023-05-31 14:25 ` [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module inv.git-commit
  2023-06-03 11:23   ` andy.shevchenko
@ 2023-06-04 11:08   ` Jonathan Cameron
  2023-06-05 19:09     ` Jean-Baptiste Maneyrol
  1 sibling, 1 reply; 17+ messages in thread
From: Jonathan Cameron @ 2023-06-04 11:08 UTC (permalink / raw)
  To: inv.git-commit; +Cc: linux-iio, lars, Jean-Baptiste Maneyrol

On Wed, 31 May 2023 14:25:13 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Replace timestamping by the new common inv_sensors timestamp
> module.
Are there functional changes as a result of this, or were the two
algorithms identical?

I don't mind changes, but should call out if there are any when
unifying code like this,

Jonathan

> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

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

* Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common
  2023-06-04 10:58   ` Jonathan Cameron
@ 2023-06-05 19:07     ` Jean-Baptiste Maneyrol
  2023-06-05 19:25       ` Jonathan Cameron
  0 siblings, 1 reply; 17+ messages in thread
From: Jean-Baptiste Maneyrol @ 2023-06-05 19:07 UTC (permalink / raw)
  To: Jonathan Cameron, INV Git Commit
  Cc: linux-iio@vger.kernel.org, lars@metafoo.de

Hello Jonathan,

currently only IMU drivers will be using this timestamp module. But we have other new chips (pressure sensor for example) that could use it in the future.

So I prefer keeping this module more general to avoid being obliged to move it in the future.

Thanks,
JB


From: Jonathan Cameron <jic23@kernel.org>
Sent: Sunday, June 4, 2023 12:58
To: INV Git Commit <INV.git-commit@tdk.com>
Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; lars@metafoo.de <lars@metafoo.de>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
Subject: Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common 
 
[CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe.

======================================================================
On Wed, 31 May 2023 14:25:11 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Create new inv_sensors common modules and move inv_icm42600
> timestamp module inside.
> Modify inv_icm42600 driver to use timestamp module.
> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
Hi Jean-Baptiste,

Any plan to use this outside of IMU drivers?  If not I'd be tempted
to keep it more local.

drivers/iio/imu/inv_common/ or similar and avoid the global
header by using a "../inv_common/" include path.

Changes themselves look fine to me.

Jonathan

> ---
>  drivers/iio/common/Kconfig                           |  1 +
>  drivers/iio/common/Makefile                          |  1 +
>  drivers/iio/common/inv_sensors/Kconfig               |  7 +++++++
>  drivers/iio/common/inv_sensors/Makefile              |  6 ++++++
>  .../inv_sensors}/inv_icm42600_timestamp.c            | 12 ++++++++++--
>  drivers/iio/imu/inv_icm42600/Kconfig                 |  1 +
>  drivers/iio/imu/inv_icm42600/Makefile                |  1 -
>  drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c    |  2 +-
>  drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c   |  2 +-
>  drivers/iio/imu/inv_icm42600/inv_icm42600_core.c     |  3 ++-
>  drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c     |  2 +-
>  .../linux/iio/common}/inv_icm42600_timestamp.h       |  0
>  12 files changed, 31 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/iio/common/inv_sensors/Kconfig
>  create mode 100644 drivers/iio/common/inv_sensors/Makefile
>  rename drivers/iio/{imu/inv_icm42600 => common/inv_sensors}/inv_icm42600_timestamp.c (91%)
>  rename {drivers/iio/imu/inv_icm42600 => include/linux/iio/common}/inv_icm42600_timestamp.h (100%)
> 
> diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> index 0334b4954773..1ccb5ccf3706 100644
> --- a/drivers/iio/common/Kconfig
> +++ b/drivers/iio/common/Kconfig
> @@ -5,6 +5,7 @@
>  
>  source "drivers/iio/common/cros_ec_sensors/Kconfig"
>  source "drivers/iio/common/hid-sensors/Kconfig"
> +source "drivers/iio/common/inv_sensors/Kconfig"
>  source "drivers/iio/common/ms_sensors/Kconfig"
>  source "drivers/iio/common/scmi_sensors/Kconfig"
>  source "drivers/iio/common/ssp_sensors/Kconfig"
> diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
> index fad40e1e1718..d3e952239a62 100644
> --- a/drivers/iio/common/Makefile
> +++ b/drivers/iio/common/Makefile
> @@ -10,6 +10,7 @@
>  # When adding new entries keep the list in alphabetical order
>  obj-y += cros_ec_sensors/
>  obj-y += hid-sensors/
> +obj-y += inv_sensors/
>  obj-y += ms_sensors/
>  obj-y += scmi_sensors/
>  obj-y += ssp_sensors/
> diff --git a/drivers/iio/common/inv_sensors/Kconfig b/drivers/iio/common/inv_sensors/Kconfig
> new file mode 100644
> index 000000000000..28815fb43157
> --- /dev/null
> +++ b/drivers/iio/common/inv_sensors/Kconfig
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# TDK-InvenSense sensors common library
> +#
> +
> +config IIO_INV_SENSORS_TIMESTAMP
> +     tristate
> diff --git a/drivers/iio/common/inv_sensors/Makefile b/drivers/iio/common/inv_sensors/Makefile
> new file mode 100644
> index 000000000000..93bddb9356b8
> --- /dev/null
> +++ b/drivers/iio/common/inv_sensors/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for TDK-InvenSense sensors module.
> +#
> +
> +obj-$(CONFIG_IIO_INV_SENSORS_TIMESTAMP) += inv_icm42600_timestamp.o
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> similarity index 91%
> rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> rename to drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> index ceae8ccb1747..411f561e1a24 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> +++ b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> @@ -5,9 +5,9 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/math64.h>
> +#include <linux/module.h>
>  #include <linux/errno.h>
> -
> -#include "inv_icm42600_timestamp.h"
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  
>  /* internal chip period is 32kHz, 31250ns */
>  #define INV_ICM42600_TIMESTAMP_PERIOD                31250
> @@ -54,6 +54,7 @@ void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts,
>        /* use theoretical value for chip period */
>        inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD);
>  }
> +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_init, IIO_INV_SENSORS_TIMESTAMP);
>  
>  int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
>                                      uint32_t period, bool fifo)
> @@ -66,6 +67,7 @@ int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
>  
>        return 0;
>  }
> +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP);
>  
>  static bool inv_validate_period(uint32_t period, uint32_t mult)
>  {
> @@ -153,6 +155,7 @@ void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
>        if (valid)
>                inv_align_timestamp_it(ts);
>  }
> +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_interrupt, IIO_INV_SENSORS_TIMESTAMP);
>  
>  void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
>                                      uint32_t fifo_period, size_t fifo_nb,
> @@ -184,3 +187,8 @@ void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
>                ts->timestamp = ts->it.up - interval;
>        }
>  }
> +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_apply_odr, IIO_INV_SENSORS_TIMESTAMP);
> +
> +MODULE_AUTHOR("InvenSense, Inc.");
> +MODULE_DESCRIPTION("InvenSense sensors timestamp module");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/iio/imu/inv_icm42600/Kconfig b/drivers/iio/imu/inv_icm42600/Kconfig
> index 50cbcfcb6cf1..f56b0816cc4d 100644
> --- a/drivers/iio/imu/inv_icm42600/Kconfig
> +++ b/drivers/iio/imu/inv_icm42600/Kconfig
> @@ -3,6 +3,7 @@
>  config INV_ICM42600
>        tristate
>        select IIO_BUFFER
> +     select IIO_INV_SENSORS_TIMESTAMP
>  
>  config INV_ICM42600_I2C
>        tristate "InvenSense ICM-426xx I2C driver"
> diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile
> index 291714d9aa54..0f49f6df3647 100644
> --- a/drivers/iio/imu/inv_icm42600/Makefile
> +++ b/drivers/iio/imu/inv_icm42600/Makefile
> @@ -6,7 +6,6 @@ inv-icm42600-y += inv_icm42600_gyro.o
>  inv-icm42600-y += inv_icm42600_accel.o
>  inv-icm42600-y += inv_icm42600_temp.o
>  inv-icm42600-y += inv_icm42600_buffer.o
> -inv-icm42600-y += inv_icm42600_timestamp.o
>  
>  obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
>  inv-icm42600-i2c-y += inv_icm42600_i2c.o
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> index c3f433ad3af6..1015de636a94 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> @@ -12,12 +12,12 @@
>  #include <linux/math64.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  #include <linux/iio/kfifo_buf.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_temp.h"
>  #include "inv_icm42600_buffer.h"
> -#include "inv_icm42600_timestamp.h"
>  
>  #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info)                \
>        {                                                               \
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> index 32d7f8364230..4a39d31e911f 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> @@ -11,9 +11,9 @@
>  #include <linux/delay.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  
>  #include "inv_icm42600.h"
> -#include "inv_icm42600_timestamp.h"
>  #include "inv_icm42600_buffer.h"
>  
>  /* FIFO header: 1 byte */
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index c34735b05830..f3e379f9733d 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -16,10 +16,10 @@
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/iio/iio.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_buffer.h"
> -#include "inv_icm42600_timestamp.h"
>  
>  static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
>        {
> @@ -799,3 +799,4 @@ EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42600_pm_ops, IIO_ICM42600) = {
>  MODULE_AUTHOR("InvenSense, Inc.");
>  MODULE_DESCRIPTION("InvenSense ICM-426xx device driver");
>  MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(IIO_INV_SENSORS_TIMESTAMP);
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> index 9d94a8518e3c..6caea7b8a344 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> @@ -12,12 +12,12 @@
>  #include <linux/math64.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/buffer.h>
> +#include <linux/iio/common/inv_icm42600_timestamp.h>
>  #include <linux/iio/kfifo_buf.h>
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_temp.h"
>  #include "inv_icm42600_buffer.h"
> -#include "inv_icm42600_timestamp.h"
>  
>  #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info)         \
>        {                                                               \
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h b/include/linux/iio/common/inv_icm42600_timestamp.h
> similarity index 100%
> rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
> rename to include/linux/iio/common/inv_icm42600_timestamp.h

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

* Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module
  2023-06-04 11:08   ` Jonathan Cameron
@ 2023-06-05 19:09     ` Jean-Baptiste Maneyrol
  2023-06-05 19:25       ` Jonathan Cameron
  0 siblings, 1 reply; 17+ messages in thread
From: Jean-Baptiste Maneyrol @ 2023-06-05 19:09 UTC (permalink / raw)
  To: Jonathan Cameron, INV Git Commit
  Cc: linux-iio@vger.kernel.org, lars@metafoo.de

Hi Jonathan,

the 2 algorithms are very similar, but the new one in module is better (less jitter, better average value using a moving window, ...).

So switching to the new one will lead to better timestamping, while keeping a very similar approach.

Thanks,
JB


From: Jonathan Cameron <jic23@kernel.org>
Sent: Sunday, June 4, 2023 13:08
To: INV Git Commit <INV.git-commit@tdk.com>
Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; lars@metafoo.de <lars@metafoo.de>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
Subject: Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module 
 
[CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe.

======================================================================
On Wed, 31 May 2023 14:25:13 +0000
inv.git-commit@tdk.com wrote:

> From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> 
> Replace timestamping by the new common inv_sensors timestamp
> module.
Are there functional changes as a result of this, or were the two
algorithms identical?

I don't mind changes, but should call out if there are any when
unifying code like this,

Jonathan

> 
> Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>

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

* Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common
  2023-06-05 19:07     ` Jean-Baptiste Maneyrol
@ 2023-06-05 19:25       ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-06-05 19:25 UTC (permalink / raw)
  To: Jean-Baptiste Maneyrol
  Cc: INV Git Commit, linux-iio@vger.kernel.org, lars@metafoo.de

On Mon, 5 Jun 2023 19:07:38 +0000
Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:

> Hello Jonathan,
> 
> currently only IMU drivers will be using this timestamp module. But we have other new chips (pressure sensor for example) that could use it in the future.
> 
> So I prefer keeping this module more general to avoid being obliged to move it in the future.

Fair enough.  Perhaps add a note on that to the patch description.

Jonathan

> 
> Thanks,
> JB
> 
> 
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, June 4, 2023 12:58
> To: INV Git Commit <INV.git-commit@tdk.com>
> Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; lars@metafoo.de <lars@metafoo.de>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
> Subject: Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common 
>  
> [CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> ======================================================================
> On Wed, 31 May 2023 14:25:11 +0000
> inv.git-commit@tdk.com wrote:
> 
> > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> > 
> > Create new inv_sensors common modules and move inv_icm42600
> > timestamp module inside.
> > Modify inv_icm42600 driver to use timestamp module.
> > 
> > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>  
> Hi Jean-Baptiste,
> 
> Any plan to use this outside of IMU drivers?  If not I'd be tempted
> to keep it more local.
> 
> drivers/iio/imu/inv_common/ or similar and avoid the global
> header by using a "../inv_common/" include path.
> 
> Changes themselves look fine to me.
> 
> Jonathan
> 
> > ---
> >  drivers/iio/common/Kconfig                           |  1 +
> >  drivers/iio/common/Makefile                          |  1 +
> >  drivers/iio/common/inv_sensors/Kconfig               |  7 +++++++
> >  drivers/iio/common/inv_sensors/Makefile              |  6 ++++++
> >  .../inv_sensors}/inv_icm42600_timestamp.c            | 12 ++++++++++--
> >  drivers/iio/imu/inv_icm42600/Kconfig                 |  1 +
> >  drivers/iio/imu/inv_icm42600/Makefile                |  1 -
> >  drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c    |  2 +-
> >  drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c   |  2 +-
> >  drivers/iio/imu/inv_icm42600/inv_icm42600_core.c     |  3 ++-
> >  drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c     |  2 +-
> >  .../linux/iio/common}/inv_icm42600_timestamp.h       |  0
> >  12 files changed, 31 insertions(+), 7 deletions(-)
> >  create mode 100644 drivers/iio/common/inv_sensors/Kconfig
> >  create mode 100644 drivers/iio/common/inv_sensors/Makefile
> >  rename drivers/iio/{imu/inv_icm42600 => common/inv_sensors}/inv_icm42600_timestamp.c (91%)
> >  rename {drivers/iio/imu/inv_icm42600 => include/linux/iio/common}/inv_icm42600_timestamp.h (100%)
> > 
> > diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig
> > index 0334b4954773..1ccb5ccf3706 100644
> > --- a/drivers/iio/common/Kconfig
> > +++ b/drivers/iio/common/Kconfig
> > @@ -5,6 +5,7 @@
> >  
> >  source "drivers/iio/common/cros_ec_sensors/Kconfig"
> >  source "drivers/iio/common/hid-sensors/Kconfig"
> > +source "drivers/iio/common/inv_sensors/Kconfig"
> >  source "drivers/iio/common/ms_sensors/Kconfig"
> >  source "drivers/iio/common/scmi_sensors/Kconfig"
> >  source "drivers/iio/common/ssp_sensors/Kconfig"
> > diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile
> > index fad40e1e1718..d3e952239a62 100644
> > --- a/drivers/iio/common/Makefile
> > +++ b/drivers/iio/common/Makefile
> > @@ -10,6 +10,7 @@
> >  # When adding new entries keep the list in alphabetical order
> >  obj-y += cros_ec_sensors/
> >  obj-y += hid-sensors/
> > +obj-y += inv_sensors/
> >  obj-y += ms_sensors/
> >  obj-y += scmi_sensors/
> >  obj-y += ssp_sensors/
> > diff --git a/drivers/iio/common/inv_sensors/Kconfig b/drivers/iio/common/inv_sensors/Kconfig
> > new file mode 100644
> > index 000000000000..28815fb43157
> > --- /dev/null
> > +++ b/drivers/iio/common/inv_sensors/Kconfig
> > @@ -0,0 +1,7 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# TDK-InvenSense sensors common library
> > +#
> > +
> > +config IIO_INV_SENSORS_TIMESTAMP
> > +     tristate
> > diff --git a/drivers/iio/common/inv_sensors/Makefile b/drivers/iio/common/inv_sensors/Makefile
> > new file mode 100644
> > index 000000000000..93bddb9356b8
> > --- /dev/null
> > +++ b/drivers/iio/common/inv_sensors/Makefile
> > @@ -0,0 +1,6 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Makefile for TDK-InvenSense sensors module.
> > +#
> > +
> > +obj-$(CONFIG_IIO_INV_SENSORS_TIMESTAMP) += inv_icm42600_timestamp.o
> > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> > similarity index 91%
> > rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> > rename to drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> > index ceae8ccb1747..411f561e1a24 100644
> > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c
> > +++ b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c
> > @@ -5,9 +5,9 @@
> >  
> >  #include <linux/kernel.h>
> >  #include <linux/math64.h>
> > +#include <linux/module.h>
> >  #include <linux/errno.h>
> > -
> > -#include "inv_icm42600_timestamp.h"
> > +#include <linux/iio/common/inv_icm42600_timestamp.h>
> >  
> >  /* internal chip period is 32kHz, 31250ns */
> >  #define INV_ICM42600_TIMESTAMP_PERIOD                31250
> > @@ -54,6 +54,7 @@ void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts,
> >        /* use theoretical value for chip period */
> >        inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD);
> >  }
> > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_init, IIO_INV_SENSORS_TIMESTAMP);
> >  
> >  int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
> >                                      uint32_t period, bool fifo)
> > @@ -66,6 +67,7 @@ int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts,
> >  
> >        return 0;
> >  }
> > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP);
> >  
> >  static bool inv_validate_period(uint32_t period, uint32_t mult)
> >  {
> > @@ -153,6 +155,7 @@ void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts,
> >        if (valid)
> >                inv_align_timestamp_it(ts);
> >  }
> > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_interrupt, IIO_INV_SENSORS_TIMESTAMP);
> >  
> >  void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
> >                                      uint32_t fifo_period, size_t fifo_nb,
> > @@ -184,3 +187,8 @@ void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts,
> >                ts->timestamp = ts->it.up - interval;
> >        }
> >  }
> > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_apply_odr, IIO_INV_SENSORS_TIMESTAMP);
> > +
> > +MODULE_AUTHOR("InvenSense, Inc.");
> > +MODULE_DESCRIPTION("InvenSense sensors timestamp module");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/iio/imu/inv_icm42600/Kconfig b/drivers/iio/imu/inv_icm42600/Kconfig
> > index 50cbcfcb6cf1..f56b0816cc4d 100644
> > --- a/drivers/iio/imu/inv_icm42600/Kconfig
> > +++ b/drivers/iio/imu/inv_icm42600/Kconfig
> > @@ -3,6 +3,7 @@
> >  config INV_ICM42600
> >        tristate
> >        select IIO_BUFFER
> > +     select IIO_INV_SENSORS_TIMESTAMP
> >  
> >  config INV_ICM42600_I2C
> >        tristate "InvenSense ICM-426xx I2C driver"
> > diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile
> > index 291714d9aa54..0f49f6df3647 100644
> > --- a/drivers/iio/imu/inv_icm42600/Makefile
> > +++ b/drivers/iio/imu/inv_icm42600/Makefile
> > @@ -6,7 +6,6 @@ inv-icm42600-y += inv_icm42600_gyro.o
> >  inv-icm42600-y += inv_icm42600_accel.o
> >  inv-icm42600-y += inv_icm42600_temp.o
> >  inv-icm42600-y += inv_icm42600_buffer.o
> > -inv-icm42600-y += inv_icm42600_timestamp.o
> >  
> >  obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
> >  inv-icm42600-i2c-y += inv_icm42600_i2c.o
> > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> > index c3f433ad3af6..1015de636a94 100644
> > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> > @@ -12,12 +12,12 @@
> >  #include <linux/math64.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/buffer.h>
> > +#include <linux/iio/common/inv_icm42600_timestamp.h>
> >  #include <linux/iio/kfifo_buf.h>
> >  
> >  #include "inv_icm42600.h"
> >  #include "inv_icm42600_temp.h"
> >  #include "inv_icm42600_buffer.h"
> > -#include "inv_icm42600_timestamp.h"
> >  
> >  #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info)                \
> >        {                                                               \
> > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> > index 32d7f8364230..4a39d31e911f 100644
> > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
> > @@ -11,9 +11,9 @@
> >  #include <linux/delay.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/buffer.h>
> > +#include <linux/iio/common/inv_icm42600_timestamp.h>
> >  
> >  #include "inv_icm42600.h"
> > -#include "inv_icm42600_timestamp.h"
> >  #include "inv_icm42600_buffer.h"
> >  
> >  /* FIFO header: 1 byte */
> > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> > index c34735b05830..f3e379f9733d 100644
> > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> > @@ -16,10 +16,10 @@
> >  #include <linux/property.h>
> >  #include <linux/regmap.h>
> >  #include <linux/iio/iio.h>
> > +#include <linux/iio/common/inv_icm42600_timestamp.h>
> >  
> >  #include "inv_icm42600.h"
> >  #include "inv_icm42600_buffer.h"
> > -#include "inv_icm42600_timestamp.h"
> >  
> >  static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = {
> >        {
> > @@ -799,3 +799,4 @@ EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42600_pm_ops, IIO_ICM42600) = {
> >  MODULE_AUTHOR("InvenSense, Inc.");
> >  MODULE_DESCRIPTION("InvenSense ICM-426xx device driver");
> >  MODULE_LICENSE("GPL");
> > +MODULE_IMPORT_NS(IIO_INV_SENSORS_TIMESTAMP);
> > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> > index 9d94a8518e3c..6caea7b8a344 100644
> > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c
> > @@ -12,12 +12,12 @@
> >  #include <linux/math64.h>
> >  #include <linux/iio/iio.h>
> >  #include <linux/iio/buffer.h>
> > +#include <linux/iio/common/inv_icm42600_timestamp.h>
> >  #include <linux/iio/kfifo_buf.h>
> >  
> >  #include "inv_icm42600.h"
> >  #include "inv_icm42600_temp.h"
> >  #include "inv_icm42600_buffer.h"
> > -#include "inv_icm42600_timestamp.h"
> >  
> >  #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info)         \
> >        {                                                               \
> > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h b/include/linux/iio/common/inv_icm42600_timestamp.h
> > similarity index 100%
> > rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h
> > rename to include/linux/iio/common/inv_icm42600_timestamp.h  


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

* Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module
  2023-06-05 19:09     ` Jean-Baptiste Maneyrol
@ 2023-06-05 19:25       ` Jonathan Cameron
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Cameron @ 2023-06-05 19:25 UTC (permalink / raw)
  To: Jean-Baptiste Maneyrol
  Cc: INV Git Commit, linux-iio@vger.kernel.org, lars@metafoo.de

On Mon, 5 Jun 2023 19:09:51 +0000
Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote:

> Hi Jonathan,
> 
> the 2 algorithms are very similar, but the new one in module is better (less jitter, better average value using a moving window, ...).
> 
> So switching to the new one will lead to better timestamping, while keeping a very similar approach.
That's fine. Add this info to the patch description for next version.

Thanks,

Jonathan

> 
> Thanks,
> JB
> 
> 
> From: Jonathan Cameron <jic23@kernel.org>
> Sent: Sunday, June 4, 2023 13:08
> To: INV Git Commit <INV.git-commit@tdk.com>
> Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; lars@metafoo.de <lars@metafoo.de>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>
> Subject: Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module 
>  
> [CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> ======================================================================
> On Wed, 31 May 2023 14:25:13 +0000
> inv.git-commit@tdk.com wrote:
> 
> > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com>
> > 
> > Replace timestamping by the new common inv_sensors timestamp
> > module.  
> Are there functional changes as a result of this, or were the two
> algorithms identical?
> 
> I don't mind changes, but should call out if there are any when
> unifying code like this,
> 
> Jonathan
> 
> > 
> > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com  


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

end of thread, other threads:[~2023-06-05 19:26 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-31 14:25 [PATCH 0/4] Factorize timestamp module inv.git-commit
2023-05-31 14:25 ` [PATCH 1/4] iio: imu: inv_icm42600: make timestamp module chip independant inv.git-commit
2023-06-03 11:10   ` andy.shevchenko
2023-05-31 14:25 ` [PATCH 2/4] iio: move inv_icm42600 timestamp module in common inv.git-commit
2023-06-03 11:13   ` andy.shevchenko
2023-06-04 10:58   ` Jonathan Cameron
2023-06-05 19:07     ` Jean-Baptiste Maneyrol
2023-06-05 19:25       ` Jonathan Cameron
2023-05-31 14:25 ` [PATCH 3/4] iio: make invensense timestamp module generic inv.git-commit
2023-06-03 11:19   ` andy.shevchenko
2023-06-04 11:06   ` Jonathan Cameron
2023-05-31 14:25 ` [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module inv.git-commit
2023-06-03 11:23   ` andy.shevchenko
2023-06-04 11:08   ` Jonathan Cameron
2023-06-05 19:09     ` Jean-Baptiste Maneyrol
2023-06-05 19:25       ` Jonathan Cameron
2023-06-03 11:24 ` [PATCH 0/4] Factorize " andy.shevchenko

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).