linux-kernel - [PATCH v3 4/7] i2c: aspeed: Acknowledge Tx done w/wo ACK irq late

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 19 May 2021 14:49:31 +0700
From:   Quan Nguyen <quan@...amperecomputing.com>
To:     Corey Minyard <minyard@....org>, Rob Herring <robh+dt@...nel.org>,
        Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>,
        Brendan Higgins <brendanhiggins@...gle.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Wolfram Sang <wsa@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        openipmi-developer@...ts.sourceforge.net,
        devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-aspeed@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        linux-i2c@...r.kernel.org
Cc:     Open Source Submission <patches@...erecomputing.com>,
        Phong Vo <phong@...amperecomputing.com>,
        "Thang Q . Nguyen" <thang@...amperecomputing.com>,
        openbmc@...ts.ozlabs.org
Subject: [PATCH v3 4/7] i2c: aspeed: Acknowledge Tx done w/wo ACK irq late

With Tx done w/wo ACK are ack'ed early at beginning of irq handler,
it is observed that, usually, the Tx done with Ack irq raises in the
READ REQUESTED state. This is unexpected and complaint as below appear:
"Unexpected Ack on read request"

Assumed that Tx done should only be ack'ed once it was truly processed,
switch to late ack'ed this two irqs and seen this issue go away through
test with AST2500..

Signed-off-by: Quan Nguyen <quan@...amperecomputing.com>
---
v3:
  + First introduce in v3 [Quan]

 drivers/i2c/busses/i2c-aspeed.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 3fb37c3f23d4..b2e9c8f0ddf7 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -606,8 +606,12 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 
 	spin_lock(&bus->lock);
 	irq_received = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
-	/* Ack all interrupts except for Rx done */
-	writel(irq_received & ~ASPEED_I2CD_INTR_RX_DONE,
+	/*
+	 * Ack all interrupts except for Rx done and
+	 * Tx done with/without ACK
+	 */
+	writel(irq_received &
+	       ~(ASPEED_I2CD_INTR_RX_DONE | ASPEED_I2CD_INTR_TX_ACK | ASPEED_I2CD_INTR_TX_NAK),
 	       bus->base + ASPEED_I2C_INTR_STS_REG);
 	readl(bus->base + ASPEED_I2C_INTR_STS_REG);
 	irq_received &= ASPEED_I2CD_INTR_RECV_MASK;
@@ -652,12 +656,18 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
 			"irq handled != irq. expected 0x%08x, but was 0x%08x\n",
 			irq_received, irq_handled);
 
-	/* Ack Rx done */
-	if (irq_received & ASPEED_I2CD_INTR_RX_DONE) {
-		writel(ASPEED_I2CD_INTR_RX_DONE,
-		       bus->base + ASPEED_I2C_INTR_STS_REG);
-		readl(bus->base + ASPEED_I2C_INTR_STS_REG);
-	}
+	/* Ack Rx done and Tx done with/without ACK */
+	/* Note: Re-use irq_handled variable */
+	irq_handled = 0;
+	if (irq_received & ASPEED_I2CD_INTR_RX_DONE)
+		irq_handled |= ASPEED_I2CD_INTR_RX_DONE;
+	if (irq_received & ASPEED_I2CD_INTR_TX_ACK)
+		irq_handled |= ASPEED_I2CD_INTR_TX_ACK;
+	if (irq_received & ASPEED_I2CD_INTR_TX_NAK)
+		irq_handled |= ASPEED_I2CD_INTR_TX_NAK;
+	writel(irq_handled, bus->base + ASPEED_I2C_INTR_STS_REG);
+	readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+
 	spin_unlock(&bus->lock);
 	return irq_remaining ? IRQ_NONE : IRQ_HANDLED;
 }
-- 
2.28.0