[PATCH 07/14] iio: pressure: mprls0025pa: make ops->write function consistent

Petre Rodan posted 14 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH 07/14] iio: pressure: mprls0025pa: make ops->write function consistent
Posted by Petre Rodan 1 month, 3 weeks ago
Make the i2c bus write operation more consistent with the rest of the
driver.
Also move defines only used by core out of the common header file.

Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
---
 drivers/iio/pressure/mprls0025pa.c     | 5 +++++
 drivers/iio/pressure/mprls0025pa.h     | 4 ----
 drivers/iio/pressure/mprls0025pa_i2c.c | 9 ++++++---
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/pressure/mprls0025pa.c b/drivers/iio/pressure/mprls0025pa.c
index 7cc8dd0d8476..7f8dc47d7073 100644
--- a/drivers/iio/pressure/mprls0025pa.c
+++ b/drivers/iio/pressure/mprls0025pa.c
@@ -39,6 +39,11 @@
 
 #define MPR_ST_ERR_FLAG  (MPR_ST_BUSY | MPR_ST_MEMORY | MPR_ST_MATH)
 
+#define MPR_CMD_NOP      0xf0
+#define MPR_CMD_SYNC     0xaa
+#define MPR_PKT_NOP_LEN  MPR_MEASUREMENT_RD_SIZE
+#define MPR_PKT_SYNC_LEN 3
+
 /*
  * support _RAW sysfs interface:
  *
diff --git a/drivers/iio/pressure/mprls0025pa.h b/drivers/iio/pressure/mprls0025pa.h
index 83dbc3ef8ba4..50c99ddc8937 100644
--- a/drivers/iio/pressure/mprls0025pa.h
+++ b/drivers/iio/pressure/mprls0025pa.h
@@ -20,10 +20,6 @@
 #include <linux/iio/iio.h>
 
 #define MPR_MEASUREMENT_RD_SIZE 4
-#define MPR_CMD_NOP      0xf0
-#define MPR_CMD_SYNC     0xaa
-#define MPR_PKT_NOP_LEN  MPR_MEASUREMENT_RD_SIZE
-#define MPR_PKT_SYNC_LEN 3
 
 struct device;
 
diff --git a/drivers/iio/pressure/mprls0025pa_i2c.c b/drivers/iio/pressure/mprls0025pa_i2c.c
index 0fe8cfe0d7e7..5508a1681b7b 100644
--- a/drivers/iio/pressure/mprls0025pa_i2c.c
+++ b/drivers/iio/pressure/mprls0025pa_i2c.c
@@ -34,16 +34,19 @@ static int mpr_i2c_read(struct mpr_data *data, const u8 unused, const u8 cnt)
 	return 0;
 }
 
-static int mpr_i2c_write(struct mpr_data *data, const u8 cmd, const u8 unused)
+static int mpr_i2c_write(struct mpr_data *data, const u8 cmd, const u8 cnt)
 {
 	int ret;
 	struct i2c_client *client = to_i2c_client(data->dev);
 
+	if (cnt > MPR_MEASUREMENT_RD_SIZE)
+		return -EOVERFLOW;
+
 	data->tx_buf[0] = cmd;
-	ret = i2c_master_send(client, data->tx_buf, MPR_PKT_SYNC_LEN);
+	ret = i2c_master_send(client, data->tx_buf, cnt);
 	if (ret < 0)
 		return ret;
-	else if (ret != MPR_PKT_SYNC_LEN)
+	else if (ret != cnt)
 		return -EIO;
 
 	return 0;

-- 
2.51.2
Re: [PATCH 07/14] iio: pressure: mprls0025pa: make ops->write function consistent
Posted by Marcelo Schmitt 1 month, 3 weeks ago
On 12/18, Petre Rodan wrote:
> Make the i2c bus write operation more consistent with the rest of the
> driver.
That's not the most appealing reason for updating driver code. Is the update
meaningful for a different purpose? Consider squashing that with another patch
that makes better use of the updated function. 

> Also move defines only used by core out of the common header file.
Do the define thing in a separate patch.
Commits/patches should be semantically concise (in other words, do one thing on each one).
Usually, the word 'also' in a commit description is a strong indicator that the
second thing being done should be on a patch of it's own.

> 
> Signed-off-by: Petre Rodan <petre.rodan@subdimension.ro>
> ---
>  drivers/iio/pressure/mprls0025pa.c     | 5 +++++
>  drivers/iio/pressure/mprls0025pa.h     | 4 ----
>  drivers/iio/pressure/mprls0025pa_i2c.c | 9 ++++++---
>  3 files changed, 11 insertions(+), 7 deletions(-)
Re: [PATCH 07/14] iio: pressure: mprls0025pa: make ops->write function consistent
Posted by Petre Rodan 1 month, 3 weeks ago
Hello,

On Sat, Dec 20, 2025 at 01:49:30AM -0300, Marcelo Schmitt wrote:
> On 12/18, Petre Rodan wrote:
> > Make the i2c bus write operation more consistent with the rest of the
> > driver.
> That's not the most appealing reason for updating driver code. Is the update
> meaningful for a different purpose? Consider squashing that with another patch
> that makes better use of the updated function. 

I see your point.

I wanted to have device specific logic in _core instead of one of the bus drivers,
but if I'm the only one upset about it then I will skip this change.

best regards,
peter

-- 
petre rodan