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 31A19C4167B for ; Mon, 27 Nov 2023 23:00:43 +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=t1W6ytHModmzJDYeUR+iO4OEMGOcPy9TYZfp1SItEnA=; b=AizpLRsQ5klBdu OSPxP28yhBcsjBvEal0bjqsfwatE6hZ8XecgICAzQMbfY6cC+sDroQIBDuIZrHYTz2MgFg18M9vBM H644vCaYtIuIlrPkPH/ppZ2EUodePBXCZX9C111j+3aP2pzlge6SfOGCTgklkZiU1Myy8rA8zcgwh JpIhKa96WNE+uUwkF/d5pqTAxnqUObx70kK5xi4Lk32w8j3JB2tqPgmiIeauCTx9XJ2+zyuYoOc7/ M4VoMeCMgXz6EHZnsZa6IpNVkT8C+Tut1hvmBipdr1NvibFLQR+ByEuiWqhdAD/gnIBW5kILc8JbN 01ttgTiUSnxnHdxmrRGA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r7kaR-003dHH-0y; Mon, 27 Nov 2023 23:00:19 +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 1r7kaN-003dGF-1Y for linux-arm-kernel@lists.infradead.org; Mon, 27 Nov 2023 23:00:17 +0000 Received: from [192.168.68.112] (ppp118-210-131-38.adl-adc-lon-bras33.tpg.internode.on.net [118.210.131.38]) by mail.codeconstruct.com.au (Postfix) with ESMTPSA id DC2FB200EF; Tue, 28 Nov 2023 07:00:02 +0800 (AWST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=codeconstruct.com.au; s=2022a; t=1701126006; bh=jeaVBZ61hYUULmiG4P1ndu1Q2m319cZm4MzoynyxVlQ=; h=Subject:From:To:Cc:Date:In-Reply-To:References; b=GcF5S01D0K9OrzUP0huZxWEqVkzKv+ALVQNJLjU286ml3+TWPtHI2Zc8t6o8wULQk KqmZkOUx0cGc5393b6urtjElVwWj3lGCg4obKPoov+MkKuHgNZZMKJV/ofccGs7cFX RFwEHtRS1QEG1fEyjt7aZe/h33NCJjvIcqSNRTRoHy2/N4rEquFYON9Y1iqm0rzJEv C5nfRoagxZnIgxEWBCdb1IL9THFhXkg6oGh70riVnXa5jPtyduaH/9GenlWyUlxYKi kIKcmX/4E5/YnNDHVydkF6JAVm+AICdzgHsd4IY2w2/68iom6sRDaqwziTv7BzQsrw JBe2POQtO3KnA== Message-ID: Subject: Re: [PATCH] i2c: aspeed: Acknowledge Tx ack late when in SLAVE_READ_PROCESSED From: Andrew Jeffery To: Quan Nguyen , Cosmo Chou Cc: linux-arm-kernel@lists.infradead.org, jae.hyun.yoo@linux.intel.com, andi.shyti@kernel.org, linux-aspeed@lists.ozlabs.org, openbmc@lists.ozlabs.org, linux-kernel@vger.kernel.org, wsa@kernel.org, brendan.higgins@linux.dev, cosmo.chou@quantatw.com, joel@jms.id.au, linux@roeck-us.net, linux-i2c@vger.kernel.org Date: Tue, 28 Nov 2023 09:30:01 +1030 In-Reply-To: <854762fb-1767-4208-a7fc-10580730c1f3@os.amperecomputing.com> References: <20231120091746.2866232-1-chou.cosmo@gmail.com> <854762fb-1767-4208-a7fc-10580730c1f3@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-20231127_150015_752012_D68FAF64 X-CRM114-Status: GOOD ( 21.55 ) 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 Mon, 2023-11-27 at 15:08 +0700, Quan Nguyen wrote: > > On 27/11/2023 14:04, Cosmo Chou wrote: > > Andrew Jeffery wrote on Mon, 2023-11-27 > > at 11:23 AM: > > > > > > On Mon, 2023-11-20 at 17:17 +0800, Cosmo Chou wrote: > > > > commit 2be6b47211e1 ("i2c: aspeed: Acknowledge most interrupts early > > > > in interrupt handler") moved most interrupt acknowledgments to the > > > > start of the interrupt handler to avoid race conditions. However, > > > > slave Tx ack status shouldn't be cleared before SLAVE_READ_PROCESSED > > > > is handled. > > > > > > > > Acknowledge Tx ack status after handling SLAVE_READ_PROCESSED to fix > > > > the problem that the next byte is not sent correctly. > > > > > > What does this mean in practice? Can you provide more details? It > > > sounds like you've seen concrete problems and it would be nice to > > > capture what it was that occurred. > > > > > > Andrew > > > > For a normal slave transaction, a master attempts to read out N bytes > > from BMC: (BMC addr: 0x20) > > [S] [21] [A] [1st_B] [1_ack] [2nd_B] [2_ack] ... [Nth_B] [N] [P] > > > > T1: when [21] [A]: Both INTR_SLAVE_MATCH and INTR_RX_DONE rise, > > INTR_RX_DONE is not cleared until BMC is ready to send the 1st_B: > > https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-aspeed.c#L294 > > That is, BMC stretches the SCL until ready to send the 1st_B. > > > > T2: when [1_ack]: INTR_TX_ACK rises, but it's cleared at the start of > > the ISR, so that BMC does not stretch the SCL, the master continues > > to read 2nd_B before BMC is ready to send the 2nd_B. > > > > To fix this, do not clear INTR_TX_ACK until BMC is ready to send data: > > https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-aspeed.c#L302 > > > > This looks like the same issue, but we chose to ack them late. Same with > INTR_RX_DONE. > > https://lore.kernel.org/all/20210616031046.2317-3-quan@os.amperecomputing.com/ >From a brief inspection I prefer the descriptions in your series Quan. Looks like we dropped the ball a bit there though on the review - can you resend your series based on 6.7-rc1 or so and Cc Cosmo? Cheers, Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel