From: Jarkko Nikula <jarkko.nikula@linux.intel.com>
To: linux-i2c@vger.kernel.org
Cc: Wolfram Sang <wsa@kernel.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
Jan Dabros <jsd@semihalf.com>, Michael Wu <michael.wu@vatics.com>,
Tian Ye <tianye@sugon.com>,
Luis Oliveira <luis.oliveira@synopsys.com>,
Jarkko Nikula <jarkko.nikula@linux.intel.com>
Subject: [PATCH 02/11] i2c: designware: Empty receive FIFO in slave interrupt handler
Date: Wed, 26 Oct 2022 15:39:03 +0300 [thread overview]
Message-ID: <20221026123912.2833271-3-jarkko.nikula@linux.intel.com> (raw)
In-Reply-To: <20221026123912.2833271-1-jarkko.nikula@linux.intel.com>
Writes from I2C bus often fail when testing the i2c-designware-slave.c
with the slave-eeprom backend. The same writes work correctly when
testing with a real 24c02 EEPROM chip.
In the tests below an i2c-designware-slave.c instance with the
slave-eeprom backend is configured to act as a simulated 24c02 at
address 0x65 on an I2C host bus 6.
1. i2cset -y 6 0x65 0x00 0x55
Single byte 0x55 write into address 0x00. No data goes into simulated
EEPROM. Debug prints from the i2c_dw_irq_handler_slave():
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
2. i2ctransfer -y 6 w9@0x65 0x00 0xff-
Write 8 bytes with decrementing value starting from 0xff at address 0x00
and forward. Only some of the data goes into arbitrary addresses.
Content is something like below but varies:
00000000 f9 f8 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000050 00 00 00 00 00 00 ff fe 00 00 00 00 00 00 00 00 |................|
000000f0 00 00 00 00 00 00 00 00 00 00 00 00 00 fc fb fa |................|
In this case debug prints were:
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x510 : INTR_STAT=0x0
Both cases show there is more data coming from the receive FIFO still
after detecting the STOP condition. This can be seen from interrupt
status bits DW_IC_INTR_STOP_DET (0x200) and DW_IC_INTR_RX_FULL (0x4).
Perhaps due interrupt latencies the receive FIFO is not read fast
enough, STOP detection happens synchronously when it occurs on the I2C
bus and the DW_IC_INTR_RX_FULL keeps coming as long as there are more
bytes in the receive FIFO.
Fix this by reading the receive FIFO completely empty whenever
DW_IC_INTR_RX_FULL occurs. Use RFNE, Receive FIFO Not Empty bit in the
DW_IC_STATUS register to loop through bytes in the FIFO.
While at it do not test the return code from i2c_slave_event() for the
I2C_SLAVE_WRITE_RECEIVED since to my understanding this hardware cannot
generate NACK to incoming bytes and debug print itself does not have
much value.
Reported-by: Tian Ye <tianye@sugon.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
Hi Tian Ye. I've been testing the i2c-designware-slave.c recently and
discovered these write issues. Your recent patch gave an idea what might
cause them. In my solution I went testing DW_IC_STATUS_RFNE since
according to datasheet it's equivalent to RX_FULL interrupt in case
interrupts are masked. Seems to work here too.
Does this fix the issue you were seeing?
---
drivers/i2c/busses/i2c-designware-core.h | 1 +
drivers/i2c/busses/i2c-designware-slave.c | 12 +++++++-----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index dbf6bdc5f01b..6d1df28dd93b 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -108,6 +108,7 @@
#define DW_IC_STATUS_ACTIVITY BIT(0)
#define DW_IC_STATUS_TFE BIT(2)
+#define DW_IC_STATUS_RFNE BIT(3)
#define DW_IC_STATUS_MASTER_ACTIVITY BIT(5)
#define DW_IC_STATUS_SLAVE_ACTIVITY BIT(6)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 1eac4f4d5573..295774a69b67 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -180,11 +180,13 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
&val);
}
- regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
- val = tmp;
- if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
- &val))
- dev_vdbg(dev->dev, "Byte %X acked!", val);
+ do {
+ regmap_read(dev->map, DW_IC_DATA_CMD, &tmp);
+ val = tmp;
+ i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
+ &val);
+ regmap_read(dev->map, DW_IC_STATUS, &tmp);
+ } while (tmp & DW_IC_STATUS_RFNE);
}
if (stat & DW_IC_INTR_RD_REQ) {
--
2.35.1
next prev parent reply other threads:[~2022-10-26 12:40 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-26 12:39 [PATCH 00/11] i2c: designware: Slave fixes and generic cleanups Jarkko Nikula
2022-10-26 12:39 ` [PATCH 01/11] i2c: designware: Fix slave state machine for sequential reads Jarkko Nikula
2022-10-26 12:39 ` Jarkko Nikula [this message]
2022-10-26 12:39 ` [PATCH 03/11] i2c: designware: Define software status flags with BIT() Jarkko Nikula
2022-10-26 12:39 ` [PATCH 04/11] i2c: designware: Remove needless initializations from i2c_dw_reg_slave() Jarkko Nikula
2022-10-26 12:39 ` [PATCH 05/11] i2c: designware: Remove unused completion code from i2c-designware-slave Jarkko Nikula
2022-10-26 12:39 ` [PATCH 06/11] i2c: designware: Simplify slave interrupt handler nesting Jarkko Nikula
2022-10-26 12:39 ` [PATCH 07/11] i2c: designware: Do not process interrupt when device is suspended Jarkko Nikula
2022-10-26 13:28 ` Andy Shevchenko
2022-10-26 13:29 ` Andy Shevchenko
2022-10-26 12:39 ` [PATCH 08/11] i2c: designware: Move debug print in i2c_dw_isr() Jarkko Nikula
2022-10-26 12:39 ` [PATCH 09/11] i2c: designware: Simplify master interrupt handler nesting Jarkko Nikula
2022-10-26 12:39 ` [PATCH 10/11] i2c: designware: Remove common i2c_dw_disable_int() Jarkko Nikula
2022-10-26 13:34 ` Andy Shevchenko
2022-10-26 14:00 ` Jarkko Nikula
2022-10-26 12:39 ` [PATCH 11/11] i2c: designware: Align defines in i2c-designware-core.h Jarkko Nikula
2022-10-26 13:38 ` Andy Shevchenko
2022-11-02 13:14 ` Jarkko Nikula
2022-10-26 12:56 ` [PATCH 00/11] i2c: designware: Slave fixes and generic cleanups Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221026123912.2833271-3-jarkko.nikula@linux.intel.com \
--to=jarkko.nikula@linux.intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=jsd@semihalf.com \
--cc=linux-i2c@vger.kernel.org \
--cc=luis.oliveira@synopsys.com \
--cc=michael.wu@vatics.com \
--cc=mika.westerberg@linux.intel.com \
--cc=tianye@sugon.com \
--cc=wsa@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.