From nobody Fri Dec 17 07:31:11 2021 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id DBF6E1903798; Fri, 17 Dec 2021 07:31:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JFgdW4T1rz3Jpl; Fri, 17 Dec 2021 07:31:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 794BB17DA5; Fri, 17 Dec 2021 07:31:11 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1BH7VB6p044944; Fri, 17 Dec 2021 07:31:11 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1BH7VBGG044943; Fri, 17 Dec 2021 07:31:11 GMT (envelope-from git) Date: Fri, 17 Dec 2021 07:31:11 GMT Message-Id: <202112170731.1BH7VBGG044943@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Andriy Gapon Subject: git: c7bf34cfe52c - stable/13 - twsi: move handling of TWSI_CONTROL_ACK into the state machine List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: avg X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: c7bf34cfe52c09abf813c784a60d7d9677afb603 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1639726271; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=PJ5sCMJPi6uCHuRxqfOznTtAyULoY0Q6MRXJl3m9o1w=; b=nVSI5KSWnqUY52jxLp0JdlZDgLls7KTUE1SCTrxaeYGyEg7uOouObhUb8JLO80AQ0bxGfc 8PxsoVDFkXNLMQ5HQmG51UQSUhn16aqXqsQbl801Q/yZxpQ5tgc5NkHqYVqTSKOeVws9pv su1/DZUwY6N0CWljrb/BGvRVBEAgQIrLpEO18s63ay6LkuYhSwFCgGVpDDSZnSCCgKKZZF 9PdDVFx8bBXFzR1p2xyqkKh8QppxET3KFFFaXz5WgqI0DllSEl72cXsfzRpyCKZ45U2EzO xWlS65Q67nB2M51CEvKXNDDBn4m+Tyo1EZOcKuF/39CXCPlEUjisiV+S8KjaWA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1639726271; a=rsa-sha256; cv=none; b=Sh3rq1x0NQ/MaPZd3OkdjMkSPwj6kT0ExxSoowXV+nzzl34TmjGI9WWLq0feYBiAn07FrB q5kptVq/1Qh0/qrAfWLj2sClxIAy93dkzTwnPuKKOw6DOt/pdXaQ6hJofzqElKpVwXK+fW 16fNDTrDeeXR6AvqyvcqFm0MwqetLbSSfc71tdW3ByrpGW0AdP5EUIbcGv/JYgiPn7ukvY KWLsBRFtW2Za2+WRKUb3mkuHg+FgXawSBFOsqM/7IvifoWVNQoXvQhuMJTNYREhQ0CDwOS JP9t0OZNUIxpon+sTGQzNaiWGlcMw4MojPnNZFMDFThfIOWDaDia1dpCViLDVA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by avg: URL: https://cgit.FreeBSD.org/src/commit/?id=c7bf34cfe52c09abf813c784a60d7d9677afb603 commit c7bf34cfe52c09abf813c784a60d7d9677afb603 Author: Andriy Gapon AuthorDate: 2021-11-26 08:07:27 +0000 Commit: Andriy Gapon CommitDate: 2021-12-17 07:30:41 +0000 twsi: move handling of TWSI_CONTROL_ACK into the state machine Previously the code set TWSI_CONTROL_ACK in twsi_transfer() based on whether the first message had a length of one. That was done regardless of whether the message was a read or write and what kind of messages followed it. Now the bit is set or cleared while handling TWSI_STATUS_ADDR_R_ACK state transition based on the current (read) message. The old code did not correctly work in a scenario where a single byte was read from an EEPROM device with two byte addressing. For example: i2c -m tr -a 0x50 -d r -w 16 -o 0 -c 1 -v The reason is that the first message (a write) has two bytes, so TWSI_CONTROL_ACK was set and never cleared. Since the controller did not send NACK the EEPROM sent more data resulting in a buffer overrun. While working on TWSI_STATUS_ADDR_R_ACK I also added support for the zero-length read access and then I did the same for zero-length write access. While rare, those types of I2C transactions are completely valid and are used by some devices. PR: 258994 (cherry picked from commit 04622a7f21157827fe75276a4520a52d3a571a86) --- sys/dev/iicbus/twsi/twsi.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/sys/dev/iicbus/twsi/twsi.c b/sys/dev/iicbus/twsi/twsi.c index ecadfade04d9..4a20d7ea35b6 100644 --- a/sys/dev/iicbus/twsi/twsi.c +++ b/sys/dev/iicbus/twsi/twsi.c @@ -516,11 +516,9 @@ twsi_transfer(device_t dev, struct iic_msg *msgs, uint32_t nmsgs) for (int i = 0; i < nmsgs; i++) debugf(sc, "msg %d is %d bytes long\n", i, msgs[i].len); #endif + /* Send start and re-enable interrupts */ - sc->control_val = TWSI_CONTROL_TWSIEN | - TWSI_CONTROL_INTEN | TWSI_CONTROL_ACK; - if (sc->msgs[0].len == 1) - sc->control_val &= ~TWSI_CONTROL_ACK; + sc->control_val = TWSI_CONTROL_TWSIEN | TWSI_CONTROL_INTEN; TWSI_WRITE(sc, sc->reg_control, sc->control_val | TWSI_CONTROL_START); msleep_sbt(sc, &sc->mutex, 0, "twsi", 3000 * SBT_1MS, SBT_1MS, 0); debugf(sc, "pause finish\n"); @@ -597,22 +595,36 @@ twsi_intr(void *arg) break; case TWSI_STATUS_ADDR_W_ACK: - debugf(sc, "Ack received after transmitting the address (write)\n"); - /* Directly send the first byte */ - sc->sent_bytes = 1; - debugf(sc, "Sending byte 0 (of %d) = %x\n", - sc->msgs[sc->msg_idx].len, - sc->msgs[sc->msg_idx].buf[0]); - TWSI_WRITE(sc, sc->reg_data, sc->msgs[sc->msg_idx].buf[0]); + debugf(sc, "Address ACK-ed (write)\n"); - TWSI_WRITE(sc, sc->reg_control, sc->control_val); + if (sc->msgs[sc->msg_idx].len > 0) { + /* Directly send the first byte */ + sc->sent_bytes = 1; + debugf(sc, "Sending byte 0 (of %d) = %x\n", + sc->msgs[sc->msg_idx].len, + sc->msgs[sc->msg_idx].buf[0]); + TWSI_WRITE(sc, sc->reg_data, + sc->msgs[sc->msg_idx].buf[0]); + } else { + debugf(sc, "Zero-length write, sending STOP\n"); + TWSI_WRITE(sc, sc->reg_control, + sc->control_val | TWSI_CONTROL_STOP); + } break; case TWSI_STATUS_ADDR_R_ACK: - debugf(sc, "Ack received after transmitting the address (read)\n"); + debugf(sc, "Address ACK-ed (read)\n"); sc->recv_bytes = 0; - TWSI_WRITE(sc, sc->reg_control, sc->control_val); + if (sc->msgs[sc->msg_idx].len == 0) { + debugf(sc, "Zero-length read, sending STOP\n"); + TWSI_WRITE(sc, sc->reg_control, + sc->control_val | TWSI_CONTROL_STOP); + } else if (sc->msgs[sc->msg_idx].len == 1) { + sc->control_val &= ~TWSI_CONTROL_ACK; + } else { + sc->control_val |= TWSI_CONTROL_ACK; + } break; case TWSI_STATUS_ADDR_W_NACK: