* [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.