Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
From:   Quan Nguyen <quan@os.amperecomputing.com>
To:     Brendan Higgins <brendan.higgins@linux.dev>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Joel Stanley <joel@jms.id.au>,
        Andi Shyti <andi.shyti@kernel.org>,
        Andrew Jeffery <andrew@codeconstruct.com.au>,
        Wolfram Sang <wsa@kernel.org>,
        Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>,
        Guenter Roeck <linux@roeck-us.net>, 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 <chou.cosmo@gmail.com>,
        Open Source Submission <patches@amperecomputing.com>,
        Phong Vo <phong@os.amperecomputing.com>,
        "Thang Q . Nguyen" <thang@os.amperecomputing.com>,
        Quan Nguyen <quan@os.amperecomputing.com>
Subject: [PATCH v3 1/2] i2c: aspeed: Handle the coalesced stop conditions with the start conditions.
Date:   Fri,  8 Dec 2023 10:31:41 +0700
Message-Id: <20231208033142.1673232-2-quan@os.amperecomputing.com>
In-Reply-To: <20231208033142.1673232-1-quan@os.amperecomputing.com>
References: <20231208033142.1673232-1-quan@os.amperecomputing.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?E7OhJtKiDhISKT4bOFMLrFCD63yiAd4gZlbsfaDMvQMHv4LbnPXjg3TqrYdR?=
 =?us-ascii?Q?rXyh56HCZc1VKwVqJIJpDmZ67OqGl4H89zV9xNImMmlCf5WSzQ6hSvVbIBUp?=
 =?us-ascii?Q?v8u5BlyoHr+1EDBjfn6Wqv/B5vbhlbmgEhyIbyKkpUvffMuftHtMj61jauuI?=
 =?us-ascii?Q?GvOe1Dx0C2UQGAuAKmtodvryoPlmnFi5+oeR65d9qUDeEeQOFtrOtiYT7UxO?=
 =?us-ascii?Q?393nE7ebQzqCFM0iFIxPkkpGJMMPSHIwj2EQi6dLwdiJOkths+VI1tMJmipp?=
 =?us-ascii?Q?723iq9HTXUbc2ksNnn8jQ9Xl8mWHvl17zIs7t6FQl6yk9yn3QxQKw6GvtaiU?=
 =?us-ascii?Q?S5AN1s5C1FPF8oLYzwXrkdyAccIJscRlNUMZGXAPCUWzIjni68gbReyHkQYo?=
 =?us-ascii?Q?9j76Z1rAU6fFWO5/Z9MFBxre2PZSJ3z59JDg7x7nPQsQqZtnZ3k0Nw0tVrDs?=
 =?us-ascii?Q?Vd6r1j4GNB2G10tYQ3FojMlkgg0Ta2D8A+I8bBQLOW0abb3hyAAuq+xBIMj/?=
 =?us-ascii?Q?H1CPzmV8aqHby2EfTZZp2BpyTSrpFGWw5p1oHuCANqR91Q4gIxYz0cxqt64p?=
 =?us-ascii?Q?x4rn3LaF+FaSZReQ1+MDppKgP2eH2j9pFcC/hikJ1C+jjMD42rhH3Kkbbeza?=
 =?us-ascii?Q?s0Tr12MlOh40pqaVRExKp9mUO5GMfEbelT7DDPsNMaw74nVcH21yU5cCNBxU?=
 =?us-ascii?Q?IHeAFN9c7/TOjWwUcDaB0f2mJtU+IBuWodne68It1BDqzSyFf9tpJN9TtmQ4?=
 =?us-ascii?Q?CE1W20HQDH/xnPVMC9iA32ZwBipKYFDcPrkKvEMdy58k8zqirF/9AX8G/ar3?=
 =?us-ascii?Q?KHTg6BrL491FcZOxqFdNXy6PHqBj3u0VhVC6JZ3EE5evqBJ1+QQXizHvEem3?=
 =?us-ascii?Q?xOocrPYlm7APLHp6LuTkA22xa77DtiIgwEqbZkEqpOAFsbTDHywR4VKWObRe?=
 =?us-ascii?Q?zqMWSdvwWCQXCd+E161p5oQHEFInzs2Vtql7BoNNo7w8G5ymWhvAL1GbtLGJ?=
 =?us-ascii?Q?xlM4X9Ik0ix0nf4mkT2U9BvWmzab/Q0sySKLlgynPGp7mtDbn09Dj/V4WAQr?=
 =?us-ascii?Q?HFN+QliROH0Bh25VwFT6m/B560f6e94+DFW3j9S2MLfrn+g0W9NgXYaAUNO6?=
 =?us-ascii?Q?V2UyNgKI9ZkscDa6ooQ9huR61tuxwn29lKsM8s4uz5sMwXQMqJMM9ytXyqAO?=
 =?us-ascii?Q?hJAoTrkzbRj48+J4teYPvqLj2BnB4oScCGyFmI+UucEOIwbUA+MGbTaYlQNQ?=
 =?us-ascii?Q?1Jypfap0kZrzW3WhylykMKNrOwk71LkDbHJBAv1y/nvmyjE6vOZpFroZrwA1?=
 =?us-ascii?Q?0HrWkCKzLElb80P++OMJFKpZQ0ciwxNauyr7/3/5rDO9H9i/eHWnkm/BFWpk?=
 =?us-ascii?Q?TXbhOoBwuxtL+1gfUMOUn8cD/76R3L0ofaTZAo2ov8p4YULHYzRK1UUtq+0y?=
 =?us-ascii?Q?r+xV/eoRMXBV5RwGpbAXqIzexzjdZem99TsUyS65meFmUAmjfrBd+NKDHL9W?=
 =?us-ascii?Q?6glLHU0QQQx/WNqZ8ynoyMcIxEccvltCpXcY1rV1rv47TmjT5HauCmvtSNw/?=
 =?us-ascii?Q?d8TgKpQoMGMIBRjlGdphjaReEkjp/JxqFX66486G3OtWqAiOz0NuKYbOAGuH?=
 =?us-ascii?Q?J/sEq3a6Q/mDECKOzza4pTU=3D?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 89633b98-bcc3-45f3-f07b-08dbf79e4240
X-MS-Exchange-CrossTenant-AuthSource: SN4PR01MB7455.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Dec 2023 03:32:09.5231
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Z6G5GZfauDSwV1pkNdn6C7ZyKp0fstj/PFCss0K9VIKwXpQ73DIk/NDj5D9nLFM5/Qlv9Xh1nPUZvNNumCLD0f7Iu0r+SMdo+Fw8AkxcvIs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR01MB7784
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Thu, 07 Dec 2023 19:32:32 -0800 (PST)

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 <quan@os.amperecomputing.com>
---
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;
+	}
+	/*
+	 * 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;
 	}
 
-	/* Slave is not currently active, irq was for someone else. */
+	/*
+	 * If the slave has been stopped and not started then slave interrupt
+	 * handling is complete.
+	 */
 	if (bus->slave_state == ASPEED_I2C_SLAVE_INACTIVE)
 		return irq_handled;
 
+	command = readl(bus->base + ASPEED_I2C_CMD_REG);
 	dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
 		irq_status, command);
 
@@ -279,17 +306,6 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		irq_handled |= ASPEED_I2CD_INTR_RX_DONE;
 	}
 
-	/* Slave was asked to stop. */
-	if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
-		irq_handled |= ASPEED_I2CD_INTR_NORMAL_STOP;
-		bus->slave_state = ASPEED_I2C_SLAVE_STOP;
-	}
-	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;
-	}
-
 	switch (bus->slave_state) {
 	case ASPEED_I2C_SLAVE_READ_REQUESTED:
 		if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_ACK))
@@ -324,8 +340,7 @@ static u32 aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
 		i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
 		break;
 	case ASPEED_I2C_SLAVE_STOP:
-		i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
-		bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
+		/* Stop event handling is done early. Unreachable. */
 		break;
 	case ASPEED_I2C_SLAVE_START:
 		/* Slave was just started. Waiting for the next event. */;
-- 
2.35.1