Re: [PATCH v2] i2c: mediatek: fix potential incorrect use of I2C_MASTER_WRRD
From: Andi Shyti <andi.shyti@kernel.org>
Date: 2025-09-08 22:17:18
Also in:
linux-i2c, linux-mediatek, lkml
Hi Leilk,
quoted hunk ↗ jump to hunk
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c index ab456c3717db..dee40704825c 100644 --- a/drivers/i2c/busses/i2c-mt65xx.c +++ b/drivers/i2c/busses/i2c-mt65xx.c@@ -1243,6 +1243,7 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap, { int ret; int left_num = num; + bool write_then_read_en = false; struct mtk_i2c *i2c = i2c_get_adapdata(adap); ret = clk_bulk_enable(I2C_MT65XX_CLK_MAX, i2c->clocks);@@ -1256,6 +1257,7 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap, if (!(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) && msgs[0].addr == msgs[1].addr) { i2c->auto_restart = 0; + write_then_read_en = true; } }@@ -1280,12 +1282,10 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap, else i2c->op = I2C_MASTER_WR; - if (!i2c->auto_restart) { - if (num > 1) { - /* combined two messages into one transaction */ - i2c->op = I2C_MASTER_WRRD; - left_num--; - } + if (write_then_read_en) { + /* combined two messages into one transaction */ + i2c->op = I2C_MASTER_WRRD;
i2c doesn't change for the whole loop so that it can be set only once outside the loop instead of setting it everytime. Something like this: if (i2c->op == I2C_MASTER_WRRD) left_num--; else if (msgs->flags & I2C_M_RD) ... else looks cleaner to me and we save the extra flag. Am I missing anything? Andi
quoted hunk ↗ jump to hunk
+ left_num--; } /* always use DMA mode. */@@ -1293,7 +1293,10 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap, if (ret < 0) goto err_exit; - msgs++; + if (i2c->op == I2C_MASTER_WRRD) + msgs += 2; + else + msgs++; } /* the return value is number of executed messages */ ret = num;-- 2.46.0