From patchwork Mon Jan 28 17:54:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corey Minyard X-Patchwork-Id: 156844 Delivered-To: patch@linaro.org Received: by 2002:a02:48:0:0:0:0:0 with SMTP id 69csp3729055jaa; Mon, 28 Jan 2019 10:14:06 -0800 (PST) X-Google-Smtp-Source: ALg8bN5ZwH9AfWbvGCuX57sh+xInctnlL5ApyqPYdbgFP6kAc7gs8A2RNxa4lGTGbtgRMU7Wdxb/ X-Received: by 2002:adf:ec83:: with SMTP id z3mr23473108wrn.264.1548699245970; Mon, 28 Jan 2019 10:14:05 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1548699245; cv=none; d=google.com; s=arc-20160816; b=hXZx320yarvfL4dhwOMzWypC9a6mEwtD39e6PGNKmCFWNRVQiViVHPCy0v2zbeaCIV LAjYBRM9rL7N6OIeND+9srpxDCo8vXvz/LEPx6/cKpWGKYonCphtVDbObshODmkApH+T 8YUx4f+OT0ejkqxWaO1fAVcE2phdmMM2gktgS1hq9/Aqqgvi0s6OJN4EN0ItFHdfsyW6 NrmUCkIAMMWG/egUZlBvQsswDIsh6ndk+nyM61rReK6uD9vkywgW2/CEMf1I/uXmLSv0 ej5dTXJ5ghruKKIADWpggyKk0XuJmOVfiVvrB/4RcriJBLEKOMcRpQArflfftgazWVNh 4Zig== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject:references:in-reply-to :message-id:date:to:from:dkim-signature; bh=vCzpvVmLyXpfuQofAv3zcqXy6IBz4pj9h28tnU3AUe8=; b=ne4dRzP87htJ5P0OTzpCblnizm49m/70OfaGK86YXzNs4z0B+XT+ck1tkhiKCMUjS0 PR6RFMoEPsO41rQdxzIlnrEZy9ytGCrEt0yko8lyVgO3RJ/etGzXurB8t85w3f+WThD5 k9Etu4Ze1b2/1zMzMd2XEOr6paEEVHFdE3aJoY+H0QqPA/u3YAWi6Dhwa9NOLwvPSbEH Cdo0MEBE7X1sNxddORVbMFLRyQE+KYzsyt451ZOAg0nPE5/vuJJ037JEp0imFCHvawZp dvFOWHtvuj51h45WXv0sgbVgEmvr4pl5lxofiEIpTKMUpW0dUMeD1Ni78k0b6+VCiFGS /mDQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b=qhL54q58; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org" Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id b8si95497wmc.115.2019.01.28.10.14.05 for (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 28 Jan 2019 10:14:05 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b=qhL54q58; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org" Received: from localhost ([127.0.0.1]:36366 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1goBQ4-0004Hj-O4 for patch@linaro.org; Mon, 28 Jan 2019 13:14:04 -0500 Received: from eggs.gnu.org ([209.51.188.92]:36897) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1goBEF-00038N-Bs for qemu-devel@nongnu.org; Mon, 28 Jan 2019 13:01:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1goB8B-0005jK-R3 for qemu-devel@nongnu.org; Mon, 28 Jan 2019 12:55:48 -0500 Received: from mail-it1-x144.google.com ([2607:f8b0:4864:20::144]:40374) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1goB7z-0005Zb-Pm for qemu-devel@nongnu.org; Mon, 28 Jan 2019 12:55:26 -0500 Received: by mail-it1-x144.google.com with SMTP id h193so20646663ita.5 for ; Mon, 28 Jan 2019 09:55:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=vCzpvVmLyXpfuQofAv3zcqXy6IBz4pj9h28tnU3AUe8=; b=qhL54q58EE7ueKU4jeWTQUK7BVJMeymUBdM38wunecUe6k75ijrl4p9ASyj8FXpnZe x3qwph41pjyNU7KUH706iwHR4udQ5ffRqtw8YeC0yGvNjz7NZ0EeofF6r9uJz4906pIS 4SbF8DoHjSEblfd56W0hHi6Q3KlaMdfgIDe/fk9EHn/3BvV1R/bvaJ1CMBJMEocVvZD2 vXrWxhyKnp7TZX1hWNrKKlvReCt0C2WyDx95LtCmjueh08/85+IRXKBH96BwS4jo7imS /PzqgZBLRjNwaA301//cnE5fKbjbzECEmeizWyCzuczoh4Xt6cRm8HoVIHOc8fRTsuil IjVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=vCzpvVmLyXpfuQofAv3zcqXy6IBz4pj9h28tnU3AUe8=; b=bf3a2QXerpXI0CZzugi+PLjH4LUJKpCE6FuoO53NomIm6P4B7/4LcLqgAvU4JKP9ZJ I/aYVgO1tcVzJWbjVhTR0yNuFd5Bsj2IurNDZ+//oPKy3e99TYq7AibZ5kB7k2bs1bNm Cc/nnRwIgMURIMEKAwKJp5v5BPZOt/rdiGKKMnZS5r5kkkWMM2t6eYde1njBbmMpMqpp mSYgPo0o3Eq0SODB1JftPzoCs2GauZeLWqZ9poPshUOd77UIVqeK20QmzFkCrXCw4MNy QsqkUtt5g55i4PZgn4zVO3iEThiJApgZdxI86h3NU18gRmJ6KL269nbJgTibK9kuuqP/ Bo+w== X-Gm-Message-State: AJcUukcLEJq22pPSS0lVyrRuC7kaPWUGro74wlL0uqVaqtiVuqMTn985 F5gatDbp7PfTc9CQnY4qUA== X-Received: by 2002:a05:660c:91:: with SMTP id t17mr8989085itj.41.1548698120942; Mon, 28 Jan 2019 09:55:20 -0800 (PST) Received: from serve.minyard.net ([47.184.128.64]) by smtp.gmail.com with ESMTPSA id x10sm13631834iop.54.2019.01.28.09.55.15 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 28 Jan 2019 09:55:16 -0800 (PST) Received: from t430.minyard.net (unknown [IPv6:2001:470:b8f6:1b:c8f8:7971:e432:a838]) by serve.minyard.net (Postfix) with ESMTPA id 54E2BFCB; Mon, 28 Jan 2019 11:55:13 -0600 (CST) Received: by t430.minyard.net (Postfix, from userid 1000) id 2CD3D30115E; Mon, 28 Jan 2019 11:55:08 -0600 (CST) From: minyard@acm.org To: qemu-devel@nongnu.org, "Dr . David Alan Gilbert" Date: Mon, 28 Jan 2019 11:54:50 -0600 Message-Id: <20190128175458.27255-12-minyard@acm.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190128175458.27255-1-minyard@acm.org> References: <20190128175458.27255-1-minyard@acm.org> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4864:20::144 Subject: [Qemu-devel] [PATCH v4 11/19] i2c:pm_smbus: Fix pm_smbus handling of I2C block read 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 , Paolo Bonzini , Corey Minyard , Corey Minyard , "Michael S . Tsirkin" Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" From: Corey Minyard The I2C block read function of pm_smbus was completely broken. It required doing some direct I2C handling because it didn't have a defined size, the OS code just reads bytes until it marks the transaction finished. This also required adjusting how the AMIBIOS workaround code worked, the I2C block mode was setting STS_HOST_BUSY during a transaction, so that bit could no longer be used to inform the host status read code to start the transaction. Create a explicit bool for that operation. Also, don't read the next byte from the device in byte-by-byte mode unless the OS is actually clearing the byte done bit. Just assuming that's what the OS is doing is a bad idea. Signed-off-by: Corey Minyard --- hw/i2c/pm_smbus.c | 86 ++++++++++++++++++++++++++++++--------- include/hw/i2c/pm_smbus.h | 6 +++ 2 files changed, 73 insertions(+), 19 deletions(-) -- 2.17.1 diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c index 6cfb7eb1b3..81d2a425ec 100644 --- a/hw/i2c/pm_smbus.c +++ b/hw/i2c/pm_smbus.c @@ -118,19 +118,30 @@ static void smb_transaction(PMSMBus *s) } break; case PROT_I2C_BLOCK_READ: - if (read) { - int xfersize = s->smb_data0; - if (xfersize > sizeof(s->smb_data)) { - xfersize = sizeof(s->smb_data); - } - ret = smbus_read_block(bus, addr, s->smb_data1, s->smb_data, - xfersize, false, true); - goto data8; - } else { - /* The manual says the behavior is undefined, just set DEV_ERR. */ + /* According to the Linux i2c-i801 driver: + * NB: page 240 of ICH5 datasheet shows that the R/#W + * bit should be cleared here, even when reading. + * However if SPD Write Disable is set (Lynx Point and later), + * the read will fail if we don't set the R/#W bit. + * So at least Linux may or may not set the read bit here. + * So just ignore the read bit for this command. + */ + if (i2c_start_transfer(bus, addr, 0)) { goto error; } - break; + ret = i2c_send(bus, s->smb_data1); + if (ret) { + goto error; + } + if (i2c_start_transfer(bus, addr, 1)) { + goto error; + } + s->in_i2c_block_read = true; + s->smb_blkdata = i2c_recv(s->smbus); + s->op_done = false; + s->smb_stat |= STS_HOST_BUSY | STS_BYTE_DONE; + goto out; + case PROT_BLOCK_DATA: if (read) { ret = smbus_read_block(bus, addr, cmd, s->smb_data, @@ -208,6 +219,7 @@ static void smb_transaction_start(PMSMBus *s) { if (s->smb_ctl & CTL_INTREN) { smb_transaction(s); + s->start_transaction_on_status_read = false; } else { /* Do not execute immediately the command; it will be * executed when guest will read SMB_STAT register. This @@ -217,6 +229,7 @@ static void smb_transaction_start(PMSMBus *s) * checking for status. If STS_HOST_BUSY doesn't get * set, it gets stuck. */ s->smb_stat |= STS_HOST_BUSY; + s->start_transaction_on_status_read = true; } } @@ -226,19 +239,38 @@ smb_irq_value(PMSMBus *s) return ((s->smb_stat & ~STS_HOST_BUSY) != 0) && (s->smb_ctl & CTL_INTREN); } +static bool +smb_byte_by_byte(PMSMBus *s) +{ + if (s->op_done) { + return false; + } + if (s->in_i2c_block_read) { + return true; + } + return !(s->smb_auxctl & AUX_BLK); +} + static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, unsigned width) { PMSMBus *s = opaque; + uint8_t clear_byte_done; SMBUS_DPRINTF("SMB writeb port=0x%04" HWADDR_PRIx " val=0x%02" PRIx64 "\n", addr, val); switch(addr) { case SMBHSTSTS: + clear_byte_done = s->smb_stat & val & STS_BYTE_DONE; s->smb_stat &= ~(val & ~STS_HOST_BUSY); - if (!s->op_done && !(s->smb_auxctl & AUX_BLK)) { + if (clear_byte_done && smb_byte_by_byte(s)) { uint8_t read = s->smb_addr & 0x01; + if (s->in_i2c_block_read) { + /* See comment below PROT_I2C_BLOCK_READ above. */ + read = 1; + } + s->smb_index++; if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { s->smb_index = 0; @@ -268,12 +300,23 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, s->smb_stat |= STS_BYTE_DONE; } else if (s->smb_ctl & CTL_LAST_BYTE) { s->op_done = true; - s->smb_blkdata = s->smb_data[s->smb_index]; + if (s->in_i2c_block_read) { + s->in_i2c_block_read = false; + s->smb_blkdata = i2c_recv(s->smbus); + i2c_nack(s->smbus); + i2c_end_transfer(s->smbus); + } else { + s->smb_blkdata = s->smb_data[s->smb_index]; + } s->smb_index = 0; s->smb_stat |= STS_INTR; s->smb_stat &= ~STS_HOST_BUSY; } else { - s->smb_blkdata = s->smb_data[s->smb_index]; + if (s->in_i2c_block_read) { + s->smb_blkdata = i2c_recv(s->smbus); + } else { + s->smb_blkdata = s->smb_data[s->smb_index]; + } s->smb_stat |= STS_BYTE_DONE; } } @@ -284,6 +327,10 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, if (!s->op_done) { s->smb_index = 0; s->op_done = true; + if (s->in_i2c_block_read) { + s->in_i2c_block_read = false; + i2c_end_transfer(s->smbus); + } } smb_transaction_start(s); } @@ -337,8 +384,9 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width) switch(addr) { case SMBHSTSTS: val = s->smb_stat; - if (s->smb_stat & STS_HOST_BUSY) { + if (s->start_transaction_on_status_read) { /* execute command now */ + s->start_transaction_on_status_read = false; s->smb_stat &= ~STS_HOST_BUSY; smb_transaction(s); } @@ -359,10 +407,10 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width) val = s->smb_data1; break; case SMBBLKDAT: - if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { - s->smb_index = 0; - } - if (s->smb_auxctl & AUX_BLK) { + if (s->smb_auxctl & AUX_BLK && !s->in_i2c_block_read) { + if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) { + s->smb_index = 0; + } val = s->smb_data[s->smb_index++]; if (!s->op_done && s->smb_index == s->smb_data0) { s->op_done = true; diff --git a/include/hw/i2c/pm_smbus.h b/include/hw/i2c/pm_smbus.h index 6dd5b7040b..7bcca97672 100644 --- a/include/hw/i2c/pm_smbus.h +++ b/include/hw/i2c/pm_smbus.h @@ -33,6 +33,12 @@ typedef struct PMSMBus { /* Set on block transfers after the last byte has been read, so the INTR bit can be set at the right time. */ bool op_done; + + /* Set during an I2C block read, so we know how to handle data. */ + bool in_i2c_block_read; + + /* Used to work around a bug in AMIBIOS, see smb_transaction_start() */ + bool start_transaction_on_status_read; } PMSMBus; void pm_smbus_init(DeviceState *parent, PMSMBus *smb, bool force_aux_blk);