From: Michael Wu <michael.wu@vatics.com>
To: Jarkko Nikula <jarkko.nikula@linux.intel.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
<linux-i2c@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: Morgan Chang <morgan.chang@vatics.com>,
Dean Hsiao <dean.hsiao@vatics.com>,
Paul Chen <paul.chen@vatics.com>,
Michael Wu <michael.wu@vatics.com>
Subject: [PATCH 1/2] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once
Date: Fri, 16 Oct 2020 14:04:01 +0800 [thread overview]
Message-ID: <20201016060402.17259-1-michael.wu@vatics.com> (raw)
i2c_dw_read_clear_intrbits_slave() was called per each interrupt handle.
It caused some interrupt bits which haven't been handled yet were cleared,
the corresponding handlers would do nothing due to interrupt bits been
discarded. For example,
$ i2cset -f -y 2 0x42 0x00 0x41; dmesg -c
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
[1][irq_handler ]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
[1][irq_handler ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
WRITE_RECEIVED
t1: ISR with the 1st IC_INTR_RX_FULL.
t2: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave().
t3: Enter i2c_dw_irq_handler_slave() and then do
i2c_slave_event(WRITE_RECEIVED) because
if (stat & DW_IC_INTR_RX_FULL).
t4: ISR with both IC_INTR_STOP_DET and the 2nd IC_INTR_RX_FULL.
t5: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave(). The
current IC_INTR_STOP_DET is cleared by this
i2c_dw_read_clear_intrbits_slave().
t6: Enter i2c_dw_irq_handler_slave() and then do
i2c_slave_event(WRITE_RECEIVED) because
if (stat & DW_IC_INTR_RX_FULL).
t7: i2c_slave_event(STOP) never be done because IC_INTR_STOP_DET was
cleared in t5.
The root cause is that i2c_dw_read_clear_intrbits_slave() was called many
times. Calling i2c_dw_read_clear_intrbits_slave() once in one ISR and take
the returned stat for later handling is the solution.
Signed-off-by: Michael Wu <michael.wu@vatics.com>
---
drivers/i2c/busses/i2c-designware-slave.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c
index 44974b53a626..02e7c5171827 100644
--- a/drivers/i2c/busses/i2c-designware-slave.c
+++ b/drivers/i2c/busses/i2c-designware-slave.c
@@ -159,7 +159,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
u32 raw_stat, stat, enabled, tmp;
u8 val = 0, slave_activity;
- regmap_read(dev->map, DW_IC_INTR_STAT, &stat);
regmap_read(dev->map, DW_IC_ENABLE, &enabled);
regmap_read(dev->map, DW_IC_RAW_INTR_STAT, &raw_stat);
regmap_read(dev->map, DW_IC_STATUS, &tmp);
@@ -168,13 +167,11 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (!enabled || !(raw_stat & ~DW_IC_INTR_ACTIVITY) || !dev->slave)
return 0;
+ stat = i2c_dw_read_clear_intrbits_slave(dev);
dev_dbg(dev->dev,
"%#x STATUS SLAVE_ACTIVITY=%#x : RAW_INTR_STAT=%#x : INTR_STAT=%#x\n",
enabled, slave_activity, raw_stat, stat);
- if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
- i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
-
if (stat & DW_IC_INTR_RD_REQ) {
if (slave_activity) {
if (stat & DW_IC_INTR_RX_FULL) {
@@ -188,11 +185,9 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
val);
}
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
- stat = i2c_dw_read_clear_intrbits_slave(dev);
} else {
regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp);
regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp);
- stat = i2c_dw_read_clear_intrbits_slave(dev);
}
if (!i2c_slave_event(dev->slave,
I2C_SLAVE_READ_REQUESTED,
@@ -207,7 +202,6 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp);
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
- stat = i2c_dw_read_clear_intrbits_slave(dev);
return 1;
}
@@ -217,10 +211,11 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_dev *dev)
if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
&val))
dev_vdbg(dev->dev, "Byte %X acked!", val);
- } else {
+ } else
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
- stat = i2c_dw_read_clear_intrbits_slave(dev);
- }
+
+ if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
+ i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
return 1;
}
@@ -230,7 +225,6 @@ static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
struct dw_i2c_dev *dev = dev_id;
int ret;
- i2c_dw_read_clear_intrbits_slave(dev);
ret = i2c_dw_irq_handler_slave(dev);
if (ret > 0)
complete(&dev->cmd_complete);
--
2.17.1
next reply other threads:[~2020-10-16 6:05 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-16 6:04 Michael Wu [this message]
2020-10-16 6:04 ` [PATCH 2/2] i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED Michael Wu
2020-10-30 8:04 [PATCH 0/2] Designware I2C slave confusing IC_INTR_STOP_DET handle Michael Wu
2020-10-30 8:04 ` [PATCH 1/2] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once Michael Wu
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=20201016060402.17259-1-michael.wu@vatics.com \
--to=michael.wu@vatics.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=dean.hsiao@vatics.com \
--cc=jarkko.nikula@linux.intel.com \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=morgan.chang@vatics.com \
--cc=paul.chen@vatics.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).