* [PATCH] iio: imu: inv_mpu6050: refactor aux read/write to use shared xfer logic
@ 2025-04-28 13:25 Isabella Caselli
2025-04-29 9:36 ` Jean-Baptiste Maneyrol
0 siblings, 1 reply; 2+ messages in thread
From: Isabella Caselli @ 2025-04-28 13:25 UTC (permalink / raw)
To: jean-baptiste.maneyrol, jic23, lars, linux-iio
Cc: rodrigo.michelassi, marcelo.schmitt1, Isabella Caselli
Extract common transfer logic from inv_mpu_aux_read() and
inv_mpu_aux_write() into a new helper function,
inv_mpu_aux_exec_xfer(), which performs the I2C transfer.
This refactoring removes code duplication and improves maintainability.
No functional changes are intended.
Signed-off-by: Isabella Caselli <bellacaselli20@gmail.com>
Co-developed-by: Rodrigo Michelassi <rodrigo.michelassi@usp.br>
Signed-off-by: Rodrigo Michelassi <rodrigo.michelassi@usp.br>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c | 41 +++++++++++------------
drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h | 2 ++
2 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c
index 8a7f29119..de013e034 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c
@@ -117,7 +117,6 @@ int inv_mpu_aux_init(const struct inv_mpu6050_state *st)
int inv_mpu_aux_read(const struct inv_mpu6050_state *st, uint8_t addr,
uint8_t reg, uint8_t *val, size_t size)
{
- unsigned int status;
int ret;
if (size > 0x0F)
@@ -136,30 +135,13 @@ int inv_mpu_aux_read(const struct inv_mpu6050_state *st, uint8_t addr,
if (ret)
return ret;
- /* do i2c xfer */
- ret = inv_mpu_i2c_master_xfer(st);
- if (ret)
- goto error_disable_i2c;
-
- /* disable i2c slave */
- ret = regmap_write(st->map, INV_MPU6050_REG_I2C_SLV_CTRL(0), 0);
- if (ret)
- goto error_disable_i2c;
-
- /* check i2c status */
- ret = regmap_read(st->map, INV_MPU6050_REG_I2C_MST_STATUS, &status);
+ ret = inv_mpu_aux_exec_xfer(st);
if (ret)
return ret;
- if (status & INV_MPU6050_BIT_I2C_SLV0_NACK)
- return -EIO;
/* read data in registers */
return regmap_bulk_read(st->map, INV_MPU6050_REG_EXT_SENS_DATA,
val, size);
-
-error_disable_i2c:
- regmap_write(st->map, INV_MPU6050_REG_I2C_SLV_CTRL(0), 0);
- return ret;
}
/**
@@ -174,7 +156,6 @@ int inv_mpu_aux_read(const struct inv_mpu6050_state *st, uint8_t addr,
int inv_mpu_aux_write(const struct inv_mpu6050_state *st, uint8_t addr,
uint8_t reg, uint8_t val)
{
- unsigned int status;
int ret;
/* setup i2c SLV0 control: i2c addr, register, value, enable + size */
@@ -192,6 +173,24 @@ int inv_mpu_aux_write(const struct inv_mpu6050_state *st, uint8_t addr,
if (ret)
return ret;
+ ret = inv_mpu_aux_exec_xfer(st);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+/**
+ * inv_mpu_aux_exec_xfer() - executes i2c auxiliary transfer and checks status
+ * @st: driver internal state.
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ */
+int inv_mpu_aux_exec_xfer(const struct inv_mpu6050_state *st)
+{
+ int ret;
+ unsigned int status;
+
/* do i2c xfer */
ret = inv_mpu_i2c_master_xfer(st);
if (ret)
@@ -209,8 +208,6 @@ int inv_mpu_aux_write(const struct inv_mpu6050_state *st, uint8_t addr,
if (status & INV_MPU6050_BIT_I2C_SLV0_NACK)
return -EIO;
- return 0;
-
error_disable_i2c:
regmap_write(st->map, INV_MPU6050_REG_I2C_SLV_CTRL(0), 0);
return ret;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h
index b66997545..0353103aa 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h
@@ -16,4 +16,6 @@ int inv_mpu_aux_read(const struct inv_mpu6050_state *st, uint8_t addr,
int inv_mpu_aux_write(const struct inv_mpu6050_state *st, uint8_t addr,
uint8_t reg, uint8_t val);
+int inv_mpu_aux_exec_xfer(const struct inv_mpu6050_state *st);
+
#endif /* INV_MPU_AUX_H_ */
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] iio: imu: inv_mpu6050: refactor aux read/write to use shared xfer logic
2025-04-28 13:25 [PATCH] iio: imu: inv_mpu6050: refactor aux read/write to use shared xfer logic Isabella Caselli
@ 2025-04-29 9:36 ` Jean-Baptiste Maneyrol
0 siblings, 0 replies; 2+ messages in thread
From: Jean-Baptiste Maneyrol @ 2025-04-29 9:36 UTC (permalink / raw)
To: Isabella Caselli, jic23@kernel.org, lars@metafoo.de,
linux-iio@vger.kernel.org
Cc: rodrigo.michelassi@usp.br, marcelo.schmitt1@gmail.com
Hello,
I don't really see the point in this patch.
We already have a common function inv_mpu_i2c_master_xfer() for handling master I2C transfer, and there is no need to create a new external function inv_mpu_aux_exec_xfer() that is never called outside the inv_mpu_aux.c file.
If you want to factorize the I2C transfer status check, better add it to the inv_mpu_i2c_master_xfer function. This way it would make sense. But you need to warn that only SLV0 slot has to be used for transfer if we are enforcing SLV0 transfer check.
In the actual implementation we're free to use the slave slot we want, since all SLV(x) calls are done in the same function. It is easier to understand like this, I think.
Thanks,
JB
________________________________________
From: Isabella Caselli <bellacaselli20@gmail.com>
Sent: Monday, April 28, 2025 15:25
To: Jean-Baptiste Maneyrol <Jean-Baptiste.Maneyrol@tdk.com>; jic23@kernel.org <jic23@kernel.org>; lars@metafoo.de <lars@metafoo.de>; linux-iio@vger.kernel.org <linux-iio@vger.kernel.org>
Cc: rodrigo.michelassi@usp.br <rodrigo.michelassi@usp.br>; marcelo.schmitt1@gmail.com <marcelo.schmitt1@gmail.com>; Isabella Caselli <bellacaselli20@gmail.com>
Subject: [PATCH] iio: imu: inv_mpu6050: refactor aux read/write to use shared xfer logic
This Message Is From an Untrusted Sender
You have not previously corresponded with this sender.
Extract common transfer logic from inv_mpu_aux_read() and
inv_mpu_aux_write() into a new helper function,
inv_mpu_aux_exec_xfer(), which performs the I2C transfer.
This refactoring removes code duplication and improves maintainability.
No functional changes are intended.
Signed-off-by: Isabella Caselli <bellacaselli20@gmail.com>
Co-developed-by: Rodrigo Michelassi <rodrigo.michelassi@usp.br>
Signed-off-by: Rodrigo Michelassi <rodrigo.michelassi@usp.br>
---
drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c | 41 +++++++++++------------
drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h | 2 ++
2 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c
index 8a7f29119..de013e034 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.c
@@ -117,7 +117,6 @@ int inv_mpu_aux_init(const struct inv_mpu6050_state *st)
int inv_mpu_aux_read(const struct inv_mpu6050_state *st, uint8_t addr,
uint8_t reg, uint8_t *val, size_t size)
{
- unsigned int status;
int ret;
if (size > 0x0F)
@@ -136,30 +135,13 @@ int inv_mpu_aux_read(const struct inv_mpu6050_state *st, uint8_t addr,
if (ret)
return ret;
- /* do i2c xfer */
- ret = inv_mpu_i2c_master_xfer(st);
- if (ret)
- goto error_disable_i2c;
-
- /* disable i2c slave */
- ret = regmap_write(st->map, INV_MPU6050_REG_I2C_SLV_CTRL(0), 0);
- if (ret)
- goto error_disable_i2c;
-
- /* check i2c status */
- ret = regmap_read(st->map, INV_MPU6050_REG_I2C_MST_STATUS, &status);
+ ret = inv_mpu_aux_exec_xfer(st);
if (ret)
return ret;
- if (status & INV_MPU6050_BIT_I2C_SLV0_NACK)
- return -EIO;
/* read data in registers */
return regmap_bulk_read(st->map, INV_MPU6050_REG_EXT_SENS_DATA,
val, size);
-
-error_disable_i2c:
- regmap_write(st->map, INV_MPU6050_REG_I2C_SLV_CTRL(0), 0);
- return ret;
}
/**
@@ -174,7 +156,6 @@ int inv_mpu_aux_read(const struct inv_mpu6050_state *st, uint8_t addr,
int inv_mpu_aux_write(const struct inv_mpu6050_state *st, uint8_t addr,
uint8_t reg, uint8_t val)
{
- unsigned int status;
int ret;
/* setup i2c SLV0 control: i2c addr, register, value, enable + size */
@@ -192,6 +173,24 @@ int inv_mpu_aux_write(const struct inv_mpu6050_state *st, uint8_t addr,
if (ret)
return ret;
+ ret = inv_mpu_aux_exec_xfer(st);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+/**
+ * inv_mpu_aux_exec_xfer() - executes i2c auxiliary transfer and checks status
+ * @st: driver internal state.
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ */
+int inv_mpu_aux_exec_xfer(const struct inv_mpu6050_state *st)
+{
+ int ret;
+ unsigned int status;
+
/* do i2c xfer */
ret = inv_mpu_i2c_master_xfer(st);
if (ret)
@@ -209,8 +208,6 @@ int inv_mpu_aux_write(const struct inv_mpu6050_state *st, uint8_t addr,
if (status & INV_MPU6050_BIT_I2C_SLV0_NACK)
return -EIO;
- return 0;
-
error_disable_i2c:
regmap_write(st->map, INV_MPU6050_REG_I2C_SLV_CTRL(0), 0);
return ret;
diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h b/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h
index b66997545..0353103aa 100644
--- a/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h
+++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_aux.h
@@ -16,4 +16,6 @@ int inv_mpu_aux_read(const struct inv_mpu6050_state *st, uint8_t addr,
int inv_mpu_aux_write(const struct inv_mpu6050_state *st, uint8_t addr,
uint8_t reg, uint8_t val);
+int inv_mpu_aux_exec_xfer(const struct inv_mpu6050_state *st);
+
#endif /* INV_MPU_AUX_H_ */
--
2.43.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-04-29 10:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-28 13:25 [PATCH] iio: imu: inv_mpu6050: refactor aux read/write to use shared xfer logic Isabella Caselli
2025-04-29 9:36 ` Jean-Baptiste Maneyrol
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox