From patchwork Mon Nov 26 20:04:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corey Minyard X-Patchwork-Id: 152069 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp326496ljp; Mon, 26 Nov 2018 12:23:08 -0800 (PST) X-Google-Smtp-Source: AJdET5dnaGVbt7rZpY+WNobG/VjR//2vJqbe/OJzTEv2S2XgVxsCbDnQV6kDqwrUfGOXIFBjIfwb X-Received: by 2002:a81:6f03:: with SMTP id k3mr29458548ywc.424.1543263788724; Mon, 26 Nov 2018 12:23:08 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1543263788; cv=none; d=google.com; s=arc-20160816; b=gbelW9pfTgfo7/AswncTk+Vlz5auJidyOeaGVkkfns0ZPdmRjAbnqTOO40QHLTxhQI iTdNqqxF9CsuYQimTZqGN+SBFyxMMADDa4eSA3wFvCwS/ncD144bSfqIRHIteotC1/Uz 9umlHEfdEtdkChnNDzngpMTiuqLlxwws0Gi+Lp9OTJxL7qrWRR05EFtGXbteyCftLLCN boj2xoUZRjwCP3I6iiFlk6sAOUMh295D0Lc1WhDuK8rOwXl3adtF9T0j/P0H2D/t57JC Rvq2q9cTgLFu9b+cSgWm3Y6gVv+MgjHhJAdN6eK08NetCR81r7uk/MMnPuMFRWHD4cNu Zywg== 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=gM/LgMxXMeTIaMuAOPS6MexKvnSoDE/MEUCH4U4EIoY=; b=UyaqX0GXcpLV/wW9b6xfLO+Lzv68alttnilldDFx1agkHPWwAUyeKTFQDJPu3gQ4Ps BTd5IqYrDmI2RL5DcMJKoxkNUoSeS8hbcei3VyZtTMjWWCiVy2FCXIEaufNLd18zJy3O uijigkwQiFoLxS6AkHqZB2p34LB0JkPz6M0/EIzPv5CPg7pF9Ef67+ygciK+9xsXGLWF Z6am7frK6z3s8JxUxLxomil1ts+6pGbxg4D+LO4cv+wtol4e2rF8noGuzt0ZYDc5EyhA 4CpYgd4OmALAp8CrVxWMvfL4zrYaN0Sc/gHF6fF0LYa0GKTNpf74WuJCm2kBHZ+Bc0AF IUUw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b=RNX9W8sr; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org" Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id n128-v6si1021534ywf.400.2018.11.26.12.23.08 for (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 26 Nov 2018 12:23:08 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b=RNX9W8sr; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org" Received: from localhost ([::1]:38646 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gRNPP-0000MR-7e for patch@linaro.org; Mon, 26 Nov 2018 15:23:07 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53706) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gRN81-0001lG-Cb for qemu-devel@nongnu.org; Mon, 26 Nov 2018 15:05:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gRN7w-0001Jb-8X for qemu-devel@nongnu.org; Mon, 26 Nov 2018 15:05:09 -0500 Received: from mail-ot1-x343.google.com ([2607:f8b0:4864:20::343]:38136) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gRN7u-0001HR-7m for qemu-devel@nongnu.org; Mon, 26 Nov 2018 15:05:02 -0500 Received: by mail-ot1-x343.google.com with SMTP id e12so13562976otl.5 for ; Mon, 26 Nov 2018 12:04:56 -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=gM/LgMxXMeTIaMuAOPS6MexKvnSoDE/MEUCH4U4EIoY=; b=RNX9W8sr+nnBRrnGL7y/9BIkTCUWnqCZLaYgACMz20b7PrVV+YZaR8rr49hZRsdqWd fOYxy0DN84Rmz5c7y+dBF00M2iBdJ205HFSMvsaQegoLY6i7/Awc35Vhbx/K+gm0WPgF Vv+fETuEDq+wo4+GqoLznkF+WSQxzmEmrCGhH4eWru3/sbq/Ci86m/WSWq+3IR6Byoma FVz8ubxUZoxl3c7EOcLVmtNuabliJxzYAWF1N6Y7uec7q0OL9+57ZfqF/X+yy81uQn6L BOycgmBxLZjFtnIXgIUsQRWQretMgJeAyA13WP3KLwjvLJml6/r8HqMfPV1IC3YTAo5C DM+g== 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=gM/LgMxXMeTIaMuAOPS6MexKvnSoDE/MEUCH4U4EIoY=; b=UUY+1FUIbxRQ6i1g9PW32dc3Q8/FhDDVmoChHkMQDQA4fEqdv0CkvcpGs5zhWX1FJ1 9Od0FrIPIyz+AI9PMz5EBiwKmhOJqXMFnQ3PD5U0rJR0HbD3ULtDdUWlMSdKvDx1Axg4 Hi5KLGEB5KTbpCGW3UI8CrzU9xQFRf8D7ttF4DNt6kZcfKpRBqROHHuUoMitCu/0mhhy wDyyxlH6spWSWrsYBrLBIysa6rxQgJPpLt4+VXmr9DboR07Etepl5t9PfJ43lguRReAK OYGWSQwzBfyvo5mMfHQvvV0dTJtOFjqq6Z/iOdnBd7htSBJxtmG9NvgQicELBCwRsHjH dgwA== X-Gm-Message-State: AA+aEWYheC0AtnbDtf1mZoD5JLla9yleAVIuNbcSsSOk2HbsdXshr8TS xVYhhh1Kq7WNBSzgpFNTMg== X-Received: by 2002:a9d:6108:: with SMTP id i8mr16943726otj.278.1543262695933; Mon, 26 Nov 2018 12:04:55 -0800 (PST) Received: from serve.minyard.net ([47.184.128.64]) by smtp.gmail.com with ESMTPSA id 4sm390426otw.39.2018.11.26.12.04.51 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 26 Nov 2018 12:04:52 -0800 (PST) Received: from t430.minyard.net (t430m.minyard.net [192.168.27.3]) by serve.minyard.net (Postfix) with ESMTPA id CA45A1DA0; Mon, 26 Nov 2018 14:04:48 -0600 (CST) Received: by t430.minyard.net (Postfix, from userid 1000) id 6B329301468; Mon, 26 Nov 2018 14:04:46 -0600 (CST) From: minyard@acm.org To: qemu-devel@nongnu.org, "Dr . David Alan Gilbert" , =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= , Peter Maydell Date: Mon, 26 Nov 2018 14:04:26 -0600 Message-Id: <20181126200435.23408-8-minyard@acm.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181126200435.23408-1-minyard@acm.org> References: <20181126200435.23408-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::343 Subject: [Qemu-devel] [PATCH v3 07/16] 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: 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 f3c6cc46f9..8793113c25 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 (!read && s->smb_index == s->smb_data0) { uint8_t prot = (s->smb_ctl >> 2) & 0x07; @@ -265,12 +297,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; } } @@ -281,6 +324,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); } @@ -334,8 +381,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); } @@ -356,10 +404,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);