From nobody Fri Dec 17 07:31:52 2021 X-Original-To: dev-commits-src-all@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 577911904831; Fri, 17 Dec 2021 07:31:54 +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 4JFgfK0LsZz3KY5; Fri, 17 Dec 2021 07:31:52 +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 229DF17973; Fri, 17 Dec 2021 07:31:52 +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 1BH7VqSC046009; Fri, 17 Dec 2021 07:31:52 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1BH7VqhA046008; Fri, 17 Dec 2021 07:31:52 GMT (envelope-from git) Date: Fri, 17 Dec 2021 07:31:52 GMT Message-Id: <202112170731.1BH7VqhA046008@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: e8dbe7bdcf5d - stable/12 - twsi: support more message combinations in transfers List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@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: e8dbe7bdcf5d9dfbe40b326001b7751e3d42c635 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1639726313; 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=evAUZp9ud/dcyHeIZBhDtxMq4c9wMCY8TqqZeOQJwu4=; b=JvACbPt2EDRQGEVjTY5Dxx0Dl44Vmd7uwei5yFCo67uyFU7ps9DsgbTjTEl68GEcYtfkl1 W6xTNLbEIzTMTEwbT4POCFiAzG6kJoeeTsIYT9ZLd9DwN78GnVb9yZlgIsMIe8pY73Moz6 7P63xHfI/kxP56AFsqCiv946XC4HA+ei7PONr52uHFZyVoPI2tvXoSg+uLK5DFbhgyGQ/H EgK/dUnTbz1AULYJpZQnyQ0Bi4air15KNuDRyOjztuRYD61mT+sFQMn26r9Z3yqYt/GviX WuXGKW4D5LMhfYplFrX4TB6DEKdFPGe0PDy8kp45rI2JPvib8HWfx7fhs7zD8Q== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1639726313; a=rsa-sha256; cv=none; b=FGftnZHhotU+jpZlAi/riC/X2OkZC7d1VIDcgFuuAZ+tJ6RURs6z7Wz+hVq+enL2EEeRoJ LfvglR0bHtqo8wxSttUbHrYVtal6d0qXQsJ3FIE13qK36GAC8rVy4fLvHSO8PiuB3yVZRn ikRba/+GFABEZNHwKoTsiG+cGkBw079+joDb3ix88oiqdrG9TurJThrQug2gOEtl2fPTaw 0eGZu8Jn+V/GqEi6RthyXlBnmntDvWE7CAWw60vQ+ODZihnfdfE1W8/SG36ZJaxerPayTk hIoFggsrOrKGWdcJNu8KaSOyR5Mis+G5m6xTUc97IZs3sU/MlfMgFQ2xSS4jQQ== 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=e8dbe7bdcf5d9dfbe40b326001b7751e3d42c635 commit e8dbe7bdcf5d9dfbe40b326001b7751e3d42c635 Author: Andriy Gapon AuthorDate: 2021-11-26 09:48:21 +0000 Commit: Andriy Gapon CommitDate: 2021-12-17 07:31:39 +0000 twsi: support more message combinations in transfers Most prominently, add support for a transfer where a write with no-stop flag is followed by a write with no-start flag. Logically, it's a single larger write, but consumers may want to split it like that because one part can be a register ID and the other part can be data to be written to (or starting at) that register. Such a transfer can be created by i2c tool and iic(4) driver, e.g., for an EEPROM write at specific offset: i2c -m tr -a 0x50 -d w -w 16 -o 0 -c 8 -v < /dev/random This should be fixed by new code that handles the end of data transfer for both reads and writes. It handles two existing conditions and one new. Namely: - the last message has been completed -- end of transfer; - a message has been completed and the next one requires the start condition; - a message has been completed and the next one should be sent without the start condition. In the last case we simply switch to the next message and start sending its data. Reads without the start condition are not supported yet, though. That's because we NACK the last byte of the previous message, so the device stops sending data. To fix this we will need to add a look-ahead at the next message when handling the penultimate byte of the current one. This change also fixed a bug where msg_idx was not incremented after a read message. Apparently, typically a read message is a last message in a transfer, so the bug did not cause much trouble. PR: 258994 (cherry picked from commit ff1e8581806f70e54fecddf8a6a23488dc7b968a) --- sys/dev/iicbus/twsi/twsi.c | 85 +++++++++++++++++++++++++++++----------------- 1 file changed, 53 insertions(+), 32 deletions(-) diff --git a/sys/dev/iicbus/twsi/twsi.c b/sys/dev/iicbus/twsi/twsi.c index 2f77e2dc69c3..ab0bb3f26961 100644 --- a/sys/dev/iicbus/twsi/twsi.c +++ b/sys/dev/iicbus/twsi/twsi.c @@ -559,9 +559,12 @@ twsi_intr(void *arg) { struct twsi_softc *sc; uint32_t status; - int transfer_done = 0; + bool message_done; + bool send_start; sc = arg; + message_done = false; + send_start = false; mtx_lock(&sc->mutex); debugf(sc, "Got interrupt, current msg=%u\n", sc->msg_idx); @@ -578,6 +581,7 @@ twsi_intr(void *arg) goto end; } +restart: switch (status) { case TWSI_STATUS_START: case TWSI_STATUS_RPTD_START: @@ -636,38 +640,30 @@ twsi_intr(void *arg) twsi_error(sc, IIC_ENOACK); break; case TWSI_STATUS_DATA_WR_ACK: - debugf(sc, "Ack received after transmitting data\n"); + KASSERT(sc->sent_bytes <= sc->msgs[sc->msg_idx].len, + ("sent_bytes beyond message length")); + debugf(sc, "ACK received after transmitting data\n"); if (sc->sent_bytes == sc->msgs[sc->msg_idx].len) { - debugf(sc, "Done sending all the bytes for msg %d\n", sc->msg_idx); + debugf(sc, "Done TX data\n"); + /* Send stop, no interrupts on stop */ if (!(sc->msgs[sc->msg_idx].flags & IIC_M_NOSTOP)) { - debugf(sc, "Done TX data, send stop\n"); TWSI_WRITE(sc, sc->reg_control, sc->control_val | TWSI_CONTROL_STOP); } else { - debugf(sc, "Done TX data with NO_STOP\n"); - TWSI_WRITE(sc, sc->reg_control, sc->control_val | TWSI_CONTROL_START); - } - sc->msg_idx++; - if (sc->msg_idx == sc->nmsgs) { - debugf(sc, "transfer_done=1\n"); - transfer_done = 1; - sc->error = 0; - } else { - debugf(sc, "Send repeated start\n"); - TWSI_WRITE(sc, sc->reg_control, sc->control_val | TWSI_CONTROL_START); + debugf(sc, "NOSTOP flag\n"); } - } else { - debugf(sc, "Sending byte %d (of %d) = %x\n", - sc->sent_bytes, - sc->msgs[sc->msg_idx].len, - sc->msgs[sc->msg_idx].buf[sc->sent_bytes]); - TWSI_WRITE(sc, sc->reg_data, - sc->msgs[sc->msg_idx].buf[sc->sent_bytes]); - TWSI_WRITE(sc, sc->reg_control, - sc->control_val); - sc->sent_bytes++; + message_done = true; + break; } + + debugf(sc, "Sending byte %d (of %d) = 0x%x\n", + sc->sent_bytes, + sc->msgs[sc->msg_idx].len, + sc->msgs[sc->msg_idx].buf[sc->sent_bytes]); + TWSI_WRITE(sc, sc->reg_data, + sc->msgs[sc->msg_idx].buf[sc->sent_bytes]); + sc->sent_bytes++; break; case TWSI_STATUS_DATA_RD_ACK: @@ -715,6 +711,7 @@ twsi_intr(void *arg) TWSI_WRITE(sc, sc->reg_control, sc->control_val | TWSI_CONTROL_STOP); } + message_done = true; } else { /* * We should not have NACK-ed yet. @@ -723,9 +720,6 @@ twsi_intr(void *arg) debugf(sc, "NACK-ed before receving all bytes?\n"); twsi_error(sc, IIC_ESTATUS); } - sc->transfer = 0; - transfer_done = 1; - sc->error = 0; break; case TWSI_STATUS_BUS_ERROR: @@ -741,17 +735,44 @@ twsi_intr(void *arg) twsi_error(sc, IIC_ESTATUS); break; } - debugf(sc, "Refresh reg_control\n"); + if (message_done) { + sc->msg_idx++; + if (sc->msg_idx == sc->nmsgs) { + debugf(sc, "All messages transmitted\n"); + sc->transfer = 0; + sc->error = 0; + } else if ((sc->msgs[sc->msg_idx].flags & IIC_M_NOSTART) == 0) { + debugf(sc, "Send (repeated) start\n"); + send_start = true; + } else { + /* Just keep transmitting data. */ + KASSERT((sc->msgs[sc->msg_idx - 1].flags & IIC_M_NOSTOP) != 0, + ("NOSTART message after STOP")); + KASSERT((sc->msgs[sc->msg_idx].flags & IIC_M_RD) == + (sc->msgs[sc->msg_idx - 1].flags & IIC_M_RD), + ("change of transfer direction without a START")); + debugf(sc, "NOSTART message after NOSTOP\n"); + sc->sent_bytes = 0; + sc->recv_bytes = 0; + if ((sc->msgs[sc->msg_idx].flags & IIC_M_RD) == 0) { + status = TWSI_STATUS_ADDR_W_ACK; + goto restart; + } else { + debugf(sc, "Read+NOSTART unsupported\n"); + twsi_error(sc, IIC_ESTATUS); + } + } + } end: /* * Newer Allwinner chips clear IFLG after writing 1 to it. */ + debugf(sc, "Refresh reg_control\n"); TWSI_WRITE(sc, sc->reg_control, sc->control_val | - (sc->iflag_w1c ? TWSI_CONTROL_IFLG : 0)); + (sc->iflag_w1c ? TWSI_CONTROL_IFLG : 0) | + (send_start ? TWSI_CONTROL_START : 0)); - if (transfer_done == 1) - sc->transfer = 0; debugf(sc, "Done with interrupt, transfer = %d\n", sc->transfer); if (sc->transfer == 0) wakeup(sc);