diff mbox series

[v2,01/12] i2c: Split smbus into parts

Message ID 20181115192446.17187-2-minyard@acm.org
State New
Headers show
Series RFC: Fix/add vmstate handling in some I2C code | expand

Commit Message

Corey Minyard Nov. 15, 2018, 7:24 p.m. UTC
From: Corey Minyard <cminyard@mvista.com>

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 <cminyard@mvista.com>
---
 hw/arm/aspeed.c                           |   2 +-
 hw/i2c/Makefile.objs                      |   2 +-
 hw/i2c/pm_smbus.c                         |   2 +-
 hw/i2c/smbus_eeprom.c                     |   3 +-
 hw/i2c/smbus_ich9.c                       |   2 -
 hw/i2c/smbus_master.c                     | 165 ++++++++++++++++++++++
 hw/i2c/{smbus.c => smbus_slave.c}         | 153 +-------------------
 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_eeprom.h             |  11 ++
 include/hw/i2c/smbus_master.h             |  55 ++++++++
 include/hw/i2c/{smbus.h => smbus_slave.h} |  35 +----
 17 files changed, 251 insertions(+), 192 deletions(-)
 create mode 100644 hw/i2c/smbus_master.c
 rename hw/i2c/{smbus.c => smbus_slave.c} (64%)
 create mode 100644 include/hw/i2c/smbus_eeprom.h
 create mode 100644 include/hw/i2c/smbus_master.h
 rename include/hw/i2c/{smbus.h => smbus_slave.h} (65%)

Comments

Philippe Mathieu-Daudé Nov. 15, 2018, 10:22 p.m. UTC | #1
On 15/11/18 20:24, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> 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.

Lovely cleanup!

> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>   hw/arm/aspeed.c                           |   2 +-
>   hw/i2c/Makefile.objs                      |   2 +-
>   hw/i2c/pm_smbus.c                         |   2 +-
>   hw/i2c/smbus_eeprom.c                     |   3 +-
>   hw/i2c/smbus_ich9.c                       |   2 -
>   hw/i2c/smbus_master.c                     | 165 ++++++++++++++++++++++
>   hw/i2c/{smbus.c => smbus_slave.c}         | 153 +-------------------
>   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_eeprom.h             |  11 ++
>   include/hw/i2c/smbus_master.h             |  55 ++++++++
>   include/hw/i2c/{smbus.h => smbus_slave.h} |  35 +----
>   17 files changed, 251 insertions(+), 192 deletions(-)
>   create mode 100644 hw/i2c/smbus_master.c
>   rename hw/i2c/{smbus.c => smbus_slave.c} (64%)
>   create mode 100644 include/hw/i2c/smbus_eeprom.h
>   create mode 100644 include/hw/i2c/smbus_master.h
>   rename include/hw/i2c/{smbus.h => smbus_slave.h} (65%)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 6b33ecd5aa..69a19df00d 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 37cacde978..8973edfa22 100644
> --- a/hw/i2c/Makefile.objs
> +++ b/hw/i2c/Makefile.objs
> @@ -1,4 +1,4 @@
> -common-obj-$(CONFIG_I2C) += core.o smbus.o smbus_eeprom.o
> +common-obj-$(CONFIG_I2C) += core.o smbus_slave.o smbus_master.o smbus_eeprom.o
>   common-obj-$(CONFIG_DDC) += i2c-ddc.o
>   common-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o
>   common-obj-$(CONFIG_ACPI_X86) += smbus_ich9.o
> diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
> index 685a2378ed..f3c6cc46f9 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_eeprom.c b/hw/i2c/smbus_eeprom.c
> index f18aa3de35..d82423aa7e 100644
> --- a/hw/i2c/smbus_eeprom.c
> +++ b/hw/i2c/smbus_eeprom.c
> @@ -25,7 +25,8 @@
>   #include "qemu/osdep.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.c b/hw/i2c/smbus_slave.c
> similarity index 64%
> rename from hw/i2c/smbus.c
> rename to hw/i2c/smbus_slave.c
> index 6ff77c582f..81d2a38111 100644
> --- a/hw/i2c/smbus.c
> +++ b/hw/i2c/smbus_slave.c
> @@ -1,6 +1,10 @@
>   /*
>    * QEMU SMBus device emulation.
>    *
> + * This code is a helper for SMBus device emulation.  It implement 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
>    *
> @@ -12,7 +16,7 @@
>   #include "qemu/osdep.h"
>   #include "hw/hw.h"
>   #include "hw/i2c/i2c.h"
> -#include "hw/i2c/smbus.h"
> +#include "hw/i2c/smbus_slave.h"
>   
>   //#define DEBUG_SMBUS 1
>   
> @@ -202,153 +206,6 @@ static int smbus_i2c_send(I2CSlave *s, uint8_t data)
>       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);
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index dc09466b3e..cb28227cc3 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 532241e3f8..90e88c9b28 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 2fbba32c48..dae8acc108 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -27,7 +27,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 c1cf0fe12e..1fb7170f5e 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 5aac58f36e..7136b23f91 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_eeprom.h b/include/hw/i2c/smbus_eeprom.h
> new file mode 100644
> index 0000000000..2f56e5dc4e
> --- /dev/null
> +++ b/include/hw/i2c/smbus_eeprom.h
> @@ -0,0 +1,11 @@

You missed the copyright notice here.

> +
> +#ifndef QEMU_SMBUS_EEPROM_H
> +#define QEMU_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);
> +
> +#endif
> diff --git a/include/hw/i2c/smbus_master.h b/include/hw/i2c/smbus_master.h
> new file mode 100644
> index 0000000000..7d8bba5e37
> --- /dev/null
> +++ b/include/hw/i2c/smbus_master.h
> @@ -0,0 +1,55 @@
> +#ifndef QEMU_SMBUS_MASTER_H
> +#define QEMU_SMBUS_MASTER_H

Maybe HW_SMBUS_MASTER_H?

(same apply for other headers: QEMU_ -> HW_).

> +
> +/*
> + * 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.
> + */

Personally I prefer the #ifndef/define HW_SMBUS_MASTER_H here, matter of 
taste.

> +
> +#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.h b/include/hw/i2c/smbus_slave.h
> similarity index 65%
> rename from include/hw/i2c/smbus.h
> rename to include/hw/i2c/smbus_slave.h
> index d8b1b9ee81..8a40fdd34e 100644
> --- a/include/hw/i2c/smbus.h
> +++ b/include/hw/i2c/smbus_slave.h
> @@ -1,8 +1,8 @@
> -#ifndef QEMU_SMBUS_H
> -#define QEMU_SMBUS_H
> +#ifndef QEMU_SMBUS_DEV_H
> +#define QEMU_SMBUS_DEV_H
>   
>   /*
> - * QEMU SMBus API
> + * QEMU SMBus device (slave) API
>    *
>    * Copyright (c) 2007 Arastra, Inc.
>    *
> @@ -64,33 +64,4 @@ struct SMBusDevice {
>       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);
> -
>   #endif
> 

LGTM, thanks,

Phil.
Corey Minyard Nov. 16, 2018, 1:20 p.m. UTC | #2
On 11/15/18 4:22 PM, Philippe Mathieu-Daudé wrote:
> On 15/11/18 20:24, minyard@acm.org wrote:
>> From: Corey Minyard <cminyard@mvista.com>
>>
>> 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.
>
> Lovely cleanup!
>
Yes, this really needed to be done.  It took me a while to understand
this code originally because it was all mixed up.

I've fixed the things you pointed out.  One thing, though:


>>
>> diff --git a/include/hw/i2c/smbus_eeprom.h 
>> b/include/hw/i2c/smbus_eeprom.h
>> new file mode 100644
>> index 0000000000..2f56e5dc4e
>> --- /dev/null
>> +++ b/include/hw/i2c/smbus_eeprom.h
>> @@ -0,0 +1,11 @@
>
> You missed the copyright notice here.

Other files don't have copyright notices (i2c.h, for instance), and for
the smbus.[ch] case the copyrights are kind of mixed up, the include
files had the big header with a copyright by one company and the C
file had a different copyright notice by a different company.

Not a huge deal, but I didn't include it in that file because I didn't
think it was necessary.  I'm wondering if it would be best to
establish a style like Linux has, with the // SPDX... thing on the
first line.

Thanks,

-corey
Peter Maydell Nov. 20, 2018, 3:47 p.m. UTC | #3
On 16 November 2018 at 13:20, Corey Minyard <minyard@acm.org> wrote:
> On 11/15/18 4:22 PM, Philippe Mathieu-Daudé wrote:
>>> --- /dev/null
>>> +++ b/include/hw/i2c/smbus_eeprom.h
>>> @@ -0,0 +1,11 @@
>>
>>
>> You missed the copyright notice here.
>
>
> Other files don't have copyright notices (i2c.h, for instance), and for
> the smbus.[ch] case the copyrights are kind of mixed up, the include
> files had the big header with a copyright by one company and the C
> file had a different copyright notice by a different company.
>
> Not a huge deal, but I didn't include it in that file because I didn't
> think it was necessary.  I'm wondering if it would be best to
> establish a style like Linux has, with the // SPDX... thing on the
> first line.

Yeah, we have some legacy files with no copyright notice, but we
usually try to avoid that for new files. New files should have
a copyright notice and a license statement. (If you copied from
a file without a license statement, LICENSE says that means
2-or-later.)

We don't yet use SPDX headers. (They're just a different and
shorter way to write the license statement.)

thanks
-- PMM
Philippe Mathieu-Daudé Nov. 20, 2018, 7:30 p.m. UTC | #4
On 20/11/18 16:47, Peter Maydell wrote:
> On 16 November 2018 at 13:20, Corey Minyard <minyard@acm.org> wrote:
>> On 11/15/18 4:22 PM, Philippe Mathieu-Daudé wrote:
>>>> --- /dev/null
>>>> +++ b/include/hw/i2c/smbus_eeprom.h
>>>> @@ -0,0 +1,11 @@
>>>
>>>
>>> You missed the copyright notice here.
>>
>>
>> Other files don't have copyright notices (i2c.h, for instance), and for
>> the smbus.[ch] case the copyrights are kind of mixed up, the include
>> files had the big header with a copyright by one company and the C
>> file had a different copyright notice by a different company.
>>
>> Not a huge deal, but I didn't include it in that file because I didn't
>> think it was necessary.  I'm wondering if it would be best to
>> establish a style like Linux has, with the // SPDX... thing on the
>> first line.
> 
> Yeah, we have some legacy files with no copyright notice, but we
> usually try to avoid that for new files. New files should have
> a copyright notice and a license statement. (If you copied from
> a file without a license statement, LICENSE says that means
> 2-or-later.)
> 
> We don't yet use SPDX headers. (They're just a different and
> shorter way to write the license statement.)

Does that mean we can use them, or you rather prefer we don't?

While they are machine parseable, I find them easier to understand than 
the big chunk of legal text that sometime are not correctly written.

Thanks,

Phil.
Peter Maydell Nov. 21, 2018, 11:59 a.m. UTC | #5
On 20 November 2018 at 19:30, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 20/11/18 16:47, Peter Maydell wrote:
>> We don't yet use SPDX headers. (They're just a different and
>> shorter way to write the license statement.)
>
>
> Does that mean we can use them, or you rather prefer we don't?
>
> While they are machine parseable, I find them easier to understand than the
> big chunk of legal text that sometime are not correctly written.

It means that I'm not going to absolutely insist on dropping
the line if somebody submits a patch with an SPDX tag, but
I probably will mention that we don't use SPDX tags, and
definitely I'm not going to ask for them.

Overall I don't think there's much point in having them
added to one or two files randomly. If we want them then
we should consistently require them as policy. But that is
work, and so I think we should not do that until/unless
somebody (probably a corporate somebody) steps forward
to make the argument for "this is why we should have them,
we as a contributor to the project think they are worthwhile
and a useful feature for us, and we will make the effort to
add them, review that they are correct, update checkpatch to
insist on tags for new files, etc". In other words, "if it
ain't broke, don't fix it"; nobody is yet complaining that
our current setup is broken.

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6b33ecd5aa..69a19df00d 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 37cacde978..8973edfa22 100644
--- a/hw/i2c/Makefile.objs
+++ b/hw/i2c/Makefile.objs
@@ -1,4 +1,4 @@ 
-common-obj-$(CONFIG_I2C) += core.o smbus.o smbus_eeprom.o
+common-obj-$(CONFIG_I2C) += core.o smbus_slave.o smbus_master.o smbus_eeprom.o
 common-obj-$(CONFIG_DDC) += i2c-ddc.o
 common-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o
 common-obj-$(CONFIG_ACPI_X86) += smbus_ich9.o
diff --git a/hw/i2c/pm_smbus.c b/hw/i2c/pm_smbus.c
index 685a2378ed..f3c6cc46f9 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_eeprom.c b/hw/i2c/smbus_eeprom.c
index f18aa3de35..d82423aa7e 100644
--- a/hw/i2c/smbus_eeprom.c
+++ b/hw/i2c/smbus_eeprom.c
@@ -25,7 +25,8 @@ 
 #include "qemu/osdep.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.c b/hw/i2c/smbus_slave.c
similarity index 64%
rename from hw/i2c/smbus.c
rename to hw/i2c/smbus_slave.c
index 6ff77c582f..81d2a38111 100644
--- a/hw/i2c/smbus.c
+++ b/hw/i2c/smbus_slave.c
@@ -1,6 +1,10 @@ 
 /*
  * QEMU SMBus device emulation.
  *
+ * This code is a helper for SMBus device emulation.  It implement 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
  *
@@ -12,7 +16,7 @@ 
 #include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/i2c/i2c.h"
-#include "hw/i2c/smbus.h"
+#include "hw/i2c/smbus_slave.h"
 
 //#define DEBUG_SMBUS 1
 
@@ -202,153 +206,6 @@  static int smbus_i2c_send(I2CSlave *s, uint8_t data)
     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);
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index dc09466b3e..cb28227cc3 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 532241e3f8..90e88c9b28 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 2fbba32c48..dae8acc108 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -27,7 +27,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 c1cf0fe12e..1fb7170f5e 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 5aac58f36e..7136b23f91 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_eeprom.h b/include/hw/i2c/smbus_eeprom.h
new file mode 100644
index 0000000000..2f56e5dc4e
--- /dev/null
+++ b/include/hw/i2c/smbus_eeprom.h
@@ -0,0 +1,11 @@ 
+
+#ifndef QEMU_SMBUS_EEPROM_H
+#define QEMU_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);
+
+#endif
diff --git a/include/hw/i2c/smbus_master.h b/include/hw/i2c/smbus_master.h
new file mode 100644
index 0000000000..7d8bba5e37
--- /dev/null
+++ b/include/hw/i2c/smbus_master.h
@@ -0,0 +1,55 @@ 
+#ifndef QEMU_SMBUS_MASTER_H
+#define QEMU_SMBUS_MASTER_H
+
+/*
+ * 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.
+ */
+
+#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.h b/include/hw/i2c/smbus_slave.h
similarity index 65%
rename from include/hw/i2c/smbus.h
rename to include/hw/i2c/smbus_slave.h
index d8b1b9ee81..8a40fdd34e 100644
--- a/include/hw/i2c/smbus.h
+++ b/include/hw/i2c/smbus_slave.h
@@ -1,8 +1,8 @@ 
-#ifndef QEMU_SMBUS_H
-#define QEMU_SMBUS_H
+#ifndef QEMU_SMBUS_DEV_H
+#define QEMU_SMBUS_DEV_H
 
 /*
- * QEMU SMBus API
+ * QEMU SMBus device (slave) API
  *
  * Copyright (c) 2007 Arastra, Inc.
  *
@@ -64,33 +64,4 @@  struct SMBusDevice {
     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);
-
 #endif