Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1265375 > unrolled thread

[PATCH v2 0/2] Mediatek I2C Fixup

Started byLiguo Zhang <liguo.zhang@mediatek.com>
First post2015-11-09 06:50 +0100
Last post2015-11-10 02:00 +0100
Articles 5 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/2] Mediatek I2C Fixup  Liguo Zhang <liguo.zhang@mediatek.com> - 2015-11-09 06:50 +0100
    [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization Liguo Zhang <liguo.zhang@mediatek.com> - 2015-11-09 06:50 +0100
      Re: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization Andy Shevchenko <andy.shevchenko@gmail.com> - 2015-11-09 15:30 +0100
        Re: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization Daniel Kurtz <djkurtz@chromium.org> - 2015-11-10 01:40 +0100
        Re: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read  optimization Yingjoe Chen <yingjoe.chen@mediatek.com> - 2015-11-10 02:00 +0100

#1265375 — [PATCH v2 0/2] Mediatek I2C Fixup

FromLiguo Zhang <liguo.zhang@mediatek.com>
Date2015-11-09 06:50 +0100
Subject[PATCH v2 0/2] Mediatek I2C Fixup
Message-ID<qsNv4-2fT-17@gated-at.bofh.it>
This series contain two patches, first is to optimize Mediatek I2C driver to use WRRD
if hardware support auto restart. Because auto restart will issue auto restart
interrupt, change to use WRRD can reduce interrupt latency. The second is to fix
multi transfer error in high speed mode. If hardware support auto restart, need driver
to send master code first.

Change in v2:
fix i2c checkpatch error.

Liguo Zhang (2):
  i2c: mediatek: add i2c first write then read optimization
  i2c: mediatek: fix i2c multi transfer issue in high speed mode

 drivers/i2c/busses/i2c-mt65xx.c | 78 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 6 deletions(-)

--
1.8.1.1.dirty

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1265378 — [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization

FromLiguo Zhang <liguo.zhang@mediatek.com>
Date2015-11-09 06:50 +0100
Subject[PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization
Message-ID<qsNv4-2fT-21@gated-at.bofh.it>
In reply to#1265375
For platform with auto restart support, between every transfer,
i2c controller will trigger an interrupt and SW need to handle
it to start new transfer. When doing write-then-read transfer,
instead of restart mechanism, using WRRD mode to have controller
send both transfer in one request to reduce latency.

Signed-off-by: Liguo Zhang <liguo.zhang@mediatek.com>
Reviewed-by: Eddie Huang <eddie.huang@mediatek.com>
---
 drivers/i2c/busses/i2c-mt65xx.c | 33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 9b86716..dc4aac6 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -132,6 +132,7 @@ struct mtk_i2c_compatible {
 	unsigned char pmic_i2c: 1;
 	unsigned char dcm: 1;
 	unsigned char auto_restart: 1;
+	unsigned char aux_len_reg: 1;
 };
 
 struct mtk_i2c {
@@ -153,6 +154,7 @@ struct mtk_i2c {
 	enum mtk_trans_op op;
 	u16 timing_reg;
 	u16 high_speed_reg;
+	unsigned char auto_restart;
 	const struct mtk_i2c_compatible *dev_comp;
 };
 
@@ -178,6 +180,7 @@ static const struct mtk_i2c_compatible mt6577_compat = {
 	.pmic_i2c = 0,
 	.dcm = 1,
 	.auto_restart = 0,
+	.aux_len_reg = 0,
 };
 
 static const struct mtk_i2c_compatible mt6589_compat = {
@@ -185,6 +188,7 @@ static const struct mtk_i2c_compatible mt6589_compat = {
 	.pmic_i2c = 1,
 	.dcm = 0,
 	.auto_restart = 0,
+	.aux_len_reg = 0,
 };
 
 static const struct mtk_i2c_compatible mt8173_compat = {
@@ -192,6 +196,7 @@ static const struct mtk_i2c_compatible mt8173_compat = {
 	.pmic_i2c = 0,
 	.dcm = 1,
 	.auto_restart = 1,
+	.aux_len_reg = 1,
 };
 
 static const struct of_device_id mtk_i2c_of_match[] = {
@@ -373,7 +378,7 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
 
 	i2c->irq_stat = 0;
 
-	if (i2c->dev_comp->auto_restart)
+	if (i2c->auto_restart)
 		restart_flag = I2C_RS_TRANSFER;
 
 	reinit_completion(&i2c->msg_complete);
@@ -411,8 +416,14 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
 
 	/* Set transfer and transaction len */
 	if (i2c->op == I2C_MASTER_WRRD) {
-		writew(msgs->len | ((msgs + 1)->len) << 8,
-		       i2c->base + OFFSET_TRANSFER_LEN);
+		if (i2c->dev_comp->aux_len_reg) {
+			writew(msgs->len, i2c->base + OFFSET_TRANSFER_LEN);
+			writew((msgs + 1)->len, i2c->base +
+			       OFFSET_TRANSFER_LEN_AUX);
+		} else {
+			writew(msgs->len | ((msgs + 1)->len) << 8,
+			       i2c->base + OFFSET_TRANSFER_LEN);
+		}
 		writew(I2C_WRRD_TRANAC_VALUE, i2c->base + OFFSET_TRANSAC_LEN);
 	} else {
 		writew(msgs->len, i2c->base + OFFSET_TRANSFER_LEN);
@@ -461,7 +472,7 @@ static int mtk_i2c_do_transfer(struct mtk_i2c *i2c, struct i2c_msg *msgs,
 
 	writel(I2C_DMA_START_EN, i2c->pdmabase + OFFSET_EN);
 
-	if (!i2c->dev_comp->auto_restart) {
+	if (!i2c->auto_restart) {
 		start_reg = I2C_TRANSAC_START;
 	} else {
 		start_reg = I2C_TRANSAC_START | I2C_RS_MUL_TRIG;
@@ -518,6 +529,16 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
 	if (ret)
 		return ret;
 
+	i2c->auto_restart = i2c->dev_comp->auto_restart;
+
+	/* checking if we can skip restart and optimize using WRRD mode */
+	if (i2c->auto_restart && num == 2) {
+		if (!(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
+		    msgs[0].addr == msgs[1].addr) {
+			i2c->auto_restart = 0;
+		}
+	}
+
 	while (left_num--) {
 		if (!msgs->buf) {
 			dev_dbg(i2c->dev, "data buffer is NULL.\n");
@@ -530,7 +551,7 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
 		else
 			i2c->op = I2C_MASTER_WR;
 
-		if (!i2c->dev_comp->auto_restart) {
+		if (!i2c->auto_restart) {
 			if (num > 1) {
 				/* combined two messages into one transaction */
 				i2c->op = I2C_MASTER_WRRD;
@@ -559,7 +580,7 @@ static irqreturn_t mtk_i2c_irq(int irqno, void *dev_id)
 	u16 restart_flag = 0;
 	u16 intr_stat;
 
-	if (i2c->dev_comp->auto_restart)
+	if (i2c->auto_restart)
 		restart_flag = I2C_RS_TRANSFER;
 
 	intr_stat = readw(i2c->base + OFFSET_INTR_STAT);
-- 
1.8.1.1.dirty

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1265713 — Re: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2015-11-09 15:30 +0100
SubjectRe: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization
Message-ID<qsVCi-7yE-9@gated-at.bofh.it>
In reply to#1265378
On Mon, Nov 9, 2015 at 7:43 AM, Liguo Zhang <liguo.zhang@mediatek.com> wrote:
> For platform with auto restart support, between every transfer,
> i2c controller will trigger an interrupt and SW need to handle
> it to start new transfer. When doing write-then-read transfer,
> instead of restart mechanism, using WRRD mode to have controller
> send both transfer in one request to reduce latency.


> @@ -518,6 +529,16 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
>         if (ret)
>                 return ret;
>
> +       i2c->auto_restart = i2c->dev_comp->auto_restart;
> +
> +       /* checking if we can skip restart and optimize using WRRD mode */
> +       if (i2c->auto_restart && num == 2) {
> +               if (!(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
> +                   msgs[0].addr == msgs[1].addr) {

Nitpick (optional):

((msgs[0].flags & msgs[1].flags) & I2C_M_RD)
?

> +                       i2c->auto_restart = 0;
> +               }
> +       }

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266125 — Re: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization

FromDaniel Kurtz <djkurtz@chromium.org>
Date2015-11-10 01:40 +0100
SubjectRe: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization
Message-ID<qt58B-68l-9@gated-at.bofh.it>
In reply to#1265713
On Mon, Nov 9, 2015 at 10:25 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Nov 9, 2015 at 7:43 AM, Liguo Zhang <liguo.zhang@mediatek.com> wrote:
>> For platform with auto restart support, between every transfer,
>> i2c controller will trigger an interrupt and SW need to handle
>> it to start new transfer. When doing write-then-read transfer,
>> instead of restart mechanism, using WRRD mode to have controller
>> send both transfer in one request to reduce latency.
>
>
>> @@ -518,6 +529,16 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
>>         if (ret)
>>                 return ret;
>>
>> +       i2c->auto_restart = i2c->dev_comp->auto_restart;
>> +
>> +       /* checking if we can skip restart and optimize using WRRD mode */
>> +       if (i2c->auto_restart && num == 2) {
>> +               if (!(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
>> +                   msgs[0].addr == msgs[1].addr) {
>
> Nitpick (optional):
>
> ((msgs[0].flags & msgs[1].flags) & I2C_M_RD)
> ?

IMHO, this makes the code less readable.
Leave this to the compiler's optimizer.

-Dan

>
>> +                       i2c->auto_restart = 0;
>> +               }
>> +       }
>
> --
> With Best Regards,
> Andy Shevchenko
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1266134 — Re: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization

FromYingjoe Chen <yingjoe.chen@mediatek.com>
Date2015-11-10 02:00 +0100
SubjectRe: [PATCH v2 1/2] i2c: mediatek: add i2c first write then read optimization
Message-ID<qt5rX-6jy-1@gated-at.bofh.it>
In reply to#1265713
On Mon, 2015-11-09 at 16:25 +0200, Andy Shevchenko wrote:
> On Mon, Nov 9, 2015 at 7:43 AM, Liguo Zhang <liguo.zhang@mediatek.com> wrote:
> > For platform with auto restart support, between every transfer,
> > i2c controller will trigger an interrupt and SW need to handle
> > it to start new transfer. When doing write-then-read transfer,
> > instead of restart mechanism, using WRRD mode to have controller
> > send both transfer in one request to reduce latency.
> 
> 
> > @@ -518,6 +529,16 @@ static int mtk_i2c_transfer(struct i2c_adapter *adap,
> >         if (ret)
> >                 return ret;
> >
> > +       i2c->auto_restart = i2c->dev_comp->auto_restart;
> > +
> > +       /* checking if we can skip restart and optimize using WRRD mode */
> > +       if (i2c->auto_restart && num == 2) {
> > +               if (!(msgs[0].flags & I2C_M_RD) && (msgs[1].flags & I2C_M_RD) &&
> > +                   msgs[0].addr == msgs[1].addr) {
> 
> Nitpick (optional):
> 
> ((msgs[0].flags & msgs[1].flags) & I2C_M_RD)
> ?

These 2 check for different conditions.
The original one check the first one must NOT set I2C_M_RD, but second
one must set I2C_M_RD.

Joe.C


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web