* [PATCH] i2c: brcmstb: Fix START and STOP conditions @ 2016-08-12 5:56 Jaedon Shin 2017-03-02 18:42 ` Kamal Dasu 0 siblings, 1 reply; 4+ messages in thread From: Jaedon Shin @ 2016-08-12 5:56 UTC (permalink / raw) To: Wolfram Sang Cc: Kamal Dasu, Florian Fainelli, bcm-kernel-feedback-list, linux-i2c, Jaedon Shin 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. Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com> --- 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: brcmstb: Fix START and STOP conditions 2016-08-12 5:56 [PATCH] i2c: brcmstb: Fix START and STOP conditions Jaedon Shin @ 2017-03-02 18:42 ` Kamal Dasu 2017-03-02 18:48 ` Florian Fainelli 0 siblings, 1 reply; 4+ messages in thread From: Kamal Dasu @ 2017-03-02 18:42 UTC (permalink / raw) To: Jaedon Shin Cc: Wolfram Sang, Kamal Dasu, Florian Fainelli, bcm-kernel-feedback-list, linux-i2c Jaedon, On Fri, Aug 12, 2016 at 1:56 AM, Jaedon Shin <jaedon.shin@gmail.com> wrote: > 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. > 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. > Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com> > --- > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: brcmstb: Fix START and STOP conditions 2017-03-02 18:42 ` Kamal Dasu @ 2017-03-02 18:48 ` Florian Fainelli 2017-03-03 1:30 ` Jaedon Shin 0 siblings, 1 reply; 4+ messages in thread From: Florian Fainelli @ 2017-03-02 18:48 UTC (permalink / raw) To: Kamal Dasu, Jaedon Shin Cc: Wolfram Sang, Kamal Dasu, bcm-kernel-feedback-list, linux-i2c On 03/02/2017 10:42 AM, Kamal Dasu wrote: > Jaedon, > > On Fri, Aug 12, 2016 at 1:56 AM, Jaedon Shin <jaedon.shin@gmail.com> wrote: >> 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. > >> 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") so this can be backported to -stable tree as well? Thanks! > >> Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com> >> --- >> 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] i2c: brcmstb: Fix START and STOP conditions 2017-03-02 18:48 ` Florian Fainelli @ 2017-03-03 1:30 ` Jaedon Shin 0 siblings, 0 replies; 4+ messages in thread From: Jaedon Shin @ 2017-03-03 1:30 UTC (permalink / raw) To: Florian Fainelli Cc: Kamal Dasu, Wolfram Sang, Kamal Dasu, bcm-kernel-feedback-list, linux-i2c Hi Kamal, Florian, > On 3 Mar 2017, at 3:48 AM, Florian Fainelli <f.fainelli@gmail.com> wrote: > > On 03/02/2017 10:42 AM, Kamal Dasu wrote: >> Jaedon, >> >> On Fri, Aug 12, 2016 at 1:56 AM, Jaedon Shin <jaedon.shin@gmail.com> wrote: >>> 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. >>> 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! > >> >>> Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com> >>> --- >>> 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-03-03 1:32 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-12 5:56 [PATCH] i2c: brcmstb: Fix START and STOP conditions Jaedon Shin 2017-03-02 18:42 ` Kamal Dasu 2017-03-02 18:48 ` Florian Fainelli 2017-03-03 1:30 ` Jaedon Shin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.