* [PATCH] iio: imu: inv_icm42600: refactor sensors config
@ 2026-04-23 2:09 João Fernandes
2026-04-23 8:09 ` Andy Shevchenko
0 siblings, 1 reply; 2+ messages in thread
From: João Fernandes @ 2026-04-23 2:09 UTC (permalink / raw)
To: jean-baptiste.maneyrol, jic23, dlechner, nuno.sa, andy
Cc: Otavio Capobianco, linux-iio
gyro and accel configs were refactored to reduce code duplication
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 | 130 +++++++++---------
1 file changed, 62 insertions(+), 68 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..e9fd97504 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
@@ -289,15 +289,24 @@ 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 *conf,
+ unsigned int *sleep_ms,
+ bool is_accel)
{
- struct inv_icm42600_sensor_conf *oldconf = &st->conf.accel;
+ struct inv_icm42600_sensor_conf *oldconf;
+ struct inv_icm42600_sensor_conf *accel_conf;
+ struct inv_icm42600_sensor_conf *gyro_conf;
unsigned int val;
int ret;
- /* Sanitize missing values with current values */
+ /* choose context for old configuration */
+ if (is_accel)
+ oldconf = &st->conf.accel;
+ else
+ oldconf = &st->conf.gyro;
+
+ /* sanitize missing values with current values */
if (conf->mode < 0)
conf->mode = oldconf->mode;
if (conf->fs < 0)
@@ -308,94 +317,79 @@ int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
conf->filter = oldconf->filter;
/* force power mode against ODR when sensor is on */
- switch (conf->mode) {
- case INV_ICM42600_SENSOR_MODE_LOW_POWER:
- case INV_ICM42600_SENSOR_MODE_LOW_NOISE:
- if (conf->odr <= INV_ICM42600_ODR_1KHZ_LN) {
- conf->mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
- conf->filter = INV_ICM42600_FILTER_BW_ODR_DIV_2;
- } else if (conf->odr >= INV_ICM42600_ODR_6_25HZ_LP &&
- conf->odr <= INV_ICM42600_ODR_1_5625HZ_LP) {
- conf->mode = INV_ICM42600_SENSOR_MODE_LOW_POWER;
- conf->filter = INV_ICM42600_FILTER_AVG_16X;
+ if (is_accel) {
+ switch (conf->mode) {
+ case INV_ICM42600_SENSOR_MODE_LOW_POWER:
+ case INV_ICM42600_SENSOR_MODE_LOW_NOISE:
+ if (conf->odr <= INV_ICM42600_ODR_1KHZ_LN) {
+ conf->mode = INV_ICM42600_SENSOR_MODE_LOW_NOISE;
+ conf->filter = INV_ICM42600_FILTER_BW_ODR_DIV_2;
+ } else if (conf->odr >= INV_ICM42600_ODR_6_25HZ_LP &&
+ conf->odr <= INV_ICM42600_ODR_1_5625HZ_LP) {
+ conf->mode = INV_ICM42600_SENSOR_MODE_LOW_POWER;
+ conf->filter = INV_ICM42600_FILTER_AVG_16X;
+ }
+ break;
+ default:
+ break;
}
- break;
- default:
- break;
}
- /* set ACCEL_CONFIG0 register (accel fullscale & odr) */
+ /* choose context */
+ if (is_accel) {
+ accel_conf = conf;
+ gyro_conf = &st->conf.gyro;
+ } else {
+ accel_conf = &st->conf.accel;
+ gyro_conf = conf;
+ }
+
+ /* set GYRO_CONFIG0 register */
if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
- 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 (is_accel) {
+ 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);
+ } else {
+ 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 (accel filter) */
+ /* set GYRO_ACCEL_CONFIG0 register */
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);
+ 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 = conf->filter;
}
- /* set PWR_MGMT0 register (accel sensor mode) */
- return inv_icm42600_set_pwr_mgmt0(st, st->conf.gyro.mode, conf->mode,
+ /* set PWR_MGMT0 register */
+ return inv_icm42600_set_pwr_mgmt0(st, gyro_conf->mode, accel_conf->mode,
st->conf.temp_en, sleep_ms);
}
+int inv_icm42600_set_accel_conf(struct inv_icm42600_state *st,
+ struct inv_icm42600_sensor_conf *conf,
+ unsigned int *sleep_ms)
+{
+ return inv_icm42600_set_sensor_conf(st, conf, sleep_ms, 1);
+}
+
int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
struct inv_icm42600_sensor_conf *conf,
unsigned int *sleep_ms)
{
- struct inv_icm42600_sensor_conf *oldconf = &st->conf.gyro;
- unsigned int val;
- int ret;
-
- /* 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;
-
- /* set GYRO_CONFIG0 register (gyro fullscale & odr) */
- if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
- 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, sleep_ms, 0);
}
-
int inv_icm42600_set_temp_conf(struct inv_icm42600_state *st, bool enable,
unsigned int *sleep_ms)
{
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] iio: imu: inv_icm42600: refactor sensors config
2026-04-23 2:09 [PATCH] iio: imu: inv_icm42600: refactor sensors config João Fernandes
@ 2026-04-23 8:09 ` Andy Shevchenko
0 siblings, 0 replies; 2+ messages in thread
From: Andy Shevchenko @ 2026-04-23 8:09 UTC (permalink / raw)
To: João Fernandes
Cc: jean-baptiste.maneyrol, jic23, dlechner, nuno.sa, andy,
Otavio Capobianco, linux-iio
On Wed, Apr 22, 2026 at 11:09:20PM -0300, João Fernandes wrote:
> gyro and accel configs were refactored to reduce code duplication
Please, use proper English grammar and punctuation.
Also Submitting Patches documentation (which is assumed that you have
studied before sending patches) points out on imperative mood.
...
> - /* Sanitize missing values with current values */
> + /* choose context for old configuration */
One has to be consistent with the existing style in the driver. Or, if you
disagree, convert the driver to a style you prefer and provide _a very good_
justification why it should be done.
...
> + if (is_accel)
> + oldconf = &st->conf.accel;
> + else
> + oldconf = &st->conf.gyro;
Here and below the function is completely uglified. Even kernel documentation
suggests to avoid bool parameters like this.
If you need a common code, make it agnostic first (using parameters), and then
use small wrappers for each of the cases (accel, gyro).
...
> int inv_icm42600_set_gyro_conf(struct inv_icm42600_state *st,
> struct inv_icm42600_sensor_conf *conf,
> unsigned int *sleep_ms)
> {
> - struct inv_icm42600_sensor_conf *oldconf = &st->conf.gyro;
You see, this has to stay in the wrapper.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-04-23 8:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-23 2:09 [PATCH] iio: imu: inv_icm42600: refactor sensors config João Fernandes
2026-04-23 8:09 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox