From: minyard@acm.org
To: Igor Mammedov <imammedo@redhat.com>,
"Michael S . Tsirkin" <mst@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
qemu-devel@nongnu.org, minyard@acm.org
Cc: Corey Minyard <cminyard@mvista.com>
Subject: [Qemu-devel] [PATCH 1/7] i2c: Fix the PM SMBus driver so it actually works correctly
Date: Wed, 11 May 2016 14:46:00 -0500 [thread overview]
Message-ID: <1462995966-1184-2-git-send-email-minyard@acm.org> (raw)
In-Reply-To: <1462995966-1184-1-git-send-email-minyard@acm.org>
From: Corey Minyard <cminyard@mvista.com>
The PM SMBus driver really just didn't work. This patch fixes it
to be fairly hardware compliant with the actual hardware. Plus
it adds interrupts and working block transfers.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
hw/i2c/pm_smbus.c | 198 ++++++++++++++++++++++++++++++++++++++++------
hw/i2c/smbus.c | 24 +++---
hw/i2c/smbus_ich9.c | 27 ++++++-
include/hw/i2c/pm_smbus.h | 23 +++++-
include/hw/i2c/smbus.h | 5 +-
5 files changed, 236 insertions(+), 41 deletions(-)
diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index 6fc3923..d8d1032 100644
--- a/hw/i2c/pm_smbus.c
+++ b/hw/i2c/pm_smbus.c
@@ -23,8 +23,6 @@
#include "hw/i2c/pm_smbus.h"
#include "hw/i2c/smbus.h"
-/* no save/load? */
-
#define SMBHSTSTS 0x00
#define SMBHSTCNT 0x02
#define SMBHSTCMD 0x03
@@ -32,8 +30,9 @@
#define SMBHSTDAT0 0x05
#define SMBHSTDAT1 0x06
#define SMBBLKDAT 0x07
+#define SMBAUXCTL 0x0d
-#define STS_HOST_BUSY (1)
+#define STS_HOST_BUSY (1<<0)
#define STS_INTR (1<<1)
#define STS_DEV_ERR (1<<2)
#define STS_BUS_ERR (1<<3)
@@ -45,10 +44,28 @@
* ByteDoneStatus = 1 (STS_BYTE_DONE) and INTR = 1 (STS_INTR )
*/
-//#define DEBUG
+#define CTL_INTREN (1<<0)
+#define CTL_KILL (1<<1)
+#define CTL_LAST_BYTE (1<<5)
+#define CTL_START (1<<6)
+#define CTL_PEC_EN (1<<7)
+
+#define PROT_QUICK 0
+#define PROT_BYTE 1
+#define PROT_BYTE_DATA 2
+#define PROT_WORD_DATA 3
+#define PROT_PROC_CALL 4
+#define PROT_BLOCK_DATA 5
+#define PROT_I2C_BLOCK_DATA 6
+
+#define AUX_PEC (1<<0)
+#define AUX_BLK (1<<1)
+#define AUX_MASK 0x3
+
+/*#define DEBUG*/
#ifdef DEBUG
-# define SMBUS_DPRINTF(format, ...) printf(format, ## __VA_ARGS__)
+# define SMBUS_DPRINTF(format, ...) fprintf(stderr, format, ## __VA_ARGS__)
#else
# define SMBUS_DPRINTF(format, ...) do { } while (0)
#endif
@@ -61,6 +78,7 @@ static void smb_transaction(PMSMBus *s)
uint8_t cmd = s->smb_cmd;
uint8_t addr = s->smb_addr >> 1;
I2CBus *bus = s->smbus;
+ bool i2c_enable = s->i2c_enable;
int ret;
SMBUS_DPRINTF("SMBus trans addr=0x%02x prot=0x%02x\n", addr, prot);
@@ -68,11 +86,12 @@ static void smb_transaction(PMSMBus *s)
if ((s->smb_stat & STS_DEV_ERR) != 0) {
goto error;
}
+
switch(prot) {
- case 0x0:
+ case PROT_QUICK:
ret = smbus_quick_command(bus, addr, read);
goto done;
- case 0x1:
+ case PROT_BYTE:
if (read) {
ret = smbus_receive_byte(bus, addr);
goto data8;
@@ -80,7 +99,7 @@ static void smb_transaction(PMSMBus *s)
ret = smbus_send_byte(bus, addr, cmd);
goto done;
}
- case 0x2:
+ case PROT_BYTE_DATA:
if (read) {
ret = smbus_read_byte(bus, addr, cmd);
goto data8;
@@ -89,22 +108,58 @@ static void smb_transaction(PMSMBus *s)
goto done;
}
break;
- case 0x3:
+ case PROT_WORD_DATA:
if (read) {
ret = smbus_read_word(bus, addr, cmd);
goto data16;
} else {
- ret = smbus_write_word(bus, addr, cmd, (s->smb_data1 << 8) | s->smb_data0);
+ ret = smbus_write_word(bus, addr, cmd,
+ (s->smb_data1 << 8) | s->smb_data0);
goto done;
}
break;
- case 0x5:
+ case PROT_I2C_BLOCK_DATA:
+ cmd = s->smb_data1;
+ if (s->smb_ctl & CTL_LAST_BYTE) {
+ s->smb_data0 = 1;
+ } else {
+ s->smb_data0 = PM_SMBUS_MAX_MSG_SIZE;
+ }
+ read = true;
+ i2c_enable = true;
+ /* Fallthrough */
+ case PROT_BLOCK_DATA:
if (read) {
- ret = smbus_read_block(bus, addr, cmd, s->smb_data);
- goto data8;
+ ret = smbus_read_block(bus, addr, cmd, s->smb_data,
+ sizeof(s->smb_data), !i2c_enable);
+ s->smb_index = 0;
+ s->op_done = false;
+ if (s->smb_auxctl & AUX_BLK) {
+ s->smb_stat |= STS_INTR;
+ } else {
+ s->smb_stat |= STS_HOST_BUSY;
+ }
+ goto datablk;
} else {
- ret = smbus_write_block(bus, addr, cmd, s->smb_data, s->smb_data0);
- goto done;
+ if (s->smb_auxctl & AUX_BLK || s->smb_index == s->smb_data0) {
+ if (s->smb_index != s->smb_data0) {
+ s->smb_index = 0;
+ goto error;
+ }
+ /* Data is already all written to the queue, just do
+ the operation. */
+ ret = smbus_write_block(bus, addr, cmd, s->smb_data,
+ s->smb_data0, !i2c_enable);
+ s->op_done = true;
+ s->smb_index = 0;
+ s->smb_stat |= STS_INTR;
+ s->smb_stat &= ~STS_HOST_BUSY;
+ } else {
+ s->op_done = false;
+ s->smb_stat |= STS_HOST_BUSY;
+ ret = 0;
+ }
+ goto doneblk;
}
break;
default:
@@ -128,11 +183,27 @@ done:
}
s->smb_stat |= STS_BYTE_DONE | STS_INTR;
return;
+datablk:
+ if (ret < 0) {
+ goto error;
+ }
+ s->smb_data0 = ret;
+doneblk:
+ if (ret < 0) {
+ goto error;
+ }
+ s->smb_stat |= STS_BYTE_DONE;
+ return;
error:
s->smb_stat |= STS_DEV_ERR;
return;
+}
+static bool
+smb_irq_value(PMSMBus *s)
+{
+ return ((s->smb_stat & ~STS_HOST_BUSY) != 0) && (s->smb_ctl & CTL_INTREN);
}
static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
@@ -140,17 +211,30 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
{
PMSMBus *s = opaque;
- SMBUS_DPRINTF("SMB writeb port=0x%04" HWADDR_PRIx
- " val=0x%02" PRIx64 "\n", addr, val);
+ SMBUS_DPRINTF("SMB writeb port=0x%04" HWADDR_PRIx " val=0x%02" PRIx64 "\n",
+ addr, val);
switch(addr) {
case SMBHSTSTS:
- s->smb_stat = (~(val & 0xff)) & s->smb_stat;
- s->smb_index = 0;
+ s->smb_stat &= ~(val & ~STS_HOST_BUSY);
+ if (!s->op_done && !(s->smb_auxctl & AUX_BLK)) {
+ s->smb_stat |= STS_BYTE_DONE;
+ }
break;
case SMBHSTCNT:
s->smb_ctl = val;
- if (val & 0x40)
+ if (s->smb_ctl & CTL_START) {
+ if (!s->op_done) {
+ s->smb_index = 0;
+ s->op_done = true;
+ }
smb_transaction(s);
+ }
+ if (s->smb_ctl & CTL_KILL) {
+ s->op_done = true;
+ s->smb_index = 0;
+ s->smb_stat |= STS_FAILED;
+ s->smb_stat &= ~STS_HOST_BUSY;
+ }
break;
case SMBHSTCMD:
s->smb_cmd = val;
@@ -165,13 +249,28 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
s->smb_data1 = val;
break;
case SMBBLKDAT:
- s->smb_data[s->smb_index++] = val;
- if (s->smb_index > 31)
+ if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) {
s->smb_index = 0;
+ }
+ s->smb_data[s->smb_index++] = val;
+ if (!(s->smb_auxctl & AUX_BLK) && s->smb_ctl & CTL_START &&
+ !s->op_done && s->smb_index == s->smb_data0) {
+ smb_transaction(s);
+ s->op_done = true;
+ s->smb_stat |= STS_INTR;
+ s->smb_stat &= ~STS_HOST_BUSY;
+ }
+ break;
+ case SMBAUXCTL:
+ s->smb_auxctl = val & AUX_MASK;
break;
default:
break;
}
+
+ if (s->set_irq) {
+ s->set_irq(s, smb_irq_value(s));
+ }
}
static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width)
@@ -184,7 +283,6 @@ static uint64_t smb_ioport_readb(void *opaque, hwaddr addr, unsigned width)
val = s->smb_stat;
break;
case SMBHSTCNT:
- s->smb_index = 0;
val = s->smb_ctl & 0x1f;
break;
case SMBHSTCMD:
@@ -200,18 +298,47 @@ 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;
+ }
val = s->smb_data[s->smb_index++];
- if (s->smb_index > 31)
+ if (s->smb_ctl & CTL_START && !s->op_done &&
+ s->smb_index == s->smb_data0) {
+ s->op_done = true;
+ s->smb_index = 0;
+ s->smb_stat &= ~STS_HOST_BUSY;
+ }
+ if (s->smb_ctl & CTL_LAST_BYTE) {
+ s->op_done = true;
s->smb_index = 0;
+ s->smb_stat |= STS_INTR;
+ s->smb_stat &= ~STS_HOST_BUSY;
+ }
+ break;
+ case SMBAUXCTL:
+ val = s->smb_auxctl;
break;
default:
val = 0;
break;
}
- SMBUS_DPRINTF("SMB readb port=0x%04" HWADDR_PRIx " val=0x%02x\n", addr, val);
+ SMBUS_DPRINTF("SMB readb port=0x%04" HWADDR_PRIx " val=0x%02x\n",
+ addr, val);
+
+ if (s->set_irq) {
+ s->set_irq(s, smb_irq_value(s));
+ }
+
return val;
}
+static void pm_smbus_reset(PMSMBus *s)
+{
+ s->op_done = true;
+ s->smb_index = 0;
+ s->smb_stat = 0;
+}
+
static const MemoryRegionOps pm_smbus_ops = {
.read = smb_ioport_readb,
.write = smb_ioport_writeb,
@@ -220,8 +347,29 @@ static const MemoryRegionOps pm_smbus_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
+const VMStateDescription pmsmb_vmstate = {
+ .name = "pmsmb",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT8(smb_stat, PMSMBus),
+ VMSTATE_UINT8(smb_ctl, PMSMBus),
+ VMSTATE_UINT8(smb_cmd, PMSMBus),
+ VMSTATE_UINT8(smb_addr, PMSMBus),
+ VMSTATE_UINT8(smb_data0, PMSMBus),
+ VMSTATE_UINT8(smb_data1, PMSMBus),
+ VMSTATE_VBUFFER_UINT32(smb_data, PMSMBus, 1, NULL, 0, smb_index),
+ VMSTATE_UINT8(smb_auxctl, PMSMBus),
+ VMSTATE_BOOL(i2c_enable, PMSMBus),
+ VMSTATE_BOOL(op_done, PMSMBus),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
void pm_smbus_init(DeviceState *parent, PMSMBus *smb)
{
+ smb->op_done = true;
+ smb->reset = pm_smbus_reset;
smb->smbus = i2c_init_bus(parent, "i2c");
memory_region_init_io(&smb->io, OBJECT(parent), &pm_smbus_ops, smb,
"pm-smbus", 64);
diff --git a/hw/i2c/smbus.c b/hw/i2c/smbus.c
index 3979b3d..7016676 100644
--- a/hw/i2c/smbus.c
+++ b/hw/i2c/smbus.c
@@ -293,9 +293,10 @@ int smbus_write_word(I2CBus *bus, uint8_t addr, uint8_t command, uint16_t data)
return 0;
}
-int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data)
+int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data,
+ int len, bool recv_len)
{
- int len;
+ int rlen;
int i;
if (i2c_start_transfer(bus, addr, 0)) {
@@ -303,20 +304,24 @@ int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data)
}
i2c_send(bus, command);
i2c_start_transfer(bus, addr, 1);
- len = i2c_recv(bus);
- if (len > 32) {
- len = 0;
+ if (recv_len) {
+ rlen = i2c_recv(bus);
+ } else {
+ rlen = len;
}
- for (i = 0; i < len; i++) {
+ if (rlen > len) {
+ rlen = 0;
+ }
+ for (i = 0; i < rlen; i++) {
data[i] = i2c_recv(bus);
}
i2c_nack(bus);
i2c_end_transfer(bus);
- return len;
+ return rlen;
}
int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data,
- int len)
+ int len, bool send_len)
{
int i;
@@ -327,7 +332,8 @@ int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data,
return -1;
}
i2c_send(bus, command);
- i2c_send(bus, len);
+ if (send_len)
+ i2c_send(bus, len);
for (i = 0; i < len; i++) {
i2c_send(bus, data[i]);
}
diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
index 498f03e..8289cd1 100644
--- a/hw/i2c/smbus_ich9.c
+++ b/hw/i2c/smbus_ich9.c
@@ -42,6 +42,8 @@
typedef struct ICH9SMBState {
PCIDevice dev;
+ bool irq_enabled;
+
PMSMBus smb;
} ICH9SMBState;
@@ -50,7 +52,9 @@ static const VMStateDescription vmstate_ich9_smbus = {
.version_id = 1,
.minimum_version_id = 1,
.fields = (VMStateField[]) {
- VMSTATE_PCI_DEVICE(dev, struct ICH9SMBState),
+ VMSTATE_PCI_DEVICE(dev, ICH9SMBState),
+ VMSTATE_BOOL(irq_enabled, ICH9SMBState),
+ VMSTATE_STRUCT(smb, ICH9SMBState, 1, pmsmb_vmstate, struct PMSMBus),
VMSTATE_END_OF_LIST()
}
};
@@ -63,12 +67,16 @@ static void ich9_smbus_write_config(PCIDevice *d, uint32_t address,
pci_default_write_config(d, address, val, len);
if (range_covers_byte(address, len, ICH9_SMB_HOSTC)) {
uint8_t hostc = s->dev.config[ICH9_SMB_HOSTC];
- if ((hostc & ICH9_SMB_HOSTC_HST_EN) &&
- !(hostc & ICH9_SMB_HOSTC_I2C_EN)) {
+ if (hostc & ICH9_SMB_HOSTC_HST_EN) {
memory_region_set_enabled(&s->smb.io, true);
} else {
memory_region_set_enabled(&s->smb.io, false);
}
+ s->smb.i2c_enable = (hostc & ICH9_SMB_HOSTC_I2C_EN) != 0;
+ if (hostc & ICH9_SMB_HOSTC_SSRESET) {
+ s->smb.reset(&s->smb);
+ s->dev.config[ICH9_SMB_HOSTC] &= ~ICH9_SMB_HOSTC_SSRESET;
+ }
}
}
@@ -107,11 +115,24 @@ static void ich9_smb_class_init(ObjectClass *klass, void *data)
dc->cannot_instantiate_with_device_add_yet = true;
}
+static void ich9_smb_set_irq(PMSMBus *pmsmb, bool enabled)
+{
+ ICH9SMBState *s = pmsmb->opaque;
+
+ if (enabled == s->irq_enabled)
+ return;
+
+ s->irq_enabled = enabled;
+ pci_set_irq(&s->dev, enabled);
+}
+
I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base)
{
PCIDevice *d =
pci_create_simple_multifunction(bus, devfn, true, TYPE_ICH9_SMB_DEVICE);
ICH9SMBState *s = ICH9_SMB_DEVICE(d);
+ s->smb.set_irq = ich9_smb_set_irq;
+ s->smb.opaque = s;
return s->smb.smbus;
}
diff --git a/include/hw/i2c/pm_smbus.h b/include/hw/i2c/pm_smbus.h
index 926603f..bfe740a 100644
--- a/include/hw/i2c/pm_smbus.h
+++ b/include/hw/i2c/pm_smbus.h
@@ -1,6 +1,8 @@
#ifndef PM_SMBUS_H
#define PM_SMBUS_H
+#define PM_SMBUS_MAX_MSG_SIZE 32
+
typedef struct PMSMBus {
I2CBus *smbus;
MemoryRegion io;
@@ -11,10 +13,27 @@ typedef struct PMSMBus {
uint8_t smb_addr;
uint8_t smb_data0;
uint8_t smb_data1;
- uint8_t smb_data[32];
- uint8_t smb_index;
+ uint8_t smb_data[PM_SMBUS_MAX_MSG_SIZE];
+ uint8_t smb_auxctl;
+ uint32_t smb_index;
+
+ /* Set by pm_smbus.c */
+ void (*reset)(struct PMSMBus *s);
+
+ /* Set by the user. */
+ bool i2c_enable;
+ void (*set_irq)(struct PMSMBus *s, bool enabled);
+ void *opaque;
+
+ /* Internally used by pm_smbus. */
+
+ /* 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;
} PMSMBus;
void pm_smbus_init(DeviceState *parent, PMSMBus *smb);
+extern const VMStateDescription pmsmb_vmstate;
+
#endif /* !PM_SMBUS_H */
diff --git a/include/hw/i2c/smbus.h b/include/hw/i2c/smbus.h
index 544bbc1..1270691 100644
--- a/include/hw/i2c/smbus.h
+++ b/include/hw/i2c/smbus.h
@@ -73,9 +73,10 @@ int smbus_read_byte(I2CBus *bus, uint8_t addr, uint8_t command);
int smbus_write_byte(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t data);
int smbus_read_word(I2CBus *bus, uint8_t addr, uint8_t command);
int smbus_write_word(I2CBus *bus, uint8_t addr, uint8_t command, uint16_t data);
-int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data);
+int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data,
+ int len, bool recv_len);
int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data,
- int len);
+ int len, bool send_len);
void smbus_eeprom_init(I2CBus *smbus, int nb_eeprom,
const uint8_t *eeprom_spd, int size);
--
2.7.4
next prev parent reply other threads:[~2016-05-11 19:46 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-11 19:45 [Qemu-devel] [PATCH 0/7] Fix PM SMBus and add IPMI over SMBus minyard
2016-05-11 19:46 ` minyard [this message]
2016-05-11 19:46 ` [Qemu-devel] [PATCH 2/7] pm_smbus: Add the ability to force block transfer enable minyard
2016-05-11 19:46 ` [Qemu-devel] [PATCH 3/7] pc: Add the SMBus device to the ACPI tables minyard
2016-05-11 19:46 ` [Qemu-devel] [PATCH 4/7] ipmi: Add an SMBus IPMI interface minyard
2016-05-11 19:46 ` [Qemu-devel] [PATCH 5/7] acpi: Add I2c serial bus CRS handling minyard
2016-05-12 7:30 ` Michael S. Tsirkin
2016-05-12 13:26 ` Corey Minyard
2016-05-12 13:33 ` Michael S. Tsirkin
2016-05-11 19:46 ` [Qemu-devel] [PATCH 6/7] ipmi: Fix SSIF ACPI handling to use the right CRS minyard
2016-05-12 7:33 ` Michael S. Tsirkin
2016-05-12 13:29 ` Corey Minyard
2016-05-12 13:39 ` Michael S. Tsirkin
2016-05-13 13:13 ` Corey Minyard
2016-05-11 19:46 ` [Qemu-devel] [PATCH 7/7] ipmi: Add ACPI to the SMBus IPMI device minyard
2016-05-12 7:36 ` Michael S. Tsirkin
2016-05-12 13:32 ` Corey Minyard
2016-05-12 13:35 ` Michael S. Tsirkin
2016-05-12 19:20 ` Corey Minyard
2016-05-12 19:34 ` Michael S. Tsirkin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1462995966-1184-2-git-send-email-minyard@acm.org \
--to=minyard@acm.org \
--cc=cminyard@mvista.com \
--cc=imammedo@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).