From patchwork Tue Jun 28 19:30:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corey Minyard X-Patchwork-Id: 9203991 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7D4CA6075F for ; Tue, 28 Jun 2016 19:55:02 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6EC111FF0B for ; Tue, 28 Jun 2016 19:55:02 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 606EA2860F; Tue, 28 Jun 2016 19:55:02 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id AA0871FF0B for ; Tue, 28 Jun 2016 19:55:01 +0000 (UTC) Received: from localhost ([::1]:39286 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHz64-0002ar-QU for patchwork-qemu-devel@patchwork.kernel.org; Tue, 28 Jun 2016 15:55:00 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43106) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHyiG-00063G-V8 for qemu-devel@nongnu.org; Tue, 28 Jun 2016 15:30:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bHyiD-00031S-Oh for qemu-devel@nongnu.org; Tue, 28 Jun 2016 15:30:24 -0400 Received: from mail-pf0-x241.google.com ([2607:f8b0:400e:c00::241]:34303) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bHyiD-00031G-DP for qemu-devel@nongnu.org; Tue, 28 Jun 2016 15:30:21 -0400 Received: by mail-pf0-x241.google.com with SMTP id 66so2471698pfy.1 for ; Tue, 28 Jun 2016 12:30:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:from:to:cc:subject:date:message-id; bh=zjrMIXLUm7LyQQYjBuWG/38IORId5kKKqA27/mkVZFY=; b=y6xuP3LPmm2sjSEoz8NLpaW5IZDfV9xx+v0gGNBD+ZlBPdaxVroKrnYFlR91735zRu xnfc1TZcusdFl5N5bkaG7SeNvluw42ybtAvB6rgLnGXAw+bfzJRoqo6Z4j6nGNBbghfn uzfnh1dRkIxNhNpDvTK5VoO+CVHrldLHafFfDch1N9zbWjZGAFN9nBmJS/AstYFFA7zn L5ubN2AWuafkPfHlDAckBQKBlnmqo2VXdumteGlQkai4LQ7dINlDMklnq7SWlL0eiEZb MUr+1JHE2CCMM8WBBfmIpq7Z/9Rlxcv2AKWY4G78o2NYag/sRrHM956e2kPYdJTfaUH6 xMyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id; bh=zjrMIXLUm7LyQQYjBuWG/38IORId5kKKqA27/mkVZFY=; b=H+3vXq9otHDTrJDUrmDjhHjUyVzlDv4Zc+kZ7Ysd5t79N8694ZHGylzomJ8GG4r7DR pY/6JZDo1H7SMTwFvyzM9QNxjBskgyb0+o9qPiWM4gbkAbkf/04K5oeEuKPJoZjaP3FS m7/Cl6kvsehpB83fzVcTnYyR6xAMkXw0usuZK62V5WP8ZSZvIntzfjfoO7FIgD+JLGmn 7EyDtrNWzl7qWe4hydctRfxoCobySVMXPanGZnCi9kPpKo/O7gHPvUDBOwOFwkVGp12R syxnY4wlqsMNkzZl/MehlJiGLJZqgVcQyQrUd8/qea0VW5fk36nxeaM45IYqXvoNihwY BzmQ== X-Gm-Message-State: ALyK8tI7KfZv3t0Wn7J1o4D3F+8O7uq69k5i21sD7+Mk7hlQuz74FMDUbePO/9jx4gXV+Q== X-Received: by 10.98.5.2 with SMTP id 2mr4796459pff.58.1467142220609; Tue, 28 Jun 2016 12:30:20 -0700 (PDT) Received: from serve.minyard.net ([173.64.235.97]) by smtp.gmail.com with ESMTPSA id an13sm1208819pac.42.2016.06.28.12.30.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 Jun 2016 12:30:19 -0700 (PDT) Received: from t430.minyard.net (t430m.minyard.net [192.168.27.3]) by serve.minyard.net (Postfix) with ESMTPA id 5CF6C79B; Tue, 28 Jun 2016 14:30:18 -0500 (CDT) Received: by t430.minyard.net (Postfix, from userid 1000) id A5ACB30267D; Tue, 28 Jun 2016 14:30:17 -0500 (CDT) From: minyard@acm.org To: qemu-devel@nongnu.org, minyard@acm.org, cminyard@mvista.com Date: Tue, 28 Jun 2016 14:30:12 -0500 Message-Id: <1467142212-29810-1-git-send-email-minyard@acm.org> X-Mailer: git-send-email 2.7.4 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:400e:c00::241 Subject: [Qemu-devel] [PATCH v2] i2c: Fix SMBus read transactions to avoid double events X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Peter Crosthwaite , Alistair Francis , Kwon , KONRAD Frederic Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Corey Minyard Change 2293c27faddf (i2c: implement broadcast write) added broadcast capability to the I2C bus, but it broke SMBus read transactions. An SMBus read transaction does two i2c_start_transaction() calls without an intervening i2c_end_transfer() call. This will result in i2c_start_transfer() adding the same device to the current_devs list twice, and then the ->event() for the same device gets called twice in the second call to i2c_start_transfer(), resulting in the smbus code getting confused. Note that this happens even with pure I2C devices when simulating SMBus over I2C. This fix only scans the bus if the current set of devices is empty. This means that the current set of devices stays fixed until i2c_end_transfer() is called, which is really what you want. This also deletes the empty check from the top of i2c_end_transfer(). It's unnecessary, and it prevents the broadcast variable from being set to false at the end of the transaction if no devices were on the bus. Cc: KONRAD Frederic Cc: Alistair Francis Cc: Peter Crosthwaite Cc: Kwon Cc: Peter Maydell Signed-off-by: Corey Minyard Reviewed-by: KONRAD Frederic Tested-by: KONRAD Frederic --- hw/i2c/core.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) This fix should work with I2C devices as well as SMBus devices. Sorry for not thinking it through all the way before. diff --git a/hw/i2c/core.c b/hw/i2c/core.c index abb3efb..6313d31 100644 --- a/hw/i2c/core.c +++ b/hw/i2c/core.c @@ -101,15 +101,21 @@ int i2c_start_transfer(I2CBus *bus, uint8_t address, int recv) bus->broadcast = true; } - QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) { - DeviceState *qdev = kid->child; - I2CSlave *candidate = I2C_SLAVE(qdev); - if ((candidate->address == address) || (bus->broadcast)) { - node = g_malloc(sizeof(struct I2CNode)); - node->elt = candidate; - QLIST_INSERT_HEAD(&bus->current_devs, node, next); - if (!bus->broadcast) { - break; + /* + * If there are already devices in the list, that means we are in + * the middle of a transaction and we shouldn't rescan the bus. + */ + if (QLIST_EMPTY(&bus->current_devs)) { + QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) { + DeviceState *qdev = kid->child; + I2CSlave *candidate = I2C_SLAVE(qdev); + if ((candidate->address == address) || (bus->broadcast)) { + node = g_malloc(sizeof(struct I2CNode)); + node->elt = candidate; + QLIST_INSERT_HEAD(&bus->current_devs, node, next); + if (!bus->broadcast) { + break; + } } } } @@ -134,10 +140,6 @@ void i2c_end_transfer(I2CBus *bus) I2CSlaveClass *sc; I2CNode *node, *next; - if (QLIST_EMPTY(&bus->current_devs)) { - return; - } - QLIST_FOREACH_SAFE(node, &bus->current_devs, next, next) { sc = I2C_SLAVE_GET_CLASS(node->elt); if (sc->event) {