* [PATCH] iio: imu: inv_icm42600: refactor accel/gyro configuration paths
@ 2026-05-03 3:12 João Fernandes
2026-05-03 9:13 ` Joshua Crofts
2026-05-04 16:17 ` Jonathan Cameron
0 siblings, 2 replies; 4+ messages in thread
From: João Fernandes @ 2026-05-03 3:12 UTC (permalink / raw)
To: jean-baptiste.maneyrol, jic23, dlechner, nuno.sa, andy
Cc: gcotavio, linux-iio
Refactor accel and gyro configuration paths to remove duplicated
logic and improve code structure.
Introduce a generic helper, inv_icm42600_set_sensor_conf(), to handle
common register programming and state updates.
Keep sensor-specific handling in inv_icm42600_set_accel_conf() and
inv_icm42600_set_gyro_conf() wrappers. Also extract configuration
sanitization into a dedicated helper inv_icm42600_sanitize_conf()
to further reduce repetition.
Signed-off-by: João Fernandes <joaovictor.fernandes@usp.br>
Co-developed-by: Otavio Capobianco <gcotavio@usp.br>
Signed-off-by: Otavio Capobianco <gcotavio@usp.br>
---
.../iio/imu/inv_icm42600/inv_icm42600_core.c | 125 +++++++++---------
1 file changed, 63 insertions(+), 62 deletions(-)
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
index 76eb22488..e326e0aae 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -289,15 +289,51 @@ static int inv_icm42600_set_pwr_mgmt0(struct inv_icm42600_state *st,
return 0;
}
-int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
- struct inv_icm42600_sensor_conf *conf,
- unsigned int *sleep_ms)
+static int inv_icm42600_set_sensor_conf(struct inv_icm42600_state *st,
+ struct inv_icm42600_sensor_conf *target_conf,
+ struct inv_icm42600_sensor_conf *accel_conf,
+ struct inv_icm42600_sensor_conf *gyro_conf,
+ struct inv_icm42600_sensor_conf *oldconf,
+ unsigned int reg_config0,
+ unsigned int target_config0_val,
+ unsigned int *sleep_ms)
{
- struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
unsigned int val;
int ret;
- /* Sanitize missing values with current values */
+ /* set target sensor's CONFIG0 register */
+ if (target_conf->fs != oldconf->fs || target_conf->odr != oldconf->odr) {
+ ret = regmap_write(st->map, reg_config0, target_config0_val);
+ if (ret)
+ return ret;
+
+ oldconf->fs = target_conf->fs;
+ oldconf->odr = target_conf->odr;
+ }
+
+ /* set GYRO_ACCEL_CONFIG0 register */
+ if (target_conf->filter != oldconf->filter) {
+ val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(accel_conf->filter) |
+ INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(gyro_conf->filter);
+
+ ret = regmap_write(st->map,
+ INV_ICM42600_REG_GYRO_ACCEL_CONFIG0,
+ val);
+ if (ret)
+ return ret;
+
+ oldconf->filter = target_conf->filter;
+ }
+
+ /* set PWR_MGMT0 register */
+ return inv_icm42600_set_pwr_mgmt0(st, gyro_conf->mode, accel_conf->mode,
+ st->conf.temp_en, sleep_ms);
+}
+
+static void inv_icm42600_sanitize_conf(
+ struct inv_icm42600_sensor_conf *conf,
+ const struct inv_icm42600_sensor_conf *oldconf)
+{
if (conf->mode < 0)
conf->mode = oldconf->mode;
if (conf->fs < 0)
@@ -306,6 +342,17 @@ int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
conf->odr = oldconf->odr;
if (conf->filter < 0)
conf->filter = oldconf->filter;
+}
+
+int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
+ struct inv_icm42600_sensor_conf *conf,
+ unsigned int *sleep_ms)
+{
+ struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
+ unsigned int config0_val;
+
+ /* Sanitize missing values with current values */
+ inv_icm42600_sanitize_conf(conf, oldconf);
/* force power mode against ODR when sensor is on */
switch (conf->mode) {
@@ -324,30 +371,12 @@ int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
break;
}
- /* set ACCEL_CONFIG0 register (accel fullscale & odr) */
- if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
- val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
+ config0_val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
INV_ICM42600_ACCEL_CONFIG0_ODR(conf->odr);
- ret = regmap_write(st->map, INV_ICM42600_REG_ACCEL_CONFIG0, val);
- if (ret)
- return ret;
- oldconf->fs = conf->fs;
- oldconf->odr = conf->odr;
- }
-
- /* set GYRO_ACCEL_CONFIG0 register (accel filter) */
- if (conf->filter != oldconf->filter) {
- val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(conf->filter) |
- INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(st->conf.gyro.filter);
- ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
- if (ret)
- return ret;
- oldconf->filter = conf->filter;
- }
- /* set PWR_MGMT0 register (accel sensor mode) */
- return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode,
- st->conf.temp_en, sleep_ms);
+ return inv_icm42600_set_sensor_conf(st, conf, conf, &st->conf.gyro, oldconf,
+ INV_ICM42600_REG_ACCEL_CONFIG0,
+ config0_val, sleep_ms);
}
int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
@@ -355,45 +384,17 @@ int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
unsigned int *sleep_ms)
{
struct inv_icm42600_sensor_conf *oldconf = &st->conf.gyro;
- unsigned int val;
- int ret;
+ unsigned int config0_val;
- /* sanitize missing values with current values */
- if (conf->mode < 0)
- conf->mode = oldconf->mode;
- if (conf->fs < 0)
- conf->fs = oldconf->fs;
- if (conf->odr < 0)
- conf->odr = oldconf->odr;
- if (conf->filter < 0)
- conf->filter = oldconf->filter;
+ /* Sanitize missing values with current values */
+ inv_icm42600_sanitize_conf(conf, oldconf);
- /* set GYRO_CONFIG0 register (gyro fullscale & odr) */
- if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
- val = INV_ICM42600_GYRO_CONFIG0_FS(conf->fs) |
+ config0_val = INV_ICM42600_GYRO_CONFIG0_FS(conf->fs) |
INV_ICM42600_GYRO_CONFIG0_ODR(conf->odr);
- ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_CONFIG0, val);
- if (ret)
- return ret;
- oldconf->fs = conf->fs;
- oldconf->odr = conf->odr;
- }
- /* set GYRO_ACCEL_CONFIG0 register (gyro filter) */
- if (conf->filter != oldconf->filter) {
- val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(st->conf.accel.filter) |
- INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(conf->filter);
- ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
- if (ret)
- return ret;
- oldconf->filter = conf->filter;
- }
-
- /* set PWR_MGMT0 register (gyro sensor mode) */
- return inv_icm42600_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
- st->conf.temp_en, sleep_ms);
-
- return 0;
+ return inv_icm42600_set_sensor_conf(st, conf, &st->conf.accel, conf, oldconf,
+ INV_ICM42600_REG_GYRO_CONFIG0,
+ config0_val, sleep_ms);
}
int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: imu: inv_icm42600: refactor accel/gyro configuration paths
2026-05-03 3:12 [PATCH] iio: imu: inv_icm42600: refactor accel/gyro configuration paths João Fernandes
@ 2026-05-03 9:13 ` Joshua Crofts
2026-05-04 14:17 ` Andy Shevchenko
2026-05-04 16:17 ` Jonathan Cameron
1 sibling, 1 reply; 4+ messages in thread
From: Joshua Crofts @ 2026-05-03 9:13 UTC (permalink / raw)
To: João Fernandes
Cc: jean-baptiste.maneyrol, jic23, dlechner, nuno.sa, andy, gcotavio,
linux-iio
On Sun, 3 May 2026 at 05:17, João Fernandes <joaovictor.fernandes@usp.br> wrote:
>
> Refactor accel and gyro configuration paths to remove duplicated
> logic and improve code structure.
> Introduce a generic helper, inv_icm42600_set_sensor_conf(), to handle
> common register programming and state updates.
>
> Keep sensor-specific handling in inv_icm42600_set_accel_conf() and
> inv_icm42600_set_gyro_conf() wrappers. Also extract configuration
> sanitization into a dedicated helper inv_icm42600_sanitize_conf()
> to further reduce repetition.
This should be (at least) a four-part patch series, please don't send multiple
logical changes as one patch. Patches should be atomic and only deal with
one change/fix. Try to introduce preparatory patches when sending, it speeds
up the reviewing process.
> -int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> - struct inv_icm42600_sensor_conf *conf,
> - unsigned int *sleep_ms)
> +static int inv_icm42600_set_sensor_conf(struct inv_icm42600_state *st,
> + struct inv_icm42600_sensor_conf *target_conf,
> + struct inv_icm42600_sensor_conf *accel_conf,
> + struct inv_icm42600_sensor_conf *gyro_conf,
> + struct inv_icm42600_sensor_conf *oldconf,
> + unsigned int reg_config0,
> + unsigned int target_config0_val,
> + unsigned int *sleep_ms)
Incorrect indentation, try to align the parameters within the parentheses
below each other.
> {
> - struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
> unsigned int val;
> int ret;
>
> - /* Sanitize missing values with current values */
> + /* set target sensor's CONFIG0 register */
> + if (target_conf->fs != oldconf->fs || target_conf->odr != oldconf->odr) {
> + ret = regmap_write(st->map, reg_config0, target_config0_val);
> + if (ret)
> + return ret;
> +
> + oldconf->fs = target_conf->fs;
> + oldconf->odr = target_conf->odr;
> + }
> +
> + /* set GYRO_ACCEL_CONFIG0 register */
> + if (target_conf->filter != oldconf->filter) {
> + val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(accel_conf->filter) |
> + INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(gyro_conf->filter);
> +
> + ret = regmap_write(st->map,
> + INV_ICM42600_REG_GYRO_ACCEL_CONFIG0,
> + val);
> + if (ret)
> + return ret;
> +
> + oldconf->filter = target_conf->filter;
> + }
> +
> + /* set PWR_MGMT0 register */
> + return inv_icm42600_set_pwr_mgmt0(st, gyro_conf->mode, accel_conf->mode,
> + st->conf.temp_en, sleep_ms);
> +}
> +
> +static void inv_icm42600_sanitize_conf(
> + struct inv_icm42600_sensor_conf *conf,
> + const struct inv_icm42600_sensor_conf *oldconf)
> +{
> if (conf->mode < 0)
> conf->mode = oldconf->mode;
> if (conf->fs < 0)
> @@ -306,6 +342,17 @@ int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> conf->odr = oldconf->odr;
> if (conf->filter < 0)
> conf->filter = oldconf->filter;
> +}
> +
> +int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> + struct inv_icm42600_sensor_conf *conf,
> + unsigned int *sleep_ms)
> +{
> + struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
> + unsigned int config0_val;
> +
> + /* Sanitize missing values with current values */
> + inv_icm42600_sanitize_conf(conf, oldconf);
>
> /* force power mode against ODR when sensor is on */
> switch (conf->mode) {
> @@ -324,30 +371,12 @@ int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> break;
> }
>
> - /* set ACCEL_CONFIG0 register (accel fullscale & odr) */
> - if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> - val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
> + config0_val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
> INV_ICM42600_ACCEL_CONFIG0_ODR(conf->odr);
> - ret = regmap_write(st->map, INV_ICM42600_REG_ACCEL_CONFIG0, val);
> - if (ret)
> - return ret;
> - oldconf->fs = conf->fs;
> - oldconf->odr = conf->odr;
> - }
> -
> - /* set GYRO_ACCEL_CONFIG0 register (accel filter) */
> - if (conf->filter != oldconf->filter) {
> - val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(conf->filter) |
> - INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(st->conf.gyro.filter);
> - ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
> - if (ret)
> - return ret;
> - oldconf->filter = conf->filter;
> - }
>
> - /* set PWR_MGMT0 register (accel sensor mode) */
> - return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode,
> - st->conf.temp_en, sleep_ms);
> + return inv_icm42600_set_sensor_conf(st, conf, conf, &st->conf.gyro, oldconf,
> + INV_ICM42600_REG_ACCEL_CONFIG0,
> + config0_val, sleep_ms);
> }
>
> int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
> @@ -355,45 +384,17 @@ int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
> unsigned int *sleep_ms)
> {
> struct inv_icm42600_sensor_conf *oldconf = &st->conf.gyro;
> - unsigned int val;
> - int ret;
> + unsigned int config0_val;
>
> - /* sanitize missing values with current values */
> - if (conf->mode < 0)
> - conf->mode = oldconf->mode;
> - if (conf->fs < 0)
> - conf->fs = oldconf->fs;
> - if (conf->odr < 0)
> - conf->odr = oldconf->odr;
> - if (conf->filter < 0)
> - conf->filter = oldconf->filter;
> + /* Sanitize missing values with current values */
> + inv_icm42600_sanitize_conf(conf, oldconf);
>
> - /* set GYRO_CONFIG0 register (gyro fullscale & odr) */
> - if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> - val = INV_ICM42600_GYRO_CONFIG0_FS(conf->fs) |
> + config0_val = INV_ICM42600_GYRO_CONFIG0_FS(conf->fs) |
> INV_ICM42600_GYRO_CONFIG0_ODR(conf->odr);
> - ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_CONFIG0, val);
> - if (ret)
> - return ret;
> - oldconf->fs = conf->fs;
> - oldconf->odr = conf->odr;
> - }
>
> - /* set GYRO_ACCEL_CONFIG0 register (gyro filter) */
> - if (conf->filter != oldconf->filter) {
> - val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(st->conf.accel.filter) |
> - INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(conf->filter);
> - ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
> - if (ret)
> - return ret;
> - oldconf->filter = conf->filter;
> - }
> -
> - /* set PWR_MGMT0 register (gyro sensor mode) */
> - return inv_icm42600_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
> - st->conf.temp_en, sleep_ms);
> -
> - return 0;
> + return inv_icm42600_set_sensor_conf(st, conf, &st->conf.accel, conf, oldconf,
> + INV_ICM42600_REG_GYRO_CONFIG0,
> + config0_val, sleep_ms);
> }
>
> int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
> --
> 2.43.0
>
--
Kind regards
CJD
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: imu: inv_icm42600: refactor accel/gyro configuration paths
2026-05-03 9:13 ` Joshua Crofts
@ 2026-05-04 14:17 ` Andy Shevchenko
0 siblings, 0 replies; 4+ messages in thread
From: Andy Shevchenko @ 2026-05-04 14:17 UTC (permalink / raw)
To: Joshua Crofts
Cc: João Fernandes, jean-baptiste.maneyrol, jic23, dlechner,
nuno.sa, andy, gcotavio, linux-iio
On Sun, May 03, 2026 at 11:13:39AM +0200, Joshua Crofts wrote:
> On Sun, 3 May 2026 at 05:17, João Fernandes <joaovictor.fernandes@usp.br> wrote:
> >
> > Refactor accel and gyro configuration paths to remove duplicated
> > logic and improve code structure.
> > Introduce a generic helper, inv_icm42600_set_sensor_conf(), to handle
> > common register programming and state updates.
> >
> > Keep sensor-specific handling in inv_icm42600_set_accel_conf() and
> > inv_icm42600_set_gyro_conf() wrappers. Also extract configuration
> > sanitization into a dedicated helper inv_icm42600_sanitize_conf()
> > to further reduce repetition.
>
> This should be (at least) a four-part patch series, please don't send multiple
> logical changes as one patch. Patches should be atomic and only deal with
> one change/fix. Try to introduce preparatory patches when sending, it speeds
> up the reviewing process.
Can you be more specific? It will help the author to split it correctly in less
amount of attempts.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] iio: imu: inv_icm42600: refactor accel/gyro configuration paths
2026-05-03 3:12 [PATCH] iio: imu: inv_icm42600: refactor accel/gyro configuration paths João Fernandes
2026-05-03 9:13 ` Joshua Crofts
@ 2026-05-04 16:17 ` Jonathan Cameron
1 sibling, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2026-05-04 16:17 UTC (permalink / raw)
To: João Fernandes
Cc: jean-baptiste.maneyrol, dlechner, nuno.sa, andy, gcotavio,
linux-iio
On Sun, 3 May 2026 00:12:36 -0300
João Fernandes <joaovictor.fernandes@usp.br> wrote:
> Refactor accel and gyro configuration paths to remove duplicated
> logic and improve code structure.
> Introduce a generic helper, inv_icm42600_set_sensor_conf(), to handle
> common register programming and state updates.
>
> Keep sensor-specific handling in inv_icm42600_set_accel_conf() and
> inv_icm42600_set_gyro_conf() wrappers. Also extract configuration
> sanitization into a dedicated helper inv_icm42600_sanitize_conf()
> to further reduce repetition.
>
> Signed-off-by: João Fernandes <joaovictor.fernandes@usp.br>
> Co-developed-by: Otavio Capobianco <gcotavio@usp.br>
> Signed-off-by: Otavio Capobianco <gcotavio@usp.br>
> ---
> .../iio/imu/inv_icm42600/inv_icm42600_core.c | 125 +++++++++---------
When you get a patch to 'remove duplicated' code and it adds 1 line,
then start questioning if it is a good idea. I think on balance, not
quite worth it here. Some feedback inline.
I haven't broken it out but maybe the sanitize section is a sensible
small deduplication patch?
Comments mostly about too much complexity.
Thanks,
Jonathan
> 1 file changed, 63 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index 76eb22488..e326e0aae 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -289,15 +289,51 @@ static int inv_icm42600_set_pwr_mgmt0(struct inv_icm42600_state *st,
> return 0;
> }
>
> -int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> - struct inv_icm42600_sensor_conf *conf,
> - unsigned int *sleep_ms)
> +static int inv_icm42600_set_sensor_conf(struct inv_icm42600_state *st,
> + struct inv_icm42600_sensor_conf *target_conf,
> + struct inv_icm42600_sensor_conf *accel_conf,
> + struct inv_icm42600_sensor_conf *gyro_conf,
> + struct inv_icm42600_sensor_conf *oldconf,
It's a bit odd to have 3 with _conf and one with no _
Maybe old_config? Given you are updating it, is it really old?
Maybe just call it conf?
> + unsigned int reg_config0,
The number of parameters is what got me first wondering if this was
worth doing. To me it feels of marginal benefit.
> + unsigned int target_config0_val,
> + unsigned int *sleep_ms)
> {
> - struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
> unsigned int val;
> int ret;
>
> - /* Sanitize missing values with current values */
> + /* set target sensor's CONFIG0 register */
> + if (target_conf->fs != oldconf->fs || target_conf->odr != oldconf->odr) {
> + ret = regmap_write(st->map, reg_config0, target_config0_val);
> + if (ret)
> + return ret;
> +
> + oldconf->fs = target_conf->fs;
> + oldconf->odr = target_conf->odr;
> + }
> +
> + /* set GYRO_ACCEL_CONFIG0 register */
Code makes that obvious. So I'd drop the comment.
> + if (target_conf->filter != oldconf->filter) {
> + val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(accel_conf->filter) |
> + INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(gyro_conf->filter);
> +
> + ret = regmap_write(st->map,
> + INV_ICM42600_REG_GYRO_ACCEL_CONFIG0,
> + val);
> + if (ret)
> + return ret;
> +
> + oldconf->filter = target_conf->filter;
> + }
> +
> + /* set PWR_MGMT0 register */
> + return inv_icm42600_set_pwr_mgmt0(st, gyro_conf->mode, accel_conf->mode,
> + st->conf.temp_en, sleep_ms);
> +}
> +
> +static void inv_icm42600_sanitize_conf(
> + struct inv_icm42600_sensor_conf *conf,
> + const struct inv_icm42600_sensor_conf *oldconf)
Maybe go a bit long on this one and use this style:
static void inv_icm42600_sanitize_conf(struct inv_icm42600_sensor_conf *conf,
const struct inv_icm42600_sensor_conf *oldconf)
> +{
> if (conf->mode < 0)
> conf->mode = oldconf->mode;
> if (conf->fs < 0)
> @@ -306,6 +342,17 @@ int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> conf->odr = oldconf->odr;
> if (conf->filter < 0)
> conf->filter = oldconf->filter;
> +}
> +
> +int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> + struct inv_icm42600_sensor_conf *conf,
> + unsigned int *sleep_ms)
> +{
> + struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
> + unsigned int config0_val;
> +
> + /* Sanitize missing values with current values */
> + inv_icm42600_sanitize_conf(conf, oldconf);
>
> /* force power mode against ODR when sensor is on */
> switch (conf->mode) {
> @@ -324,30 +371,12 @@ int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
> break;
> }
>
> - /* set ACCEL_CONFIG0 register (accel fullscale & odr) */
> - if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> - val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
> + config0_val = INV_ICM42600_ACCEL_CONFIG0_FS(conf->fs) |
> INV_ICM42600_ACCEL_CONFIG0_ODR(conf->odr);
> - ret = regmap_write(st->map, INV_ICM42600_REG_ACCEL_CONFIG0, val);
> - if (ret)
> - return ret;
> - oldconf->fs = conf->fs;
> - oldconf->odr = conf->odr;
> - }
> -
> - /* set GYRO_ACCEL_CONFIG0 register (accel filter) */
> - if (conf->filter != oldconf->filter) {
> - val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(conf->filter) |
> - INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(st->conf.gyro.filter);
> - ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
> - if (ret)
> - return ret;
> - oldconf->filter = conf->filter;
> - }
>
> - /* set PWR_MGMT0 register (accel sensor mode) */
> - return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode,
> - st->conf.temp_en, sleep_ms);
> + return inv_icm42600_set_sensor_conf(st, conf, conf, &st->conf.gyro, oldconf,
> + INV_ICM42600_REG_ACCEL_CONFIG0,
> + config0_val, sleep_ms);
> }
>
> int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
> @@ -355,45 +384,17 @@ int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
> unsigned int *sleep_ms)
> {
> struct inv_icm42600_sensor_conf *oldconf = &st->conf.gyro;
> - unsigned int val;
> - int ret;
> + unsigned int config0_val;
>
> - /* sanitize missing values with current values */
> - if (conf->mode < 0)
> - conf->mode = oldconf->mode;
> - if (conf->fs < 0)
> - conf->fs = oldconf->fs;
> - if (conf->odr < 0)
> - conf->odr = oldconf->odr;
> - if (conf->filter < 0)
> - conf->filter = oldconf->filter;
> + /* Sanitize missing values with current values */
> + inv_icm42600_sanitize_conf(conf, oldconf);
>
> - /* set GYRO_CONFIG0 register (gyro fullscale & odr) */
> - if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> - val = INV_ICM42600_GYRO_CONFIG0_FS(conf->fs) |
> + config0_val = INV_ICM42600_GYRO_CONFIG0_FS(conf->fs) |
> INV_ICM42600_GYRO_CONFIG0_ODR(conf->odr);
> - ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_CONFIG0, val);
> - if (ret)
> - return ret;
> - oldconf->fs = conf->fs;
> - oldconf->odr = conf->odr;
> - }
>
> - /* set GYRO_ACCEL_CONFIG0 register (gyro filter) */
> - if (conf->filter != oldconf->filter) {
> - val = INV_ICM42600_GYRO_ACCEL_CONFIG0_ACCEL_FILT(st->conf.accel.filter) |
> - INV_ICM42600_GYRO_ACCEL_CONFIG0_GYRO_FILT(conf->filter);
> - ret = regmap_write(st->map, INV_ICM42600_REG_GYRO_ACCEL_CONFIG0, val);
> - if (ret)
> - return ret;
> - oldconf->filter = conf->filter;
> - }
> -
> - /* set PWR_MGMT0 register (gyro sensor mode) */
> - return inv_icm42600_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
> - st->conf.temp_en, sleep_ms);
> -
> - return 0;
> + return inv_icm42600_set_sensor_conf(st, conf, &st->conf.accel, conf, oldconf,
> + INV_ICM42600_REG_GYRO_CONFIG0,
> + config0_val, sleep_ms);
> }
>
> int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-04 16:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-03 3:12 [PATCH] iio: imu: inv_icm42600: refactor accel/gyro configuration paths João Fernandes
2026-05-03 9:13 ` Joshua Crofts
2026-05-04 14:17 ` Andy Shevchenko
2026-05-04 16:17 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox