From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0D547C4167B for ; Fri, 8 Dec 2023 03:57:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=pOA9WBRxCTPa6TL3abHcc1YhBLDZwCYSacn1ObSlUtg=; b=GVkgrnVHndVdFv c457SrUK2aUffs143v1xyxCt3A8O5HXFRsGS0bnr/P2BdGgV/JF4AGRS+q3bsF9P2jOGB0q/s/trK Aq9ZziiAuwajcc3yFqPL+2ehX3Xva23mzRE3CGi8gUuFOc0SgdxjlPv7S31XZkovdRJ0/Qt8WhsFl BH2Fy3FNoyHvJql8Zrqqu0zA2F3dnTSzyTMKmgxhZMt377vI9NP3Q3vYJMPgxGSMXgVe/RTLr/XLb rhiJys/QBKtAVqzabjYe1sfTY41xOT5o9R0YSlEv6jDb8jKN3WpXZ7YrOJgGAxHvorH/pzf5i6MPS brsE6nyRa5fiEog4a4iw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rBRyy-00Ee1R-3B; Fri, 08 Dec 2023 03:56:56 +0000 Received: from pi.codeconstruct.com.au ([203.29.241.158] helo=codeconstruct.com.au) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rBRyu-00Ee08-1t for linux-arm-kernel@lists.infradead.org; Fri, 08 Dec 2023 03:56:54 +0000 Received: from [192.168.68.112] (ppp118-210-181-59.adl-adc-lon-bras34.tpg.internode.on.net [118.210.181.59]) by mail.codeconstruct.com.au (Postfix) with ESMTPSA id E526920016; Fri, 8 Dec 2023 11:56:33 +0800 (AWST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1702007800; bh=tyllIWdZHFdzvGcGSWMb1b2dzbhWG0Dcolc/GscISJs=; h=Subject:From:To:Cc:Date:In-Reply-To:References; b=IpkBpjVlJqrjJbUQM32dDSX0YGxz6gBIhe/tBGjUEzsc8/aqUO/E57MQSLhQJmW+g HARu1kXb4P3QDUh5Jg2MiyauB8vTYbUlMHFgGCqgQJK+DltJy6AQd0+N7gPyfPTqH7 USmh9YZfNr0wzfkWYs/l9AtBgIPZhIlp490vLAPCX+oRx5vkE5mCKSdC3v/fn/jLuf S3xtdMlrNhHdpFc+PpuQkJl15BtayAhWbaq0qmGZbOcxl7xIU+7769WluDh2AkGiy+ lnvUXItICteaSiZzEKyHAZ3wE1cVuWuQcVg7wGScpPnypxaBnY9ML6O/VrzBUlHox/ snvJVTJb19R2A== Message-ID: <79ce9162faeb113ecb13efeb58d95f8a71e1a060.camel@codeconstruct.com.au> Subject: Re: [PATCH v3 1/2] i2c: aspeed: Handle the coalesced stop conditions with the start conditions. From: Andrew Jeffery To: Quan Nguyen , Brendan Higgins , Benjamin Herrenschmidt , Joel Stanley , Andi Shyti , Wolfram Sang , Jae Hyun Yoo , Guenter Roeck , linux-i2c@vger.kernel.org, openbmc@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org Cc: Cosmo Chou , Open Source Submission , Phong Vo , "Thang Q . Nguyen" Date: Fri, 08 Dec 2023 14:26:31 +1030 In-Reply-To: <20231208033142.1673232-2-quan@os.amperecomputing.com> References: <20231208033142.1673232-1-quan@os.amperecomputing.com> <20231208033142.1673232-2-quan@os.amperecomputing.com> User-Agent: Evolution 3.46.4-2 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231207_195652_854533_46BA8119 X-CRM114-Status: GOOD ( 21.66 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 2023-12-08 at 10:31 +0700, Quan Nguyen wrote: > Some masters may drive the transfers with low enough latency between > the nak/stop phase of the current command and the start/address phase > of the following command that the interrupts are coalesced by the > time we process them. > Handle the stop conditions before processing SLAVE_MATCH to fix the > complaints that sometimes occur below. > > "aspeed-i2c-bus 1e78a040.i2c-bus: irq handled != irq. Expected > 0x00000086, but was 0x00000084" > > Fixes: f9eb91350bb2 ("i2c: aspeed: added slave support for Aspeed I2C driver") > Signed-off-by: Quan Nguyen > --- > v3: > + Change to handle the coalesced stop condition with the start > conditions [Andrew] > + Revised commit message [Quan] > > v2: > + Split to separate series [Joel] > + Added the Fixes line [Joel] > + Revised commit message [Quan] > > v1: > + First introduced in > https://lore.kernel.org/all/20210519074934.20712-1-quan@os.amperecomputing.com/ > --- > drivers/i2c/busses/i2c-aspeed.c | 47 ++++++++++++++++++++++----------- > 1 file changed, 31 insertions(+), 16 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c > index 28e2a5fc4528..1c2a4f4c4e1b 100644 > --- a/drivers/i2c/busses/i2c-aspeed.c > +++ b/drivers/i2c/busses/i2c-aspeed.c > @@ -249,18 +249,45 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status) > if (!slave) > return 0; > > - command = readl(bus->base + ASPEED_I2C_CMD_REG); > + /* > + * Handle stop conditions early, prior to SLAVE_MATCH. Some masters may drive > + * transfers with low enough latency between the nak/stop phase of the current > + * command and the start/address phase of the following command that the > + * interrupts are coalesced by the time we process them. > + */ > + if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) { > + irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP; > + bus->slave_state = ASPEED_I2C_SLAVE_STOP; > + } > > - /* Slave was requested, restart state machine. */ > + if (irq_status & ASPEED_I2CD_INTR_TX_NAK && > + bus->slave_state == ASPEED_I2C_SLAVE_READ_PROCESSED) { > + irq_handled |= ASPEED_I2CD_INTR_TX_NAK; > + bus->slave_state = ASPEED_I2C_SLAVE_STOP; > + } > + > + /* Propagate any stop conditions to the slave implementation. */ > + if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) { > + i2c_slave_event(slave, I2C_SLAVE_STOP, &value); > + bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE; > + } > + /* If there's a reason to do a v4 then an extra empty line above the comment would be nice. But let's not get hung up on that if everyone else is happy. Thanks for the fixes! Reviewed-by: Andrew Jeffery > + * Now that we've dealt with any potentially coalesced stop conditions, > + * address any start conditions. > + */ > if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) { > irq_handled |= ASPEED_I2CD_INTR_SLAVE_MATCH; > bus->slave_state = ASPEED_I2C_SLAVE_START; > } > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel