From nobody Fri Nov 26 14:24:03 2021 X-Original-To: dev-commits-src-main@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 8456E18A2D06; Fri, 26 Nov 2021 14:24:06 +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 4J0xnc2sdhz4cV7; Fri, 26 Nov 2021 14:24:04 +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 593002056; Fri, 26 Nov 2021 14:24:03 +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 1AQEO3WW002495; Fri, 26 Nov 2021 14:24:03 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1AQEO3dV002494; Fri, 26 Nov 2021 14:24:03 GMT (envelope-from git) Date: Fri, 26 Nov 2021 14:24:03 GMT Message-Id: <202111261424.1AQEO3dV002494@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Andriy Gapon Subject: git: 04622a7f2115 - main - twsi: move handling of TWSI_CONTROL_ACK into the state machine List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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/main X-Git-Reftype: branch X-Git-Commit: 04622a7f21157827fe75276a4520a52d3a571a86 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1637936645; 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=mqTnaffWmokw4JcGp4dw5pWHTuJXnrRzvNIeB8TavD0=; b=Q9m6azwkkMwG9+v9QIXMJO0T0eeNJ6zFswz+2uTMxClb2YntNDC09pixmsfzZg42EEyLhm vSnJTHQqEASZYglOOzidv05oWscRIErC91XF1EiA5zBtdFVVGBlIFdal2gfrjkCUqb/Sj1 AhzrFy+qUQZZZsDK0B09zcNAeO3CogMmbBvgRQDoZKG9ik0rABtmRps7nwbKtGfSoSQ4Rq Ue1h5cqFrxqZvRHIQjDZA57MuyWkktDXaJmbeiC7HjK4F4njhQGybYWSEOd1eC0VTuUfln kol9PQ+eNidWjXO27ERbr9qHr4yt5uaFnTIRAb9pKMqlaDMxnK1Ti84nQlL1cQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1637936645; a=rsa-sha256; cv=none; b=Q9eTDUfLQCwpncxo58NKmMd/llNuWWB08oQ2jc9gfDUD9h++HJMbMlebJ/61yib9bFVb+o YlMEq1pJD4Lnnr7qbxaONYnyYMWwT6oHjXdUbdDkKiRhknMZy7/R9dKhtQsi8+E2A7ia15 RHkWQ6bi6wiVSGQ97UKCb9bgiflZVG5c8CGTjQYYJFcORTrv/oO65esOtyeeZLfjiZynX+ 6pBQgZx8q388n57i684zRuvwJJKrJ8Oa18YpCIwd80UT0WJURgxxPfxEs8IiurNJsjGKYy M4+xBNGv6O51nqgx5RTqLEgYoH7X+RIojTQihaewGfPCdpNsQGogKLIlSr6OPQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by avg: URL: https://cgit.FreeBSD.org/src/commit/?id=04622a7f21157827fe75276a4520a52d3a571a86 commit 04622a7f21157827fe75276a4520a52d3a571a86 Author: Andriy Gapon AuthorDate: 2021-11-26 08:07:27 +0000 Commit: Andriy Gapon CommitDate: 2021-11-26 14:17:24 +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 MFC after: 3 weeks --- 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: