linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
@ 2019-09-30  6:44 Rayagonda Kokatanur
  2019-10-10  9:32 ` Rayagonda Kokatanur
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Rayagonda Kokatanur @ 2019-09-30  6:44 UTC (permalink / raw)
  To: Ray Jui, Scott Branden, bcm-kernel-feedback-list, Wolfram Sang,
	Michael Cheng, Shreesha Rajashekar, Lori Hikichi, linux-i2c,
	linux-arm-kernel, linux-kernel
  Cc: Rayagonda Kokatanur, Icarus Chau, Ray Jui, Shivaraj Shetty

From: Lori Hikichi <lori.hikichi@broadcom.com>

Enable handling of i2c repeated start. The current code
handles a multi msg i2c transfer as separate i2c bus
transactions. This change will now handle this case
using the i2c repeated start protocol. The number of msgs
in a transfer is limited to two, and must be a write
followed by a read.

Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>
---
changes from v1:
 - Address following review comments from Wolfarm Sang,
   Use i2c_8bit_addr_from_msg() api instead of decoding i2c_msg struct and
   remove check against number of i2c message as it will be taken care
   by core using quirks flags. 

 drivers/i2c/busses/i2c-bcm-iproc.c | 63 ++++++++++++++++++++++++++++++--------
 1 file changed, 50 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index d7fd76b..e478db7 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -81,6 +81,7 @@
 #define M_CMD_PROTOCOL_MASK          0xf
 #define M_CMD_PROTOCOL_BLK_WR        0x7
 #define M_CMD_PROTOCOL_BLK_RD        0x8
+#define M_CMD_PROTOCOL_PROCESS       0xa
 #define M_CMD_PEC_SHIFT              8
 #define M_CMD_RD_CNT_SHIFT           0
 #define M_CMD_RD_CNT_MASK            0xff
@@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
 	return 0;
 }
 
-static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
-					 struct i2c_msg *msg)
+/*
+ * If 'process_call' is true, then this is a multi-msg transfer that requires
+ * a repeated start between the messages.
+ * More specifically, it must be a write (reg) followed by a read (data).
+ * The i2c quirks are set to enforce this rule.
+ */
+static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
+					struct i2c_msg *msgs, bool process_call)
 {
 	int i;
 	u8 addr;
 	u32 val, tmp, val_intr_en;
 	unsigned int tx_bytes;
+	struct i2c_msg *msg = &msgs[0];
 
 	/* check if bus is busy */
 	if (!!(iproc_i2c_rd_reg(iproc_i2c,
@@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 			val = msg->buf[i];
 
 			/* mark the last byte */
-			if (i == msg->len - 1)
-				val |= BIT(M_TX_WR_STATUS_SHIFT);
+			if (!process_call && (i == msg->len - 1))
+				val |= 1 << M_TX_WR_STATUS_SHIFT;
 
 			iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
 		}
 		iproc_i2c->tx_bytes = tx_bytes;
 	}
 
+	/* Process the read message if this is process call */
+	if (process_call) {
+		msg++;
+		iproc_i2c->msg = msg;  /* point to second msg */
+
+		/*
+		 * The last byte to be sent out should be a slave
+		 * address with read operation
+		 */
+		addr = i2c_8bit_addr_from_msg(msg);
+		/* mark it the last byte out */
+		val = addr | (1 << M_TX_WR_STATUS_SHIFT);
+		iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
+	}
+
 	/* mark as incomplete before starting the transaction */
 	if (iproc_i2c->irq)
 		reinit_completion(&iproc_i2c->done);
@@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 	 * underrun interrupt, which will be triggerred when the TX FIFO is
 	 * empty. When that happens we can then pump more data into the FIFO
 	 */
-	if (!(msg->flags & I2C_M_RD) &&
+	if (!process_call && !(msg->flags & I2C_M_RD) &&
 	    msg->len > iproc_i2c->tx_bytes)
 		val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
 
@@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 	 */
 	val = BIT(M_CMD_START_BUSY_SHIFT);
 	if (msg->flags & I2C_M_RD) {
+		u32 protocol;
+
 		iproc_i2c->rx_bytes = 0;
 		if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
 			iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
@@ -758,7 +783,10 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 		/* enable the RX threshold interrupt */
 		val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
 
-		val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
+		protocol = process_call ?
+				M_CMD_PROTOCOL_PROCESS : M_CMD_PROTOCOL_BLK_RD;
+
+		val |= (protocol << M_CMD_PROTOCOL_SHIFT) |
 		       (msg->len << M_CMD_RD_CNT_SHIFT);
 	} else {
 		val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOCOL_SHIFT);
@@ -774,17 +802,24 @@ static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
 			      struct i2c_msg msgs[], int num)
 {
 	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adapter);
-	int ret, i;
+	bool process_call = false;
+	int ret;
 
-	/* go through all messages */
-	for (i = 0; i < num; i++) {
-		ret = bcm_iproc_i2c_xfer_single_msg(iproc_i2c, &msgs[i]);
-		if (ret) {
-			dev_dbg(iproc_i2c->device, "xfer failed\n");
-			return ret;
+	if (num == 2) {
+		/* Repeated start, use process call */
+		process_call = true;
+		if (msgs[1].flags & I2C_M_NOSTART) {
+			dev_err(iproc_i2c->device, "Invalid repeated start\n");
+			return -EOPNOTSUPP;
 		}
 	}
 
+	ret = bcm_iproc_i2c_xfer_internal(iproc_i2c, msgs, process_call);
+	if (ret) {
+		dev_dbg(iproc_i2c->device, "xfer failed\n");
+		return ret;
+	}
+
 	return num;
 }
 
@@ -806,6 +841,8 @@ static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
 };
 
 static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
+	.flags = I2C_AQ_COMB_WRITE_THEN_READ,
+	.max_comb_1st_msg_len = M_TX_RX_FIFO_SIZE,
 	.max_read_len = M_RX_MAX_READ_LEN,
 };
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
  2019-09-30  6:44 [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability Rayagonda Kokatanur
@ 2019-10-10  9:32 ` Rayagonda Kokatanur
  2019-10-17  5:58   ` Rayagonda Kokatanur
  2019-10-24 18:55 ` Wolfram Sang
  2019-11-17 11:24 ` Wolfram Sang
  2 siblings, 1 reply; 9+ messages in thread
From: Rayagonda Kokatanur @ 2019-10-10  9:32 UTC (permalink / raw)
  To: Ray Jui, Scott Branden, BCM Kernel Feedback, Wolfram Sang,
	Michael Cheng, Shreesha Rajashekar, Lori Hikichi, linux-i2c,
	linux-arm-kernel, Linux Kernel Mailing List
  Cc: Icarus Chau, Ray Jui, Shivaraj Shetty

Hi Wolfram,

Did you get a chance to review this patch.

Best regards,
Rayagonda


On Mon, Sep 30, 2019 at 12:19 PM Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> From: Lori Hikichi <lori.hikichi@broadcom.com>
>
> Enable handling of i2c repeated start. The current code
> handles a multi msg i2c transfer as separate i2c bus
> transactions. This change will now handle this case
> using the i2c repeated start protocol. The number of msgs
> in a transfer is limited to two, and must be a write
> followed by a read.
>
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>
> ---
> changes from v1:
>  - Address following review comments from Wolfarm Sang,
>    Use i2c_8bit_addr_from_msg() api instead of decoding i2c_msg struct and
>    remove check against number of i2c message as it will be taken care
>    by core using quirks flags.
>
>  drivers/i2c/busses/i2c-bcm-iproc.c | 63 ++++++++++++++++++++++++++++++--------
>  1 file changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index d7fd76b..e478db7 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -81,6 +81,7 @@
>  #define M_CMD_PROTOCOL_MASK          0xf
>  #define M_CMD_PROTOCOL_BLK_WR        0x7
>  #define M_CMD_PROTOCOL_BLK_RD        0x8
> +#define M_CMD_PROTOCOL_PROCESS       0xa
>  #define M_CMD_PEC_SHIFT              8
>  #define M_CMD_RD_CNT_SHIFT           0
>  #define M_CMD_RD_CNT_MASK            0xff
> @@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
>         return 0;
>  }
>
> -static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
> -                                        struct i2c_msg *msg)
> +/*
> + * If 'process_call' is true, then this is a multi-msg transfer that requires
> + * a repeated start between the messages.
> + * More specifically, it must be a write (reg) followed by a read (data).
> + * The i2c quirks are set to enforce this rule.
> + */
> +static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
> +                                       struct i2c_msg *msgs, bool process_call)
>  {
>         int i;
>         u8 addr;
>         u32 val, tmp, val_intr_en;
>         unsigned int tx_bytes;
> +       struct i2c_msg *msg = &msgs[0];
>
>         /* check if bus is busy */
>         if (!!(iproc_i2c_rd_reg(iproc_i2c,
> @@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>                         val = msg->buf[i];
>
>                         /* mark the last byte */
> -                       if (i == msg->len - 1)
> -                               val |= BIT(M_TX_WR_STATUS_SHIFT);
> +                       if (!process_call && (i == msg->len - 1))
> +                               val |= 1 << M_TX_WR_STATUS_SHIFT;
>
>                         iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
>                 }
>                 iproc_i2c->tx_bytes = tx_bytes;
>         }
>
> +       /* Process the read message if this is process call */
> +       if (process_call) {
> +               msg++;
> +               iproc_i2c->msg = msg;  /* point to second msg */
> +
> +               /*
> +                * The last byte to be sent out should be a slave
> +                * address with read operation
> +                */
> +               addr = i2c_8bit_addr_from_msg(msg);
> +               /* mark it the last byte out */
> +               val = addr | (1 << M_TX_WR_STATUS_SHIFT);
> +               iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
> +       }
> +
>         /* mark as incomplete before starting the transaction */
>         if (iproc_i2c->irq)
>                 reinit_completion(&iproc_i2c->done);
> @@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>          * underrun interrupt, which will be triggerred when the TX FIFO is
>          * empty. When that happens we can then pump more data into the FIFO
>          */
> -       if (!(msg->flags & I2C_M_RD) &&
> +       if (!process_call && !(msg->flags & I2C_M_RD) &&
>             msg->len > iproc_i2c->tx_bytes)
>                 val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
>
> @@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>          */
>         val = BIT(M_CMD_START_BUSY_SHIFT);
>         if (msg->flags & I2C_M_RD) {
> +               u32 protocol;
> +
>                 iproc_i2c->rx_bytes = 0;
>                 if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
>                         iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
> @@ -758,7 +783,10 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
>                 /* enable the RX threshold interrupt */
>                 val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
>
> -               val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
> +               protocol = process_call ?
> +                               M_CMD_PROTOCOL_PROCESS : M_CMD_PROTOCOL_BLK_RD;
> +
> +               val |= (protocol << M_CMD_PROTOCOL_SHIFT) |
>                        (msg->len << M_CMD_RD_CNT_SHIFT);
>         } else {
>                 val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOCOL_SHIFT);
> @@ -774,17 +802,24 @@ static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
>                               struct i2c_msg msgs[], int num)
>  {
>         struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adapter);
> -       int ret, i;
> +       bool process_call = false;
> +       int ret;
>
> -       /* go through all messages */
> -       for (i = 0; i < num; i++) {
> -               ret = bcm_iproc_i2c_xfer_single_msg(iproc_i2c, &msgs[i]);
> -               if (ret) {
> -                       dev_dbg(iproc_i2c->device, "xfer failed\n");
> -                       return ret;
> +       if (num == 2) {
> +               /* Repeated start, use process call */
> +               process_call = true;
> +               if (msgs[1].flags & I2C_M_NOSTART) {
> +                       dev_err(iproc_i2c->device, "Invalid repeated start\n");
> +                       return -EOPNOTSUPP;
>                 }
>         }
>
> +       ret = bcm_iproc_i2c_xfer_internal(iproc_i2c, msgs, process_call);
> +       if (ret) {
> +               dev_dbg(iproc_i2c->device, "xfer failed\n");
> +               return ret;
> +       }
> +
>         return num;
>  }
>
> @@ -806,6 +841,8 @@ static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
>  };
>
>  static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> +       .flags = I2C_AQ_COMB_WRITE_THEN_READ,
> +       .max_comb_1st_msg_len = M_TX_RX_FIFO_SIZE,
>         .max_read_len = M_RX_MAX_READ_LEN,
>  };
>
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
  2019-10-10  9:32 ` Rayagonda Kokatanur
@ 2019-10-17  5:58   ` Rayagonda Kokatanur
  0 siblings, 0 replies; 9+ messages in thread
From: Rayagonda Kokatanur @ 2019-10-17  5:58 UTC (permalink / raw)
  To: Ray Jui, Scott Branden, BCM Kernel Feedback, Wolfram Sang,
	Michael Cheng, Shreesha Rajashekar, Lori Hikichi, linux-i2c,
	linux-arm-kernel, Linux Kernel Mailing List
  Cc: Icarus Chau, Ray Jui, Shivaraj Shetty

Hi Wolfram,

Please review the patch and let me know if you still have any review comments.

Best regards,
Rayagonda

On Thu, Oct 10, 2019 at 3:02 PM Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> Hi Wolfram,
>
> Did you get a chance to review this patch.
>
> Best regards,
> Rayagonda
>
>
> On Mon, Sep 30, 2019 at 12:19 PM Rayagonda Kokatanur
> <rayagonda.kokatanur@broadcom.com> wrote:
> >
> > From: Lori Hikichi <lori.hikichi@broadcom.com>
> >
> > Enable handling of i2c repeated start. The current code
> > handles a multi msg i2c transfer as separate i2c bus
> > transactions. This change will now handle this case
> > using the i2c repeated start protocol. The number of msgs
> > in a transfer is limited to two, and must be a write
> > followed by a read.
> >
> > Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> > Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
> > Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> > Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>
> > ---
> > changes from v1:
> >  - Address following review comments from Wolfarm Sang,
> >    Use i2c_8bit_addr_from_msg() api instead of decoding i2c_msg struct and
> >    remove check against number of i2c message as it will be taken care
> >    by core using quirks flags.
> >
> >  drivers/i2c/busses/i2c-bcm-iproc.c | 63 ++++++++++++++++++++++++++++++--------
> >  1 file changed, 50 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> > index d7fd76b..e478db7 100644
> > --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> > +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> > @@ -81,6 +81,7 @@
> >  #define M_CMD_PROTOCOL_MASK          0xf
> >  #define M_CMD_PROTOCOL_BLK_WR        0x7
> >  #define M_CMD_PROTOCOL_BLK_RD        0x8
> > +#define M_CMD_PROTOCOL_PROCESS       0xa
> >  #define M_CMD_PEC_SHIFT              8
> >  #define M_CMD_RD_CNT_SHIFT           0
> >  #define M_CMD_RD_CNT_MASK            0xff
> > @@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
> >         return 0;
> >  }
> >
> > -static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
> > -                                        struct i2c_msg *msg)
> > +/*
> > + * If 'process_call' is true, then this is a multi-msg transfer that requires
> > + * a repeated start between the messages.
> > + * More specifically, it must be a write (reg) followed by a read (data).
> > + * The i2c quirks are set to enforce this rule.
> > + */
> > +static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
> > +                                       struct i2c_msg *msgs, bool process_call)
> >  {
> >         int i;
> >         u8 addr;
> >         u32 val, tmp, val_intr_en;
> >         unsigned int tx_bytes;
> > +       struct i2c_msg *msg = &msgs[0];
> >
> >         /* check if bus is busy */
> >         if (!!(iproc_i2c_rd_reg(iproc_i2c,
> > @@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
> >                         val = msg->buf[i];
> >
> >                         /* mark the last byte */
> > -                       if (i == msg->len - 1)
> > -                               val |= BIT(M_TX_WR_STATUS_SHIFT);
> > +                       if (!process_call && (i == msg->len - 1))
> > +                               val |= 1 << M_TX_WR_STATUS_SHIFT;
> >
> >                         iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
> >                 }
> >                 iproc_i2c->tx_bytes = tx_bytes;
> >         }
> >
> > +       /* Process the read message if this is process call */
> > +       if (process_call) {
> > +               msg++;
> > +               iproc_i2c->msg = msg;  /* point to second msg */
> > +
> > +               /*
> > +                * The last byte to be sent out should be a slave
> > +                * address with read operation
> > +                */
> > +               addr = i2c_8bit_addr_from_msg(msg);
> > +               /* mark it the last byte out */
> > +               val = addr | (1 << M_TX_WR_STATUS_SHIFT);
> > +               iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
> > +       }
> > +
> >         /* mark as incomplete before starting the transaction */
> >         if (iproc_i2c->irq)
> >                 reinit_completion(&iproc_i2c->done);
> > @@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
> >          * underrun interrupt, which will be triggerred when the TX FIFO is
> >          * empty. When that happens we can then pump more data into the FIFO
> >          */
> > -       if (!(msg->flags & I2C_M_RD) &&
> > +       if (!process_call && !(msg->flags & I2C_M_RD) &&
> >             msg->len > iproc_i2c->tx_bytes)
> >                 val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
> >
> > @@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
> >          */
> >         val = BIT(M_CMD_START_BUSY_SHIFT);
> >         if (msg->flags & I2C_M_RD) {
> > +               u32 protocol;
> > +
> >                 iproc_i2c->rx_bytes = 0;
> >                 if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
> >                         iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
> > @@ -758,7 +783,10 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
> >                 /* enable the RX threshold interrupt */
> >                 val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
> >
> > -               val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
> > +               protocol = process_call ?
> > +                               M_CMD_PROTOCOL_PROCESS : M_CMD_PROTOCOL_BLK_RD;
> > +
> > +               val |= (protocol << M_CMD_PROTOCOL_SHIFT) |
> >                        (msg->len << M_CMD_RD_CNT_SHIFT);
> >         } else {
> >                 val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOCOL_SHIFT);
> > @@ -774,17 +802,24 @@ static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
> >                               struct i2c_msg msgs[], int num)
> >  {
> >         struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adapter);
> > -       int ret, i;
> > +       bool process_call = false;
> > +       int ret;
> >
> > -       /* go through all messages */
> > -       for (i = 0; i < num; i++) {
> > -               ret = bcm_iproc_i2c_xfer_single_msg(iproc_i2c, &msgs[i]);
> > -               if (ret) {
> > -                       dev_dbg(iproc_i2c->device, "xfer failed\n");
> > -                       return ret;
> > +       if (num == 2) {
> > +               /* Repeated start, use process call */
> > +               process_call = true;
> > +               if (msgs[1].flags & I2C_M_NOSTART) {
> > +                       dev_err(iproc_i2c->device, "Invalid repeated start\n");
> > +                       return -EOPNOTSUPP;
> >                 }
> >         }
> >
> > +       ret = bcm_iproc_i2c_xfer_internal(iproc_i2c, msgs, process_call);
> > +       if (ret) {
> > +               dev_dbg(iproc_i2c->device, "xfer failed\n");
> > +               return ret;
> > +       }
> > +
> >         return num;
> >  }
> >
> > @@ -806,6 +841,8 @@ static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
> >  };
> >
> >  static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
> > +       .flags = I2C_AQ_COMB_WRITE_THEN_READ,
> > +       .max_comb_1st_msg_len = M_TX_RX_FIFO_SIZE,
> >         .max_read_len = M_RX_MAX_READ_LEN,
> >  };
> >
> > --
> > 1.9.1
> >

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
  2019-09-30  6:44 [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability Rayagonda Kokatanur
  2019-10-10  9:32 ` Rayagonda Kokatanur
@ 2019-10-24 18:55 ` Wolfram Sang
  2019-10-25  4:51   ` Rayagonda Kokatanur
  2019-11-17 11:24 ` Wolfram Sang
  2 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2019-10-24 18:55 UTC (permalink / raw)
  To: Rayagonda Kokatanur
  Cc: Ray Jui, Scott Branden, bcm-kernel-feedback-list, Michael Cheng,
	Shreesha Rajashekar, Lori Hikichi, linux-i2c, linux-arm-kernel,
	linux-kernel, Icarus Chau, Ray Jui, Shivaraj Shetty

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

On Mon, Sep 30, 2019 at 12:14:29PM +0530, Rayagonda Kokatanur wrote:
> From: Lori Hikichi <lori.hikichi@broadcom.com>
> 
> Enable handling of i2c repeated start. The current code
> handles a multi msg i2c transfer as separate i2c bus
> transactions. This change will now handle this case
> using the i2c repeated start protocol. The number of msgs
> in a transfer is limited to two, and must be a write
> followed by a read.
> 
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>

Patch looks good but doesn't apply for me on top of v5.4-rc4? What was
your base?

Also, I will apply it to for-next (v5.5). If you want it for v5.4, then
please add a Fixes tag.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
  2019-10-24 18:55 ` Wolfram Sang
@ 2019-10-25  4:51   ` Rayagonda Kokatanur
  0 siblings, 0 replies; 9+ messages in thread
From: Rayagonda Kokatanur @ 2019-10-25  4:51 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Ray Jui, Scott Branden, BCM Kernel Feedback, Michael Cheng,
	Shreesha Rajashekar, Lori Hikichi, linux-i2c, linux-arm-kernel,
	Linux Kernel Mailing List, Icarus Chau, Ray Jui, Shivaraj Shetty

On Fri, Oct 25, 2019 at 12:25 AM Wolfram Sang <wsa@the-dreams.de> wrote:
>
> On Mon, Sep 30, 2019 at 12:14:29PM +0530, Rayagonda Kokatanur wrote:
> > From: Lori Hikichi <lori.hikichi@broadcom.com>
> >
> > Enable handling of i2c repeated start. The current code
> > handles a multi msg i2c transfer as separate i2c bus
> > transactions. This change will now handle this case
> > using the i2c repeated start protocol. The number of msgs
> > in a transfer is limited to two, and must be a write
> > followed by a read.
> >
> > Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> > Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
> > Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> > Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>
>
> Patch looks good but doesn't apply for me on top of v5.4-rc4? What was
> your base?
>
> Also, I will apply it to for-next (v5.5). If you want it for v5.4, then
> please add a Fixes tag.
>
thank you, please apply it to for-next (v5.5).
Do I need to resend patch again for you to apply it to v5.5 ?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
  2019-09-30  6:44 [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability Rayagonda Kokatanur
  2019-10-10  9:32 ` Rayagonda Kokatanur
  2019-10-24 18:55 ` Wolfram Sang
@ 2019-11-17 11:24 ` Wolfram Sang
  2 siblings, 0 replies; 9+ messages in thread
From: Wolfram Sang @ 2019-11-17 11:24 UTC (permalink / raw)
  To: Rayagonda Kokatanur
  Cc: Ray Jui, Scott Branden, bcm-kernel-feedback-list, Michael Cheng,
	Shreesha Rajashekar, Lori Hikichi, linux-i2c, linux-arm-kernel,
	linux-kernel, Icarus Chau, Ray Jui, Shivaraj Shetty

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

On Mon, Sep 30, 2019 at 12:14:29PM +0530, Rayagonda Kokatanur wrote:
> From: Lori Hikichi <lori.hikichi@broadcom.com>
> 
> Enable handling of i2c repeated start. The current code
> handles a multi msg i2c transfer as separate i2c bus
> transactions. This change will now handle this case
> using the i2c repeated start protocol. The number of msgs
> in a transfer is limited to two, and must be a write
> followed by a read.
> 
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>

For the record, applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
  2019-09-28 18:23 ` Wolfram Sang
@ 2019-09-30  6:52   ` Rayagonda Kokatanur
  0 siblings, 0 replies; 9+ messages in thread
From: Rayagonda Kokatanur @ 2019-09-30  6:52 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Ray Jui, Scott Branden, BCM Kernel Feedback, Michael Cheng,
	Shreesha Rajashekar, Lori Hikichi, linux-i2c, linux-arm-kernel,
	Linux Kernel Mailing List, Icarus Chau, Ray Jui, Shivaraj Shetty

On Sat, Sep 28, 2019 at 11:53 PM Wolfram Sang <wsa@the-dreams.de> wrote:
>
> On Thu, Sep 26, 2019 at 10:10:08AM +0530, Rayagonda Kokatanur wrote:
> > From: Lori Hikichi <lori.hikichi@broadcom.com>
> >
> > Enable handling of i2c repeated start. The current code
> > handles a multi msg i2c transfer as separate i2c bus
> > transactions. This change will now handle this case
> > using the i2c repeated start protocol. The number of msgs
> > in a transfer is limited to two, and must be a write
> > followed by a read.
> >
> > Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> > Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> > Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
> > Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> > Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>
> > ---
> > changes from v1:
> >  - Address code review comment from Wolfram Sang
>
> No, sorry, this is not a proper changelog. I review so many patches, I
> can't recall what I suggested to do for every patch. Please describe
> what changes you actually made. It is also better when digging through
> mail archives.
>
Sorry for inconvenience, I updated the changelog and resent the patch.
I have kept the patch version as v2 only. Hope that is fine.

Best regards,
Rayagonda

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
  2019-09-26  4:40 Rayagonda Kokatanur
@ 2019-09-28 18:23 ` Wolfram Sang
  2019-09-30  6:52   ` Rayagonda Kokatanur
  0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2019-09-28 18:23 UTC (permalink / raw)
  To: Rayagonda Kokatanur
  Cc: Ray Jui, Scott Branden, bcm-kernel-feedback-list, Michael Cheng,
	Shreesha Rajashekar, Lori Hikichi, linux-i2c, linux-arm-kernel,
	linux-kernel, Icarus Chau, Ray Jui, Shivaraj Shetty

[-- Attachment #1: Type: text/plain, Size: 1047 bytes --]

On Thu, Sep 26, 2019 at 10:10:08AM +0530, Rayagonda Kokatanur wrote:
> From: Lori Hikichi <lori.hikichi@broadcom.com>
> 
> Enable handling of i2c repeated start. The current code
> handles a multi msg i2c transfer as separate i2c bus
> transactions. This change will now handle this case
> using the i2c repeated start protocol. The number of msgs
> in a transfer is limited to two, and must be a write
> followed by a read.
> 
> Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>
> ---
> changes from v1:
>  - Address code review comment from Wolfram Sang

No, sorry, this is not a proper changelog. I review so many patches, I
can't recall what I suggested to do for every patch. Please describe
what changes you actually made. It is also better when digging through
mail archives.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability
@ 2019-09-26  4:40 Rayagonda Kokatanur
  2019-09-28 18:23 ` Wolfram Sang
  0 siblings, 1 reply; 9+ messages in thread
From: Rayagonda Kokatanur @ 2019-09-26  4:40 UTC (permalink / raw)
  To: Ray Jui, Scott Branden, bcm-kernel-feedback-list, Wolfram Sang,
	Michael Cheng, Shreesha Rajashekar, Lori Hikichi, linux-i2c,
	linux-arm-kernel, linux-kernel
  Cc: Rayagonda Kokatanur, Icarus Chau, Ray Jui, Shivaraj Shetty

From: Lori Hikichi <lori.hikichi@broadcom.com>

Enable handling of i2c repeated start. The current code
handles a multi msg i2c transfer as separate i2c bus
transactions. This change will now handle this case
using the i2c repeated start protocol. The number of msgs
in a transfer is limited to two, and must be a write
followed by a read.

Signed-off-by: Lori Hikichi <lori.hikichi@broadcom.com>
Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
Signed-off-by: Icarus Chau <icarus.chau@broadcom.com>
Signed-off-by: Ray Jui <ray.jui@broadcom.com>
Signed-off-by: Shivaraj Shetty <sshetty1@broadcom.com>
---
changes from v1:
 - Address code review comment from Wolfram Sang

 drivers/i2c/busses/i2c-bcm-iproc.c | 63 ++++++++++++++++++++++++++++++--------
 1 file changed, 50 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index d7fd76b..e478db7 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -81,6 +81,7 @@
 #define M_CMD_PROTOCOL_MASK          0xf
 #define M_CMD_PROTOCOL_BLK_WR        0x7
 #define M_CMD_PROTOCOL_BLK_RD        0x8
+#define M_CMD_PROTOCOL_PROCESS       0xa
 #define M_CMD_PEC_SHIFT              8
 #define M_CMD_RD_CNT_SHIFT           0
 #define M_CMD_RD_CNT_MASK            0xff
@@ -675,13 +676,20 @@ static int bcm_iproc_i2c_xfer_wait(struct bcm_iproc_i2c_dev *iproc_i2c,
 	return 0;
 }
 
-static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
-					 struct i2c_msg *msg)
+/*
+ * If 'process_call' is true, then this is a multi-msg transfer that requires
+ * a repeated start between the messages.
+ * More specifically, it must be a write (reg) followed by a read (data).
+ * The i2c quirks are set to enforce this rule.
+ */
+static int bcm_iproc_i2c_xfer_internal(struct bcm_iproc_i2c_dev *iproc_i2c,
+					struct i2c_msg *msgs, bool process_call)
 {
 	int i;
 	u8 addr;
 	u32 val, tmp, val_intr_en;
 	unsigned int tx_bytes;
+	struct i2c_msg *msg = &msgs[0];
 
 	/* check if bus is busy */
 	if (!!(iproc_i2c_rd_reg(iproc_i2c,
@@ -707,14 +715,29 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 			val = msg->buf[i];
 
 			/* mark the last byte */
-			if (i == msg->len - 1)
-				val |= BIT(M_TX_WR_STATUS_SHIFT);
+			if (!process_call && (i == msg->len - 1))
+				val |= 1 << M_TX_WR_STATUS_SHIFT;
 
 			iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
 		}
 		iproc_i2c->tx_bytes = tx_bytes;
 	}
 
+	/* Process the read message if this is process call */
+	if (process_call) {
+		msg++;
+		iproc_i2c->msg = msg;  /* point to second msg */
+
+		/*
+		 * The last byte to be sent out should be a slave
+		 * address with read operation
+		 */
+		addr = i2c_8bit_addr_from_msg(msg);
+		/* mark it the last byte out */
+		val = addr | (1 << M_TX_WR_STATUS_SHIFT);
+		iproc_i2c_wr_reg(iproc_i2c, M_TX_OFFSET, val);
+	}
+
 	/* mark as incomplete before starting the transaction */
 	if (iproc_i2c->irq)
 		reinit_completion(&iproc_i2c->done);
@@ -733,7 +756,7 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 	 * underrun interrupt, which will be triggerred when the TX FIFO is
 	 * empty. When that happens we can then pump more data into the FIFO
 	 */
-	if (!(msg->flags & I2C_M_RD) &&
+	if (!process_call && !(msg->flags & I2C_M_RD) &&
 	    msg->len > iproc_i2c->tx_bytes)
 		val_intr_en |= BIT(IE_M_TX_UNDERRUN_SHIFT);
 
@@ -743,6 +766,8 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 	 */
 	val = BIT(M_CMD_START_BUSY_SHIFT);
 	if (msg->flags & I2C_M_RD) {
+		u32 protocol;
+
 		iproc_i2c->rx_bytes = 0;
 		if (msg->len > M_RX_FIFO_MAX_THLD_VALUE)
 			iproc_i2c->thld_bytes = M_RX_FIFO_THLD_VALUE;
@@ -758,7 +783,10 @@ static int bcm_iproc_i2c_xfer_single_msg(struct bcm_iproc_i2c_dev *iproc_i2c,
 		/* enable the RX threshold interrupt */
 		val_intr_en |= BIT(IE_M_RX_THLD_SHIFT);
 
-		val |= (M_CMD_PROTOCOL_BLK_RD << M_CMD_PROTOCOL_SHIFT) |
+		protocol = process_call ?
+				M_CMD_PROTOCOL_PROCESS : M_CMD_PROTOCOL_BLK_RD;
+
+		val |= (protocol << M_CMD_PROTOCOL_SHIFT) |
 		       (msg->len << M_CMD_RD_CNT_SHIFT);
 	} else {
 		val |= (M_CMD_PROTOCOL_BLK_WR << M_CMD_PROTOCOL_SHIFT);
@@ -774,17 +802,24 @@ static int bcm_iproc_i2c_xfer(struct i2c_adapter *adapter,
 			      struct i2c_msg msgs[], int num)
 {
 	struct bcm_iproc_i2c_dev *iproc_i2c = i2c_get_adapdata(adapter);
-	int ret, i;
+	bool process_call = false;
+	int ret;
 
-	/* go through all messages */
-	for (i = 0; i < num; i++) {
-		ret = bcm_iproc_i2c_xfer_single_msg(iproc_i2c, &msgs[i]);
-		if (ret) {
-			dev_dbg(iproc_i2c->device, "xfer failed\n");
-			return ret;
+	if (num == 2) {
+		/* Repeated start, use process call */
+		process_call = true;
+		if (msgs[1].flags & I2C_M_NOSTART) {
+			dev_err(iproc_i2c->device, "Invalid repeated start\n");
+			return -EOPNOTSUPP;
 		}
 	}
 
+	ret = bcm_iproc_i2c_xfer_internal(iproc_i2c, msgs, process_call);
+	if (ret) {
+		dev_dbg(iproc_i2c->device, "xfer failed\n");
+		return ret;
+	}
+
 	return num;
 }
 
@@ -806,6 +841,8 @@ static uint32_t bcm_iproc_i2c_functionality(struct i2c_adapter *adap)
 };
 
 static struct i2c_adapter_quirks bcm_iproc_i2c_quirks = {
+	.flags = I2C_AQ_COMB_WRITE_THEN_READ,
+	.max_comb_1st_msg_len = M_TX_RX_FIFO_SIZE,
 	.max_read_len = M_RX_MAX_READ_LEN,
 };
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-11-17 11:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-30  6:44 [PATCH v2 1/1] i2c: iproc: Add i2c repeated start capability Rayagonda Kokatanur
2019-10-10  9:32 ` Rayagonda Kokatanur
2019-10-17  5:58   ` Rayagonda Kokatanur
2019-10-24 18:55 ` Wolfram Sang
2019-10-25  4:51   ` Rayagonda Kokatanur
2019-11-17 11:24 ` Wolfram Sang
  -- strict thread matches above, loose matches on Subject: below --
2019-09-26  4:40 Rayagonda Kokatanur
2019-09-28 18:23 ` Wolfram Sang
2019-09-30  6:52   ` Rayagonda Kokatanur

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).