From patchwork Wed Feb 20 13:59:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Corey Minyard X-Patchwork-Id: 158825 Delivered-To: patch@linaro.org Received: by 2002:a02:48:0:0:0:0:0 with SMTP id 69csp5070607jaa; Wed, 20 Feb 2019 07:43:47 -0800 (PST) X-Google-Smtp-Source: AHgI3IZ/0XXCzuTxguzITg6bWv0L4M5wqqRxsZ+VGe4nmZWfC+szPM0c/c7o6TSzKsCGD6Mfacaz X-Received: by 2002:a25:4056:: with SMTP id n83mr9652830yba.471.1550677427027; Wed, 20 Feb 2019 07:43:47 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1550677427; cv=none; d=google.com; s=arc-20160816; b=xdcBoZZxMtj+m7eaF9GjbpVR1I33wLh8lGrJBxOoYJzmnFnDX9FfA6y4kO+KMWCs3M OI4QPNwsr/cKW+Kz8nEx1RzQtqgNS7P2jsJ9DE/y8yO7XC7mIYayQxf/6kXgS2NOnIAj KfhZRvgTYDH3EHDRmIuIEcitZ+pOl8uoD1HkLGhJxJwsTZcWWe5lpzTCSrtmS5iG3PAM HBGJwdPVtZlDloF2KTHxvIhyN+IY4hTo+Q3sLetZz2FJZyulgcUGqmGp1oecbBFUnaa+ D9OAAdmy9oZFcdCaCnYAIHybf+c00hDRtqhYwwbMHxxz+RnlTgDmWoA8o4gNXW6S0ebs 5w2g== 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=dOJuzU+xgz8lvq3FDLd5MKADrDVYfX2XNIhbX1U+mlnxTHrB8kF3ycZQEGvyVi9sHo R4btQko0k2pUDHXh8Dw2X4J7wVNQ1B6Bx+n+4n8J1tjNDBEcC3REIM7uZ8NzZ1ZWjXSq EDGzafdt27SONuSkyNCiTUq4N2k0UHnP9WykvQDw5c0mt8Qj8i16i7XSlhDSPEQJgu8z jYpy4hKp9e2RRagsLqryPjXQMGreoqjfCYVVZtKOfnG9Nkf92gWaHCd1YQxELKDYUqyc 60A1BF8HsJw/u1gt5sHyY0uvrgMA40hoCcSr/POPK52HjHH5kLOqmCBR0qKuN+DaguDP X3tQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@gmail.com header.s=20161025 header.b="rj3W/50K"; 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 t8si11303522yba.36.2019.02.20.07.43.46 for (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 20 Feb 2019 07:43:47 -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="rj3W/50K"; 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]:41120 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gwU2E-0005xb-En for patch@linaro.org; Wed, 20 Feb 2019 10:43:46 -0500 Received: from eggs.gnu.org ([209.51.188.92]:43840) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gwTmu-0002Rh-B7 for qemu-devel@nongnu.org; Wed, 20 Feb 2019 10:27:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gwTmq-0000Pu-MJ for qemu-devel@nongnu.org; Wed, 20 Feb 2019 10:27:56 -0500 Received: from mail-ot1-x341.google.com ([2607:f8b0:4864:20::341]:35329) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gwTmc-0008L5-LI for qemu-devel@nongnu.org; Wed, 20 Feb 2019 10:27:42 -0500 Received: by mail-ot1-x341.google.com with SMTP id z19so40819061otm.2 for ; Wed, 20 Feb 2019 07:26:43 -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=rj3W/50KIP/g8l/XLh/SwpEYTSiJ2jwxLtth67WOegVWfb+j8ijKwy+ab41k4TboMR maIhUgzOido6BuWYt3an+n5xraXdVfe0ydpgPnkWyTWrvvMiD4Pf/XjUXbbg2SJ1ANkn oOgsY2IVENI+zDGdkU0XYxkWoF1z9qBX72x6G5LWpFYiVsUUjne477XYMDFijyVgKUvw IjX/JNwmhXe1I1wcWk/InpIpzF7LiRbHPZkous/O4M9a7n0gvIKRXf22qrL+7Yc4w5ag A+HLDMTeGvKuHVg9uP7lcKn8tqxWFPgu9FG7JI+iOG+nyqvvu1rV/aJdyP5BtnKIKzxf O/7Q== 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=mqHOMFK2Oe0vQev0AfP9f0YSq1VN1Y1nG8VFdMiMLC2hWRp1Cc+pi1J/ZPWiSnKZky 5pjAptsGl8MhcAf6Pi6h5GRgQ6BldYwadB3+x5MbA9UwyAaz0+wwovDhb1p+RE583krS zH3ZqSb5bs827hK9PhQhZ60k2HYYe8nM3ibJDnn3XFKCn9T9QKGVeIKSTXn0n//qDRB3 Ew/1FjMIxjflXpwMW46ZF0KVm4Gt50IrKiLC4NRJ8Fg6S0NWuSE/P0HaC7PeUmcwMdg5 n3cYZncxUiMoiypM6HJG14k65bAxbjliEkXeeJuicEIy2PHSdCYrYJm8i3sIcoNUFxew tfJA== X-Gm-Message-State: AHQUAubWCvmMD2/weWFfUnJQxV8c6znu03OrZ/uZ1D7zW7hjafm7ybeZ QVIYZ4OjjEKSvgXgcoA7oBZsGYUAFQ== X-Received: by 2002:a05:6830:e:: with SMTP id c14mr6651857otp.144.1550671220522; Wed, 20 Feb 2019 06:00:20 -0800 (PST) Received: from serve.minyard.net (serve.minyard.net. [2001:470:b8f6:1b::1]) by smtp.gmail.com with ESMTPSA id r124sm3990133oih.7.2019.02.20.06.00.13 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 20 Feb 2019 06:00:16 -0800 (PST) Received: from t430.minyard.net (unknown [IPv6:2001:470:b8f6:1b:d1f6:7172:48bb:512b]) by serve.minyard.net (Postfix) with ESMTPA id 9F9E81808EF; Wed, 20 Feb 2019 14:00:10 +0000 (UTC) Received: by t430.minyard.net (Postfix, from userid 1000) id 6B8BD302A6A; Wed, 20 Feb 2019 08:00:09 -0600 (CST) From: minyard@acm.org To: qemu-devel@nongnu.org Date: Wed, 20 Feb 2019 07:59:48 -0600 Message-Id: <20190220135956.22589-12-minyard@acm.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190220135956.22589-1-minyard@acm.org> References: <20190220135956.22589-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::341 Subject: [Qemu-devel] [PATCH 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 , Corey Minyard , Corey Minyard , "Michael S . Tsirkin" , "Dr . David Alan Gilbert" , Paolo Bonzini 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);