From patchwork Mon Oct 24 18:12:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 79046 Delivered-To: patches@linaro.org Received: by 10.80.142.83 with SMTP id 19csp2935643edx; Mon, 24 Oct 2016 11:12:31 -0700 (PDT) X-Received: by 10.28.63.199 with SMTP id m190mr22525183wma.96.1477332751489; Mon, 24 Oct 2016 11:12:31 -0700 (PDT) Return-Path: Received: from orth.archaic.org.uk (orth.archaic.org.uk. [2001:8b0:1d0::2]) by mx.google.com with ESMTPS id y200si13478496wme.30.2016.10.24.11.12.31 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Oct 2016 11:12:31 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 2001:8b0:1d0::2 as permitted sender) client-ip=2001:8b0:1d0::2; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of pm215@archaic.org.uk designates 2001:8b0:1d0::2 as permitted sender) smtp.mailfrom=pm215@archaic.org.uk; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: from pm215 by orth.archaic.org.uk with local (Exim 4.84_2) (envelope-from ) id 1byjjZ-0002WU-Ml; Mon, 24 Oct 2016 19:12:29 +0100 From: Peter Maydell To: qemu-devel@nongnu.org Cc: patches@linaro.org, Jan Kiszka Subject: [PATCH] hw/i2c/bitbang_i2c: Handle NACKs from devices Date: Mon, 24 Oct 2016 19:12:29 +0100 Message-Id: <1477332749-27098-1-git-send-email-peter.maydell@linaro.org> X-Mailer: git-send-email 2.7.4 If the guest attempts to talk to a nonexistent device over i2c, the i2c_start_transfer() function will return non-zero, indicating that the bus is signalling a NACK. Similarly, if the i2c_send() function returns nonzero then the target device returned a NACK. Handle this possibility in the bitbang_i2c code, by returning the state machine to the STOPPED state and returning the NACK bit to the guest. This bit of missing functionality was spotted by Coverity (it noticed that we weren't checking the return value from i2c_start_transfer()). Signed-off-by: Peter Maydell --- Lightly tested using the musicpal board, which is the only one using the bitbang_i2c code. If you make the board put the Wolfson 8750 at the wrong I2C address then the guest will retry the transaction a few times and give up, as expected. Without this patch it will send a bunch of data out into the void without realizing there's a problem. --- hw/i2c/bitbang_i2c.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) -- 2.7.4 diff --git a/hw/i2c/bitbang_i2c.c b/hw/i2c/bitbang_i2c.c index d3a2989..8be88ee 100644 --- a/hw/i2c/bitbang_i2c.c +++ b/hw/i2c/bitbang_i2c.c @@ -130,14 +130,25 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int level) return bitbang_i2c_ret(i2c, 1); case WAITING_FOR_ACK: + { + int ret; + if (i2c->current_addr < 0) { i2c->current_addr = i2c->buffer; DPRINTF("Address 0x%02x\n", i2c->current_addr); - i2c_start_transfer(i2c->bus, i2c->current_addr >> 1, - i2c->current_addr & 1); + ret = i2c_start_transfer(i2c->bus, i2c->current_addr >> 1, + i2c->current_addr & 1); } else { DPRINTF("Sent 0x%02x\n", i2c->buffer); - i2c_send(i2c->bus, i2c->buffer); + ret = i2c_send(i2c->bus, i2c->buffer); + } + if (ret) { + /* NACK (either addressing a nonexistent device, or the + * device we were sending to decided to NACK us). + */ + DPRINTF("Got NACK\n"); + bitbang_i2c_enter_stop(i2c); + return bitbang_i2c_ret(i2c, 1); } if (i2c->current_addr & 1) { i2c->state = RECEIVING_BIT7; @@ -145,7 +156,7 @@ int bitbang_i2c_set(bitbang_i2c_interface *i2c, int line, int level) i2c->state = SENDING_BIT7; } return bitbang_i2c_ret(i2c, 0); - + } case RECEIVING_BIT7: i2c->buffer = i2c_recv(i2c->bus); DPRINTF("RX byte 0x%02x\n", i2c->buffer);