* [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
* 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
* [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
* 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 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 2/4] iio: move inv_icm42600 timestamp module in common 2023-06-04 10:58 ` Jonathan Cameron @ 2023-06-05 19:07 ` Jean-Baptiste Maneyrol 2023-06-05 19:25 ` Jonathan Cameron 0 siblings, 1 reply; 17+ messages in thread From: Jean-Baptiste Maneyrol @ 2023-06-05 19:07 UTC (permalink / raw) To: Jonathan Cameron, INV Git Commit Cc: linux-iio@vger.kernel.org, lars@metafoo.de Hello Jonathan, currently only IMU drivers will be using this timestamp module. But we have other new chips (pressure sensor for example) that could use it in the future. So I prefer keeping this module more general to avoid being obliged to move it in the future. Thanks, JB From: Jonathan Cameron <jic23@kernel.org> Sent: Sunday, June 4, 2023 12:58 To: INV Git Commit <INV.git-commit@tdk.com> Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; lars@metafoo.de <lars@metafoo.de>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> Subject: Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common [CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe. ====================================================================== On Wed, 31 May 2023 14:25:11 +0000 inv.git-commit@tdk.com wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> > > Create new inv_sensors common modules and move inv_icm42600 > timestamp module inside. > Modify inv_icm42600 driver to use timestamp module. > > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> Hi Jean-Baptiste, Any plan to use this outside of IMU drivers? If not I'd be tempted to keep it more local. drivers/iio/imu/inv_common/ or similar and avoid the global header by using a "../inv_common/" include path. Changes themselves look fine to me. Jonathan > --- > drivers/iio/common/Kconfig | 1 + > drivers/iio/common/Makefile | 1 + > drivers/iio/common/inv_sensors/Kconfig | 7 +++++++ > drivers/iio/common/inv_sensors/Makefile | 6 ++++++ > .../inv_sensors}/inv_icm42600_timestamp.c | 12 ++++++++++-- > drivers/iio/imu/inv_icm42600/Kconfig | 1 + > drivers/iio/imu/inv_icm42600/Makefile | 1 - > drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 2 +- > drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c | 2 +- > drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 3 ++- > drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 2 +- > .../linux/iio/common}/inv_icm42600_timestamp.h | 0 > 12 files changed, 31 insertions(+), 7 deletions(-) > create mode 100644 drivers/iio/common/inv_sensors/Kconfig > create mode 100644 drivers/iio/common/inv_sensors/Makefile > rename drivers/iio/{imu/inv_icm42600 => common/inv_sensors}/inv_icm42600_timestamp.c (91%) > rename {drivers/iio/imu/inv_icm42600 => include/linux/iio/common}/inv_icm42600_timestamp.h (100%) > > diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig > index 0334b4954773..1ccb5ccf3706 100644 > --- a/drivers/iio/common/Kconfig > +++ b/drivers/iio/common/Kconfig > @@ -5,6 +5,7 @@ > > source "drivers/iio/common/cros_ec_sensors/Kconfig" > source "drivers/iio/common/hid-sensors/Kconfig" > +source "drivers/iio/common/inv_sensors/Kconfig" > source "drivers/iio/common/ms_sensors/Kconfig" > source "drivers/iio/common/scmi_sensors/Kconfig" > source "drivers/iio/common/ssp_sensors/Kconfig" > diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile > index fad40e1e1718..d3e952239a62 100644 > --- a/drivers/iio/common/Makefile > +++ b/drivers/iio/common/Makefile > @@ -10,6 +10,7 @@ > # When adding new entries keep the list in alphabetical order > obj-y += cros_ec_sensors/ > obj-y += hid-sensors/ > +obj-y += inv_sensors/ > obj-y += ms_sensors/ > obj-y += scmi_sensors/ > obj-y += ssp_sensors/ > diff --git a/drivers/iio/common/inv_sensors/Kconfig b/drivers/iio/common/inv_sensors/Kconfig > new file mode 100644 > index 000000000000..28815fb43157 > --- /dev/null > +++ b/drivers/iio/common/inv_sensors/Kconfig > @@ -0,0 +1,7 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +# > +# TDK-InvenSense sensors common library > +# > + > +config IIO_INV_SENSORS_TIMESTAMP > + tristate > diff --git a/drivers/iio/common/inv_sensors/Makefile b/drivers/iio/common/inv_sensors/Makefile > new file mode 100644 > index 000000000000..93bddb9356b8 > --- /dev/null > +++ b/drivers/iio/common/inv_sensors/Makefile > @@ -0,0 +1,6 @@ > +# SPDX-License-Identifier: GPL-2.0 > +# > +# Makefile for TDK-InvenSense sensors module. > +# > + > +obj-$(CONFIG_IIO_INV_SENSORS_TIMESTAMP) += inv_icm42600_timestamp.o > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c > similarity index 91% > rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c > rename to drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c > index ceae8ccb1747..411f561e1a24 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c > +++ b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c > @@ -5,9 +5,9 @@ > > #include <linux/kernel.h> > #include <linux/math64.h> > +#include <linux/module.h> > #include <linux/errno.h> > - > -#include "inv_icm42600_timestamp.h" > +#include <linux/iio/common/inv_icm42600_timestamp.h> > > /* internal chip period is 32kHz, 31250ns */ > #define INV_ICM42600_TIMESTAMP_PERIOD 31250 > @@ -54,6 +54,7 @@ void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts, > /* use theoretical value for chip period */ > inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD); > } > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_init, IIO_INV_SENSORS_TIMESTAMP); > > int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts, > uint32_t period, bool fifo) > @@ -66,6 +67,7 @@ int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts, > > return 0; > } > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP); > > static bool inv_validate_period(uint32_t period, uint32_t mult) > { > @@ -153,6 +155,7 @@ void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts, > if (valid) > inv_align_timestamp_it(ts); > } > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_interrupt, IIO_INV_SENSORS_TIMESTAMP); > > void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts, > uint32_t fifo_period, size_t fifo_nb, > @@ -184,3 +187,8 @@ void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts, > ts->timestamp = ts->it.up - interval; > } > } > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_apply_odr, IIO_INV_SENSORS_TIMESTAMP); > + > +MODULE_AUTHOR("InvenSense, Inc."); > +MODULE_DESCRIPTION("InvenSense sensors timestamp module"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/iio/imu/inv_icm42600/Kconfig b/drivers/iio/imu/inv_icm42600/Kconfig > index 50cbcfcb6cf1..f56b0816cc4d 100644 > --- a/drivers/iio/imu/inv_icm42600/Kconfig > +++ b/drivers/iio/imu/inv_icm42600/Kconfig > @@ -3,6 +3,7 @@ > config INV_ICM42600 > tristate > select IIO_BUFFER > + select IIO_INV_SENSORS_TIMESTAMP > > config INV_ICM42600_I2C > tristate "InvenSense ICM-426xx I2C driver" > diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile > index 291714d9aa54..0f49f6df3647 100644 > --- a/drivers/iio/imu/inv_icm42600/Makefile > +++ b/drivers/iio/imu/inv_icm42600/Makefile > @@ -6,7 +6,6 @@ inv-icm42600-y += inv_icm42600_gyro.o > inv-icm42600-y += inv_icm42600_accel.o > inv-icm42600-y += inv_icm42600_temp.o > inv-icm42600-y += inv_icm42600_buffer.o > -inv-icm42600-y += inv_icm42600_timestamp.o > > obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o > inv-icm42600-i2c-y += inv_icm42600_i2c.o > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > index c3f433ad3af6..1015de636a94 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > @@ -12,12 +12,12 @@ > #include <linux/math64.h> > #include <linux/iio/iio.h> > #include <linux/iio/buffer.h> > +#include <linux/iio/common/inv_icm42600_timestamp.h> > #include <linux/iio/kfifo_buf.h> > > #include "inv_icm42600.h" > #include "inv_icm42600_temp.h" > #include "inv_icm42600_buffer.h" > -#include "inv_icm42600_timestamp.h" > > #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info) \ > { \ > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c > index 32d7f8364230..4a39d31e911f 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c > @@ -11,9 +11,9 @@ > #include <linux/delay.h> > #include <linux/iio/iio.h> > #include <linux/iio/buffer.h> > +#include <linux/iio/common/inv_icm42600_timestamp.h> > > #include "inv_icm42600.h" > -#include "inv_icm42600_timestamp.h" > #include "inv_icm42600_buffer.h" > > /* FIFO header: 1 byte */ > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > index c34735b05830..f3e379f9733d 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > @@ -16,10 +16,10 @@ > #include <linux/property.h> > #include <linux/regmap.h> > #include <linux/iio/iio.h> > +#include <linux/iio/common/inv_icm42600_timestamp.h> > > #include "inv_icm42600.h" > #include "inv_icm42600_buffer.h" > -#include "inv_icm42600_timestamp.h" > > static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = { > { > @@ -799,3 +799,4 @@ EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42600_pm_ops, IIO_ICM42600) = { > MODULE_AUTHOR("InvenSense, Inc."); > MODULE_DESCRIPTION("InvenSense ICM-426xx device driver"); > MODULE_LICENSE("GPL"); > +MODULE_IMPORT_NS(IIO_INV_SENSORS_TIMESTAMP); > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > index 9d94a8518e3c..6caea7b8a344 100644 > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > @@ -12,12 +12,12 @@ > #include <linux/math64.h> > #include <linux/iio/iio.h> > #include <linux/iio/buffer.h> > +#include <linux/iio/common/inv_icm42600_timestamp.h> > #include <linux/iio/kfifo_buf.h> > > #include "inv_icm42600.h" > #include "inv_icm42600_temp.h" > #include "inv_icm42600_buffer.h" > -#include "inv_icm42600_timestamp.h" > > #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info) \ > { \ > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h b/include/linux/iio/common/inv_icm42600_timestamp.h > similarity index 100% > rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h > rename to include/linux/iio/common/inv_icm42600_timestamp.h ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common 2023-06-05 19:07 ` Jean-Baptiste Maneyrol @ 2023-06-05 19:25 ` Jonathan Cameron 0 siblings, 0 replies; 17+ messages in thread From: Jonathan Cameron @ 2023-06-05 19:25 UTC (permalink / raw) To: Jean-Baptiste Maneyrol Cc: INV Git Commit, linux-iio@vger.kernel.org, lars@metafoo.de On Mon, 5 Jun 2023 19:07:38 +0000 Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote: > Hello Jonathan, > > currently only IMU drivers will be using this timestamp module. But we have other new chips (pressure sensor for example) that could use it in the future. > > So I prefer keeping this module more general to avoid being obliged to move it in the future. Fair enough. Perhaps add a note on that to the patch description. Jonathan > > Thanks, > JB > > > From: Jonathan Cameron <jic23@kernel.org> > Sent: Sunday, June 4, 2023 12:58 > To: INV Git Commit <INV.git-commit@tdk.com> > Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; lars@metafoo.de <lars@metafoo.de>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> > Subject: Re: [PATCH 2/4] iio: move inv_icm42600 timestamp module in common > > [CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > ====================================================================== > On Wed, 31 May 2023 14:25:11 +0000 > inv.git-commit@tdk.com wrote: > > > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> > > > > Create new inv_sensors common modules and move inv_icm42600 > > timestamp module inside. > > Modify inv_icm42600 driver to use timestamp module. > > > > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> > Hi Jean-Baptiste, > > Any plan to use this outside of IMU drivers? If not I'd be tempted > to keep it more local. > > drivers/iio/imu/inv_common/ or similar and avoid the global > header by using a "../inv_common/" include path. > > Changes themselves look fine to me. > > Jonathan > > > --- > > drivers/iio/common/Kconfig | 1 + > > drivers/iio/common/Makefile | 1 + > > drivers/iio/common/inv_sensors/Kconfig | 7 +++++++ > > drivers/iio/common/inv_sensors/Makefile | 6 ++++++ > > .../inv_sensors}/inv_icm42600_timestamp.c | 12 ++++++++++-- > > drivers/iio/imu/inv_icm42600/Kconfig | 1 + > > drivers/iio/imu/inv_icm42600/Makefile | 1 - > > drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c | 2 +- > > drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c | 2 +- > > drivers/iio/imu/inv_icm42600/inv_icm42600_core.c | 3 ++- > > drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c | 2 +- > > .../linux/iio/common}/inv_icm42600_timestamp.h | 0 > > 12 files changed, 31 insertions(+), 7 deletions(-) > > create mode 100644 drivers/iio/common/inv_sensors/Kconfig > > create mode 100644 drivers/iio/common/inv_sensors/Makefile > > rename drivers/iio/{imu/inv_icm42600 => common/inv_sensors}/inv_icm42600_timestamp.c (91%) > > rename {drivers/iio/imu/inv_icm42600 => include/linux/iio/common}/inv_icm42600_timestamp.h (100%) > > > > diff --git a/drivers/iio/common/Kconfig b/drivers/iio/common/Kconfig > > index 0334b4954773..1ccb5ccf3706 100644 > > --- a/drivers/iio/common/Kconfig > > +++ b/drivers/iio/common/Kconfig > > @@ -5,6 +5,7 @@ > > > > source "drivers/iio/common/cros_ec_sensors/Kconfig" > > source "drivers/iio/common/hid-sensors/Kconfig" > > +source "drivers/iio/common/inv_sensors/Kconfig" > > source "drivers/iio/common/ms_sensors/Kconfig" > > source "drivers/iio/common/scmi_sensors/Kconfig" > > source "drivers/iio/common/ssp_sensors/Kconfig" > > diff --git a/drivers/iio/common/Makefile b/drivers/iio/common/Makefile > > index fad40e1e1718..d3e952239a62 100644 > > --- a/drivers/iio/common/Makefile > > +++ b/drivers/iio/common/Makefile > > @@ -10,6 +10,7 @@ > > # When adding new entries keep the list in alphabetical order > > obj-y += cros_ec_sensors/ > > obj-y += hid-sensors/ > > +obj-y += inv_sensors/ > > obj-y += ms_sensors/ > > obj-y += scmi_sensors/ > > obj-y += ssp_sensors/ > > diff --git a/drivers/iio/common/inv_sensors/Kconfig b/drivers/iio/common/inv_sensors/Kconfig > > new file mode 100644 > > index 000000000000..28815fb43157 > > --- /dev/null > > +++ b/drivers/iio/common/inv_sensors/Kconfig > > @@ -0,0 +1,7 @@ > > +# SPDX-License-Identifier: GPL-2.0-only > > +# > > +# TDK-InvenSense sensors common library > > +# > > + > > +config IIO_INV_SENSORS_TIMESTAMP > > + tristate > > diff --git a/drivers/iio/common/inv_sensors/Makefile b/drivers/iio/common/inv_sensors/Makefile > > new file mode 100644 > > index 000000000000..93bddb9356b8 > > --- /dev/null > > +++ b/drivers/iio/common/inv_sensors/Makefile > > @@ -0,0 +1,6 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > +# > > +# Makefile for TDK-InvenSense sensors module. > > +# > > + > > +obj-$(CONFIG_IIO_INV_SENSORS_TIMESTAMP) += inv_icm42600_timestamp.o > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c > > similarity index 91% > > rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c > > rename to drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c > > index ceae8ccb1747..411f561e1a24 100644 > > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.c > > +++ b/drivers/iio/common/inv_sensors/inv_icm42600_timestamp.c > > @@ -5,9 +5,9 @@ > > > > #include <linux/kernel.h> > > #include <linux/math64.h> > > +#include <linux/module.h> > > #include <linux/errno.h> > > - > > -#include "inv_icm42600_timestamp.h" > > +#include <linux/iio/common/inv_icm42600_timestamp.h> > > > > /* internal chip period is 32kHz, 31250ns */ > > #define INV_ICM42600_TIMESTAMP_PERIOD 31250 > > @@ -54,6 +54,7 @@ void inv_icm42600_timestamp_init(struct inv_icm42600_timestamp *ts, > > /* use theoretical value for chip period */ > > inv_update_acc(&ts->chip_period, INV_ICM42600_TIMESTAMP_PERIOD); > > } > > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_init, IIO_INV_SENSORS_TIMESTAMP); > > > > int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts, > > uint32_t period, bool fifo) > > @@ -66,6 +67,7 @@ int inv_icm42600_timestamp_update_odr(struct inv_icm42600_timestamp *ts, > > > > return 0; > > } > > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_update_odr, IIO_INV_SENSORS_TIMESTAMP); > > > > static bool inv_validate_period(uint32_t period, uint32_t mult) > > { > > @@ -153,6 +155,7 @@ void inv_icm42600_timestamp_interrupt(struct inv_icm42600_timestamp *ts, > > if (valid) > > inv_align_timestamp_it(ts); > > } > > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_interrupt, IIO_INV_SENSORS_TIMESTAMP); > > > > void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts, > > uint32_t fifo_period, size_t fifo_nb, > > @@ -184,3 +187,8 @@ void inv_icm42600_timestamp_apply_odr(struct inv_icm42600_timestamp *ts, > > ts->timestamp = ts->it.up - interval; > > } > > } > > +EXPORT_SYMBOL_NS_GPL(inv_icm42600_timestamp_apply_odr, IIO_INV_SENSORS_TIMESTAMP); > > + > > +MODULE_AUTHOR("InvenSense, Inc."); > > +MODULE_DESCRIPTION("InvenSense sensors timestamp module"); > > +MODULE_LICENSE("GPL"); > > diff --git a/drivers/iio/imu/inv_icm42600/Kconfig b/drivers/iio/imu/inv_icm42600/Kconfig > > index 50cbcfcb6cf1..f56b0816cc4d 100644 > > --- a/drivers/iio/imu/inv_icm42600/Kconfig > > +++ b/drivers/iio/imu/inv_icm42600/Kconfig > > @@ -3,6 +3,7 @@ > > config INV_ICM42600 > > tristate > > select IIO_BUFFER > > + select IIO_INV_SENSORS_TIMESTAMP > > > > config INV_ICM42600_I2C > > tristate "InvenSense ICM-426xx I2C driver" > > diff --git a/drivers/iio/imu/inv_icm42600/Makefile b/drivers/iio/imu/inv_icm42600/Makefile > > index 291714d9aa54..0f49f6df3647 100644 > > --- a/drivers/iio/imu/inv_icm42600/Makefile > > +++ b/drivers/iio/imu/inv_icm42600/Makefile > > @@ -6,7 +6,6 @@ inv-icm42600-y += inv_icm42600_gyro.o > > inv-icm42600-y += inv_icm42600_accel.o > > inv-icm42600-y += inv_icm42600_temp.o > > inv-icm42600-y += inv_icm42600_buffer.o > > -inv-icm42600-y += inv_icm42600_timestamp.o > > > > obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o > > inv-icm42600-i2c-y += inv_icm42600_i2c.o > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > > index c3f433ad3af6..1015de636a94 100644 > > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c > > @@ -12,12 +12,12 @@ > > #include <linux/math64.h> > > #include <linux/iio/iio.h> > > #include <linux/iio/buffer.h> > > +#include <linux/iio/common/inv_icm42600_timestamp.h> > > #include <linux/iio/kfifo_buf.h> > > > > #include "inv_icm42600.h" > > #include "inv_icm42600_temp.h" > > #include "inv_icm42600_buffer.h" > > -#include "inv_icm42600_timestamp.h" > > > > #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info) \ > > { \ > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c > > index 32d7f8364230..4a39d31e911f 100644 > > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c > > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c > > @@ -11,9 +11,9 @@ > > #include <linux/delay.h> > > #include <linux/iio/iio.h> > > #include <linux/iio/buffer.h> > > +#include <linux/iio/common/inv_icm42600_timestamp.h> > > > > #include "inv_icm42600.h" > > -#include "inv_icm42600_timestamp.h" > > #include "inv_icm42600_buffer.h" > > > > /* FIFO header: 1 byte */ > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > > index c34735b05830..f3e379f9733d 100644 > > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c > > @@ -16,10 +16,10 @@ > > #include <linux/property.h> > > #include <linux/regmap.h> > > #include <linux/iio/iio.h> > > +#include <linux/iio/common/inv_icm42600_timestamp.h> > > > > #include "inv_icm42600.h" > > #include "inv_icm42600_buffer.h" > > -#include "inv_icm42600_timestamp.h" > > > > static const struct regmap_range_cfg inv_icm42600_regmap_ranges[] = { > > { > > @@ -799,3 +799,4 @@ EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42600_pm_ops, IIO_ICM42600) = { > > MODULE_AUTHOR("InvenSense, Inc."); > > MODULE_DESCRIPTION("InvenSense ICM-426xx device driver"); > > MODULE_LICENSE("GPL"); > > +MODULE_IMPORT_NS(IIO_INV_SENSORS_TIMESTAMP); > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > > index 9d94a8518e3c..6caea7b8a344 100644 > > --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > > +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c > > @@ -12,12 +12,12 @@ > > #include <linux/math64.h> > > #include <linux/iio/iio.h> > > #include <linux/iio/buffer.h> > > +#include <linux/iio/common/inv_icm42600_timestamp.h> > > #include <linux/iio/kfifo_buf.h> > > > > #include "inv_icm42600.h" > > #include "inv_icm42600_temp.h" > > #include "inv_icm42600_buffer.h" > > -#include "inv_icm42600_timestamp.h" > > > > #define INV_ICM42600_GYRO_CHAN(_modifier, _index, _ext_info) \ > > { \ > > diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h b/include/linux/iio/common/inv_icm42600_timestamp.h > > similarity index 100% > > rename from drivers/iio/imu/inv_icm42600/inv_icm42600_timestamp.h > > rename to include/linux/iio/common/inv_icm42600_timestamp.h ^ permalink raw reply [flat|nested] 17+ messages in thread
* [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
* 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 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
* [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, ×tamp); /* 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 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 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 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module 2023-06-04 11:08 ` Jonathan Cameron @ 2023-06-05 19:09 ` Jean-Baptiste Maneyrol 2023-06-05 19:25 ` Jonathan Cameron 0 siblings, 1 reply; 17+ messages in thread From: Jean-Baptiste Maneyrol @ 2023-06-05 19:09 UTC (permalink / raw) To: Jonathan Cameron, INV Git Commit Cc: linux-iio@vger.kernel.org, lars@metafoo.de Hi Jonathan, the 2 algorithms are very similar, but the new one in module is better (less jitter, better average value using a moving window, ...). So switching to the new one will lead to better timestamping, while keeping a very similar approach. Thanks, JB From: Jonathan Cameron <jic23@kernel.org> Sent: Sunday, June 4, 2023 13:08 To: INV Git Commit <INV.git-commit@tdk.com> Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; lars@metafoo.de <lars@metafoo.de>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> Subject: Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module [CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe. ====================================================================== On Wed, 31 May 2023 14:25:13 +0000 inv.git-commit@tdk.com wrote: > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> > > Replace timestamping by the new common inv_sensors timestamp > module. Are there functional changes as a result of this, or were the two algorithms identical? I don't mind changes, but should call out if there are any when unifying code like this, Jonathan > > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module 2023-06-05 19:09 ` Jean-Baptiste Maneyrol @ 2023-06-05 19:25 ` Jonathan Cameron 0 siblings, 0 replies; 17+ messages in thread From: Jonathan Cameron @ 2023-06-05 19:25 UTC (permalink / raw) To: Jean-Baptiste Maneyrol Cc: INV Git Commit, linux-iio@vger.kernel.org, lars@metafoo.de On Mon, 5 Jun 2023 19:09:51 +0000 Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> wrote: > Hi Jonathan, > > the 2 algorithms are very similar, but the new one in module is better (less jitter, better average value using a moving window, ...). > > So switching to the new one will lead to better timestamping, while keeping a very similar approach. That's fine. Add this info to the patch description for next version. Thanks, Jonathan > > Thanks, > JB > > > From: Jonathan Cameron <jic23@kernel.org> > Sent: Sunday, June 4, 2023 13:08 > To: INV Git Commit <INV.git-commit@tdk.com> > Cc: linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>; lars@metafoo.de <lars@metafoo.de>; Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com> > Subject: Re: [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module > > [CAUTION] This is an EXTERNAL email. Do not click links or open attachments unless you recognize the sender and know the content is safe. > > ====================================================================== > On Wed, 31 May 2023 14:25:13 +0000 > inv.git-commit@tdk.com wrote: > > > From: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com> > > > > Replace timestamping by the new common inv_sensors timestamp > > module. > Are there functional changes as a result of this, or were the two > algorithms identical? > > I don't mind changes, but should call out if there are any when > unifying code like this, > > Jonathan > > > > > Signed-off-by: Jean-Baptiste Maneyrol <jean-baptiste.maneyrol@tdk.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* 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
end of thread, other threads:[~2023-06-05 19:26 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-05-31 14:25 [PATCH 0/4] Factorize timestamp module inv.git-commit 2023-05-31 14:25 ` [PATCH 1/4] iio: imu: inv_icm42600: make timestamp module chip independant inv.git-commit 2023-06-03 11:10 ` andy.shevchenko 2023-05-31 14:25 ` [PATCH 2/4] iio: move inv_icm42600 timestamp module in common inv.git-commit 2023-06-03 11:13 ` andy.shevchenko 2023-06-04 10:58 ` Jonathan Cameron 2023-06-05 19:07 ` Jean-Baptiste Maneyrol 2023-06-05 19:25 ` Jonathan Cameron 2023-05-31 14:25 ` [PATCH 3/4] iio: make invensense timestamp module generic inv.git-commit 2023-06-03 11:19 ` andy.shevchenko 2023-06-04 11:06 ` Jonathan Cameron 2023-05-31 14:25 ` [PATCH 4/4] iio: imu: inv_mpu6050: use the common inv_sensors timestamp module inv.git-commit 2023-06-03 11:23 ` andy.shevchenko 2023-06-04 11:08 ` Jonathan Cameron 2023-06-05 19:09 ` Jean-Baptiste Maneyrol 2023-06-05 19:25 ` Jonathan Cameron 2023-06-03 11:24 ` [PATCH 0/4] Factorize " andy.shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).