From nobody Fri Dec 17 07:31:48 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 46EDA1904595; Fri, 17 Dec 2021 07:31:50 +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 4JFgfF1nx8z3KRp; Fri, 17 Dec 2021 07:31:49 +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 AEA81179E2; Fri, 17 Dec 2021 07:31:48 +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 1BH7VmwJ045937; Fri, 17 Dec 2021 07:31:48 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1BH7Vm4M045936; Fri, 17 Dec 2021 07:31:48 GMT (envelope-from git) Date: Fri, 17 Dec 2021 07:31:48 GMT Message-Id: <202112170731.1BH7Vm4M045936@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: 074248b948b6 - stable/12 - 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/12 X-Git-Reftype: branch X-Git-Commit: 074248b948b6fab3ec3a84e2573201fe82190cf3 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1639726309; 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=PsLaN+/jU24xGXKC79/DmZPzhwsE7KhzSwj20yK52WQ=; b=WCR0EzZdleQmlc8rGwtnkKV1IMGNxdr+VaUuOm2E/zTK81/CWAQ8nRKocOtzKD1qUXDBHW aK0r3qjF0CCiQpK/rXYQ1iVdtkuLcAIiLYrk6weqcVL6tKtyWIPKppN6sLlxscGf1/QhMM gAjkvf8seDQtXZA7dKAlwJw2KCenptvvM/KYuMCF7vc96WuDm5z2l1agDyoWzOXyrng3Jq gsddRqIp9JLCmarnydVBNa3y45p8GxoJEF7Q1f6DScub4JIZ/X1a50dXhPGTNmh6xx1CZV JEsCgSZp3mx64bM0Fn2Wh3Hl5YJHH5DUpvAm8dEHFitM/FkTztRfBfbbMlXhag== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1639726309; a=rsa-sha256; cv=none; b=pdudoPi0lXYhVEiLfYj/eS4Uc2Y+v8Az1+yikw17fJiIw1wubgM8KVatxrjsAj1W1SnRyS 9sn7q7Ex+2AZHGDJ+KHDavo/lL2uasG7GIEfpog+OpmB7bPqTnbsfdTIgrspP5sj/1v1bH nPYJdHQpvpp5Q4K6CPuQdoD9Dp2IeCjqs64YmwNy75/gveR37uqCi75h2Q9/P7Wp75V25d eJB9XaVFa2gJUGe3kxuds4n2OEvJP7eD8UwoqeYxHRJwt64Zwccz+Tq8Cet748HWy5yFaI 0+3esD63VeruYTzAAThAnx2b6sL0hVONdYJ5nvTLsP2WZch1lcjb4v4OsZPNXA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/12 has been updated by avg: URL: https://cgit.FreeBSD.org/src/commit/?id=074248b948b6fab3ec3a84e2573201fe82190cf3 commit 074248b948b6fab3ec3a84e2573201fe82190cf3 Author: Andriy Gapon AuthorDate: 2021-11-26 08:07:27 +0000 Commit: Andriy Gapon CommitDate: 2021-12-17 07:31:21 +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: