diff mbox series

[v3,05/12] i2c: wmt: Reduce redundant: function parameter

Message ID a37db0cdf8d5a43cd90430361aac24cb4362847b.1698889581.git.hanshu-oc@zhaoxin.com
State New
Headers show
Series i2c: add zhaoxin i2c controller driver | expand

Commit Message

Hans Hu Nov. 2, 2023, 2:53 a.m. UTC 
Use more appropriate parameter passing to reduce the amount of code

Signed-off-by: Hans Hu <hanshu-oc@zhaoxin.com>
---
 drivers/i2c/busses/i2c-wmt.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

Comments

Wolfram Sang Dec. 22, 2023, 9:48 a.m. UTC | #1 
On Thu, Nov 02, 2023 at 10:53:55AM +0800, Hans Hu wrote:
> Use more appropriate parameter passing to reduce the amount of code
> 
> Signed-off-by: Hans Hu <hanshu-oc@zhaoxin.com>
> ---

Because I like most cleanups here: applied to for-next, thanks!

But this patch likely reveals a bug in the driver:

> -static int wmt_i2c_read(struct i2c_adapter *adap, struct i2c_msg *pmsg,
> -			int last)
> +static int wmt_i2c_read(struct wmt_i2c_dev *i2c_dev, struct i2c_msg *pmsg)

It is valid for an I2C transfer, that the last message is a read
message. So, instead of dropping it, I think the read function needs to
handle 'last' as well.

Because this bug is already present in the driver, and I don't want to
make rebasing a next version of this series too difficult, I argue that
we fix it after your changes are included.

Happy hacking!
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-wmt.c b/drivers/i2c/busses/i2c-wmt.c
index d503e85752ea..ec2a8da134e5 100644
--- a/drivers/i2c/busses/i2c-wmt.c
+++ b/drivers/i2c/busses/i2c-wmt.c
@@ -122,10 +122,9 @@  static int wmt_check_status(struct wmt_i2c_dev *i2c_dev)
 	return ret;
 }
 
-static int wmt_i2c_write(struct i2c_adapter *adap, struct i2c_msg *pmsg,
+static int wmt_i2c_write(struct wmt_i2c_dev *i2c_dev, struct i2c_msg *pmsg,
 			 int last)
 {
-	struct wmt_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
 	u16 val, tcr_val = i2c_dev->tcr;
 	int ret;
 	int xfer_len = 0;
@@ -192,10 +191,8 @@  static int wmt_i2c_write(struct i2c_adapter *adap, struct i2c_msg *pmsg,
 	return 0;
 }
 
-static int wmt_i2c_read(struct i2c_adapter *adap, struct i2c_msg *pmsg,
-			int last)
+static int wmt_i2c_read(struct wmt_i2c_dev *i2c_dev, struct i2c_msg *pmsg)
 {
-	struct wmt_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
 	u16 val, tcr_val = i2c_dev->tcr;
 	int ret;
 	u32 xfer_len = 0;
@@ -245,13 +242,11 @@  static int wmt_i2c_xfer(struct i2c_adapter *adap,
 			int num)
 {
 	struct i2c_msg *pmsg;
-	int i, is_last;
+	int i;
 	int ret = 0;
 	struct wmt_i2c_dev *i2c_dev = i2c_get_adapdata(adap);
 
 	for (i = 0; ret >= 0 && i < num; i++) {
-		is_last = ((i + 1) == num);
-
 		pmsg = &msgs[i];
 		if (!(pmsg->flags & I2C_M_NOSTART)) {
 			ret = wmt_i2c_wait_bus_not_busy(i2c_dev);
@@ -260,9 +255,9 @@  static int wmt_i2c_xfer(struct i2c_adapter *adap,
 		}
 
 		if (pmsg->flags & I2C_M_RD)
-			ret = wmt_i2c_read(adap, pmsg, is_last);
+			ret = wmt_i2c_read(i2c_dev, pmsg);
 		else
-			ret = wmt_i2c_write(adap, pmsg, is_last);
+			ret = wmt_i2c_write(i2c_dev, pmsg, (i + 1) == num);
 	}
 
 	return (ret < 0) ? ret : i;