From 93198b6cad8af03996373584284a1673ad6000cb Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 13 Nov 2018 18:31:27 -0600 Subject: i2c: Split smbus into parts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit smbus.c and smbus.h had device side code, master side code, and smbus.h has some smbus_eeprom.c definitions. Split them into separate files. Signed-off-by: Corey Minyard Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- MAINTAINERS | 12 ++ hw/arm/aspeed.c | 2 +- hw/i2c/Makefile.objs | 2 +- hw/i2c/pm_smbus.c | 2 +- hw/i2c/smbus.c | 379 ------------------------------------------ hw/i2c/smbus_eeprom.c | 3 +- hw/i2c/smbus_ich9.c | 2 - hw/i2c/smbus_master.c | 165 ++++++++++++++++++ hw/i2c/smbus_slave.c | 236 ++++++++++++++++++++++++++ hw/i386/pc_piix.c | 2 +- hw/i386/pc_q35.c | 2 +- hw/isa/vt82c686.c | 1 - hw/mips/mips_fulong2e.c | 2 +- hw/mips/mips_malta.c | 2 +- hw/ppc/sam460ex.c | 2 +- include/hw/i2c/pm_smbus.h | 2 + include/hw/i2c/smbus.h | 101 ----------- include/hw/i2c/smbus_eeprom.h | 35 ++++ include/hw/i2c/smbus_master.h | 55 ++++++ include/hw/i2c/smbus_slave.h | 69 ++++++++ 20 files changed, 584 insertions(+), 492 deletions(-) delete mode 100644 hw/i2c/smbus.c create mode 100644 hw/i2c/smbus_master.c create mode 100644 hw/i2c/smbus_slave.c delete mode 100644 include/hw/i2c/smbus.h create mode 100644 include/hw/i2c/smbus_eeprom.h create mode 100644 include/hw/i2c/smbus_master.h create mode 100644 include/hw/i2c/smbus_slave.h diff --git a/MAINTAINERS b/MAINTAINERS index 7be8c578ea..04246a63cf 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2160,6 +2160,18 @@ M: Viktor Prutyanov S: Maintained F: contrib/elf2dmp/ +I2C and SMBus +M: Corey Minyard +S: Maintained +F: hw/i2c/core.c +F: hw/i2c/smbus_slave.c +F: hw/i2c/smbus_master.c +F: hw/i2c/smbus_eeprom.c +F: include/hw/i2c/i2c.h +F: include/hw/i2c/smbus_master.h +F: include/hw/i2c/smbus_slave.h +F: include/hw/i2c/smbus_eeprom.h + Usermode Emulation ------------------ Overall diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 5158985482..996812498d 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -18,7 +18,7 @@ #include "hw/arm/aspeed.h" #include "hw/arm/aspeed_soc.h" #include "hw/boards.h" -#include "hw/i2c/smbus.h" +#include "hw/i2c/smbus_eeprom.h" #include "qemu/log.h" #include "sysemu/block-backend.h" #include "hw/loader.h" diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs index cecee486f7..9205cbee16 100644 --- a/hw/i2c/Makefile.objs +++ b/hw/i2c/Makefile.objs @@ -1,4 +1,4 @@ -common-obj-$(CONFIG_I2C) += core.o smbus.o +common-obj-$(CONFIG_I2C) += core.o smbus_slave.o smbus_master.o common-obj-$(CONFIG_SMBUS_EEPROM) += smbus_eeprom.o common-obj-$(CONFIG_DDC) += i2c-ddc.o common-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c index 03062740cc..6cfb7eb1b3 100644 --- a/hw/i2c/pm_smbus.c +++ b/hw/i2c/pm_smbus.c @@ -20,7 +20,7 @@ #include "qemu/osdep.h" #include "hw/hw.h" #include "hw/i2c/pm_smbus.h" -#include "hw/i2c/smbus.h" +#include "hw/i2c/smbus_master.h" #define SMBHSTSTS 0x00 #define SMBHSTCNT 0x02 diff --git a/hw/i2c/smbus.c b/hw/i2c/smbus.c deleted file mode 100644 index 30028bfcc2..0000000000 --- a/hw/i2c/smbus.c +++ /dev/null @@ -1,379 +0,0 @@ -/* - * QEMU SMBus device emulation. - * - * Copyright (c) 2007 CodeSourcery. - * Written by Paul Brook - * - * This code is licensed under the LGPL. - */ - -/* TODO: Implement PEC. */ - -#include "qemu/osdep.h" -#include "hw/hw.h" -#include "hw/i2c/i2c.h" -#include "hw/i2c/smbus.h" - -//#define DEBUG_SMBUS 1 - -#ifdef DEBUG_SMBUS -#define DPRINTF(fmt, ...) \ -do { printf("smbus(%02x): " fmt , dev->i2c.address, ## __VA_ARGS__); } while (0) -#define BADF(fmt, ...) \ -do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__); exit(1);} while (0) -#else -#define DPRINTF(fmt, ...) do {} while(0) -#define BADF(fmt, ...) \ -do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__);} while (0) -#endif - -enum { - SMBUS_IDLE, - SMBUS_WRITE_DATA, - SMBUS_RECV_BYTE, - SMBUS_READ_DATA, - SMBUS_DONE, - SMBUS_CONFUSED = -1 -}; - -static void smbus_do_quick_cmd(SMBusDevice *dev, int recv) -{ - SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); - - DPRINTF("Quick Command %d\n", recv); - if (sc->quick_cmd) { - sc->quick_cmd(dev, recv); - } -} - -static void smbus_do_write(SMBusDevice *dev) -{ - SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); - - if (dev->data_len == 0) { - smbus_do_quick_cmd(dev, 0); - } else if (dev->data_len == 1) { - DPRINTF("Send Byte\n"); - if (sc->send_byte) { - sc->send_byte(dev, dev->data_buf[0]); - } - } else { - dev->command = dev->data_buf[0]; - DPRINTF("Command %d len %d\n", dev->command, dev->data_len - 1); - if (sc->write_data) { - sc->write_data(dev, dev->command, dev->data_buf + 1, - dev->data_len - 1); - } - } -} - -static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) -{ - SMBusDevice *dev = SMBUS_DEVICE(s); - - switch (event) { - case I2C_START_SEND: - switch (dev->mode) { - case SMBUS_IDLE: - DPRINTF("Incoming data\n"); - dev->mode = SMBUS_WRITE_DATA; - break; - default: - BADF("Unexpected send start condition in state %d\n", dev->mode); - dev->mode = SMBUS_CONFUSED; - break; - } - break; - - case I2C_START_RECV: - switch (dev->mode) { - case SMBUS_IDLE: - DPRINTF("Read mode\n"); - dev->mode = SMBUS_RECV_BYTE; - break; - case SMBUS_WRITE_DATA: - if (dev->data_len == 0) { - BADF("Read after write with no data\n"); - dev->mode = SMBUS_CONFUSED; - } else { - if (dev->data_len > 1) { - smbus_do_write(dev); - } else { - dev->command = dev->data_buf[0]; - DPRINTF("%02x: Command %d\n", dev->i2c.address, - dev->command); - } - DPRINTF("Read mode\n"); - dev->data_len = 0; - dev->mode = SMBUS_READ_DATA; - } - break; - default: - BADF("Unexpected recv start condition in state %d\n", dev->mode); - dev->mode = SMBUS_CONFUSED; - break; - } - break; - - case I2C_FINISH: - switch (dev->mode) { - case SMBUS_WRITE_DATA: - smbus_do_write(dev); - break; - case SMBUS_RECV_BYTE: - smbus_do_quick_cmd(dev, 1); - break; - case SMBUS_READ_DATA: - BADF("Unexpected stop during receive\n"); - break; - default: - /* Nothing to do. */ - break; - } - dev->mode = SMBUS_IDLE; - dev->data_len = 0; - break; - - case I2C_NACK: - switch (dev->mode) { - case SMBUS_DONE: - /* Nothing to do. */ - break; - case SMBUS_READ_DATA: - dev->mode = SMBUS_DONE; - break; - default: - BADF("Unexpected NACK in state %d\n", dev->mode); - dev->mode = SMBUS_CONFUSED; - break; - } - } - - return 0; -} - -static int smbus_i2c_recv(I2CSlave *s) -{ - SMBusDevice *dev = SMBUS_DEVICE(s); - SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); - int ret; - - switch (dev->mode) { - case SMBUS_RECV_BYTE: - if (sc->receive_byte) { - ret = sc->receive_byte(dev); - } else { - ret = 0; - } - DPRINTF("Receive Byte %02x\n", ret); - dev->mode = SMBUS_DONE; - break; - case SMBUS_READ_DATA: - if (sc->read_data) { - ret = sc->read_data(dev, dev->command, dev->data_len); - dev->data_len++; - } else { - ret = 0; - } - DPRINTF("Read data %02x\n", ret); - break; - default: - BADF("Unexpected read in state %d\n", dev->mode); - dev->mode = SMBUS_CONFUSED; - ret = 0; - break; - } - return ret; -} - -static int smbus_i2c_send(I2CSlave *s, uint8_t data) -{ - SMBusDevice *dev = SMBUS_DEVICE(s); - - switch (dev->mode) { - case SMBUS_WRITE_DATA: - DPRINTF("Write data %02x\n", data); - if (dev->data_len >= sizeof(dev->data_buf)) { - BADF("Too many bytes sent\n"); - } else { - dev->data_buf[dev->data_len++] = data; - } - break; - default: - BADF("Unexpected write in state %d\n", dev->mode); - break; - } - return 0; -} - -/* Master device commands. */ -int smbus_quick_command(I2CBus *bus, uint8_t addr, int read) -{ - if (i2c_start_transfer(bus, addr, read)) { - return -1; - } - i2c_end_transfer(bus); - return 0; -} - -int smbus_receive_byte(I2CBus *bus, uint8_t addr) -{ - uint8_t data; - - if (i2c_start_transfer(bus, addr, 1)) { - return -1; - } - data = i2c_recv(bus); - i2c_nack(bus); - i2c_end_transfer(bus); - return data; -} - -int smbus_send_byte(I2CBus *bus, uint8_t addr, uint8_t data) -{ - if (i2c_start_transfer(bus, addr, 0)) { - return -1; - } - i2c_send(bus, data); - i2c_end_transfer(bus); - return 0; -} - -int smbus_read_byte(I2CBus *bus, uint8_t addr, uint8_t command) -{ - uint8_t data; - if (i2c_start_transfer(bus, addr, 0)) { - return -1; - } - i2c_send(bus, command); - if (i2c_start_transfer(bus, addr, 1)) { - i2c_end_transfer(bus); - return -1; - } - data = i2c_recv(bus); - i2c_nack(bus); - i2c_end_transfer(bus); - return data; -} - -int smbus_write_byte(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t data) -{ - if (i2c_start_transfer(bus, addr, 0)) { - return -1; - } - i2c_send(bus, command); - i2c_send(bus, data); - i2c_end_transfer(bus); - return 0; -} - -int smbus_read_word(I2CBus *bus, uint8_t addr, uint8_t command) -{ - uint16_t data; - if (i2c_start_transfer(bus, addr, 0)) { - return -1; - } - i2c_send(bus, command); - if (i2c_start_transfer(bus, addr, 1)) { - i2c_end_transfer(bus); - return -1; - } - data = i2c_recv(bus); - data |= i2c_recv(bus) << 8; - i2c_nack(bus); - i2c_end_transfer(bus); - return data; -} - -int smbus_write_word(I2CBus *bus, uint8_t addr, uint8_t command, uint16_t data) -{ - if (i2c_start_transfer(bus, addr, 0)) { - return -1; - } - i2c_send(bus, command); - i2c_send(bus, data & 0xff); - i2c_send(bus, data >> 8); - i2c_end_transfer(bus); - return 0; -} - -int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data, - int len, bool recv_len, bool send_cmd) -{ - int rlen; - int i; - - if (send_cmd) { - if (i2c_start_transfer(bus, addr, 0)) { - return -1; - } - i2c_send(bus, command); - } - if (i2c_start_transfer(bus, addr, 1)) { - if (send_cmd) { - i2c_end_transfer(bus); - } - return -1; - } - if (recv_len) { - rlen = i2c_recv(bus); - } else { - rlen = len; - } - if (rlen > len) { - rlen = 0; - } - for (i = 0; i < rlen; i++) { - data[i] = i2c_recv(bus); - } - i2c_nack(bus); - i2c_end_transfer(bus); - return rlen; -} - -int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data, - int len, bool send_len) -{ - int i; - - if (len > 32) - len = 32; - - if (i2c_start_transfer(bus, addr, 0)) { - return -1; - } - i2c_send(bus, command); - if (send_len) { - i2c_send(bus, len); - } - for (i = 0; i < len; i++) { - i2c_send(bus, data[i]); - } - i2c_end_transfer(bus); - return 0; -} - -static void smbus_device_class_init(ObjectClass *klass, void *data) -{ - I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); - - sc->event = smbus_i2c_event; - sc->recv = smbus_i2c_recv; - sc->send = smbus_i2c_send; -} - -static const TypeInfo smbus_device_type_info = { - .name = TYPE_SMBUS_DEVICE, - .parent = TYPE_I2C_SLAVE, - .instance_size = sizeof(SMBusDevice), - .abstract = true, - .class_size = sizeof(SMBusDeviceClass), - .class_init = smbus_device_class_init, -}; - -static void smbus_device_register_types(void) -{ - type_register_static(&smbus_device_type_info); -} - -type_init(smbus_device_register_types) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 01b9439014..c8f6f4b442 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -27,7 +27,8 @@ #include "qapi/error.h" #include "hw/hw.h" #include "hw/i2c/i2c.h" -#include "hw/i2c/smbus.h" +#include "hw/i2c/smbus_slave.h" +#include "hw/i2c/smbus_eeprom.h" //#define DEBUG diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c index 2a8b49e02f..e6d8d28194 100644 --- a/hw/i2c/smbus_ich9.c +++ b/hw/i2c/smbus_ich9.c @@ -29,8 +29,6 @@ #include "hw/i2c/pm_smbus.h" #include "hw/pci/pci.h" #include "sysemu/sysemu.h" -#include "hw/i2c/i2c.h" -#include "hw/i2c/smbus.h" #include "hw/i386/ich9.h" diff --git a/hw/i2c/smbus_master.c b/hw/i2c/smbus_master.c new file mode 100644 index 0000000000..0a6223744c --- /dev/null +++ b/hw/i2c/smbus_master.c @@ -0,0 +1,165 @@ +/* + * QEMU SMBus host (master) emulation. + * + * This code emulates SMBus transactions from the master point of view, + * it runs the individual I2C transaction to do the SMBus protocol + * over I2C. + * + * Copyright (c) 2007 CodeSourcery. + * Written by Paul Brook + * + * This code is licensed under the LGPL. + */ + +#include "qemu/osdep.h" +#include "hw/hw.h" +#include "hw/i2c/i2c.h" +#include "hw/i2c/smbus_master.h" + +/* Master device commands. */ +int smbus_quick_command(I2CBus *bus, uint8_t addr, int read) +{ + if (i2c_start_transfer(bus, addr, read)) { + return -1; + } + i2c_end_transfer(bus); + return 0; +} + +int smbus_receive_byte(I2CBus *bus, uint8_t addr) +{ + uint8_t data; + + if (i2c_start_transfer(bus, addr, 1)) { + return -1; + } + data = i2c_recv(bus); + i2c_nack(bus); + i2c_end_transfer(bus); + return data; +} + +int smbus_send_byte(I2CBus *bus, uint8_t addr, uint8_t data) +{ + if (i2c_start_transfer(bus, addr, 0)) { + return -1; + } + i2c_send(bus, data); + i2c_end_transfer(bus); + return 0; +} + +int smbus_read_byte(I2CBus *bus, uint8_t addr, uint8_t command) +{ + uint8_t data; + if (i2c_start_transfer(bus, addr, 0)) { + return -1; + } + i2c_send(bus, command); + if (i2c_start_transfer(bus, addr, 1)) { + i2c_end_transfer(bus); + return -1; + } + data = i2c_recv(bus); + i2c_nack(bus); + i2c_end_transfer(bus); + return data; +} + +int smbus_write_byte(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t data) +{ + if (i2c_start_transfer(bus, addr, 0)) { + return -1; + } + i2c_send(bus, command); + i2c_send(bus, data); + i2c_end_transfer(bus); + return 0; +} + +int smbus_read_word(I2CBus *bus, uint8_t addr, uint8_t command) +{ + uint16_t data; + if (i2c_start_transfer(bus, addr, 0)) { + return -1; + } + i2c_send(bus, command); + if (i2c_start_transfer(bus, addr, 1)) { + i2c_end_transfer(bus); + return -1; + } + data = i2c_recv(bus); + data |= i2c_recv(bus) << 8; + i2c_nack(bus); + i2c_end_transfer(bus); + return data; +} + +int smbus_write_word(I2CBus *bus, uint8_t addr, uint8_t command, uint16_t data) +{ + if (i2c_start_transfer(bus, addr, 0)) { + return -1; + } + i2c_send(bus, command); + i2c_send(bus, data & 0xff); + i2c_send(bus, data >> 8); + i2c_end_transfer(bus); + return 0; +} + +int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data, + int len, bool recv_len, bool send_cmd) +{ + int rlen; + int i; + + if (send_cmd) { + if (i2c_start_transfer(bus, addr, 0)) { + return -1; + } + i2c_send(bus, command); + } + if (i2c_start_transfer(bus, addr, 1)) { + if (send_cmd) { + i2c_end_transfer(bus); + } + return -1; + } + if (recv_len) { + rlen = i2c_recv(bus); + } else { + rlen = len; + } + if (rlen > len) { + rlen = 0; + } + for (i = 0; i < rlen; i++) { + data[i] = i2c_recv(bus); + } + i2c_nack(bus); + i2c_end_transfer(bus); + return rlen; +} + +int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data, + int len, bool send_len) +{ + int i; + + if (len > 32) { + len = 32; + } + + if (i2c_start_transfer(bus, addr, 0)) { + return -1; + } + i2c_send(bus, command); + if (send_len) { + i2c_send(bus, len); + } + for (i = 0; i < len; i++) { + i2c_send(bus, data[i]); + } + i2c_end_transfer(bus); + return 0; +} diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c new file mode 100644 index 0000000000..463fafe3c5 --- /dev/null +++ b/hw/i2c/smbus_slave.c @@ -0,0 +1,236 @@ +/* + * QEMU SMBus device emulation. + * + * This code is a helper for SMBus device emulation. It implements an + * I2C device inteface and runs the SMBus protocol from the device + * point of view and maps those to simple calls to emulate. + * + * Copyright (c) 2007 CodeSourcery. + * Written by Paul Brook + * + * This code is licensed under the LGPL. + */ + +/* TODO: Implement PEC. */ + +#include "qemu/osdep.h" +#include "hw/hw.h" +#include "hw/i2c/i2c.h" +#include "hw/i2c/smbus_slave.h" + +//#define DEBUG_SMBUS 1 + +#ifdef DEBUG_SMBUS +#define DPRINTF(fmt, ...) \ +do { printf("smbus(%02x): " fmt , dev->i2c.address, ## __VA_ARGS__); } while (0) +#define BADF(fmt, ...) \ +do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__); exit(1);} while (0) +#else +#define DPRINTF(fmt, ...) do {} while(0) +#define BADF(fmt, ...) \ +do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__);} while (0) +#endif + +enum { + SMBUS_IDLE, + SMBUS_WRITE_DATA, + SMBUS_RECV_BYTE, + SMBUS_READ_DATA, + SMBUS_DONE, + SMBUS_CONFUSED = -1 +}; + +static void smbus_do_quick_cmd(SMBusDevice *dev, int recv) +{ + SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); + + DPRINTF("Quick Command %d\n", recv); + if (sc->quick_cmd) { + sc->quick_cmd(dev, recv); + } +} + +static void smbus_do_write(SMBusDevice *dev) +{ + SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); + + if (dev->data_len == 0) { + smbus_do_quick_cmd(dev, 0); + } else if (dev->data_len == 1) { + DPRINTF("Send Byte\n"); + if (sc->send_byte) { + sc->send_byte(dev, dev->data_buf[0]); + } + } else { + dev->command = dev->data_buf[0]; + DPRINTF("Command %d len %d\n", dev->command, dev->data_len - 1); + if (sc->write_data) { + sc->write_data(dev, dev->command, dev->data_buf + 1, + dev->data_len - 1); + } + } +} + +static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) +{ + SMBusDevice *dev = SMBUS_DEVICE(s); + + switch (event) { + case I2C_START_SEND: + switch (dev->mode) { + case SMBUS_IDLE: + DPRINTF("Incoming data\n"); + dev->mode = SMBUS_WRITE_DATA; + break; + default: + BADF("Unexpected send start condition in state %d\n", dev->mode); + dev->mode = SMBUS_CONFUSED; + break; + } + break; + + case I2C_START_RECV: + switch (dev->mode) { + case SMBUS_IDLE: + DPRINTF("Read mode\n"); + dev->mode = SMBUS_RECV_BYTE; + break; + case SMBUS_WRITE_DATA: + if (dev->data_len == 0) { + BADF("Read after write with no data\n"); + dev->mode = SMBUS_CONFUSED; + } else { + if (dev->data_len > 1) { + smbus_do_write(dev); + } else { + dev->command = dev->data_buf[0]; + DPRINTF("%02x: Command %d\n", dev->i2c.address, + dev->command); + } + DPRINTF("Read mode\n"); + dev->data_len = 0; + dev->mode = SMBUS_READ_DATA; + } + break; + default: + BADF("Unexpected recv start condition in state %d\n", dev->mode); + dev->mode = SMBUS_CONFUSED; + break; + } + break; + + case I2C_FINISH: + switch (dev->mode) { + case SMBUS_WRITE_DATA: + smbus_do_write(dev); + break; + case SMBUS_RECV_BYTE: + smbus_do_quick_cmd(dev, 1); + break; + case SMBUS_READ_DATA: + BADF("Unexpected stop during receive\n"); + break; + default: + /* Nothing to do. */ + break; + } + dev->mode = SMBUS_IDLE; + dev->data_len = 0; + break; + + case I2C_NACK: + switch (dev->mode) { + case SMBUS_DONE: + /* Nothing to do. */ + break; + case SMBUS_READ_DATA: + dev->mode = SMBUS_DONE; + break; + default: + BADF("Unexpected NACK in state %d\n", dev->mode); + dev->mode = SMBUS_CONFUSED; + break; + } + } + + return 0; +} + +static int smbus_i2c_recv(I2CSlave *s) +{ + SMBusDevice *dev = SMBUS_DEVICE(s); + SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); + int ret; + + switch (dev->mode) { + case SMBUS_RECV_BYTE: + if (sc->receive_byte) { + ret = sc->receive_byte(dev); + } else { + ret = 0; + } + DPRINTF("Receive Byte %02x\n", ret); + dev->mode = SMBUS_DONE; + break; + case SMBUS_READ_DATA: + if (sc->read_data) { + ret = sc->read_data(dev, dev->command, dev->data_len); + dev->data_len++; + } else { + ret = 0; + } + DPRINTF("Read data %02x\n", ret); + break; + default: + BADF("Unexpected read in state %d\n", dev->mode); + dev->mode = SMBUS_CONFUSED; + ret = 0; + break; + } + return ret; +} + +static int smbus_i2c_send(I2CSlave *s, uint8_t data) +{ + SMBusDevice *dev = SMBUS_DEVICE(s); + + switch (dev->mode) { + case SMBUS_WRITE_DATA: + DPRINTF("Write data %02x\n", data); + if (dev->data_len >= sizeof(dev->data_buf)) { + BADF("Too many bytes sent\n"); + } else { + dev->data_buf[dev->data_len++] = data; + } + break; + default: + BADF("Unexpected write in state %d\n", dev->mode); + break; + } + return 0; +} + +static void smbus_device_class_init(ObjectClass *klass, void *data) +{ + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); + + sc->event = smbus_i2c_event; + sc->recv = smbus_i2c_recv; + sc->send = smbus_i2c_send; +} + +static const TypeInfo smbus_device_type_info = { + .name = TYPE_SMBUS_DEVICE, + .parent = TYPE_I2C_SLAVE, + .instance_size = sizeof(SMBusDevice), + .abstract = true, + .class_size = sizeof(SMBusDeviceClass), + .class_init = smbus_device_class_init, +}; + +static void smbus_device_register_types(void) +{ + type_register_static(&smbus_device_type_info); +} + +type_init(smbus_device_register_types) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index fd0f2c268f..3e9037a19a 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -42,7 +42,7 @@ #include "sysemu/sysemu.h" #include "hw/sysbus.h" #include "sysemu/arch_init.h" -#include "hw/i2c/smbus.h" +#include "hw/i2c/smbus_eeprom.h" #include "hw/xen/xen.h" #include "exec/memory.h" #include "exec/address-spaces.h" diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 4a175ea50e..503eeb46f5 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -33,7 +33,7 @@ #include "hw/hw.h" #include "hw/loader.h" #include "sysemu/arch_init.h" -#include "hw/i2c/smbus.h" +#include "hw/i2c/smbus_eeprom.h" #include "hw/boards.h" #include "hw/timer/mc146818rtc.h" #include "hw/xen/xen.h" diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index 7302f6d74b..85d0532dd5 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -14,7 +14,6 @@ #include "hw/hw.h" #include "hw/isa/vt82c686.h" #include "hw/i2c/i2c.h" -#include "hw/i2c/smbus.h" #include "hw/pci/pci.h" #include "hw/isa/isa.h" #include "hw/isa/superio.h" diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c index eec6fd02c8..fbbc543eed 100644 --- a/hw/mips/mips_fulong2e.c +++ b/hw/mips/mips_fulong2e.c @@ -28,7 +28,7 @@ #include "hw/isa/superio.h" #include "net/net.h" #include "hw/boards.h" -#include "hw/i2c/smbus.h" +#include "hw/i2c/smbus_eeprom.h" #include "hw/block/flash.h" #include "hw/mips/mips.h" #include "hw/mips/cpudevs.h" diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index 7a403ef1ce..39aef4b6db 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -33,7 +33,7 @@ #include "hw/char/serial.h" #include "net/net.h" #include "hw/boards.h" -#include "hw/i2c/smbus.h" +#include "hw/i2c/smbus_eeprom.h" #include "hw/block/flash.h" #include "hw/mips/mips.h" #include "hw/mips/cpudevs.h" diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c index 75250d49e4..d455c4bd07 100644 --- a/hw/ppc/sam460ex.c +++ b/hw/ppc/sam460ex.c @@ -34,7 +34,7 @@ #include "hw/sysbus.h" #include "hw/char/serial.h" #include "hw/i2c/ppc4xx_i2c.h" -#include "hw/i2c/smbus.h" +#include "hw/i2c/smbus_eeprom.h" #include "hw/usb/hcd-ehci.h" #include "hw/ppc/fdt.h" diff --git a/include/hw/i2c/pm_smbus.h b/include/hw/i2c/pm_smbus.h index 060d3c6ac0..6dd5b7040b 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 +#include "hw/i2c/smbus_master.h" + #define PM_SMBUS_MAX_MSG_SIZE 32 typedef struct PMSMBus { diff --git a/include/hw/i2c/smbus.h b/include/hw/i2c/smbus.h deleted file mode 100644 index 89dfea1a08..0000000000 --- a/include/hw/i2c/smbus.h +++ /dev/null @@ -1,101 +0,0 @@ -#ifndef QEMU_SMBUS_H -#define QEMU_SMBUS_H - -/* - * QEMU SMBus API - * - * Copyright (c) 2007 Arastra, Inc. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -#include "hw/i2c/i2c.h" - -#define TYPE_SMBUS_DEVICE "smbus-device" -#define SMBUS_DEVICE(obj) \ - OBJECT_CHECK(SMBusDevice, (obj), TYPE_SMBUS_DEVICE) -#define SMBUS_DEVICE_CLASS(klass) \ - OBJECT_CLASS_CHECK(SMBusDeviceClass, (klass), TYPE_SMBUS_DEVICE) -#define SMBUS_DEVICE_GET_CLASS(obj) \ - OBJECT_GET_CLASS(SMBusDeviceClass, (obj), TYPE_SMBUS_DEVICE) - -typedef struct SMBusDevice SMBusDevice; - -typedef struct SMBusDeviceClass -{ - I2CSlaveClass parent_class; - void (*quick_cmd)(SMBusDevice *dev, uint8_t read); - void (*send_byte)(SMBusDevice *dev, uint8_t val); - uint8_t (*receive_byte)(SMBusDevice *dev); - /* We can't distinguish between a word write and a block write with - length 1, so pass the whole data block including the length byte - (if present). The device is responsible figuring out what type of - command this is. */ - void (*write_data)(SMBusDevice *dev, uint8_t cmd, uint8_t *buf, int len); - /* Likewise we can't distinguish between different reads, or even know - the length of the read until the read is complete, so read data a - byte at a time. The device is responsible for adding the length - byte on block reads. */ - uint8_t (*read_data)(SMBusDevice *dev, uint8_t cmd, int n); -} SMBusDeviceClass; - -struct SMBusDevice { - /* The SMBus protocol is implemented on top of I2C. */ - I2CSlave i2c; - - /* Remaining fields for internal use only. */ - int mode; - int data_len; - uint8_t data_buf[34]; /* command + len + 32 bytes of data. */ - uint8_t command; -}; - -/* Master device commands. */ -int smbus_quick_command(I2CBus *bus, uint8_t addr, int read); -int smbus_receive_byte(I2CBus *bus, uint8_t addr); -int smbus_send_byte(I2CBus *bus, uint8_t addr, uint8_t data); -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); - -/* - * Do a block transfer from an I2C device. If recv_len is set, then the - * first received byte is a length field and is used to know how much data - * to receive. Otherwise receive "len" bytes. If send_cmd is set, send - * the command byte first before receiving the data. - */ -int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data, - int len, bool recv_len, bool send_cmd); - -/* - * Do a block transfer to an I2C device. If send_len is set, send the - * "len" value before the data. - */ -int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data, - int len, bool send_len); - -void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf); -void smbus_eeprom_init(I2CBus *smbus, int nb_eeprom, - const uint8_t *eeprom_spd, int size); - -enum sdram_type { SDR = 0x4, DDR = 0x7, DDR2 = 0x8 }; -uint8_t *spd_data_generate(enum sdram_type type, ram_addr_t size, Error **errp); - -#endif diff --git a/include/hw/i2c/smbus_eeprom.h b/include/hw/i2c/smbus_eeprom.h new file mode 100644 index 0000000000..0f96836bab --- /dev/null +++ b/include/hw/i2c/smbus_eeprom.h @@ -0,0 +1,35 @@ +/* + * QEMU SMBus EEPROM API + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef HW_SMBUS_EEPROM_H +#define HW_SMBUS_EEPROM_H + +#include "hw/i2c/i2c.h" + +void smbus_eeprom_init_one(I2CBus *bus, uint8_t address, uint8_t *eeprom_buf); +void smbus_eeprom_init(I2CBus *bus, int nb_eeprom, + const uint8_t *eeprom_spd, int size); + +enum sdram_type { SDR = 0x4, DDR = 0x7, DDR2 = 0x8 }; +uint8_t *spd_data_generate(enum sdram_type type, ram_addr_t size, Error **errp); + +#endif diff --git a/include/hw/i2c/smbus_master.h b/include/hw/i2c/smbus_master.h new file mode 100644 index 0000000000..bb13bc423c --- /dev/null +++ b/include/hw/i2c/smbus_master.h @@ -0,0 +1,55 @@ +/* + * QEMU SMBus host (master) API + * + * Copyright (c) 2007 Arastra, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef HW_SMBUS_MASTER_H +#define HW_SMBUS_MASTER_H + +#include "hw/i2c/i2c.h" + +/* Master device commands. */ +int smbus_quick_command(I2CBus *bus, uint8_t addr, int read); +int smbus_receive_byte(I2CBus *bus, uint8_t addr); +int smbus_send_byte(I2CBus *bus, uint8_t addr, uint8_t data); +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); + +/* + * Do a block transfer from an I2C device. If recv_len is set, then the + * first received byte is a length field and is used to know how much data + * to receive. Otherwise receive "len" bytes. If send_cmd is set, send + * the command byte first before receiving the data. + */ +int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data, + int len, bool recv_len, bool send_cmd); + +/* + * Do a block transfer to an I2C device. If send_len is set, send the + * "len" value before the data. + */ +int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data, + int len, bool send_len); + +#endif diff --git a/include/hw/i2c/smbus_slave.h b/include/hw/i2c/smbus_slave.h new file mode 100644 index 0000000000..46b8948f0f --- /dev/null +++ b/include/hw/i2c/smbus_slave.h @@ -0,0 +1,69 @@ +/* + * QEMU SMBus device (slave) API + * + * Copyright (c) 2007 Arastra, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef HW_SMBUS_SLAVE_H +#define HW_SMBUS_SLAVE_H + +#include "hw/i2c/i2c.h" + +#define TYPE_SMBUS_DEVICE "smbus-device" +#define SMBUS_DEVICE(obj) \ + OBJECT_CHECK(SMBusDevice, (obj), TYPE_SMBUS_DEVICE) +#define SMBUS_DEVICE_CLASS(klass) \ + OBJECT_CLASS_CHECK(SMBusDeviceClass, (klass), TYPE_SMBUS_DEVICE) +#define SMBUS_DEVICE_GET_CLASS(obj) \ + OBJECT_GET_CLASS(SMBusDeviceClass, (obj), TYPE_SMBUS_DEVICE) + +typedef struct SMBusDevice SMBusDevice; + +typedef struct SMBusDeviceClass +{ + I2CSlaveClass parent_class; + void (*quick_cmd)(SMBusDevice *dev, uint8_t read); + void (*send_byte)(SMBusDevice *dev, uint8_t val); + uint8_t (*receive_byte)(SMBusDevice *dev); + /* We can't distinguish between a word write and a block write with + length 1, so pass the whole data block including the length byte + (if present). The device is responsible figuring out what type of + command this is. */ + void (*write_data)(SMBusDevice *dev, uint8_t cmd, uint8_t *buf, int len); + /* Likewise we can't distinguish between different reads, or even know + the length of the read until the read is complete, so read data a + byte at a time. The device is responsible for adding the length + byte on block reads. */ + uint8_t (*read_data)(SMBusDevice *dev, uint8_t cmd, int n); +} SMBusDeviceClass; + +struct SMBusDevice { + /* The SMBus protocol is implemented on top of I2C. */ + I2CSlave i2c; + + /* Remaining fields for internal use only. */ + int mode; + int data_len; + uint8_t data_buf[34]; /* command + len + 32 bytes of data. */ + uint8_t command; +}; + +#endif -- cgit v1.2.3 From 2ac4c5f4d2415116d3f417a32311d437791dcfce Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 14 Nov 2018 11:50:50 -0600 Subject: i2c: have I2C receive operation return uint8_t MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is never supposed to fail and cannot return an error, so just have it return the proper type. Have it return 0xff on nothing available, since that's what would happen on a real bus. Signed-off-by: Corey Minyard Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- hw/arm/pxa2xx.c | 2 +- hw/arm/tosa.c | 4 ++-- hw/arm/z2.c | 2 +- hw/audio/wm8750.c | 2 +- hw/display/sii9022.c | 2 +- hw/display/ssd0303.c | 4 ++-- hw/gpio/max7310.c | 2 +- hw/i2c/core.c | 32 +++++++++++++------------------- hw/i2c/i2c-ddc.c | 2 +- hw/i2c/smbus_slave.c | 4 ++-- hw/input/lm832x.c | 2 +- hw/misc/pca9552.c | 2 +- hw/misc/tmp105.c | 2 +- hw/misc/tmp421.c | 2 +- hw/nvram/eeprom_at24c.c | 4 ++-- hw/timer/ds1338.c | 2 +- hw/timer/m41t80.c | 2 +- hw/timer/twl92230.c | 2 +- include/hw/i2c/i2c.h | 7 +++---- 19 files changed, 37 insertions(+), 44 deletions(-) diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c index f598a1c053..3d7c88910e 100644 --- a/hw/arm/pxa2xx.c +++ b/hw/arm/pxa2xx.c @@ -1286,7 +1286,7 @@ static int pxa2xx_i2c_event(I2CSlave *i2c, enum i2c_event event) return 0; } -static int pxa2xx_i2c_rx(I2CSlave *i2c) +static uint8_t pxa2xx_i2c_rx(I2CSlave *i2c) { PXA2xxI2CSlaveState *slave = PXA2XX_I2C_SLAVE(i2c); PXA2xxI2CState *s = slave->host; diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c index 7a925fa5e6..eef9d427e7 100644 --- a/hw/arm/tosa.c +++ b/hw/arm/tosa.c @@ -197,10 +197,10 @@ static int tosa_dac_event(I2CSlave *i2c, enum i2c_event event) return 0; } -static int tosa_dac_recv(I2CSlave *s) +static uint8_t tosa_dac_recv(I2CSlave *s) { printf("%s: recv not supported!!!\n", __func__); - return -1; + return 0xff; } static void tosa_tg_init(PXA2xxState *cpu) diff --git a/hw/arm/z2.c b/hw/arm/z2.c index 697a822f1e..6f18d924df 100644 --- a/hw/arm/z2.c +++ b/hw/arm/z2.c @@ -243,7 +243,7 @@ static int aer915_event(I2CSlave *i2c, enum i2c_event event) return 0; } -static int aer915_recv(I2CSlave *slave) +static uint8_t aer915_recv(I2CSlave *slave) { AER915State *s = AER915(slave); int retval = 0x00; diff --git a/hw/audio/wm8750.c b/hw/audio/wm8750.c index f4aa838f62..169b006ade 100644 --- a/hw/audio/wm8750.c +++ b/hw/audio/wm8750.c @@ -561,7 +561,7 @@ static int wm8750_tx(I2CSlave *i2c, uint8_t data) return 0; } -static int wm8750_rx(I2CSlave *i2c) +static uint8_t wm8750_rx(I2CSlave *i2c) { return 0x00; } diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c index eaf11a6e7b..9994385c35 100644 --- a/hw/display/sii9022.c +++ b/hw/display/sii9022.c @@ -79,7 +79,7 @@ static int sii9022_event(I2CSlave *i2c, enum i2c_event event) return 0; } -static int sii9022_rx(I2CSlave *i2c) +static uint8_t sii9022_rx(I2CSlave *i2c) { sii9022_state *s = SII9022(i2c); uint8_t res = 0x00; diff --git a/hw/display/ssd0303.c b/hw/display/ssd0303.c index eb90ba26be..8edf34986c 100644 --- a/hw/display/ssd0303.c +++ b/hw/display/ssd0303.c @@ -62,10 +62,10 @@ typedef struct { uint8_t framebuffer[132*8]; } ssd0303_state; -static int ssd0303_recv(I2CSlave *i2c) +static uint8_t ssd0303_recv(I2CSlave *i2c) { BADF("Reads not implemented\n"); - return -1; + return 0xff; } static int ssd0303_send(I2CSlave *i2c, uint8_t data) diff --git a/hw/gpio/max7310.c b/hw/gpio/max7310.c index 1a2478b5a9..c6f686c3eb 100644 --- a/hw/gpio/max7310.c +++ b/hw/gpio/max7310.c @@ -39,7 +39,7 @@ static void max7310_reset(DeviceState *dev) s->command = 0x00; } -static int max7310_rx(I2CSlave *i2c) +static uint8_t max7310_rx(I2CSlave *i2c) { MAX7310State *s = MAX7310(i2c); diff --git a/hw/i2c/core.c b/hw/i2c/core.c index b54725985a..15237ad073 100644 --- a/hw/i2c/core.c +++ b/hw/i2c/core.c @@ -191,23 +191,17 @@ int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send) } return ret ? -1 : 0; } else { - if ((QLIST_EMPTY(&bus->current_devs)) || (bus->broadcast)) { - return -1; - } - - sc = I2C_SLAVE_GET_CLASS(QLIST_FIRST(&bus->current_devs)->elt); - if (sc->recv) { - s = QLIST_FIRST(&bus->current_devs)->elt; - ret = sc->recv(s); - trace_i2c_recv(s->address, ret); - if (ret < 0) { - return ret; - } else { - *data = ret; - return 0; + ret = 0xff; + if (!QLIST_EMPTY(&bus->current_devs) && !bus->broadcast) { + sc = I2C_SLAVE_GET_CLASS(QLIST_FIRST(&bus->current_devs)->elt); + if (sc->recv) { + s = QLIST_FIRST(&bus->current_devs)->elt; + ret = sc->recv(s); + trace_i2c_recv(s->address, ret); } } - return -1; + *data = ret; + return 0; } } @@ -216,12 +210,12 @@ int i2c_send(I2CBus *bus, uint8_t data) return i2c_send_recv(bus, &data, true); } -int i2c_recv(I2CBus *bus) +uint8_t i2c_recv(I2CBus *bus) { - uint8_t data; - int ret = i2c_send_recv(bus, &data, false); + uint8_t data = 0xff; - return ret < 0 ? ret : data; + i2c_send_recv(bus, &data, false); + return data; } void i2c_nack(I2CBus *bus) diff --git a/hw/i2c/i2c-ddc.c b/hw/i2c/i2c-ddc.c index 0a0367ff38..7aa8727771 100644 --- a/hw/i2c/i2c-ddc.c +++ b/hw/i2c/i2c-ddc.c @@ -51,7 +51,7 @@ static int i2c_ddc_event(I2CSlave *i2c, enum i2c_event event) return 0; } -static int i2c_ddc_rx(I2CSlave *i2c) +static uint8_t i2c_ddc_rx(I2CSlave *i2c) { I2CDDCState *s = I2CDDC(i2c); diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c index 463fafe3c5..6e4d542f51 100644 --- a/hw/i2c/smbus_slave.c +++ b/hw/i2c/smbus_slave.c @@ -156,11 +156,11 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) return 0; } -static int smbus_i2c_recv(I2CSlave *s) +static uint8_t smbus_i2c_recv(I2CSlave *s) { SMBusDevice *dev = SMBUS_DEVICE(s); SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); - int ret; + uint8_t ret; switch (dev->mode) { case SMBUS_RECV_BYTE: diff --git a/hw/input/lm832x.c b/hw/input/lm832x.c index cffbf586d4..1fc7b86f19 100644 --- a/hw/input/lm832x.c +++ b/hw/input/lm832x.c @@ -401,7 +401,7 @@ static int lm_i2c_event(I2CSlave *i2c, enum i2c_event event) return 0; } -static int lm_i2c_rx(I2CSlave *i2c) +static uint8_t lm_i2c_rx(I2CSlave *i2c) { LM823KbdState *s = LM8323(i2c); diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c index 9775d5274a..7325d3f287 100644 --- a/hw/misc/pca9552.c +++ b/hw/misc/pca9552.c @@ -115,7 +115,7 @@ static void pca9552_autoinc(PCA9552State *s) } } -static int pca9552_recv(I2CSlave *i2c) +static uint8_t pca9552_recv(I2CSlave *i2c) { PCA9552State *s = PCA9552(i2c); uint8_t ret; diff --git a/hw/misc/tmp105.c b/hw/misc/tmp105.c index f6d7163273..0c32f6f8b6 100644 --- a/hw/misc/tmp105.c +++ b/hw/misc/tmp105.c @@ -147,7 +147,7 @@ static void tmp105_write(TMP105State *s) } } -static int tmp105_rx(I2CSlave *i2c) +static uint8_t tmp105_rx(I2CSlave *i2c) { TMP105State *s = TMP105(i2c); diff --git a/hw/misc/tmp421.c b/hw/misc/tmp421.c index eeb11000f0..ce6d40ac9c 100644 --- a/hw/misc/tmp421.c +++ b/hw/misc/tmp421.c @@ -249,7 +249,7 @@ static void tmp421_write(TMP421State *s) } } -static int tmp421_rx(I2CSlave *i2c) +static uint8_t tmp421_rx(I2CSlave *i2c) { TMP421State *s = TMP421(i2c); diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c index 27cd01e615..d1456dafbd 100644 --- a/hw/nvram/eeprom_at24c.c +++ b/hw/nvram/eeprom_at24c.c @@ -74,10 +74,10 @@ int at24c_eeprom_event(I2CSlave *s, enum i2c_event event) } static -int at24c_eeprom_recv(I2CSlave *s) +uint8_t at24c_eeprom_recv(I2CSlave *s) { EEPROMState *ee = AT24C_EE(s); - int ret; + uint8_t ret; ret = ee->mem[ee->cur]; diff --git a/hw/timer/ds1338.c b/hw/timer/ds1338.c index 3849b74a68..03da75486b 100644 --- a/hw/timer/ds1338.c +++ b/hw/timer/ds1338.c @@ -117,7 +117,7 @@ static int ds1338_event(I2CSlave *i2c, enum i2c_event event) return 0; } -static int ds1338_recv(I2CSlave *i2c) +static uint8_t ds1338_recv(I2CSlave *i2c) { DS1338State *s = DS1338(i2c); uint8_t res; diff --git a/hw/timer/m41t80.c b/hw/timer/m41t80.c index 734d7d95fc..c45b9297d8 100644 --- a/hw/timer/m41t80.c +++ b/hw/timer/m41t80.c @@ -40,7 +40,7 @@ static int m41t80_send(I2CSlave *i2c, uint8_t data) return 0; } -static int m41t80_recv(I2CSlave *i2c) +static uint8_t m41t80_recv(I2CSlave *i2c) { M41t80State *s = M41T80(i2c); struct tm now; diff --git a/hw/timer/twl92230.c b/hw/timer/twl92230.c index 51ec355f3f..c83d803dd8 100644 --- a/hw/timer/twl92230.c +++ b/hw/timer/twl92230.c @@ -737,7 +737,7 @@ static int menelaus_tx(I2CSlave *i2c, uint8_t data) return 0; } -static int menelaus_rx(I2CSlave *i2c) +static uint8_t menelaus_rx(I2CSlave *i2c) { MenelausState *s = TWL92230(i2c); diff --git a/include/hw/i2c/i2c.h b/include/hw/i2c/i2c.h index cf4c45a98f..8e236f7bb4 100644 --- a/include/hw/i2c/i2c.h +++ b/include/hw/i2c/i2c.h @@ -33,10 +33,9 @@ typedef struct I2CSlaveClass { /* * Slave to master. This cannot fail, the device should always - * return something here. Negative values probably result in 0xff - * and a possible log from the driver, and shouldn't be used. + * return something here. */ - int (*recv)(I2CSlave *s); + uint8_t (*recv)(I2CSlave *s); /* * Notify the slave of a bus state change. For start event, @@ -78,7 +77,7 @@ void i2c_end_transfer(I2CBus *bus); void i2c_nack(I2CBus *bus); int i2c_send_recv(I2CBus *bus, uint8_t *data, bool send); int i2c_send(I2CBus *bus, uint8_t data); -int i2c_recv(I2CBus *bus); +uint8_t i2c_recv(I2CBus *bus); DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr); -- cgit v1.2.3 From 05f9f17e2caf249e9454f43e0c3751b2d06e1c06 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 20 Nov 2018 11:10:58 -0600 Subject: arm:i2c: Don't mask return from i2c_recv() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It can't fail, and now that it returns a uint8_t a 0xff mask is unnecessary. Signed-off-by: Corey Minyard Suggested-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell --- hw/arm/stellaris.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 442529cc65..7b45fe3ccf 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -811,7 +811,7 @@ static void stellaris_i2c_write(void *opaque, hwaddr offset, /* TODO: Handle errors. */ if (s->msa & 1) { /* Recv */ - s->mdr = i2c_recv(s->bus) & 0xff; + s->mdr = i2c_recv(s->bus); } else { /* Send */ i2c_send(s->bus, s->mdr); -- cgit v1.2.3 From bc15cde0c4acc9128f26ff78f99ce34a59200d5e Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Tue, 20 Nov 2018 11:13:42 -0600 Subject: i2c: Don't check return value from i2c_recv() i2c_recv() cannot fail, so there is no need to check the return value. It also returns unt8_t, so comparing with < 0 is not meaningful. Fix up various I2C controllers to remove the unneeded code. Signed-off-by: Corey Minyard Suggested-by: Peter Maydell --- hw/i2c/aspeed_i2c.c | 9 ++------- hw/i2c/exynos4210_i2c.c | 8 +------- hw/i2c/imx_i2c.c | 12 ++---------- 3 files changed, 5 insertions(+), 24 deletions(-) diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c index a2dfa82760..a085510cfd 100644 --- a/hw/i2c/aspeed_i2c.c +++ b/hw/i2c/aspeed_i2c.c @@ -189,16 +189,11 @@ static uint8_t aspeed_i2c_get_state(AspeedI2CBus *bus) static void aspeed_i2c_handle_rx_cmd(AspeedI2CBus *bus) { - int ret; + uint8_t ret; aspeed_i2c_set_state(bus, I2CD_MRXD); ret = i2c_recv(bus->bus); - if (ret < 0) { - qemu_log_mask(LOG_GUEST_ERROR, "%s: read failed\n", __func__); - ret = 0xff; - } else { - bus->intr_status |= I2CD_INTR_RX_DONE; - } + bus->intr_status |= I2CD_INTR_RX_DONE; bus->buf = (ret & I2CD_BYTE_BUF_RX_MASK) << I2CD_BYTE_BUF_RX_SHIFT; if (bus->cmd & I2CD_M_S_RX_CMD_LAST) { i2c_nack(bus->bus); diff --git a/hw/i2c/exynos4210_i2c.c b/hw/i2c/exynos4210_i2c.c index c96fa7d7be..d154b05739 100644 --- a/hw/i2c/exynos4210_i2c.c +++ b/hw/i2c/exynos4210_i2c.c @@ -106,16 +106,10 @@ static inline void exynos4210_i2c_raise_interrupt(Exynos4210I2CState *s) static void exynos4210_i2c_data_receive(void *opaque) { Exynos4210I2CState *s = (Exynos4210I2CState *)opaque; - int ret; s->i2cstat &= ~I2CSTAT_LAST_BIT; s->scl_free = false; - ret = i2c_recv(s->bus); - if (ret < 0 && (s->i2ccon & I2CCON_ACK_GEN)) { - s->i2cstat |= I2CSTAT_LAST_BIT; /* Data is not acknowledged */ - } else { - s->i2cds = ret; - } + s->i2cds = i2c_recv(s->bus); exynos4210_i2c_raise_interrupt(s); } diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c index 6c81b98ebd..6da5224e2e 100644 --- a/hw/i2c/imx_i2c.c +++ b/hw/i2c/imx_i2c.c @@ -120,7 +120,7 @@ static uint64_t imx_i2c_read(void *opaque, hwaddr offset, value = s->i2dr_read; if (imx_i2c_is_master(s)) { - int ret = 0xff; + uint8_t ret = 0xff; if (s->address == ADDR_RESET) { /* something is wrong as the address is not set */ @@ -133,15 +133,7 @@ static uint64_t imx_i2c_read(void *opaque, hwaddr offset, } else { /* get the next byte */ ret = i2c_recv(s->bus); - - if (ret >= 0) { - imx_i2c_raise_interrupt(s); - } else { - qemu_log_mask(LOG_GUEST_ERROR, "[%s]%s: read failed " - "for device 0x%02x\n", TYPE_IMX_I2C, - __func__, s->address); - ret = 0xff; - } + imx_i2c_raise_interrupt(s); } s->i2dr_read = ret; -- cgit v1.2.3 From 905cec6d11088e585fcedb3fd606510959ee50ff Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Fri, 30 Nov 2018 13:20:12 -0600 Subject: i2c:smbus: Correct the working of quick commands The logic of handling quick SMBus commands was wrong. If you get a finish event with no data, that's a quick command. Document the quick command while we are at it. Signed-off-by: Corey Minyard --- hw/i2c/smbus_slave.c | 35 +++++++++++++++++++---------------- include/hw/i2c/smbus_slave.h | 5 +++++ 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c index 6e4d542f51..6a89a286e3 100644 --- a/hw/i2c/smbus_slave.c +++ b/hw/i2c/smbus_slave.c @@ -54,9 +54,7 @@ static void smbus_do_write(SMBusDevice *dev) { SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); - if (dev->data_len == 0) { - smbus_do_quick_cmd(dev, 0); - } else if (dev->data_len == 1) { + if (dev->data_len == 1) { DPRINTF("Send Byte\n"); if (sc->send_byte) { sc->send_byte(dev, dev->data_buf[0]); @@ -120,19 +118,24 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) break; case I2C_FINISH: - switch (dev->mode) { - case SMBUS_WRITE_DATA: - smbus_do_write(dev); - break; - case SMBUS_RECV_BYTE: - smbus_do_quick_cmd(dev, 1); - break; - case SMBUS_READ_DATA: - BADF("Unexpected stop during receive\n"); - break; - default: - /* Nothing to do. */ - break; + if (dev->data_len == 0) { + if (dev->mode == SMBUS_WRITE_DATA || dev->mode == SMBUS_READ_DATA) { + smbus_do_quick_cmd(dev, dev->mode == SMBUS_READ_DATA); + } + } else { + switch (dev->mode) { + case SMBUS_WRITE_DATA: + smbus_do_write(dev); + break; + + case SMBUS_READ_DATA: + BADF("Unexpected stop during receive\n"); + break; + + default: + /* Nothing to do. */ + break; + } } dev->mode = SMBUS_IDLE; dev->data_len = 0; diff --git a/include/hw/i2c/smbus_slave.h b/include/hw/i2c/smbus_slave.h index 46b8948f0f..5ef1c72ad0 100644 --- a/include/hw/i2c/smbus_slave.h +++ b/include/hw/i2c/smbus_slave.h @@ -40,6 +40,11 @@ typedef struct SMBusDevice SMBusDevice; typedef struct SMBusDeviceClass { I2CSlaveClass parent_class; + + /* + * An operation with no data, special in SMBus. + * This may be NULL, quick commands are ignore in that case. + */ void (*quick_cmd)(SMBusDevice *dev, uint8_t read); void (*send_byte)(SMBusDevice *dev, uint8_t val); uint8_t (*receive_byte)(SMBusDevice *dev); -- cgit v1.2.3 From 9cf27d74a829f651c0da5d80c014a6cef9d4cbd8 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Fri, 30 Nov 2018 13:38:21 -0600 Subject: i2c:smbus: Simplify write operation There were two different write functions and the SMBus code kept track of the command. Keeping track of the command wasn't useful, in fact it wasn't quite correct for the eeprom_smbus code. And there is no need for two write functions. Just have one write function and the first byte in the buffer is the command. Signed-off-by: Corey Minyard --- hw/i2c/smbus_eeprom.c | 47 +++++++++++++++----------------------------- hw/i2c/smbus_slave.c | 25 +++++------------------ include/hw/i2c/smbus_slave.h | 21 ++++++++++++-------- 3 files changed, 34 insertions(+), 59 deletions(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index c8f6f4b442..7fbb5ca27f 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -45,16 +45,6 @@ static void eeprom_quick_cmd(SMBusDevice *dev, uint8_t read) #endif } -static void eeprom_send_byte(SMBusDevice *dev, uint8_t val) -{ - SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *) dev; -#ifdef DEBUG - printf("eeprom_send_byte: addr=0x%02x val=0x%02x\n", - dev->i2c.address, val); -#endif - eeprom->offset = val; -} - static uint8_t eeprom_receive_byte(SMBusDevice *dev) { SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *) dev; @@ -67,34 +57,30 @@ static uint8_t eeprom_receive_byte(SMBusDevice *dev) return val; } -static void eeprom_write_data(SMBusDevice *dev, uint8_t cmd, uint8_t *buf, int len) +static int eeprom_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len) { SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *) dev; - int n; + uint8_t *data = eeprom->data; + #ifdef DEBUG printf("eeprom_write_byte: addr=0x%02x cmd=0x%02x val=0x%02x\n", - dev->i2c.address, cmd, buf[0]); + dev->i2c.address, buf[0], buf[1]); #endif - /* A page write operation is not a valid SMBus command. - It is a block write without a length byte. Fortunately we - get the full block anyway. */ - /* TODO: Should this set the current location? */ - if (cmd + len > 256) - n = 256 - cmd; - else - n = len; - memcpy(eeprom->data + cmd, buf, n); - len -= n; - if (len) - memcpy(eeprom->data, buf + n, len); + /* len is guaranteed to be > 0 */ + eeprom->offset = buf[0]; + buf++; + len--; + + for (; len > 0; len--) { + data[eeprom->offset] = *buf++; + eeprom->offset = (eeprom->offset + 1) % 256; + } + + return 0; } -static uint8_t eeprom_read_data(SMBusDevice *dev, uint8_t cmd, int n) +static uint8_t eeprom_read_data(SMBusDevice *dev, int n) { - SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *) dev; - /* If this is the first byte then set the current position. */ - if (n == 0) - eeprom->offset = cmd; /* As with writes, we implement block reads without the SMBus length byte. */ return eeprom_receive_byte(dev); @@ -119,7 +105,6 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) dc->realize = smbus_eeprom_realize; sc->quick_cmd = eeprom_quick_cmd; - sc->send_byte = eeprom_send_byte; sc->receive_byte = eeprom_receive_byte; sc->write_data = eeprom_write_data; sc->read_data = eeprom_read_data; diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c index 6a89a286e3..92c7a5086c 100644 --- a/hw/i2c/smbus_slave.c +++ b/hw/i2c/smbus_slave.c @@ -54,18 +54,9 @@ static void smbus_do_write(SMBusDevice *dev) { SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); - if (dev->data_len == 1) { - DPRINTF("Send Byte\n"); - if (sc->send_byte) { - sc->send_byte(dev, dev->data_buf[0]); - } - } else { - dev->command = dev->data_buf[0]; - DPRINTF("Command %d len %d\n", dev->command, dev->data_len - 1); - if (sc->write_data) { - sc->write_data(dev, dev->command, dev->data_buf + 1, - dev->data_len - 1); - } + DPRINTF("Command %d len %d\n", dev->data_buf[0], dev->data_len); + if (sc->write_data) { + sc->write_data(dev, dev->data_buf, dev->data_len); } } @@ -98,13 +89,7 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) BADF("Read after write with no data\n"); dev->mode = SMBUS_CONFUSED; } else { - if (dev->data_len > 1) { - smbus_do_write(dev); - } else { - dev->command = dev->data_buf[0]; - DPRINTF("%02x: Command %d\n", dev->i2c.address, - dev->command); - } + smbus_do_write(dev); DPRINTF("Read mode\n"); dev->data_len = 0; dev->mode = SMBUS_READ_DATA; @@ -177,7 +162,7 @@ static uint8_t smbus_i2c_recv(I2CSlave *s) break; case SMBUS_READ_DATA: if (sc->read_data) { - ret = sc->read_data(dev, dev->command, dev->data_len); + ret = sc->read_data(dev, dev->data_len); dev->data_len++; } else { ret = 0; diff --git a/include/hw/i2c/smbus_slave.h b/include/hw/i2c/smbus_slave.h index 5ef1c72ad0..fa92201ec6 100644 --- a/include/hw/i2c/smbus_slave.h +++ b/include/hw/i2c/smbus_slave.h @@ -46,18 +46,24 @@ typedef struct SMBusDeviceClass * This may be NULL, quick commands are ignore in that case. */ void (*quick_cmd)(SMBusDevice *dev, uint8_t read); - void (*send_byte)(SMBusDevice *dev, uint8_t val); + uint8_t (*receive_byte)(SMBusDevice *dev); - /* We can't distinguish between a word write and a block write with - length 1, so pass the whole data block including the length byte - (if present). The device is responsible figuring out what type of - command this is. */ - void (*write_data)(SMBusDevice *dev, uint8_t cmd, uint8_t *buf, int len); + + /* + * We can't distinguish between a word write and a block write with + * length 1, so pass the whole data block including the length byte + * (if present). The device is responsible figuring out what type of + * command this is. + * This may be NULL if no data is written to the device. Writes + * will be ignore in that case. + */ + int (*write_data)(SMBusDevice *dev, uint8_t *buf, uint8_t len); + /* Likewise we can't distinguish between different reads, or even know the length of the read until the read is complete, so read data a byte at a time. The device is responsible for adding the length byte on block reads. */ - uint8_t (*read_data)(SMBusDevice *dev, uint8_t cmd, int n); + uint8_t (*read_data)(SMBusDevice *dev, int n); } SMBusDeviceClass; struct SMBusDevice { @@ -68,7 +74,6 @@ struct SMBusDevice { int mode; int data_len; uint8_t data_buf[34]; /* command + len + 32 bytes of data. */ - uint8_t command; }; #endif -- cgit v1.2.3 From 031ac49886684ae6d6b88f37779cad2ffd1e4d9b Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Fri, 30 Nov 2018 13:49:31 -0600 Subject: i2c:smbus: Simplify read handling There were two different read functions, and with the removal of the command passed in there is no functional difference. So remove one of them. With that you don't need one of the states, so that can be removed, too. Signed-off-by: Corey Minyard --- hw/i2c/smbus_eeprom.c | 8 -------- hw/i2c/smbus_slave.c | 21 +++------------------ include/hw/i2c/smbus_slave.h | 17 ++++++++++------- 3 files changed, 13 insertions(+), 33 deletions(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 7fbb5ca27f..760594b3f1 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -79,13 +79,6 @@ static int eeprom_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len) return 0; } -static uint8_t eeprom_read_data(SMBusDevice *dev, int n) -{ - /* As with writes, we implement block reads without the - SMBus length byte. */ - return eeprom_receive_byte(dev); -} - static void smbus_eeprom_realize(DeviceState *dev, Error **errp) { SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *)dev; @@ -107,7 +100,6 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) sc->quick_cmd = eeprom_quick_cmd; sc->receive_byte = eeprom_receive_byte; sc->write_data = eeprom_write_data; - sc->read_data = eeprom_read_data; dc->props = smbus_eeprom_properties; /* Reason: pointer property "data" */ dc->user_creatable = false; diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c index 92c7a5086c..52a57423ee 100644 --- a/hw/i2c/smbus_slave.c +++ b/hw/i2c/smbus_slave.c @@ -34,7 +34,6 @@ do { fprintf(stderr, "smbus: error: " fmt , ## __VA_ARGS__);} while (0) enum { SMBUS_IDLE, SMBUS_WRITE_DATA, - SMBUS_RECV_BYTE, SMBUS_READ_DATA, SMBUS_DONE, SMBUS_CONFUSED = -1 @@ -82,7 +81,7 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) switch (dev->mode) { case SMBUS_IDLE: DPRINTF("Read mode\n"); - dev->mode = SMBUS_RECV_BYTE; + dev->mode = SMBUS_READ_DATA; break; case SMBUS_WRITE_DATA: if (dev->data_len == 0) { @@ -91,7 +90,6 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) } else { smbus_do_write(dev); DPRINTF("Read mode\n"); - dev->data_len = 0; dev->mode = SMBUS_READ_DATA; } break; @@ -148,31 +146,18 @@ static uint8_t smbus_i2c_recv(I2CSlave *s) { SMBusDevice *dev = SMBUS_DEVICE(s); SMBusDeviceClass *sc = SMBUS_DEVICE_GET_CLASS(dev); - uint8_t ret; + uint8_t ret = 0xff; switch (dev->mode) { - case SMBUS_RECV_BYTE: + case SMBUS_READ_DATA: if (sc->receive_byte) { ret = sc->receive_byte(dev); - } else { - ret = 0; - } - DPRINTF("Receive Byte %02x\n", ret); - dev->mode = SMBUS_DONE; - break; - case SMBUS_READ_DATA: - if (sc->read_data) { - ret = sc->read_data(dev, dev->data_len); - dev->data_len++; - } else { - ret = 0; } DPRINTF("Read data %02x\n", ret); break; default: BADF("Unexpected read in state %d\n", dev->mode); dev->mode = SMBUS_CONFUSED; - ret = 0; break; } return ret; diff --git a/include/hw/i2c/smbus_slave.h b/include/hw/i2c/smbus_slave.h index fa92201ec6..79050fb92d 100644 --- a/include/hw/i2c/smbus_slave.h +++ b/include/hw/i2c/smbus_slave.h @@ -47,8 +47,6 @@ typedef struct SMBusDeviceClass */ void (*quick_cmd)(SMBusDevice *dev, uint8_t read); - uint8_t (*receive_byte)(SMBusDevice *dev); - /* * We can't distinguish between a word write and a block write with * length 1, so pass the whole data block including the length byte @@ -59,11 +57,16 @@ typedef struct SMBusDeviceClass */ int (*write_data)(SMBusDevice *dev, uint8_t *buf, uint8_t len); - /* Likewise we can't distinguish between different reads, or even know - the length of the read until the read is complete, so read data a - byte at a time. The device is responsible for adding the length - byte on block reads. */ - uint8_t (*read_data)(SMBusDevice *dev, int n); + /* + * Likewise we can't distinguish between different reads, or even know + * the length of the read until the read is complete, so read data a + * byte at a time. The device is responsible for adding the length + * byte on block reads. This call cannot fail, it should return + * something, preferably 0xff if nothing is available. + * This may be NULL if no data is read from the device. Reads will + * return 0xff in that case. + */ + uint8_t (*receive_byte)(SMBusDevice *dev); } SMBusDeviceClass; struct SMBusDevice { -- cgit v1.2.3 From 08a8a4d45050f837d560f1aa23ee0ffff4a2c1d7 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Fri, 30 Nov 2018 13:58:27 -0600 Subject: i2c:smbus_eeprom: Get rid of the quick command It's not necessary, it won't be called if it's NULL. Signed-off-by: Corey Minyard --- hw/i2c/smbus_eeprom.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 760594b3f1..2f90287b69 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -38,13 +38,6 @@ typedef struct SMBusEEPROMDevice { uint8_t offset; } SMBusEEPROMDevice; -static void eeprom_quick_cmd(SMBusDevice *dev, uint8_t read) -{ -#ifdef DEBUG - printf("eeprom_quick_cmd: addr=0x%02x read=%d\n", dev->i2c.address, read); -#endif -} - static uint8_t eeprom_receive_byte(SMBusDevice *dev) { SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *) dev; @@ -97,7 +90,6 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) SMBusDeviceClass *sc = SMBUS_DEVICE_CLASS(klass); dc->realize = smbus_eeprom_realize; - sc->quick_cmd = eeprom_quick_cmd; sc->receive_byte = eeprom_receive_byte; sc->write_data = eeprom_write_data; dc->props = smbus_eeprom_properties; -- cgit v1.2.3 From 8b38e532b58d9e314d95aa9845cc89243ee60acc Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Fri, 30 Nov 2018 14:04:19 -0600 Subject: i2c:smbus: Make white space in switch statements consistent It had spaces between cases in some places and not others. Add a space for every one. Signed-off-by: Corey Minyard --- hw/i2c/smbus_eeprom.c | 1 + hw/i2c/smbus_slave.c | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 2f90287b69..2816e35bcc 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -43,6 +43,7 @@ static uint8_t eeprom_receive_byte(SMBusDevice *dev) SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *) dev; uint8_t *data = eeprom->data; uint8_t val = data[eeprom->offset++]; + #ifdef DEBUG printf("eeprom_receive_byte: addr=0x%02x val=0x%02x\n", dev->i2c.address, val); diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c index 52a57423ee..5b3046f6a3 100644 --- a/hw/i2c/smbus_slave.c +++ b/hw/i2c/smbus_slave.c @@ -70,6 +70,7 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) DPRINTF("Incoming data\n"); dev->mode = SMBUS_WRITE_DATA; break; + default: BADF("Unexpected send start condition in state %d\n", dev->mode); dev->mode = SMBUS_CONFUSED; @@ -83,6 +84,7 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) DPRINTF("Read mode\n"); dev->mode = SMBUS_READ_DATA; break; + case SMBUS_WRITE_DATA: if (dev->data_len == 0) { BADF("Read after write with no data\n"); @@ -93,6 +95,7 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) dev->mode = SMBUS_READ_DATA; } break; + default: BADF("Unexpected recv start condition in state %d\n", dev->mode); dev->mode = SMBUS_CONFUSED; @@ -129,9 +132,11 @@ static int smbus_i2c_event(I2CSlave *s, enum i2c_event event) case SMBUS_DONE: /* Nothing to do. */ break; + case SMBUS_READ_DATA: dev->mode = SMBUS_DONE; break; + default: BADF("Unexpected NACK in state %d\n", dev->mode); dev->mode = SMBUS_CONFUSED; @@ -155,11 +160,13 @@ static uint8_t smbus_i2c_recv(I2CSlave *s) } DPRINTF("Read data %02x\n", ret); break; + default: BADF("Unexpected read in state %d\n", dev->mode); dev->mode = SMBUS_CONFUSED; break; } + return ret; } @@ -176,10 +183,12 @@ static int smbus_i2c_send(I2CSlave *s, uint8_t data) dev->data_buf[dev->data_len++] = data; } break; + default: BADF("Unexpected write in state %d\n", dev->mode); break; } + return 0; } -- cgit v1.2.3 From 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 14 Nov 2018 14:41:01 -0600 Subject: boards.h: Ignore migration for SMBus devices on older machines Migration capability is being added for pm_smbus and SMBus devices. This change will allow backwards compatibility to be kept when migrating back to an old qemu version. Add a bool to the machine class tho keep smbus migration from happening. Future changes will use this. Signed-off-by: Corey Minyard Cc: Eduardo Habkost Cc: Marcel Apfelbaum Reviewed-by: Dr. David Alan Gilbert --- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c | 1 + include/hw/boards.h | 1 + 3 files changed, 3 insertions(+) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 3e9037a19a..8770ecada9 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -444,6 +444,7 @@ static void pc_i440fx_3_1_machine_options(MachineClass *m) pc_i440fx_4_0_machine_options(m); m->is_default = 0; + m->smbus_no_migration_support = true; m->alias = NULL; pcmc->pvh_enabled = false; compat_props_add(m->compat_props, hw_compat_3_1, hw_compat_3_1_len); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 503eeb46f5..cfb9043e12 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -380,6 +380,7 @@ static void pc_q35_3_1_machine_options(MachineClass *m) pc_q35_4_0_machine_options(m); m->default_kernel_irqchip_split = false; + m->smbus_no_migration_support = true; m->alias = NULL; pcmc->pvh_enabled = false; compat_props_add(m->compat_props, hw_compat_3_1, hw_compat_3_1_len); diff --git a/include/hw/boards.h b/include/hw/boards.h index 05f9f45c3d..21212f0859 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -206,6 +206,7 @@ struct MachineClass { void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes, int nb_nodes, ram_addr_t size); bool ignore_boot_device_suffixes; + bool smbus_no_migration_support; HotplugHandler *(*get_hotplug_handler)(MachineState *machine, DeviceState *dev); -- cgit v1.2.3 From 52cc6a492b6c6e668f99e13d3cfad5caf7fbfd1c Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Wed, 14 Nov 2018 14:41:01 -0600 Subject: i2c:pm_smbus: Fix pm_smbus handling of I2C block read 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(-) 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); -- cgit v1.2.3 From 8e995f34031d66666b45b779458cb370e86cfe2d Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 26 Nov 2018 12:54:45 -0600 Subject: migration: Add a VMSTATE_BOOL_TEST() macro This will be needed by coming I2C changes. Signed-off-by: Corey Minyard Reviewed-by: Dr. David Alan Gilbert --- include/migration/vmstate.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 067b126cf1..a668ec75b8 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -851,6 +851,9 @@ extern const VMStateInfo vmstate_info_qtailq; #define VMSTATE_INT32_POSITIVE_LE(_f, _s) \ VMSTATE_SINGLE(_f, _s, 0, vmstate_info_int32_le, int32_t) +#define VMSTATE_BOOL_TEST(_f, _s, _t) \ + VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_info_bool, bool) + #define VMSTATE_INT8_TEST(_f, _s, _t) \ VMSTATE_SINGLE_TEST(_f, _s, _t, 0, vmstate_info_int8, int8_t) -- cgit v1.2.3 From 4ab2f2a8aabfea95cc53c64e13b3f67960b27fdf Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 22 Dec 2016 12:28:23 -0600 Subject: i2c:pm_smbus: Fix state transfer Transfer the state information for the SMBus registers and internal data so it will work on a VM transfer. Signed-off-by: Corey Minyard Cc: Michael S. Tsirkin Cc: Paolo Bonzini Cc: Dr. David Alan Gilbert Reviewed-by: Dr. David Alan Gilbert --- hw/acpi/piix4.c | 7 +++++++ hw/i2c/pm_smbus.c | 31 +++++++++++++++++++++++++++++++ hw/i2c/smbus_ich9.c | 10 +++++++++- include/hw/i2c/pm_smbus.h | 9 +++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index df8c0db909..8fd25a5926 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -302,6 +302,11 @@ static const VMStateDescription vmstate_cpuhp_state = { } }; +static bool piix4_vmstate_need_smbus(void *opaque, int version_id) +{ + return pm_smbus_vmstate_needed(); +} + /* qemu-kvm 1.2 uses version 3 but advertised as 2 * To support incoming qemu-kvm 1.2 migration, change version_id * and minimum_version_id to 2 below (which breaks migration from @@ -321,6 +326,8 @@ static const VMStateDescription vmstate_acpi = { VMSTATE_UINT16(ar.pm1.evt.en, PIIX4PMState), VMSTATE_UINT16(ar.pm1.cnt.cnt, PIIX4PMState), VMSTATE_STRUCT(apm, PIIX4PMState, 0, vmstate_apm, APMState), + VMSTATE_STRUCT_TEST(smb, PIIX4PMState, piix4_vmstate_need_smbus, 3, + pmsmb_vmstate, PMSMBus), VMSTATE_TIMER_PTR(ar.tmr.timer, PIIX4PMState), VMSTATE_INT64(ar.tmr.overflow_time, PIIX4PMState), VMSTATE_STRUCT(ar.gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE), diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c index 81d2a425ec..e48544f909 100644 --- a/hw/i2c/pm_smbus.c +++ b/hw/i2c/pm_smbus.c @@ -19,6 +19,7 @@ */ #include "qemu/osdep.h" #include "hw/hw.h" +#include "hw/boards.h" #include "hw/i2c/pm_smbus.h" #include "hw/i2c/smbus_master.h" @@ -453,6 +454,36 @@ static const MemoryRegionOps pm_smbus_ops = { .endianness = DEVICE_LITTLE_ENDIAN, }; +bool pm_smbus_vmstate_needed(void) +{ + MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); + + return !mc->smbus_no_migration_support; +} + +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_UINT32(smb_index, PMSMBus), + VMSTATE_UINT8_ARRAY(smb_data, PMSMBus, PM_SMBUS_MAX_MSG_SIZE), + VMSTATE_UINT8(smb_auxctl, PMSMBus), + VMSTATE_UINT8(smb_blkdata, PMSMBus), + VMSTATE_BOOL(i2c_enable, PMSMBus), + VMSTATE_BOOL(op_done, PMSMBus), + VMSTATE_BOOL(in_i2c_block_read, PMSMBus), + VMSTATE_BOOL(start_transaction_on_status_read, PMSMBus), + VMSTATE_END_OF_LIST() + } +}; + void pm_smbus_init(DeviceState *parent, PMSMBus *smb, bool force_aux_blk) { smb->op_done = true; diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c index e6d8d28194..7b24be8256 100644 --- a/hw/i2c/smbus_ich9.c +++ b/hw/i2c/smbus_ich9.c @@ -43,12 +43,20 @@ typedef struct ICH9SMBState { PMSMBus smb; } ICH9SMBState; +static bool ich9_vmstate_need_smbus(void *opaque, int version_id) +{ + return pm_smbus_vmstate_needed(); +} + static const VMStateDescription vmstate_ich9_smbus = { .name = "ich9_smb", .version_id = 1, .minimum_version_id = 1, .fields = (VMStateField[]) { - VMSTATE_PCI_DEVICE(dev, struct ICH9SMBState), + VMSTATE_PCI_DEVICE(dev, ICH9SMBState), + VMSTATE_BOOL_TEST(irq_enabled, ICH9SMBState, ich9_vmstate_need_smbus), + VMSTATE_STRUCT_TEST(smb, ICH9SMBState, ich9_vmstate_need_smbus, 1, + pmsmb_vmstate, PMSMBus), VMSTATE_END_OF_LIST() } }; diff --git a/include/hw/i2c/pm_smbus.h b/include/hw/i2c/pm_smbus.h index 7bcca97672..fb55c44444 100644 --- a/include/hw/i2c/pm_smbus.h +++ b/include/hw/i2c/pm_smbus.h @@ -43,4 +43,13 @@ typedef struct PMSMBus { void pm_smbus_init(DeviceState *parent, PMSMBus *smb, bool force_aux_blk); +/* + * For backwards compatibility on migration, older versions don't have + * working migration for pm_smbus, this lets us ignore the migrations + * for older machine versions. + */ +bool pm_smbus_vmstate_needed(void); + +extern const VMStateDescription pmsmb_vmstate; + #endif /* PM_SMBUS_H */ -- cgit v1.2.3 From 547db24a17c22c47f762aae3a0a6f518f574e7f2 Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 7 Dec 2017 09:34:59 -0600 Subject: i2c:smbus_slave: Add an SMBus vmstate structure There is no vmstate handling for SMBus, so no device sitting on SMBus can have a state transfer that works reliably. So add it. Signed-off-by: Corey Minyard Cc: Paolo Bonzini Cc: Michael S. Tsirkin Cc: Dr. David Alan Gilbert Reviewed-by: Dr. David Alan Gilbert --- hw/i2c/smbus_slave.c | 18 ++++++++++++++++++ include/hw/i2c/smbus_slave.h | 24 +++++++++++++++++++++--- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c index 5b3046f6a3..9a2d314d1a 100644 --- a/hw/i2c/smbus_slave.c +++ b/hw/i2c/smbus_slave.c @@ -201,6 +201,24 @@ static void smbus_device_class_init(ObjectClass *klass, void *data) sc->send = smbus_i2c_send; } +bool smbus_vmstate_needed(SMBusDevice *dev) +{ + return dev->mode != SMBUS_IDLE; +} + +const VMStateDescription vmstate_smbus_device = { + .name = TYPE_SMBUS_DEVICE, + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_I2C_SLAVE(i2c, SMBusDevice), + VMSTATE_INT32(mode, SMBusDevice), + VMSTATE_INT32(data_len, SMBusDevice), + VMSTATE_UINT8_ARRAY(data_buf, SMBusDevice, SMBUS_DATA_MAX_LEN), + VMSTATE_END_OF_LIST() + } +}; + static const TypeInfo smbus_device_type_info = { .name = TYPE_SMBUS_DEVICE, .parent = TYPE_I2C_SLAVE, diff --git a/include/hw/i2c/smbus_slave.h b/include/hw/i2c/smbus_slave.h index 79050fb92d..ebe068304e 100644 --- a/include/hw/i2c/smbus_slave.h +++ b/include/hw/i2c/smbus_slave.h @@ -69,14 +69,32 @@ typedef struct SMBusDeviceClass uint8_t (*receive_byte)(SMBusDevice *dev); } SMBusDeviceClass; +#define SMBUS_DATA_MAX_LEN 34 /* command + len + 32 bytes of data. */ + struct SMBusDevice { /* The SMBus protocol is implemented on top of I2C. */ I2CSlave i2c; /* Remaining fields for internal use only. */ - int mode; - int data_len; - uint8_t data_buf[34]; /* command + len + 32 bytes of data. */ + int32_t mode; + int32_t data_len; + uint8_t data_buf[SMBUS_DATA_MAX_LEN]; }; +extern const VMStateDescription vmstate_smbus_device; + +#define VMSTATE_SMBUS_DEVICE(_field, _state) { \ + .name = (stringify(_field)), \ + .size = sizeof(SMBusDevice), \ + .vmsd = &vmstate_smbus_device, \ + .flags = VMS_STRUCT, \ + .offset = vmstate_offset_value(_state, _field, SMBusDevice), \ +} + +/* + * Users should call this in their .needed functions to know if the + * SMBus slave data needs to be transferred. + */ +bool smbus_vmstate_needed(SMBusDevice *dev); + #endif -- cgit v1.2.3 From b398a92440ef8283532b0cd3ec0197993232f28f Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 8 Nov 2018 11:31:31 -0600 Subject: i2c:smbus_eeprom: Add normal type name and cast to smbus_eeprom.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create a type name and a cast macro and use those through the code. Signed-off-by: Corey Minyard Reviewed-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé --- hw/i2c/smbus_eeprom.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 2816e35bcc..76f0d4aab0 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -32,6 +32,11 @@ //#define DEBUG +#define TYPE_SMBUS_EEPROM "smbus-eeprom" + +#define SMBUS_EEPROM(obj) \ + OBJECT_CHECK(SMBusEEPROMDevice, (obj), TYPE_SMBUS_EEPROM) + typedef struct SMBusEEPROMDevice { SMBusDevice smbusdev; void *data; @@ -40,7 +45,7 @@ typedef struct SMBusEEPROMDevice { static uint8_t eeprom_receive_byte(SMBusDevice *dev) { - SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *) dev; + SMBusEEPROMDevice *eeprom = SMBUS_EEPROM(dev); uint8_t *data = eeprom->data; uint8_t val = data[eeprom->offset++]; @@ -53,7 +58,7 @@ static uint8_t eeprom_receive_byte(SMBusDevice *dev) static int eeprom_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len) { - SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *) dev; + SMBusEEPROMDevice *eeprom = SMBUS_EEPROM(dev); uint8_t *data = eeprom->data; #ifdef DEBUG @@ -75,7 +80,7 @@ static int eeprom_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len) static void smbus_eeprom_realize(DeviceState *dev, Error **errp) { - SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *)dev; + SMBusEEPROMDevice *eeprom = SMBUS_EEPROM(dev); eeprom->offset = 0; } @@ -99,7 +104,7 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) } static const TypeInfo smbus_eeprom_info = { - .name = "smbus-eeprom", + .name = TYPE_SMBUS_EEPROM, .parent = TYPE_SMBUS_DEVICE, .instance_size = sizeof(SMBusEEPROMDevice), .class_init = smbus_eeprom_class_initfn, @@ -116,7 +121,7 @@ void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf) { DeviceState *dev; - dev = qdev_create((BusState *) smbus, "smbus-eeprom"); + dev = qdev_create((BusState *) smbus, TYPE_SMBUS_EEPROM); qdev_prop_set_uint8(dev, "address", address); qdev_prop_set_ptr(dev, "data", eeprom_buf); qdev_init_nofail(dev); -- cgit v1.2.3 From 0cf487e5bd802fccb7db9d47fdf3dad15deb273e Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 8 Nov 2018 11:54:15 -0600 Subject: i2c:smbus_eeprom: Add a size constant for the smbus_eeprom size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was hard-coded to 256 in a number of places, create a constant for that. Signed-off-by: Corey Minyard Reviewed-by: Philippe Mathieu-Daudé --- hw/i2c/smbus_eeprom.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 76f0d4aab0..3fbbffb4de 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -37,6 +37,8 @@ #define SMBUS_EEPROM(obj) \ OBJECT_CHECK(SMBusEEPROMDevice, (obj), TYPE_SMBUS_EEPROM) +#define SMBUS_EEPROM_SIZE 256 + typedef struct SMBusEEPROMDevice { SMBusDevice smbusdev; void *data; @@ -72,7 +74,7 @@ static int eeprom_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len) for (; len > 0; len--) { data[eeprom->offset] = *buf++; - eeprom->offset = (eeprom->offset + 1) % 256; + eeprom->offset = (eeprom->offset + 1) % SMBUS_EEPROM_SIZE; } return 0; @@ -131,13 +133,15 @@ void smbus_eeprom_init(I2CBus *smbus, int nb_eeprom, const uint8_t *eeprom_spd, int eeprom_spd_size) { int i; - uint8_t *eeprom_buf = g_malloc0(8 * 256); /* XXX: make this persistent */ + /* XXX: make this persistent */ + uint8_t *eeprom_buf = g_malloc0(8 * SMBUS_EEPROM_SIZE); if (eeprom_spd_size > 0) { memcpy(eeprom_buf, eeprom_spd, eeprom_spd_size); } for (i = 0; i < nb_eeprom; i++) { - smbus_eeprom_init_one(smbus, 0x50 + i, eeprom_buf + (i * 256)); + smbus_eeprom_init_one(smbus, 0x50 + i, + eeprom_buf + (i * SMBUS_EEPROM_SIZE)); } } -- cgit v1.2.3 From fd9df33f0829fd93d02f48fe650be057f8d52dfd Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 7 Dec 2017 15:40:53 -0600 Subject: i2c:smbus_eeprom: Add vmstate handling to the smbus eeprom Transfer the state of the EEPROM on a migration. This way the data remains consistent on migration. This required moving the actual data to a separate array and using the data provided in the init function as a separate initialization array, since a pointer property has to be a void * and the array needs to be uint8_t[]. Signed-off-by: Corey Minyard Cc: Paolo Bonzini Cc: Michael S. Tsirkin Cc: Dr. David Alan Gilbert Cc: Peter Maydell Reviewed-by: Dr. David Alan Gilbert --- hw/i2c/smbus_eeprom.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 3fbbffb4de..5b10c31ae7 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -26,6 +26,7 @@ #include "qemu/units.h" #include "qapi/error.h" #include "hw/hw.h" +#include "hw/boards.h" #include "hw/i2c/i2c.h" #include "hw/i2c/smbus_slave.h" #include "hw/i2c/smbus_eeprom.h" @@ -41,8 +42,10 @@ typedef struct SMBusEEPROMDevice { SMBusDevice smbusdev; - void *data; + uint8_t data[SMBUS_EEPROM_SIZE]; + void *init_data; uint8_t offset; + bool accessed; } SMBusEEPROMDevice; static uint8_t eeprom_receive_byte(SMBusDevice *dev) @@ -51,6 +54,7 @@ static uint8_t eeprom_receive_byte(SMBusDevice *dev) uint8_t *data = eeprom->data; uint8_t val = data[eeprom->offset++]; + eeprom->accessed = true; #ifdef DEBUG printf("eeprom_receive_byte: addr=0x%02x val=0x%02x\n", dev->i2c.address, val); @@ -63,6 +67,7 @@ static int eeprom_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len) SMBusEEPROMDevice *eeprom = SMBUS_EEPROM(dev); uint8_t *data = eeprom->data; + eeprom->accessed = true; #ifdef DEBUG printf("eeprom_write_byte: addr=0x%02x cmd=0x%02x val=0x%02x\n", dev->i2c.address, buf[0], buf[1]); @@ -80,15 +85,39 @@ static int eeprom_write_data(SMBusDevice *dev, uint8_t *buf, uint8_t len) return 0; } +static bool smbus_eeprom_vmstate_needed(void *opaque) +{ + MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); + SMBusEEPROMDevice *eeprom = opaque; + + return (eeprom->accessed || smbus_vmstate_needed(&eeprom->smbusdev)) && + !mc->smbus_no_migration_support; +} + +static const VMStateDescription vmstate_smbus_eeprom = { + .name = "smbus-eeprom", + .version_id = 1, + .minimum_version_id = 1, + .needed = smbus_eeprom_vmstate_needed, + .fields = (VMStateField[]) { + VMSTATE_SMBUS_DEVICE(smbusdev, SMBusEEPROMDevice), + VMSTATE_UINT8_ARRAY(data, SMBusEEPROMDevice, SMBUS_EEPROM_SIZE), + VMSTATE_UINT8(offset, SMBusEEPROMDevice), + VMSTATE_BOOL(accessed, SMBusEEPROMDevice), + VMSTATE_END_OF_LIST() + } +}; + static void smbus_eeprom_realize(DeviceState *dev, Error **errp) { SMBusEEPROMDevice *eeprom = SMBUS_EEPROM(dev); + memcpy(eeprom->data, eeprom->init_data, SMBUS_EEPROM_SIZE); eeprom->offset = 0; } static Property smbus_eeprom_properties[] = { - DEFINE_PROP_PTR("data", SMBusEEPROMDevice, data), + DEFINE_PROP_PTR("data", SMBusEEPROMDevice, init_data), DEFINE_PROP_END_OF_LIST(), }; @@ -101,6 +130,7 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) sc->receive_byte = eeprom_receive_byte; sc->write_data = eeprom_write_data; dc->props = smbus_eeprom_properties; + dc->vmsd = &vmstate_smbus_eeprom; /* Reason: pointer property "data" */ dc->user_creatable = false; } -- cgit v1.2.3 From 1042b22dcbc23be124b2ebfb15a363cf7a36914c Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Thu, 15 Nov 2018 08:31:11 -0600 Subject: i2c:smbus_eeprom: Add a reset function to smbus_eeprom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reset the contents to init data and reset the offset on a machine reset. Signed-off-by: Corey Minyard Reviewed-by: Philippe Mathieu-Daudé --- hw/i2c/smbus_eeprom.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 5b10c31ae7..0ba5763fc2 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -108,7 +108,17 @@ static const VMStateDescription vmstate_smbus_eeprom = { } }; -static void smbus_eeprom_realize(DeviceState *dev, Error **errp) +/* + * Reset the EEPROM contents to the initial state on a reset. This + * isn't really how an EEPROM works, of course, but the general + * principle of QEMU is to restore function on reset to what it would + * be if QEMU was stopped and started. + * + * The proper thing to do would be to have a backing blockdev to hold + * the contents and restore that on startup, and not do this on reset. + * But until that time, act as if we had been stopped and restarted. + */ +static void smbus_eeprom_reset(DeviceState *dev) { SMBusEEPROMDevice *eeprom = SMBUS_EEPROM(dev); @@ -116,6 +126,11 @@ static void smbus_eeprom_realize(DeviceState *dev, Error **errp) eeprom->offset = 0; } +static void smbus_eeprom_realize(DeviceState *dev, Error **errp) +{ + smbus_eeprom_reset(dev); +} + static Property smbus_eeprom_properties[] = { DEFINE_PROP_PTR("data", SMBusEEPROMDevice, init_data), DEFINE_PROP_END_OF_LIST(), @@ -127,6 +142,7 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) SMBusDeviceClass *sc = SMBUS_DEVICE_CLASS(klass); dc->realize = smbus_eeprom_realize; + dc->reset = smbus_eeprom_reset; sc->receive_byte = eeprom_receive_byte; sc->write_data = eeprom_write_data; dc->props = smbus_eeprom_properties; -- cgit v1.2.3 From c203d4514b9c8c1c3bf25988a81edf3813eb3c6d Mon Sep 17 00:00:00 2001 From: Corey Minyard Date: Mon, 28 Jan 2019 11:48:19 -0600 Subject: i2c: Verify that the count passed in to smbus_eeprom_init() is valid Keep someone from passing in a bogus number Signed-off-by: Corey Minyard --- hw/i2c/smbus_eeprom.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 0ba5763fc2..37167e7244 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -180,6 +180,8 @@ void smbus_eeprom_init(I2CBus *smbus, int nb_eeprom, { int i; /* XXX: make this persistent */ + + assert(nb_eeprom <= 8); uint8_t *eeprom_buf = g_malloc0(8 * SMBUS_EEPROM_SIZE); if (eeprom_spd_size > 0) { memcpy(eeprom_buf, eeprom_spd, eeprom_spd_size); -- cgit v1.2.3