All of lore.kernel.org
 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, lars

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, lars

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, lars

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, lars

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.