public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
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>,
	Wolfram Sang <wsa@kernel.org>, <linux-i2c@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Cc: Morgan Chang <morgan.chang@vatics.com>,
	Michael Wu <michael.wu@vatics.com>
Subject: [PATCH 0/2] Designware I2C slave confusing IC_INTR_STOP_DET handle
Date: Fri, 30 Oct 2020 16:04:18 +0800	[thread overview]
Message-ID: <20201030080420.28016-1-michael.wu@vatics.com> (raw)

When an I2C slave works, sometimes both IC_INTR_RX_FULL and
IC_INTR_STOP_DET may be rising during an IRQ handle, especially when
system is busy or too late to handle interrupts.

If IC_INTR_RX_FULL is rising and the system doesn't handle immediately,
IC_INTR_STOP_DET may be rising and the system has to handle these two
events. For this there may be two problems:

1. IC_INTR_STOP_DET is rising after i2c_dw_read_clear_intrbits_slave()
   done: It seems invalidated because I2C_SLAVE_WRITE_REQUESTED is
   reported after the 1st I2C_SLAVE_WRITE_RECEIVED.

$ 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
I2C_SLAVE_WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204
I2C_SLAVE_WRITE_REQUESTED
I2C_SLAVE_WRITE_RECEIVED
[0][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x710 : INTR_STAT=0x200
[1][irq_handler   ]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x510 : INTR_STAT=0x0
I2C_SLAVE_STOP
[2][clear_intrbits]0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x510 : INTR_STAT=0x0

  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 report
      I2C_SLAVE_WRITE_RECEIVED due to 'if (stat & DW_IC_INTR_RX_FULL)'
      matched.
  t4: ISR with the 2nd IC_INTR_RX_FULL.
  t5: Clear listed IC_INTR bits by i2c_dw_read_clear_intrbits_slave() while
      IC_INTR_STOP_DET has not risen yet.
  t6: IC_INTR_STOP_DET is rising after entering i2c_dw_irq_handler_slave().
      The driver reports I2C_SLAVE_WRITE_REQUESTED first due to
      'if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))'
      matched and then reports I2C_SLAVE_WRITE_RECEIVED.
  t7: Reports I2C_SLAVE_STOP due to IC_INTR_STOP_DET not be cleared yet.

2. Both IC_INTR_STOP_DET and IC_INTR_RX_FULL are rising before
   i2c_dw_read_clear_intrbits_slave(): I2C_SLAVE_STOP never be reported
   because IC_INTR_STOP_DET was cleared by
   i2c_dw_read_clear_intrbits_slave().

$ 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
I2C_SLAVE_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
I2C_SLAVE_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 report
      I2C_SLAVE_WRITE_RECEIVED due to 'if (stat & DW_IC_INTR_RX_FULL)'
      matched.
  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 was cleared by this
      i2c_dw_read_clear_intrbits_slave().
  t6: Enter i2c_dw_irq_handler_slave() and then report
      i2c_slave_event(WRITE_RECEIVED) due to
      'if (stat & DW_IC_INTR_RX_FULL)' matched.
  t7: I2C_SLAVE_STOP never be reported because IC_INTR_STOP_DET was
      cleared in t5.

In order to resolve these problems, i2c_dw_read_clear_intrbits_slave()
should be called only once in an ISR and take the returned stat to handle
those occurred events. The ISR handling has to be adjusted to conform event
reporting described in Documentation/i2c/slave-interface.rst.

Michael Wu (2):
  i2c: designware: call i2c_dw_read_clear_intrbits_slave() once
  i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED

 drivers/i2c/busses/i2c-designware-slave.c | 52 +++++++++--------------
 1 file changed, 19 insertions(+), 33 deletions(-)

-- 
2.17.1


             reply	other threads:[~2020-10-30  8:05 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-30  8:04 Michael Wu [this message]
2020-10-30  8:04 ` [PATCH 1/2] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once Michael Wu
2020-10-30  8:04 ` [PATCH 2/2] i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED Michael Wu
2020-10-30 14:46   ` Jarkko Nikula
2020-11-03 21:03   ` Wolfram Sang
2020-11-04 10:17     ` Michael.Wu
2020-11-04 10:35       ` Wolfram Sang
2020-11-04 10:51         ` Michael.Wu
2020-11-05  9:13       ` Jarkko Nikula

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=20201030080420.28016-1-michael.wu@vatics.com \
    --to=michael.wu@vatics.com \
    --cc=andriy.shevchenko@linux.intel.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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox