[U-Boot] [PATCH v3 1/2] xilinx_xiic: Fix fill tx fifo loop

Comparison should be against the actual message length, not loop index.
len is used for stopping while loop, pos is position in message. stop should be sent when entire message is sent, not when len and pos meet.
Signed-off-by: Tomas Melin tomas.melin@vaisala.com --- Changes in v2: - Added reasoning to commit message
Changes in v3: - None
drivers/i2c/xilinx_xiic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/xilinx_xiic.c b/drivers/i2c/xilinx_xiic.c index 83114ed510..e4ca0ab936 100644 --- a/drivers/i2c/xilinx_xiic.c +++ b/drivers/i2c/xilinx_xiic.c @@ -149,7 +149,7 @@ static void xiic_fill_tx_fifo(struct xilinx_xiic_priv *priv, while (len--) { u16 data = msg->buf[pos++];
- if (pos == len && nmsgs == 1) { + if ((msg->len - pos == 0) && nmsgs == 1) { /* last message in transfer -> STOP */ data |= XIIC_TX_DYN_STOP_MASK; }

Prior to starting a new transfer, conditionally wait for bus to not be busy.
Reinitialise controller as otherwise operation is not stable. For reference, see linux kernel commit: 9656eeebf3f1 ("i2c: Revert "i2c: xiic: Do not reset controller before every transfer"")
Signed-off-by: Tomas Melin tomas.melin@vaisala.com ---
Changes in v2: - Change variable declaration order - Change timeout to 3ms
Changes in v3: - Change timeout to 1000ms - Add print in case of timeout
drivers/i2c/xilinx_xiic.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/i2c/xilinx_xiic.c b/drivers/i2c/xilinx_xiic.c index e4ca0ab936..afb5f21b75 100644 --- a/drivers/i2c/xilinx_xiic.c +++ b/drivers/i2c/xilinx_xiic.c @@ -266,8 +266,20 @@ static void xiic_reinit(struct xilinx_xiic_priv *priv)
static int xilinx_xiic_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs) { + struct xilinx_xiic_priv *priv = dev_get_priv(dev); int ret = 0;
+ ret = wait_for_bit_8(priv->base + XIIC_SR_REG_OFFSET, + XIIC_SR_BUS_BUSY_MASK, false, 1000, true); + + if (ret == -ETIMEDOUT) + dev_err(dev, "timeout waiting for bus not busy condition"); + + if (ret) + return ret; + + xiic_reinit(priv); + for (; nmsgs > 0; nmsgs--, msg++) { if (msg->flags & I2C_M_RD) ret = xilinx_xiic_read_common(dev, msg, nmsgs);

Hi,
On 6/28/19 3:08 PM, Melin Tomas wrote:
Prior to starting a new transfer, conditionally wait for bus to not be busy.
Reinitialise controller as otherwise operation is not stable. For reference, see linux kernel commit: 9656eeebf3f1 ("i2c: Revert "i2c: xiic: Do not reset controller before every transfer"")
Signed-off-by: Tomas Melin tomas.melin@vaisala.com
Changes in v2:
- Change variable declaration order
- Change timeout to 3ms
Changes in v3:
Change timeout to 1000ms
Add print in case of timeout
drivers/i2c/xilinx_xiic.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/i2c/xilinx_xiic.c b/drivers/i2c/xilinx_xiic.c index e4ca0ab936..afb5f21b75 100644 --- a/drivers/i2c/xilinx_xiic.c +++ b/drivers/i2c/xilinx_xiic.c @@ -266,8 +266,20 @@ static void xiic_reinit(struct xilinx_xiic_priv *priv)
static int xilinx_xiic_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs) {
struct xilinx_xiic_priv *priv = dev_get_priv(dev); int ret = 0;
ret = wait_for_bit_8(priv->base + XIIC_SR_REG_OFFSET,
XIIC_SR_BUS_BUSY_MASK, false, 1000, true);
if (ret == -ETIMEDOUT)
dev_err(dev, "timeout waiting for bus not busy condition");
If patch otherwise ok, please amend this line before applying by adding missing '\n'.
if required, I can ofcourse send a v4.
thanks,
Tomas
- if (ret)
return ret;
- xiic_reinit(priv);
- for (; nmsgs > 0; nmsgs--, msg++) { if (msg->flags & I2C_M_RD) ret = xilinx_xiic_read_common(dev, msg, nmsgs);

Hello Tomas,
Am 28.06.2019 um 14:13 schrieb Melin Tomas:
Hi,
On 6/28/19 3:08 PM, Melin Tomas wrote:
Prior to starting a new transfer, conditionally wait for bus to not be busy.
Reinitialise controller as otherwise operation is not stable. For reference, see linux kernel commit: 9656eeebf3f1 ("i2c: Revert "i2c: xiic: Do not reset controller before every transfer"")
Signed-off-by: Tomas Melin tomas.melin@vaisala.com
Changes in v2:
- Change variable declaration order
- Change timeout to 3ms
Changes in v3:
Change timeout to 1000ms
Add print in case of timeout
drivers/i2c/xilinx_xiic.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/i2c/xilinx_xiic.c b/drivers/i2c/xilinx_xiic.c index e4ca0ab936..afb5f21b75 100644 --- a/drivers/i2c/xilinx_xiic.c +++ b/drivers/i2c/xilinx_xiic.c @@ -266,8 +266,20 @@ static void xiic_reinit(struct xilinx_xiic_priv *priv)
static int xilinx_xiic_xfer(struct udevice *dev, struct i2c_msg *msg, int nmsgs) {
struct xilinx_xiic_priv *priv = dev_get_priv(dev); int ret = 0;
ret = wait_for_bit_8(priv->base + XIIC_SR_REG_OFFSET,
XIIC_SR_BUS_BUSY_MASK, false, 1000, true);
if (ret == -ETIMEDOUT)
dev_err(dev, "timeout waiting for bus not busy condition");
If patch otherwise ok, please amend this line before applying by adding missing '\n'.
if required, I can ofcourse send a v4.
I fixed that, also this and patch 1/2 has some style issues:
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit fatal: mehrd ("1': unbekannter Commit oder Pfad existiert nicht")' #127: For reference, see linux kernel commit: 9656eeebf3f1 ("i2c: Revert
ERROR: DOS line endings #152: FILE: drivers/i2c/xilinx_xiic.c:269: +^Istruct xilinx_xiic_priv *priv = dev_get_priv(dev);^M$
ERROR: DOS line endings #155: FILE: drivers/i2c/xilinx_xiic.c:272: +^Iret = wait_for_bit_8(priv->base + XIIC_SR_REG_OFFSET,^M$
ERROR: DOS line endings #156: FILE: drivers/i2c/xilinx_xiic.c:273: +^I^I^I XIIC_SR_BUS_BUSY_MASK, false, 1000, true);^M$
ERROR: DOS line endings #157: FILE: drivers/i2c/xilinx_xiic.c:274: +^M$
ERROR: DOS line endings #158: FILE: drivers/i2c/xilinx_xiic.c:275: +^Iif (ret == -ETIMEDOUT)^M$
ERROR: DOS line endings #159: FILE: drivers/i2c/xilinx_xiic.c:276: +^I^Idev_err(dev, "timeout waiting for bus not busy condition");^M$
ERROR: DOS line endings #160: FILE: drivers/i2c/xilinx_xiic.c:277: +^M$
ERROR: DOS line endings #161: FILE: drivers/i2c/xilinx_xiic.c:278: +^Iif (ret)^M$
ERROR: DOS line endings #162: FILE: drivers/i2c/xilinx_xiic.c:279: +^I^Ireturn ret;^M$
ERROR: DOS line endings #163: FILE: drivers/i2c/xilinx_xiic.c:280: +^M$
ERROR: DOS line endings #164: FILE: drivers/i2c/xilinx_xiic.c:281: +^Ixiic_reinit(priv);^M$
ERROR: DOS line endings #165: FILE: drivers/i2c/xilinx_xiic.c:282: +^M$
total: 13 errors, 0 warnings, 0 checks, 20 lines checked
patch 1/2: ERROR: DOS line endings #147: FILE: drivers/i2c/xilinx_xiic.c:152: +^I^Iif ((msg->len - pos == 0) && nmsgs == 1) {^M$
I fixed them too, see: https://github.com/hsdenx/u-boot-i2c/commits/devel
travis build started: https://travis-ci.org/hsdenx/u-boot-i2c/builds/551762033
If there is a need for v4 please use them as base, thanks!
bye, Heiko

Hello Heiko,
On 6/28/19 3:43 PM, Heiko Schocher wrote:
I fixed that, also this and patch 1/2 has some style issues:
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit fatal: mehrd ("1': unbekannter Commit oder Pfad existiert nicht")' #127: For reference, see linux kernel commit: 9656eeebf3f1 ("i2c: Revert
FWIW, I did run checkpatch on these patches prior to sending, with no findings. Do you have some stricter settings or what might be the reason?
ERROR: DOS line endings #165: FILE: drivers/i2c/xilinx_xiic.c:282: +^M$
total: 13 errors, 0 warnings, 0 checks, 20 lines checked
patch 1/2: ERROR: DOS line endings #147: FILE: drivers/i2c/xilinx_xiic.c:152: +^I^Iif ((msg->len - pos == 0) && nmsgs == 1) {^M$
Strange, I don't see these errors either.
I fixed them too, see: https://github.com/hsdenx/u-boot-i2c/commits/devel
travis build started: https://travis-ci.org/hsdenx/u-boot-i2c/builds/551762033
If there is a need for v4 please use them as base, thanks!
Looks as it passed, thanks!
Tomas

Hello Tomas,
Am 01.07.2019 um 08:04 schrieb Melin Tomas:
Hello Heiko,
On 6/28/19 3:43 PM, Heiko Schocher wrote:
I fixed that, also this and patch 1/2 has some style issues:
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit fatal: mehrd ("1': unbekannter Commit oder Pfad existiert nicht")' #127: For reference, see linux kernel commit: 9656eeebf3f1 ("i2c: Revert
FWIW, I did run checkpatch on these patches prior to sending, with no findings. Do you have some stricter settings or what might be the reason?
Hmm.. no, I only use
scripts/checkpatch.pl mbox --fix-inplace
after I downloaded the patch from patchwork with:
wget http://patchwork.ozlabs.org/patch/$PW_NR/mbox
ERROR: DOS line endings #165: FILE: drivers/i2c/xilinx_xiic.c:282: +^M$
total: 13 errors, 0 warnings, 0 checks, 20 lines checked
patch 1/2: ERROR: DOS line endings #147: FILE: drivers/i2c/xilinx_xiic.c:152: +^I^Iif ((msg->len - pos == 0) && nmsgs == 1) {^M$
Strange, I don't see these errors either.
Hmm...
I fixed them too, see: https://github.com/hsdenx/u-boot-i2c/commits/devel
travis build started: https://travis-ci.org/hsdenx/u-boot-i2c/builds/551762033
If there is a need for v4 please use them as base, thanks!
Looks as it passed, thanks!
Yep.
bye, Heiko
participants (2)
-
Heiko Schocher
-
Melin Tomas