* [PATCH] i2c: mediatek: Add to support continuous mode
@ 2020-06-19 8:06 Qiangming Xia
2020-06-20 1:53 ` Qii Wang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Qiangming Xia @ 2020-06-19 8:06 UTC (permalink / raw)
To: wsa, Wolfram Sang
Cc: qiangming.xia, devicetree, Qii Wang, srv_heupstream,
linux-kernel, linux-mediatek, linux-i2c, linux-arm-kernel
From: "qiangming.xia" <qiangming.xia@mediatek.com>
Mediatek i2c controller support for continuous mode,
it allow to transfer once multiple writing messages of equal length.
For example, a slave need write a serial of non-continuous
offset range in chip,e.g. writing offset 0,offset 2 and offset 4.
Normally, it need three times i2c write operation. However,it can
use once transfer to finish it by using continuous mode.
Change-Id: If06991e3fd32867bdeaacf15bb24864d5c5904d0
Signed-off-by: Qiangming Xia <qiangming.xia@mediatek.com>
---
drivers/i2c/busses/i2c-mt65xx.c | 67 +++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index deef69e56906..76ec65d869f6 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -97,6 +97,7 @@ enum mtk_trans_op {
I2C_MASTER_WR = 1,
I2C_MASTER_RD,
I2C_MASTER_WRRD,
+ I2C_MASTER_CONTINUOUS_WR,
};
enum I2C_REGS_OFFSET {
@@ -846,6 +847,9 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
OFFSET_TRANSFER_LEN);
}
mtk_i2c_writew(i2c, I2C_WRRD_TRANAC_VALUE, OFFSET_TRANSAC_LEN);
+ } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
+ mtk_i2c_writew(i2c, msgs->len / num, OFFSET_TRANSFER_LEN);
+ mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
} else {
mtk_i2c_writew(i2c, msgs->len, OFFSET_TRANSFER_LEN);
mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
@@ -896,6 +900,23 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
}
+ writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
+ writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
+ } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
+ writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
+ writel(I2C_DMA_CON_TX, i2c->pdmabase + OFFSET_CON);
+ wpaddr = dma_map_single(i2c->dev, msgs->buf,
+ msgs->len, DMA_TO_DEVICE);
+ if (dma_mapping_error(i2c->dev, wpaddr)) {
+ kfree(msgs->buf);
+ return -ENOMEM;
+ }
+
+ if (i2c->dev_comp->support_33bits) {
+ reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr);
+ writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
+ }
+
writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
} else {
@@ -979,6 +1000,11 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
msgs->len, DMA_FROM_DEVICE);
i2c_put_dma_safe_msg_buf(dma_rd_buf, msgs, true);
+ } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
+ dma_unmap_single(i2c->dev, wpaddr,
+ msgs->len, DMA_TO_DEVICE);
+
+ kfree(msgs->buf);
} else {
dma_unmap_single(i2c->dev, wpaddr, msgs->len,
DMA_TO_DEVICE);
@@ -1009,6 +1035,9 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
{
int ret;
int left_num = num;
+ int i, j;
+ u8 *dma_multi_wr_buf;
+ struct i2c_msg multi_msg[1];
struct mtk_i2c *i2c = i2c_get_adapdata(adap);
ret = mtk_i2c_clock_enable(i2c);
@@ -1025,6 +1054,44 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
}
}
+ if (num > 1) {
+ for (i = 0; i < num - 1; i++) {
+ if (!(msgs[i].flags & I2C_M_RD) && !(msgs[i+1].flags &
+ I2C_M_RD) && (msgs[i].addr == msgs[i+1].addr)
+ && (msgs[i].len == msgs[i+1].len)) {
+ continue;
+ } else
+ break;
+ }
+ if (i >= num - 1) {
+ i2c->op = I2C_MASTER_CONTINUOUS_WR;
+ j = 0;
+ dma_multi_wr_buf = kzalloc(msgs->len * num, GFP_KERNEL);
+ if (!dma_multi_wr_buf) {
+ ret = -ENOMEM;
+ goto err_exit;
+ }
+ multi_msg->addr = msgs->addr;
+ multi_msg->len = msgs->len * num;
+ multi_msg->buf = dma_multi_wr_buf;
+ multi_msg->flags = 0;
+ while (j < num) {
+ memcpy(dma_multi_wr_buf + msgs->len * j,
+ msgs->buf, msgs->len);
+ j++;
+ msgs++;
+ }
+
+ i2c->ignore_restart_irq = false;
+ ret = mtk_i2c_do_transfer(i2c, multi_msg, num, 0);
+ if (ret < 0)
+ goto err_exit;
+ ret = num;
+ goto err_exit;
+
+ }
+ }
+
if (i2c->auto_restart && num >= 2 && i2c->speed_hz > I2C_MAX_FAST_MODE_FREQ)
/* ignore the first restart irq after the master code,
* otherwise the first transfer will be discarded.
--
2.18.0
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] i2c: mediatek: Add to support continuous mode
2020-06-19 8:06 [PATCH] i2c: mediatek: Add to support continuous mode Qiangming Xia
@ 2020-06-20 1:53 ` Qii Wang
2020-06-20 7:30 ` Yingjoe Chen
2020-06-29 2:00 ` Qii Wang
2 siblings, 0 replies; 5+ messages in thread
From: Qii Wang @ 2020-06-20 1:53 UTC (permalink / raw)
To: Qiangming Xia
Cc: devicetree, srv_heupstream, wsa, linux-kernel, Wolfram Sang,
linux-mediatek, linux-i2c, linux-arm-kernel
On Fri, 2020-06-19 at 16:06 +0800, Qiangming Xia wrote:
> From: "qiangming.xia" <qiangming.xia@mediatek.com>
>
> Mediatek i2c controller support for continuous mode,
> it allow to transfer once multiple writing messages of equal length.
> For example, a slave need write a serial of non-continuous
> offset range in chip,e.g. writing offset 0,offset 2 and offset 4.
> Normally, it need three times i2c write operation. However,it can
> use once transfer to finish it by using continuous mode.
>
> Change-Id: If06991e3fd32867bdeaacf15bb24864d5c5904d0
> Signed-off-by: Qiangming Xia <qiangming.xia@mediatek.com>
> ---
> drivers/i2c/busses/i2c-mt65xx.c | 67 +++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index deef69e56906..76ec65d869f6 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -97,6 +97,7 @@ enum mtk_trans_op {
> I2C_MASTER_WR = 1,
> I2C_MASTER_RD,
> I2C_MASTER_WRRD,
> + I2C_MASTER_CONTINUOUS_WR,
> };
>
> enum I2C_REGS_OFFSET {
> @@ -846,6 +847,9 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> OFFSET_TRANSFER_LEN);
> }
> mtk_i2c_writew(i2c, I2C_WRRD_TRANAC_VALUE, OFFSET_TRANSAC_LEN);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + mtk_i2c_writew(i2c, msgs->len / num, OFFSET_TRANSFER_LEN);
> + mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
> } else {
> mtk_i2c_writew(i2c, msgs->len, OFFSET_TRANSFER_LEN);
> mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
> @@ -896,6 +900,23 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
> }
>
> + writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
> + writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
> + writel(I2C_DMA_CON_TX, i2c->pdmabase + OFFSET_CON);
> + wpaddr = dma_map_single(i2c->dev, msgs->buf,
> + msgs->len, DMA_TO_DEVICE);
> + if (dma_mapping_error(i2c->dev, wpaddr)) {
> + kfree(msgs->buf);
> + return -ENOMEM;
> + }
> +
> + if (i2c->dev_comp->support_33bits) {
> + reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr);
> + writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
> + }
> +
> writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
> writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
> } else {
> @@ -979,6 +1000,11 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> msgs->len, DMA_FROM_DEVICE);
>
> i2c_put_dma_safe_msg_buf(dma_rd_buf, msgs, true);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + dma_unmap_single(i2c->dev, wpaddr,
> + msgs->len, DMA_TO_DEVICE);
> +
> + kfree(msgs->buf);
> } else {
> dma_unmap_single(i2c->dev, wpaddr, msgs->len,
> DMA_TO_DEVICE);
> @@ -1009,6 +1035,9 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> {
> int ret;
> int left_num = num;
> + int i, j;
> + u8 *dma_multi_wr_buf;
> + struct i2c_msg multi_msg[1];
> struct mtk_i2c *i2c = i2c_get_adapdata(adap);
>
> ret = mtk_i2c_clock_enable(i2c);
> @@ -1025,6 +1054,44 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> }
> }
>
> + if (num > 1) {
> + for (i = 0; i < num - 1; i++) {
> + if (!(msgs[i].flags & I2C_M_RD) && !(msgs[i+1].flags &
> + I2C_M_RD) && (msgs[i].addr == msgs[i+1].addr)
> + && (msgs[i].len == msgs[i+1].len)) {
if these conditions are not met, Can these transfers work?
> + continue;
> + } else
> + break;
> + }
> + if (i >= num - 1) {
> + i2c->op = I2C_MASTER_CONTINUOUS_WR;
> + j = 0;
> + dma_multi_wr_buf = kzalloc(msgs->len * num, GFP_KERNEL);
> + if (!dma_multi_wr_buf) {
> + ret = -ENOMEM;
> + goto err_exit;
> + }
> + multi_msg->addr = msgs->addr;
> + multi_msg->len = msgs->len * num;
> + multi_msg->buf = dma_multi_wr_buf;
> + multi_msg->flags = 0;
> + while (j < num) {
> + memcpy(dma_multi_wr_buf + msgs->len * j,
> + msgs->buf, msgs->len);
> + j++;
> + msgs++;
> + }
> +
> + i2c->ignore_restart_irq = false;
> + ret = mtk_i2c_do_transfer(i2c, multi_msg, num, 0);
> + if (ret < 0)
> + goto err_exit;
> + ret = num;
> + goto err_exit;
> +
> + }
> + }
> +
> if (i2c->auto_restart && num >= 2 && i2c->speed_hz > I2C_MAX_FAST_MODE_FREQ)
> /* ignore the first restart irq after the master code,
> * otherwise the first transfer will be discarded.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] i2c: mediatek: Add to support continuous mode
2020-06-19 8:06 [PATCH] i2c: mediatek: Add to support continuous mode Qiangming Xia
2020-06-20 1:53 ` Qii Wang
@ 2020-06-20 7:30 ` Yingjoe Chen
2020-06-29 2:00 ` Qii Wang
2 siblings, 0 replies; 5+ messages in thread
From: Yingjoe Chen @ 2020-06-20 7:30 UTC (permalink / raw)
To: Qiangming Xia
Cc: linux-arm-kernel, devicetree, srv_heupstream, wsa, linux-kernel,
Wolfram Sang, linux-mediatek, linux-i2c, Qii Wang
On Fri, 2020-06-19 at 16:06 +0800, Qiangming Xia wrote:
> From: "qiangming.xia" <qiangming.xia@mediatek.com>
Please make 'From:' the same to Signed-off-by.
> Mediatek i2c controller support for continuous mode,
> it allow to transfer once multiple writing messages of equal length.
So the limitations are writing to same address, all in same length.
I think this is strict limitation. Do we have many this kind of usage?
How about change this to:
MediaTek i2c controller support continuous mode. This allows to write
multiple same length messages to single address with only one setup.
> For example, a slave need write a serial of non-continuous
> offset range in chip,e.g. writing offset 0,offset 2 and offset 4.
> Normally, it need three times i2c write operation. However,it can
> use once transfer to finish it by using continuous mode.
>
> Change-Id: If06991e3fd32867bdeaacf15bb24864d5c5904d0
Please drop Change-Id:
> Signed-off-by: Qiangming Xia <qiangming.xia@mediatek.com>
> ---
> drivers/i2c/busses/i2c-mt65xx.c | 67 +++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index deef69e56906..76ec65d869f6 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -97,6 +97,7 @@ enum mtk_trans_op {
> I2C_MASTER_WR = 1,
> I2C_MASTER_RD,
> I2C_MASTER_WRRD,
> + I2C_MASTER_CONTINUOUS_WR,
> };
>
> enum I2C_REGS_OFFSET {
> @@ -846,6 +847,9 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> OFFSET_TRANSFER_LEN);
> }
> mtk_i2c_writew(i2c, I2C_WRRD_TRANAC_VALUE, OFFSET_TRANSAC_LEN);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + mtk_i2c_writew(i2c, msgs->len / num, OFFSET_TRANSFER_LEN);
> + mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
> } else {
> mtk_i2c_writew(i2c, msgs->len, OFFSET_TRANSFER_LEN);
> mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
> @@ -896,6 +900,23 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
> }
>
> + writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
> + writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
> + writel(I2C_DMA_CON_TX, i2c->pdmabase + OFFSET_CON);
> + wpaddr = dma_map_single(i2c->dev, msgs->buf,
> + msgs->len, DMA_TO_DEVICE);
> + if (dma_mapping_error(i2c->dev, wpaddr)) {
> + kfree(msgs->buf);
> + return -ENOMEM;
> + }
> +
> + if (i2c->dev_comp->support_33bits) {
> + reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr);
> + writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
> + }
> +
> writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
> writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
> } else {
> @@ -979,6 +1000,11 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> msgs->len, DMA_FROM_DEVICE);
>
> i2c_put_dma_safe_msg_buf(dma_rd_buf, msgs, true);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + dma_unmap_single(i2c->dev, wpaddr,
> + msgs->len, DMA_TO_DEVICE);
> +
> + kfree(msgs->buf);
> } else {
> dma_unmap_single(i2c->dev, wpaddr, msgs->len,
> DMA_TO_DEVICE);
> @@ -1009,6 +1035,9 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> {
> int ret;
> int left_num = num;
> + int i, j;
> + u8 *dma_multi_wr_buf;
> + struct i2c_msg multi_msg[1];
> struct mtk_i2c *i2c = i2c_get_adapdata(adap);
>
> ret = mtk_i2c_clock_enable(i2c);
> @@ -1025,6 +1054,44 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> }
> }
>
> + if (num > 1) {
> + for (i = 0; i < num - 1; i++) {
> + if (!(msgs[i].flags & I2C_M_RD) && !(msgs[i+1].flags &
> + I2C_M_RD) && (msgs[i].addr == msgs[i+1].addr)
> + && (msgs[i].len == msgs[i+1].len)) {
Since this block is all for continuous mode check, we could move first
flags check out and made the if in for loop simpler to read:
if (num > 1 && !(msgs[0].flags & I2C_M_RD)) {
for (i = 0; i < num - 1; i++) {
if (!(msgs[i+1].flags & I2C_M_RD) &&
msgs[i].addr == msgs[i+1].addr &&
msgs[i].len == msgs[i+1].len) {
> + continue;
> l+ } else
> + break;
> + }
> + if (i >= num - 1) {
just check i == num -1 is enough.
> + i2c->op = I2C_MASTER_CONTINUOUS_WR;
> + j = 0;
> + dma_multi_wr_buf = kzalloc(msgs->len * num, GFP_KERNEL);
don't need to zero it out. kmalloc is enough.
> + if (!dma_multi_wr_buf) {
> + ret = -ENOMEM;
> + goto err_exit;
> + }
> + multi_msg->addr = msgs->addr;
> + multi_msg->len = msgs->len * num;
> + multi_msg->buf = dma_multi_wr_buf;
> + multi_msg->flags = 0;
> + while (j < num) {
> + memcpy(dma_multi_wr_buf + msgs->len * j,
> + msgs->buf, msgs->len);
> + j++;
> + msgs++;
> + }
extra tab before the closing '}'
> +
> + i2c->ignore_restart_irq = false;
> + ret = mtk_i2c_do_transfer(i2c, multi_msg, num, 0);
> + if (ret < 0)
> + goto err_exit;
> + ret = num;
> + goto err_exit;
> +
extra indent level for last goto and blank line after it.
Joe.C
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] i2c: mediatek: Add to support continuous mode
2020-06-19 8:06 [PATCH] i2c: mediatek: Add to support continuous mode Qiangming Xia
2020-06-20 1:53 ` Qii Wang
2020-06-20 7:30 ` Yingjoe Chen
@ 2020-06-29 2:00 ` Qii Wang
2 siblings, 0 replies; 5+ messages in thread
From: Qii Wang @ 2020-06-29 2:00 UTC (permalink / raw)
To: Qiangming Xia
Cc: devicetree, srv_heupstream, wsa, linux-kernel, Wolfram Sang,
linux-mediatek, linux-i2c, linux-arm-kernel
Hi Qiangming:
Do you have the specific timing cost data about the "continuous mode"?
Is it better than the default multi-write mode(one message by one
message) ?I need to know if this patch is very necessary.
On Fri, 2020-06-19 at 16:06 +0800, Qiangming Xia wrote:
> From: "qiangming.xia" <qiangming.xia@mediatek.com>
>
> Mediatek i2c controller support for continuous mode,
> it allow to transfer once multiple writing messages of equal length.
> For example, a slave need write a serial of non-continuous
> offset range in chip,e.g. writing offset 0,offset 2 and offset 4.
> Normally, it need three times i2c write operation. However,it can
> use once transfer to finish it by using continuous mode.
>
> Change-Id: If06991e3fd32867bdeaacf15bb24864d5c5904d0
> Signed-off-by: Qiangming Xia <qiangming.xia@mediatek.com>
> ---
> drivers/i2c/busses/i2c-mt65xx.c | 67 +++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index deef69e56906..76ec65d869f6 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -97,6 +97,7 @@ enum mtk_trans_op {
> I2C_MASTER_WR = 1,
> I2C_MASTER_RD,
> I2C_MASTER_WRRD,
> + I2C_MASTER_CONTINUOUS_WR,
> };
>
> enum I2C_REGS_OFFSET {
> @@ -846,6 +847,9 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> OFFSET_TRANSFER_LEN);
> }
> mtk_i2c_writew(i2c, I2C_WRRD_TRANAC_VALUE, OFFSET_TRANSAC_LEN);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + mtk_i2c_writew(i2c, msgs->len / num, OFFSET_TRANSFER_LEN);
> + mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
> } else {
> mtk_i2c_writew(i2c, msgs->len, OFFSET_TRANSFER_LEN);
> mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
> @@ -896,6 +900,23 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
> }
>
> + writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
> + writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
> + writel(I2C_DMA_CON_TX, i2c->pdmabase + OFFSET_CON);
> + wpaddr = dma_map_single(i2c->dev, msgs->buf,
> + msgs->len, DMA_TO_DEVICE);
> + if (dma_mapping_error(i2c->dev, wpaddr)) {
> + kfree(msgs->buf);
> + return -ENOMEM;
> + }
> +
> + if (i2c->dev_comp->support_33bits) {
> + reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr);
> + writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
> + }
> +
> writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
> writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
> } else {
> @@ -979,6 +1000,11 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> msgs->len, DMA_FROM_DEVICE);
>
> i2c_put_dma_safe_msg_buf(dma_rd_buf, msgs, true);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + dma_unmap_single(i2c->dev, wpaddr,
> + msgs->len, DMA_TO_DEVICE);
> +
> + kfree(msgs->buf);
> } else {
> dma_unmap_single(i2c->dev, wpaddr, msgs->len,
> DMA_TO_DEVICE);
> @@ -1009,6 +1035,9 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> {
> int ret;
> int left_num = num;
> + int i, j;
> + u8 *dma_multi_wr_buf;
> + struct i2c_msg multi_msg[1];
> struct mtk_i2c *i2c = i2c_get_adapdata(adap);
>
> ret = mtk_i2c_clock_enable(i2c);
> @@ -1025,6 +1054,44 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> }
> }
>
> + if (num > 1) {
> + for (i = 0; i < num - 1; i++) {
> + if (!(msgs[i].flags & I2C_M_RD) && !(msgs[i+1].flags &
> + I2C_M_RD) && (msgs[i].addr == msgs[i+1].addr)
> + && (msgs[i].len == msgs[i+1].len)) {
> + continue;
> + } else
> + break;
> + }
> + if (i >= num - 1) {
> + i2c->op = I2C_MASTER_CONTINUOUS_WR;
> + j = 0;
> + dma_multi_wr_buf = kzalloc(msgs->len * num, GFP_KERNEL);
> + if (!dma_multi_wr_buf) {
> + ret = -ENOMEM;
> + goto err_exit;
> + }
> + multi_msg->addr = msgs->addr;
> + multi_msg->len = msgs->len * num;
> + multi_msg->buf = dma_multi_wr_buf;
> + multi_msg->flags = 0;
> + while (j < num) {
> + memcpy(dma_multi_wr_buf + msgs->len * j,
> + msgs->buf, msgs->len);
> + j++;
> + msgs++;
> + }
> +
> + i2c->ignore_restart_irq = false;
> + ret = mtk_i2c_do_transfer(i2c, multi_msg, num, 0);
> + if (ret < 0)
> + goto err_exit;
> + ret = num;
> + goto err_exit;
> +
> + }
> + }
> +
> if (i2c->auto_restart && num >= 2 && i2c->speed_hz > I2C_MAX_FAST_MODE_FREQ)
> /* ignore the first restart irq after the master code,
> * otherwise the first transfer will be discarded.
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] i2c: mediatek: Add to support continuous mode
[not found] <20200623074245.24513-1-qiangming.xia@mediatek.com>
@ 2020-06-30 7:33 ` Yingjoe Chen
0 siblings, 0 replies; 5+ messages in thread
From: Yingjoe Chen @ 2020-06-30 7:33 UTC (permalink / raw)
To: Qiangming Xia
Cc: devicetree, srv_heupstream, wsa, linux-kernel, Wolfram Sang,
Qii Wang, linux-i2c, linux-mediatek, linux-arm-kernel
Hi Qiangming,
When you send new version, you should
- Add version number in subject, so we know this is a new one.
- List what's changed in this patch after ---, so reviewer knows where
we should check.
On Tue, 2020-06-23 at 15:42 +0800, Qiangming Xia wrote:
> From: "qiangming.xia" <qiangming.xia@mediatek.com>
>
> Mediatek i2c controller support for continuous mode,
> it allow to transfer once multiple writing messages of equal
> length.
> A i2c slave sometimes need write a serial of non-continuous
> offset range in chip,e.g. camera sensor imx586,imx576. It need
> transfer 294 writing messages when initiate setting. It can use
> this mode to improve performance.
>
> Change-Id: If473d96d2b76e9d51f20741a9380db4fcad15dbd
Remove this.
> Signed-off-by: Qiangming Xia <qiangming.xia@mediatek.com>
> ---
> drivers/i2c/busses/i2c-mt65xx.c | 66 +++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index deef69e56906..108bca1a4042 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -97,6 +97,7 @@ enum mtk_trans_op {
> I2C_MASTER_WR = 1,
> I2C_MASTER_RD,
> I2C_MASTER_WRRD,
> + I2C_MASTER_CONTINUOUS_WR,
> };
>
> enum I2C_REGS_OFFSET {
> @@ -846,6 +847,9 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> OFFSET_TRANSFER_LEN);
> }
> mtk_i2c_writew(i2c, I2C_WRRD_TRANAC_VALUE, OFFSET_TRANSAC_LEN);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + mtk_i2c_writew(i2c, msgs->len / num, OFFSET_TRANSFER_LEN);
> + mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
> } else {
> mtk_i2c_writew(i2c, msgs->len, OFFSET_TRANSFER_LEN);
> mtk_i2c_writew(i2c, num, OFFSET_TRANSAC_LEN);
> @@ -896,6 +900,23 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
> }
>
> + writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
> + writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + writel(I2C_DMA_INT_FLAG_NONE, i2c->pdmabase + OFFSET_INT_FLAG);
> + writel(I2C_DMA_CON_TX, i2c->pdmabase + OFFSET_CON);
> + wpaddr = dma_map_single(i2c->dev, msgs->buf,
> + msgs->len, DMA_TO_DEVICE);
> + if (dma_mapping_error(i2c->dev, wpaddr)) {
> + kfree(msgs->buf);
> + return -ENOMEM;
> + }
> +
> + if (i2c->dev_comp->support_33bits) {
> + reg_4g_mode = mtk_i2c_set_4g_mode(wpaddr);
> + writel(reg_4g_mode, i2c->pdmabase + OFFSET_TX_4G_MODE);
> + }
> +
> writel((u32)wpaddr, i2c->pdmabase + OFFSET_TX_MEM_ADDR);
> writel(msgs->len, i2c->pdmabase + OFFSET_TX_LEN);
> } else {
> @@ -979,6 +1000,11 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
> msgs->len, DMA_FROM_DEVICE);
>
> i2c_put_dma_safe_msg_buf(dma_rd_buf, msgs, true);
> + } else if (i2c->op == I2C_MASTER_CONTINUOUS_WR) {
> + dma_unmap_single(i2c->dev, wpaddr,
> + msgs->len, DMA_TO_DEVICE);
> +
> + kfree(msgs->buf);
> } else {
> dma_unmap_single(i2c->dev, wpaddr, msgs->len,
> DMA_TO_DEVICE);
> @@ -1009,6 +1035,9 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> {
> int ret;
> int left_num = num;
> + int i, j;
> + u8 *dma_multi_wr_buf;
> + struct i2c_msg multi_msg[1];
> struct mtk_i2c *i2c = i2c_get_adapdata(adap);
>
> ret = mtk_i2c_clock_enable(i2c);
> @@ -1025,6 +1054,43 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> }
> }
>
> + if (num > 1 && !(msgs[0].flags & I2C_M_RD)) {
> + for (i = 0; i < num - 1; i++) {
> + if (!(msgs[i+1].flags & I2C_M_RD) &&
> + (msgs[i].addr == msgs[i+1].addr)
> + && (msgs[i].len == msgs[i+1].len)) {
> + continue;
Don't need () for addr/len check.
When wrap, please put operator at end of previous line.
The 2nd line of if is at the same indent level with continue, that
doesn't look good. Let's use 4 spaces as indent for addr and len check,
so this should be:
if (num > 1 && !(msgs[0].flags & I2C_M_RD)) {
for (i = 0; i < num - 1; i++) {
if (!(msgs[i+1].flags & I2C_M_RD) &&
msgs[i].addr == msgs[i+1].addr &&
msgs[i].len == msgs[i+1].len) {
Joe.C
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-30 7:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-19 8:06 [PATCH] i2c: mediatek: Add to support continuous mode Qiangming Xia
2020-06-20 1:53 ` Qii Wang
2020-06-20 7:30 ` Yingjoe Chen
2020-06-29 2:00 ` Qii Wang
[not found] <20200623074245.24513-1-qiangming.xia@mediatek.com>
2020-06-30 7:33 ` Yingjoe Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).