From patchwork Fri Oct 16 06:04:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Wu X-Patchwork-Id: 293596 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5DEF2C433E7 for ; Fri, 16 Oct 2020 06:05:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E675920848 for ; Fri, 16 Oct 2020 06:05:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=vivotek.com header.i=@vivotek.com header.b="dTUyTtYZ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391296AbgJPGFV (ORCPT ); Fri, 16 Oct 2020 02:05:21 -0400 Received: from mail.vivotek.com ([60.248.39.150]:33136 "EHLO mail.vivotek.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2391295AbgJPGFV (ORCPT ); Fri, 16 Oct 2020 02:05:21 -0400 Received: from pps.filterd (vivotekpps.vivotek.com [127.0.0.1]) by vivotekpps.vivotek.com (8.16.0.42/8.16.0.42) with SMTP id 09G65Bvu030142; Fri, 16 Oct 2020 14:05:11 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vivotek.com; h=from : to : cc : subject : date : message-id : mime-version : content-type; s=dkim; bh=OiryTol3lsa7n+iIcBzbc/EuEG5m0GVQYbD/SFQ9KYU=; b=dTUyTtYZpVgADzGjzJbvl3EYgaP59swy4Mm5Hh6zjNgIrtNkQpZI0gGy+N9nUSLpx5jy NChgifm7iyKhctuzG4B7BsrWIHrHuAPt1qOtTNhWBTb9KeakEfcO2quVxRiGSouT5KxQ E354osAhLn9Qu+9Ijk1SJdTJNVb6GtfRr5s= Received: from cas01.vivotek.tw ([192.168.0.58]) by vivotekpps.vivotek.com with ESMTP id 342yd1ccpm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Fri, 16 Oct 2020 14:05:11 +0800 Received: from localhost.localdomain (192.168.17.134) by CAS01.vivotek.tw (192.168.0.58) with Microsoft SMTP Server (TLS) id 14.3.487.0; Fri, 16 Oct 2020 14:05:10 +0800 From: Michael Wu To: Jarkko Nikula , Andy Shevchenko , Mika Westerberg , , CC: Morgan Chang , Dean Hsiao , Paul Chen , Michael Wu Subject: [PATCH 1/2] i2c: designware: call i2c_dw_read_clear_intrbits_slave() once Date: Fri, 16 Oct 2020 14:04:01 +0800 Message-ID: <20201016060402.17259-1-michael.wu@vatics.com> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 X-Originating-IP: [192.168.17.134] X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-10-16_02:2020-10-16,2020-10-16 signatures=0 X-Proofpoint-Spam-Reason: safe Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org 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 --- 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); From patchwork Fri Oct 16 06:04:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Wu X-Patchwork-Id: 285697 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8D299C43457 for ; Fri, 16 Oct 2020 06:05:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 300A9207F7 for ; Fri, 16 Oct 2020 06:05:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=vivotek.com header.i=@vivotek.com header.b="NeQHtHZC" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391469AbgJPGFa (ORCPT ); Fri, 16 Oct 2020 02:05:30 -0400 Received: from mail.vivotek.com ([60.248.39.150]:33148 "EHLO mail.vivotek.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2391458AbgJPGF3 (ORCPT ); Fri, 16 Oct 2020 02:05:29 -0400 Received: from pps.filterd (vivotekpps.vivotek.com [127.0.0.1]) by vivotekpps.vivotek.com (8.16.0.42/8.16.0.42) with SMTP id 09G63I6l027232; Fri, 16 Oct 2020 14:05:23 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vivotek.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-type; s=dkim; bh=clrtvEG+8dD3iBlUZaf/DbFmGcIlTBkvmWkQUpnUe3w=; b=NeQHtHZCuvRNSDAAoXVmsTxkSlNvI/ULyHU+brfx3ajX3vSFR0o8RbEohY6ynAf7GrGr 5vlDHwJTyQPiue3yiBsPmrI5ksDp0k098uvZjVzLjqBP+PruZcgLw19fvrVXN6dzNpqx yAY9beHSzkfSFUwc6Rjl1q7CH6FJwMYq0vM= Received: from cas01.vivotek.tw ([192.168.0.58]) by vivotekpps.vivotek.com with ESMTP id 342yd1ccpt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Fri, 16 Oct 2020 14:05:23 +0800 Received: from localhost.localdomain (192.168.17.134) by CAS01.vivotek.tw (192.168.0.58) with Microsoft SMTP Server (TLS) id 14.3.487.0; Fri, 16 Oct 2020 14:05:22 +0800 From: Michael Wu To: Jarkko Nikula , Andy Shevchenko , Mika Westerberg , , CC: Morgan Chang , Dean Hsiao , Paul Chen , Michael Wu Subject: [PATCH 2/2] i2c: designware: slave should do WRITE_REQUESTED before WRITE_RECEIVED Date: Fri, 16 Oct 2020 14:04:02 +0800 Message-ID: <20201016060402.17259-2-michael.wu@vatics.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20201016060402.17259-1-michael.wu@vatics.com> References: <20201016060402.17259-1-michael.wu@vatics.com> MIME-Version: 1.0 X-Originating-IP: [192.168.17.134] X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235, 18.0.687 definitions=2020-10-16_02:2020-10-16,2020-10-16 signatures=0 X-Proofpoint-Spam-Reason: safe Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org Sometime we would get the following flows when doing an i2cset: 1. No any WRITE_REQUESTED 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4 WRITE_RECEIVED 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4 WRITE_RECEIVED 0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x710 : INTR_STAT=0x200 STOP 2. WRITE_REQUESTED after WRITE_RECEIVED 0x1 STATUS SLAVE_ACTIVITY=0x1 : RAW_INTR_STAT=0x514 : INTR_STAT=0x4 WRITE_RECEIVED 0x1 STATUS SLAVE_ACTIVITY=0x0 : RAW_INTR_STAT=0x714 : INTR_STAT=0x204 WRITE_RECEIVED WRITE_REQUESTED Documentation/i2c/slave-interface.rst said that I2C_SLAVE_WRITE_REQUESTED, which is mandatory, comes without any data arrived. It means I2C_SLAVE_WRITE_REQUESTED should be in front of any I2C_SLAVE_WRITE_RECEIVED in a write-request. Obviously the 1st log shows no I2C_SLAVE_WRITE_REQUESTED, and 2nd log shows I2C_SLAVE_WRITE_REQUESTED occurred after I2C_SLAVE_WRITE_RECEIVED. I2C_SLAVE_STOP is mandatory to notify a request finish to reset the state machine of the backend. In case 2 it never do I2C_SLAVE_STOP when STOP condition is received. This is one of illegal issues to be fixed. dev->status can be used to record the current machine state, especially DesignWare I2C controller has no interrupts to notify a new write-request coming, an IC_INTR_RX_FULL rising while dev->status isn't STATUS_WRITE_IN_PROGRESS is notified to do an I2C_SLAVE_WRITE_REQUESTED by checking dev->status. dev->status also helps to resolve the omitted I2C_SLAVE_STOP in case 2. Signed-off-by: Michael Wu --- drivers/i2c/busses/i2c-designware-slave.c | 73 ++++++++++++----------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/drivers/i2c/busses/i2c-designware-slave.c b/drivers/i2c/busses/i2c-designware-slave.c index 02e7c5171827..57ce1f0b403c 100644 --- a/drivers/i2c/busses/i2c-designware-slave.c +++ b/drivers/i2c/busses/i2c-designware-slave.c @@ -172,50 +172,55 @@ static int i2c_dw_irq_handler_slave(struct dw_i2c_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) { + if (dev->status != STATUS_WRITE_IN_PROGRESS) { + if (dev->status != STATUS_IDLE) + i2c_slave_event(dev->slave, I2C_SLAVE_STOP, + &val); + + dev->status = STATUS_WRITE_IN_PROGRESS; + i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, + &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); + } + if (stat & DW_IC_INTR_RD_REQ) { + if (dev->status != STATUS_IDLE) { + dev->status = STATUS_IDLE; + i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val); + } + if (slave_activity) { - if (stat & DW_IC_INTR_RX_FULL) { - 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); - } - regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp); - } else { - regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp); - regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp); - } - if (!i2c_slave_event(dev->slave, - I2C_SLAVE_READ_REQUESTED, - &val)) - regmap_write(dev->map, DW_IC_DATA_CMD, val); + regmap_read(dev->map, DW_IC_CLR_RD_REQ, &tmp); + regmap_read(dev->map, DW_IC_CLR_RX_UNDER, &tmp); + + dev->status = STATUS_READ_IN_PROGRESS; + i2c_slave_event(dev->slave, I2C_SLAVE_READ_REQUESTED, + &val); + regmap_write(dev->map, DW_IC_DATA_CMD, val); } } if (stat & DW_IC_INTR_RX_DONE) { - if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED, - &val)) - regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp); + i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED, &val); + regmap_read(dev->map, DW_IC_CLR_RX_DONE, &tmp); + dev->status = STATUS_IDLE; i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val); - return 1; } - if (stat & DW_IC_INTR_RX_FULL) { - 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); - } else - i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val); - - 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_STOP_DET) { + if (dev->status != STATUS_IDLE) { + dev->status = STATUS_IDLE; + i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val); + } + } return 1; }