Hi Kamal, Florian,

On 3 Mar 2017, at 3:48 AM, Florian Fainelli [off-list ref] wrote:

On 03/02/2017 10:42 AM, Kamal Dasu wrote:

quoted

Jaedon,

On Fri, Aug 12, 2016 at 1:56 AM, Jaedon Shin [off-list ref] wrote:

quoted

Fixes always repeated START when process multiple bytes with a message
in combined transactions.

The BSC has multiple data stroage that send or receive data at once, and
it has the RESTART, NOSTART, NOSTOP to the condition flags. The problem
is that begin repeated START for all the messages in combined
transaction. If length of a data is over 32bytes, The BSC transmit
repeated START in every 32bytes.

Can you rephrase the commit message to something  like below ?

The BSC data buffers to send and receive data are each of size 32 bytes
or 8 bytes 'xfersz' depending on SoC. The problem observed for all the
combined message transfer was if length of data transfer was a multiple of
'xfersz'  a repeated START was being transmitted by BSC driver. Fixed this
by appropriately setting START/STOP conditions for such transfers.

No problem.

quoted

quoted

Fixes always repeated START when process multiple bytes with a message
in combined transactions.

The BSC has multiple data stroage that send or receive data at once, and
it has the RESTART, NOSTART, NOSTOP to the condition flags. The problem
is that begin repeated START for all the messages in combined
transaction. If length of a data is over 32bytes, The BSC transmit
repeated START in every 32bytes.

The changes looks good to me.

Do you mind adding:

Fixes: dd1aa2524bc5 ("i2c: brcmstb: Add Broadcom settop SoC i2c
controller driver")

Add that also.

Thanks,
Jaedon

so this can be backported to -stable tree as well? Thanks!

quoted

quoted

Signed-off-by: Jaedon Shin <redacted>
---
drivers/i2c/busses/i2c-brcmstb.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-brcmstb.c b/drivers/i2c/busses/i2c-brcmstb.c
index 3f5a4d71d3bf..51c5f0bd361d 100644
--- a/drivers/i2c/busses/i2c-brcmstb.c
+++ b/drivers/i2c/busses/i2c-brcmstb.c

@@ -465,6 +465,7 @@ static int brcmstb_i2c_xfer(struct i2c_adapter *adapter,
       u8 *tmp_buf;
       int len = 0;
       int xfersz = brcmstb_i2c_get_xfersz(dev);
+       u32 cond, cond_per_msg;

       if (dev->is_suspended)
               return -EBUSY;

@@ -481,10 +482,11 @@ static int brcmstb_i2c_xfer(struct i2c_adapter *adapter,
                       pmsg->buf ? pmsg->buf[0] : '0', pmsg->len);

               if (i < (num - 1) && (msgs[i + 1].flags & I2C_M_NOSTART))
-                       brcmstb_set_i2c_start_stop(dev, ~(COND_START_STOP));
+                       cond = ~COND_START_STOP;
               else
-                       brcmstb_set_i2c_start_stop(dev,
-                                                  COND_RESTART | COND_NOSTOP);
+                       cond = COND_RESTART | COND_NOSTOP;
+
+               brcmstb_set_i2c_start_stop(dev, cond);

               /* Send slave address */
               if (!(pmsg->flags & I2C_M_NOSTART)) {

@@ -497,13 +499,24 @@ static int brcmstb_i2c_xfer(struct i2c_adapter *adapter,
                       }
               }

+               cond_per_msg = cond;
+
               /* Perform data transfer */
               while (len) {
                       bytes_to_xfer = min(len, xfersz);

-                       if (len <= xfersz && i == (num - 1))
-                               brcmstb_set_i2c_start_stop(dev,
-                                                          ~(COND_START_STOP));
+                       if (len <= xfersz) {
+                               if (i == (num - 1))
+                                       cond_per_msg = cond_per_msg &
+                                               ~(COND_RESTART | COND_NOSTOP);
+                               else
+                                       cond_per_msg = cond;
+                       } else {
+                               cond_per_msg = (cond_per_msg & ~COND_RESTART) |
+                                       COND_NOSTOP;
+                       }
+
+                       brcmstb_set_i2c_start_stop(dev, cond_per_msg);

                       rc = brcmstb_i2c_xfer_bsc_data(dev, tmp_buf,
                                                      bytes_to_xfer, pmsg);

@@ -512,6 +525,8 @@ static int brcmstb_i2c_xfer(struct i2c_adapter *adapter,

                       len -=  bytes_to_xfer;
                       tmp_buf += bytes_to_xfer;
+
+                       cond_per_msg = COND_NOSTART | COND_NOSTOP;
               }
       }

--

2.9.2

Thanks
Kamal

-- 
Florian