| Message ID | 661021052fba1fed3d9d872b47ae050c5593ebfc.1361565596.git.jan.kiszka@siemens.com |
|---|---|
| State | New |
| Headers | show |
Am 22.02.2013 21:39, schrieb Jan Kiszka: > This implements I2C EEPROMs of the AT24Cxx series. Sizes from 1Kbit to > 1024Kbit are supported. Each EEPROM is backed by a block device. Its > size can be explicitly specified by the "size" property (required for > sizes < 512, the blockdev sector size) or is derived from the size of > the backing block device. Device addresses are built from the device > number property. Write protection can be configured by declaring the > block device read-only. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/Makefile.objs | 2 +- > hw/at24.c | 363 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 364 insertions(+), 1 deletions(-) > create mode 100644 hw/at24.c > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs > index a1f3a80..a64700c 100644 > --- a/hw/Makefile.objs > +++ b/hw/Makefile.objs > @@ -178,7 +178,7 @@ common-obj-$(CONFIG_SSD0323) += ssd0323.o > common-obj-$(CONFIG_ADS7846) += ads7846.o > common-obj-$(CONFIG_MAX111X) += max111x.o > common-obj-$(CONFIG_DS1338) += ds1338.o > -common-obj-y += i2c.o smbus.o smbus_eeprom.o > +common-obj-y += i2c.o smbus.o smbus_eeprom.o at24.o > common-obj-y += eeprom93xx.o > common-obj-y += scsi-disk.o cdrom.o hd-geometry.o block-common.o > common-obj-y += scsi-generic.o scsi-bus.o > diff --git a/hw/at24.c b/hw/at24.c > new file mode 100644 > index 0000000..0dfefd4 > --- /dev/null > +++ b/hw/at24.c > @@ -0,0 +1,363 @@ > +/* > + * AT24Cxx EEPROM emulation > + * > + * Copyright (c) Siemens AG, 2012 > + * Author: Jan Kiszka > + * > + * 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.h" > +#include "i2c.h" Please use "hw/hw.h" and "hw/i2c.h" since Paolo is planning to move I2C devices into hw/i2c/. > +#include "sysemu/blockdev.h" > +#include "hw/block-common.h" > + > +#define AT24_BASE_ADDRESS 0x50 > +#define AT24_MAX_PAGE_LEN 256 > + > +typedef enum AT24TransferState { > + AT24_IDLE, > + AT24_RD_ADDR, > + AT24_WR_ADDR_HI, > + AT24_WR_ADDR_LO, > + AT24_RW_DATA0, > + AT24_RD_DATA, > + AT24_WR_DATA, > +} AT24TransferState; > + > +typedef struct AT24State { > + I2CSlave i2c; parent_obj and separate from remaining fields please. All uses of this field except in VMState will be wrong. http://wiki.qemu.org/QOMConventions > + BlockConf block_conf; > + BlockDriverState *bs; > + uint32_t size; > + bool wp; > + unsigned int addr_mask; > + unsigned int page_mask; > + bool addr16; > + unsigned int hi_addr_mask; > + uint8_t device; > + AT24TransferState transfer_state; > + uint8_t sector_buffer[BDRV_SECTOR_SIZE]; > + int cached_sector; > + bool cache_dirty; > + uint32_t transfer_addr; > +} AT24State; > + > +static void at24_flush_transfer_buffer(AT24State *s) > +{ > + if (s->cached_sector < 0 || !s->cache_dirty) { > + return; > + } > + bdrv_write(s->bs, s->cached_sector, s->sector_buffer, 1); > + s->cache_dirty = false; > +} > + > +static void at24_event(I2CSlave *i2c, enum i2c_event event, uint8_t param) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); Please don't use DO_UPCAST(), add a QOM cast macro AT24() instead. > + > + switch (event) { > + case I2C_START_SEND: > + switch (s->transfer_state) { > + case AT24_IDLE: > + if (s->addr16) { > + s->transfer_addr = (param & s->hi_addr_mask) << 16; > + s->transfer_state = AT24_WR_ADDR_HI; > + } else { > + s->transfer_addr = (param & s->hi_addr_mask) << 8; > + s->transfer_state = AT24_WR_ADDR_LO; > + } > + break; > + default: > + s->transfer_state = AT24_IDLE; > + break; > + } > + break; > + case I2C_START_RECV: > + switch (s->transfer_state) { > + case AT24_IDLE: > + s->transfer_state = AT24_RD_ADDR; > + break; > + case AT24_RW_DATA0: > + s->transfer_state = AT24_RD_DATA; > + break; > + default: > + s->transfer_state = AT24_IDLE; > + break; > + } > + break; > + case I2C_FINISH: > + switch (s->transfer_state) { > + case AT24_WR_DATA: > + at24_flush_transfer_buffer(s); > + /* fall through */ > + default: > + s->transfer_state = AT24_IDLE; > + break; > + } > + break; > + default: > + s->transfer_state = AT24_IDLE; > + break; > + } > +} > + > +static int at24_cache_sector(AT24State *s, int sector) > +{ > + int ret; > + > + if (s->cached_sector == sector) { > + return 0; > + } > + ret = bdrv_read(s->bs, sector, s->sector_buffer, 1); > + if (ret < 0) { > + s->cached_sector = -1; > + return ret; > + } > + s->cached_sector = sector; > + s->cache_dirty = false; > + return 0; > +} > + > +static int at24_tx(I2CSlave *i2c, uint8_t data) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > + > + switch (s->transfer_state) { > + case AT24_WR_ADDR_HI: > + s->transfer_addr |= (data << 8) & s->addr_mask; > + s->transfer_state = AT24_WR_ADDR_LO; > + break; > + case AT24_WR_ADDR_LO: > + s->transfer_addr |= data & s->addr_mask; > + s->transfer_state = AT24_RW_DATA0; > + break; > + case AT24_RW_DATA0: > + s->transfer_state = AT24_WR_DATA; > + if (at24_cache_sector(s, s->transfer_addr >> BDRV_SECTOR_BITS) < 0) { > + s->transfer_state = AT24_IDLE; > + return -1; > + } > + /* fall through */ > + case AT24_WR_DATA: > + if (!s->wp) { > + s->sector_buffer[s->transfer_addr & ~BDRV_SECTOR_MASK] = data; > + s->cache_dirty = true; > + } > + s->transfer_addr = (s->transfer_addr & s->page_mask) | > + ((s->transfer_addr + 1) & ~s->page_mask); > + break; > + default: > + s->transfer_state = AT24_IDLE; > + return -1; > + } > + return 0; > +} > + > +static int at24_rx(I2CSlave *i2c) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > + unsigned int sector, offset; > + int result; > + > + switch (s->transfer_state) { > + case AT24_RD_ADDR: > + s->transfer_state = AT24_IDLE; > + result = s->transfer_addr; > + break; > + case AT24_RD_DATA: > + sector = s->transfer_addr >> BDRV_SECTOR_BITS; > + offset = s->transfer_addr & ~BDRV_SECTOR_MASK; > + s->transfer_addr = (s->transfer_addr + 1) & s->addr_mask; > + if (at24_cache_sector(s, sector) < 0) { > + result = 0; > + break; > + } > + result = s->sector_buffer[offset]; > + break; > + default: > + s->transfer_state = AT24_IDLE; > + result = 0; > + break; > + } > + return result; > +} > + > +static void at24_reset(DeviceState *d) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c.qdev, d); > + > + s->transfer_state = AT24_IDLE; > + s->cached_sector = -1; > +} > + > +static int at24_init(I2CSlave *i2c) hw/i2c.c:i2c_slave_qdev_init() calls I2CSlaveClass::init only, so please use a realize function instead. Cf. hw/qdev-core.h. > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > + unsigned int page_size; > + int64_t image_size; > + int device_bits; > + int hi_addr_bits; > + int dev_no; > + > + assert(AT24_MAX_PAGE_LEN <= BDRV_SECTOR_SIZE); This should instead do error_setg(errp, "...") then. > + > + s->bs = s->block_conf.bs; > + if (!s->bs) { > + error_report("drive property not set"); Dito, same below. > + return -1; > + } > + > + s->wp = bdrv_is_read_only(s->bs); > + > + image_size = bdrv_getlength(s->bs); > + if (s->size == 0) { > + s->size = image_size; > + } else if (image_size < s->size) { > + error_report("image file smaller than specified EEPROM size"); > + return -1; > + } > + > + switch (s->size * 8) { > + case 1*1024: > + case 2*1024: > + page_size = 8; > + device_bits = 3; > + hi_addr_bits = 0; > + break; > + case 4*1024: > + page_size = 16; > + device_bits = 2; > + hi_addr_bits = 1; > + break; > + case 8*1024: > + page_size = 16; > + device_bits = 1; > + hi_addr_bits = 2; > + break; > + case 16*1024: > + page_size = 16; > + device_bits = 0; > + hi_addr_bits = 3; > + break; > + case 32*1024: > + case 64*1024: > + page_size = 32; > + device_bits = 3; > + hi_addr_bits = 0; > + s->addr16 = true; > + break; > + case 128*1024: > + case 256*1024: > + page_size = 64; > + device_bits = 2; > + hi_addr_bits = 0; > + s->addr16 = true; > + break; > + case 512*1024: > + page_size = 128; > + device_bits = 2; > + hi_addr_bits = 0; > + s->addr16 = true; > + break; > + case 1024*1024: > + page_size = 256; > + device_bits = 1; > + hi_addr_bits = 1; > + s->addr16 = true; > + break; > + default: > + error_report("invalid EEPROM size, must be 2^(10..17)"); > + return -1; > + } > + s->addr_mask = s->size - 1; > + s->page_mask = ~(page_size - 1); > + s->hi_addr_mask = (1 << hi_addr_bits) - 1; > + > + dev_no = (s->device & ((1 << device_bits) - 1)) << hi_addr_bits; > + i2c->address = AT24_BASE_ADDRESS | dev_no; > + i2c->address_mask &= ~s->hi_addr_mask; > + > + return 0; > +} > + > +static void at24_pre_save(void *opaque) > +{ > + AT24State *s = opaque; > + > + at24_flush_transfer_buffer(s); > +} > + > +static int at24_post_load(void *opaque, int version_id) > +{ > + AT24State *s = opaque; > + > + s->cached_sector = -1; > + return 0; > +} > + > +static const VMStateDescription vmstate_at24 = { > + .name = "at24", > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .pre_save = at24_pre_save, > + .post_load = at24_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(transfer_state, AT24State), > + VMSTATE_UINT32(transfer_addr, AT24State), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static Property at24_properties[] = { > + DEFINE_BLOCK_PROPERTIES(AT24State, block_conf), > + DEFINE_PROP_UINT8("device", AT24State, device, 0), > + DEFINE_PROP_UINT32("size", AT24State, size, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void at24_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); > + > + sc->init = at24_init; > + sc->event = at24_event; > + sc->recv = at24_rx; > + sc->send = at24_tx; > + dc->desc = "AT24Cxx EEPROM"; > + dc->reset = at24_reset; > + dc->vmsd = &vmstate_at24; > + dc->props = at24_properties; > +} > + > +static TypeInfo at24_info = { http://git.qemu.org/?p=qemu.git;a=commit;h=8c43a6f05d5ef3c9484bd2be9d4e818d58e62016 updated all TypeInfos to be static const, please don't regress. > + .name = "at24", > + .parent = TYPE_I2C_SLAVE, > + .instance_size = sizeof(AT24State), > + .class_init = at24_class_init, > +}; > + > +static void at24_register_types(void) > +{ > + type_register_static(&at24_info); > +} > + > +type_init(at24_register_types) My I2C libqos has landed in qemu.git a while ago, so this new device might be a candidate for a qtest case now, no? The easiest way I see would be arm with -M n810 -device at24,device=x,size=y since OMAP has an I2C libqos implementation and has same endianness as x86. Regards, Andreas
> > +#include "hw.h" > > +#include "i2c.h" > > Please use "hw/hw.h" and "hw/i2c.h" since Paolo is planning to move > I2C devices into hw/i2c/. No, I2C _masters_ move into hw/i2c. This would probably move into hw/nvram. Paolo > > +#include "sysemu/blockdev.h" > > +#include "hw/block-common.h" > > + > > +#define AT24_BASE_ADDRESS 0x50 > > +#define AT24_MAX_PAGE_LEN 256 > > + > > +typedef enum AT24TransferState { > > + AT24_IDLE, > > + AT24_RD_ADDR, > > + AT24_WR_ADDR_HI, > > + AT24_WR_ADDR_LO, > > + AT24_RW_DATA0, > > + AT24_RD_DATA, > > + AT24_WR_DATA, > > +} AT24TransferState; > > + > > +typedef struct AT24State { > > + I2CSlave i2c; > > parent_obj and separate from remaining fields please. All uses of > this > field except in VMState will be wrong. > > http://wiki.qemu.org/QOMConventions > > > + BlockConf block_conf; > > + BlockDriverState *bs; > > + uint32_t size; > > + bool wp; > > + unsigned int addr_mask; > > + unsigned int page_mask; > > + bool addr16; > > + unsigned int hi_addr_mask; > > + uint8_t device; > > + AT24TransferState transfer_state; > > + uint8_t sector_buffer[BDRV_SECTOR_SIZE]; > > + int cached_sector; > > + bool cache_dirty; > > + uint32_t transfer_addr; > > +} AT24State; > > + > > +static void at24_flush_transfer_buffer(AT24State *s) > > +{ > > + if (s->cached_sector < 0 || !s->cache_dirty) { > > + return; > > + } > > + bdrv_write(s->bs, s->cached_sector, s->sector_buffer, 1); > > + s->cache_dirty = false; > > +} > > + > > +static void at24_event(I2CSlave *i2c, enum i2c_event event, > > uint8_t param) > > +{ > > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > > Please don't use DO_UPCAST(), add a QOM cast macro AT24() instead. > > > + > > + switch (event) { > > + case I2C_START_SEND: > > + switch (s->transfer_state) { > > + case AT24_IDLE: > > + if (s->addr16) { > > + s->transfer_addr = (param & s->hi_addr_mask) << > > 16; > > + s->transfer_state = AT24_WR_ADDR_HI; > > + } else { > > + s->transfer_addr = (param & s->hi_addr_mask) << 8; > > + s->transfer_state = AT24_WR_ADDR_LO; > > + } > > + break; > > + default: > > + s->transfer_state = AT24_IDLE; > > + break; > > + } > > + break; > > + case I2C_START_RECV: > > + switch (s->transfer_state) { > > + case AT24_IDLE: > > + s->transfer_state = AT24_RD_ADDR; > > + break; > > + case AT24_RW_DATA0: > > + s->transfer_state = AT24_RD_DATA; > > + break; > > + default: > > + s->transfer_state = AT24_IDLE; > > + break; > > + } > > + break; > > + case I2C_FINISH: > > + switch (s->transfer_state) { > > + case AT24_WR_DATA: > > + at24_flush_transfer_buffer(s); > > + /* fall through */ > > + default: > > + s->transfer_state = AT24_IDLE; > > + break; > > + } > > + break; > > + default: > > + s->transfer_state = AT24_IDLE; > > + break; > > + } > > +} > > + > > +static int at24_cache_sector(AT24State *s, int sector) > > +{ > > + int ret; > > + > > + if (s->cached_sector == sector) { > > + return 0; > > + } > > + ret = bdrv_read(s->bs, sector, s->sector_buffer, 1); > > + if (ret < 0) { > > + s->cached_sector = -1; > > + return ret; > > + } > > + s->cached_sector = sector; > > + s->cache_dirty = false; > > + return 0; > > +} > > + > > +static int at24_tx(I2CSlave *i2c, uint8_t data) > > +{ > > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > > + > > + switch (s->transfer_state) { > > + case AT24_WR_ADDR_HI: > > + s->transfer_addr |= (data << 8) & s->addr_mask; > > + s->transfer_state = AT24_WR_ADDR_LO; > > + break; > > + case AT24_WR_ADDR_LO: > > + s->transfer_addr |= data & s->addr_mask; > > + s->transfer_state = AT24_RW_DATA0; > > + break; > > + case AT24_RW_DATA0: > > + s->transfer_state = AT24_WR_DATA; > > + if (at24_cache_sector(s, s->transfer_addr >> > > BDRV_SECTOR_BITS) < 0) { > > + s->transfer_state = AT24_IDLE; > > + return -1; > > + } > > + /* fall through */ > > + case AT24_WR_DATA: > > + if (!s->wp) { > > + s->sector_buffer[s->transfer_addr & ~BDRV_SECTOR_MASK] > > = data; > > + s->cache_dirty = true; > > + } > > + s->transfer_addr = (s->transfer_addr & s->page_mask) | > > + ((s->transfer_addr + 1) & ~s->page_mask); > > + break; > > + default: > > + s->transfer_state = AT24_IDLE; > > + return -1; > > + } > > + return 0; > > +} > > + > > +static int at24_rx(I2CSlave *i2c) > > +{ > > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > > + unsigned int sector, offset; > > + int result; > > + > > + switch (s->transfer_state) { > > + case AT24_RD_ADDR: > > + s->transfer_state = AT24_IDLE; > > + result = s->transfer_addr; > > + break; > > + case AT24_RD_DATA: > > + sector = s->transfer_addr >> BDRV_SECTOR_BITS; > > + offset = s->transfer_addr & ~BDRV_SECTOR_MASK; > > + s->transfer_addr = (s->transfer_addr + 1) & s->addr_mask; > > + if (at24_cache_sector(s, sector) < 0) { > > + result = 0; > > + break; > > + } > > + result = s->sector_buffer[offset]; > > + break; > > + default: > > + s->transfer_state = AT24_IDLE; > > + result = 0; > > + break; > > + } > > + return result; > > +} > > + > > +static void at24_reset(DeviceState *d) > > +{ > > + AT24State *s = DO_UPCAST(AT24State, i2c.qdev, d); > > + > > + s->transfer_state = AT24_IDLE; > > + s->cached_sector = -1; > > +} > > + > > +static int at24_init(I2CSlave *i2c) > > hw/i2c.c:i2c_slave_qdev_init() calls I2CSlaveClass::init only, so > please > use a realize function instead. Cf. hw/qdev-core.h. > > > +{ > > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > > + unsigned int page_size; > > + int64_t image_size; > > + int device_bits; > > + int hi_addr_bits; > > + int dev_no; > > + > > + assert(AT24_MAX_PAGE_LEN <= BDRV_SECTOR_SIZE); > > This should instead do error_setg(errp, "...") then. > > > + > > + s->bs = s->block_conf.bs; > > + if (!s->bs) { > > + error_report("drive property not set"); > > Dito, same below. > > > + return -1; > > + } > > + > > + s->wp = bdrv_is_read_only(s->bs); > > + > > + image_size = bdrv_getlength(s->bs); > > + if (s->size == 0) { > > + s->size = image_size; > > + } else if (image_size < s->size) { > > + error_report("image file smaller than specified EEPROM > > size"); > > + return -1; > > + } > > + > > + switch (s->size * 8) { > > + case 1*1024: > > + case 2*1024: > > + page_size = 8; > > + device_bits = 3; > > + hi_addr_bits = 0; > > + break; > > + case 4*1024: > > + page_size = 16; > > + device_bits = 2; > > + hi_addr_bits = 1; > > + break; > > + case 8*1024: > > + page_size = 16; > > + device_bits = 1; > > + hi_addr_bits = 2; > > + break; > > + case 16*1024: > > + page_size = 16; > > + device_bits = 0; > > + hi_addr_bits = 3; > > + break; > > + case 32*1024: > > + case 64*1024: > > + page_size = 32; > > + device_bits = 3; > > + hi_addr_bits = 0; > > + s->addr16 = true; > > + break; > > + case 128*1024: > > + case 256*1024: > > + page_size = 64; > > + device_bits = 2; > > + hi_addr_bits = 0; > > + s->addr16 = true; > > + break; > > + case 512*1024: > > + page_size = 128; > > + device_bits = 2; > > + hi_addr_bits = 0; > > + s->addr16 = true; > > + break; > > + case 1024*1024: > > + page_size = 256; > > + device_bits = 1; > > + hi_addr_bits = 1; > > + s->addr16 = true; > > + break; > > + default: > > + error_report("invalid EEPROM size, must be 2^(10..17)"); > > + return -1; > > + } > > + s->addr_mask = s->size - 1; > > + s->page_mask = ~(page_size - 1); > > + s->hi_addr_mask = (1 << hi_addr_bits) - 1; > > + > > + dev_no = (s->device & ((1 << device_bits) - 1)) << > > hi_addr_bits; > > + i2c->address = AT24_BASE_ADDRESS | dev_no; > > + i2c->address_mask &= ~s->hi_addr_mask; > > + > > + return 0; > > +} > > + > > +static void at24_pre_save(void *opaque) > > +{ > > + AT24State *s = opaque; > > + > > + at24_flush_transfer_buffer(s); > > +} > > + > > +static int at24_post_load(void *opaque, int version_id) > > +{ > > + AT24State *s = opaque; > > + > > + s->cached_sector = -1; > > + return 0; > > +} > > + > > +static const VMStateDescription vmstate_at24 = { > > + .name = "at24", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .minimum_version_id_old = 1, > > + .pre_save = at24_pre_save, > > + .post_load = at24_post_load, > > + .fields = (VMStateField[]) { > > + VMSTATE_UINT32(transfer_state, AT24State), > > + VMSTATE_UINT32(transfer_addr, AT24State), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > +static Property at24_properties[] = { > > + DEFINE_BLOCK_PROPERTIES(AT24State, block_conf), > > + DEFINE_PROP_UINT8("device", AT24State, device, 0), > > + DEFINE_PROP_UINT32("size", AT24State, size, 0), > > + DEFINE_PROP_END_OF_LIST(), > > +}; > > + > > +static void at24_class_init(ObjectClass *klass, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); > > + > > + sc->init = at24_init; > > + sc->event = at24_event; > > + sc->recv = at24_rx; > > + sc->send = at24_tx; > > + dc->desc = "AT24Cxx EEPROM"; > > + dc->reset = at24_reset; > > + dc->vmsd = &vmstate_at24; > > + dc->props = at24_properties; > > +} > > + > > +static TypeInfo at24_info = { > > http://git.qemu.org/?p=qemu.git;a=commit;h=8c43a6f05d5ef3c9484bd2be9d4e818d58e62016 > updated all TypeInfos to be static const, please don't regress. > > > + .name = "at24", > > + .parent = TYPE_I2C_SLAVE, > > + .instance_size = sizeof(AT24State), > > + .class_init = at24_class_init, > > +}; > > + > > +static void at24_register_types(void) > > +{ > > + type_register_static(&at24_info); > > +} > > + > > +type_init(at24_register_types) > > My I2C libqos has landed in qemu.git a while ago, so this new device > might be a candidate for a qtest case now, no? The easiest way I see > would be arm with -M n810 -device at24,device=x,size=y since OMAP has > an > I2C libqos implementation and has same endianness as x86. > > Regards, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG > Nürnberg >
Am 25.02.2013 08:49, schrieb Paolo Bonzini: > >>> +#include "hw.h" >>> +#include "i2c.h" >> >> Please use "hw/hw.h" and "hw/i2c.h" since Paolo is planning to move >> I2C devices into hw/i2c/. > > No, I2C _masters_ move into hw/i2c. This would probably move into > hw/nvram. OK, but my point of using hw/ is valid whatever the directory name. :) Andreas
Hi Jan, I actually have my own subset implementation of this currently on list. Yours is more complete however, So id prefer to drop mine in favour of yours and reverify Zynq as working with yours. On Sat, Feb 23, 2013 at 6:39 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: > This implements I2C EEPROMs of the AT24Cxx series. Sizes from 1Kbit to > 1024Kbit are supported. Each EEPROM is backed by a block device. Its > size can be explicitly specified by the "size" property (required for > sizes < 512, the blockdev sector size) or is derived from the size of > the backing block device. Device addresses are built from the device > number property. Write protection can be configured by declaring the > block device read-only. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/Makefile.objs | 2 +- > hw/at24.c | 363 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 364 insertions(+), 1 deletions(-) > create mode 100644 hw/at24.c > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs > index a1f3a80..a64700c 100644 > --- a/hw/Makefile.objs > +++ b/hw/Makefile.objs > @@ -178,7 +178,7 @@ common-obj-$(CONFIG_SSD0323) += ssd0323.o > common-obj-$(CONFIG_ADS7846) += ads7846.o > common-obj-$(CONFIG_MAX111X) += max111x.o > common-obj-$(CONFIG_DS1338) += ds1338.o > -common-obj-y += i2c.o smbus.o smbus_eeprom.o > +common-obj-y += i2c.o smbus.o smbus_eeprom.o at24.o > common-obj-y += eeprom93xx.o > common-obj-y += scsi-disk.o cdrom.o hd-geometry.o block-common.o > common-obj-y += scsi-generic.o scsi-bus.o > diff --git a/hw/at24.c b/hw/at24.c > new file mode 100644 > index 0000000..0dfefd4 > --- /dev/null > +++ b/hw/at24.c > @@ -0,0 +1,363 @@ > +/* > + * AT24Cxx EEPROM emulation > + * > + * Copyright (c) Siemens AG, 2012 > + * Author: Jan Kiszka > + * > + * 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.h" > +#include "i2c.h" > +#include "sysemu/blockdev.h" > +#include "hw/block-common.h" > + > +#define AT24_BASE_ADDRESS 0x50 > +#define AT24_MAX_PAGE_LEN 256 > + > +typedef enum AT24TransferState { > + AT24_IDLE, > + AT24_RD_ADDR, > + AT24_WR_ADDR_HI, > + AT24_WR_ADDR_LO, > + AT24_RW_DATA0, > + AT24_RD_DATA, > + AT24_WR_DATA, > +} AT24TransferState; > + > +typedef struct AT24State { > + I2CSlave i2c; > + BlockConf block_conf; > + BlockDriverState *bs; > + uint32_t size; > + bool wp; > + unsigned int addr_mask; > + unsigned int page_mask; > + bool addr16; > + unsigned int hi_addr_mask; > + uint8_t device; > + AT24TransferState transfer_state; > + uint8_t sector_buffer[BDRV_SECTOR_SIZE]; > + int cached_sector; > + bool cache_dirty; > + uint32_t transfer_addr; > +} AT24State; > + > +static void at24_flush_transfer_buffer(AT24State *s) > +{ > + if (s->cached_sector < 0 || !s->cache_dirty) { > + return; > + } > + bdrv_write(s->bs, s->cached_sector, s->sector_buffer, 1); > + s->cache_dirty = false; > +} > + > +static void at24_event(I2CSlave *i2c, enum i2c_event event, uint8_t param) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > + > + switch (event) { > + case I2C_START_SEND: > + switch (s->transfer_state) { > + case AT24_IDLE: > + if (s->addr16) { > + s->transfer_addr = (param & s->hi_addr_mask) << 16; > + s->transfer_state = AT24_WR_ADDR_HI; > + } else { > + s->transfer_addr = (param & s->hi_addr_mask) << 8; > + s->transfer_state = AT24_WR_ADDR_LO; > + } > + break; > + default: This looks like a guest error. Could add a qemu_log_mask(LOG_GUEST_ERROR for completeness. Same for other default-back-to-idle conditions below. > + s->transfer_state = AT24_IDLE; > + break; > + } > + break; > + case I2C_START_RECV: > + switch (s->transfer_state) { > + case AT24_IDLE: > + s->transfer_state = AT24_RD_ADDR; > + break; > + case AT24_RW_DATA0: > + s->transfer_state = AT24_RD_DATA; > + break; > + default: > + s->transfer_state = AT24_IDLE; > + break; > + } > + break; > + case I2C_FINISH: > + switch (s->transfer_state) { > + case AT24_WR_DATA: > + at24_flush_transfer_buffer(s); > + /* fall through */ > + default: > + s->transfer_state = AT24_IDLE; > + break; > + } > + break; > + default: > + s->transfer_state = AT24_IDLE; > + break; > + } > +} > + > +static int at24_cache_sector(AT24State *s, int sector) > +{ > + int ret; > + > + if (s->cached_sector == sector) { > + return 0; > + } > + ret = bdrv_read(s->bs, sector, s->sector_buffer, 1); > + if (ret < 0) { > + s->cached_sector = -1; > + return ret; > + } > + s->cached_sector = sector; > + s->cache_dirty = false; > + return 0; > +} > + > +static int at24_tx(I2CSlave *i2c, uint8_t data) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > + > + switch (s->transfer_state) { > + case AT24_WR_ADDR_HI: > + s->transfer_addr |= (data << 8) & s->addr_mask; > + s->transfer_state = AT24_WR_ADDR_LO; > + break; > + case AT24_WR_ADDR_LO: > + s->transfer_addr |= data & s->addr_mask; > + s->transfer_state = AT24_RW_DATA0; > + break; > + case AT24_RW_DATA0: > + s->transfer_state = AT24_WR_DATA; > + if (at24_cache_sector(s, s->transfer_addr >> BDRV_SECTOR_BITS) < 0) { > + s->transfer_state = AT24_IDLE; > + return -1; > + } Minor nitpick - theres an assumption here that page_size < BDRV_SECTOR_SIZE. The device will bug if larger page devices are added in the future. > + /* fall through */ > + case AT24_WR_DATA: > + if (!s->wp) { > + s->sector_buffer[s->transfer_addr & ~BDRV_SECTOR_MASK] = data; > + s->cache_dirty = true; > + } > + s->transfer_addr = (s->transfer_addr & s->page_mask) | > + ((s->transfer_addr + 1) & ~s->page_mask); > + break; > + default: > + s->transfer_state = AT24_IDLE; > + return -1; > + } > + return 0; > +} > + > +static int at24_rx(I2CSlave *i2c) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > + unsigned int sector, offset; > + int result; > + > + switch (s->transfer_state) { > + case AT24_RD_ADDR: > + s->transfer_state = AT24_IDLE; > + result = s->transfer_addr; > + break; > + case AT24_RD_DATA: > + sector = s->transfer_addr >> BDRV_SECTOR_BITS; > + offset = s->transfer_addr & ~BDRV_SECTOR_MASK; > + s->transfer_addr = (s->transfer_addr + 1) & s->addr_mask; > + if (at24_cache_sector(s, sector) < 0) { > + result = 0; > + break; > + } > + result = s->sector_buffer[offset]; > + break; > + default: > + s->transfer_state = AT24_IDLE; > + result = 0; > + break; > + } > + return result; > +} > + > +static void at24_reset(DeviceState *d) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c.qdev, d); > + > + s->transfer_state = AT24_IDLE; > + s->cached_sector = -1; > +} > + > +static int at24_init(I2CSlave *i2c) > +{ > + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); > + unsigned int page_size; > + int64_t image_size; > + int device_bits; > + int hi_addr_bits; > + int dev_no; > + > + assert(AT24_MAX_PAGE_LEN <= BDRV_SECTOR_SIZE); > + > + s->bs = s->block_conf.bs; > + if (!s->bs) { > + error_report("drive property not set"); > + return -1; > + } > + > + s->wp = bdrv_is_read_only(s->bs); > + > + image_size = bdrv_getlength(s->bs); > + if (s->size == 0) { > + s->size = image_size; > + } else if (image_size < s->size) { > + error_report("image file smaller than specified EEPROM size"); > + return -1; > + } > + > + switch (s->size * 8) { > + case 1*1024: > + case 2*1024: > + page_size = 8; > + device_bits = 3; > + hi_addr_bits = 0; > + break; > + case 4*1024: > + page_size = 16; > + device_bits = 2; > + hi_addr_bits = 1; > + break; > + case 8*1024: > + page_size = 16; > + device_bits = 1; > + hi_addr_bits = 2; > + break; > + case 16*1024: > + page_size = 16; > + device_bits = 0; > + hi_addr_bits = 3; > + break; > + case 32*1024: > + case 64*1024: > + page_size = 32; > + device_bits = 3; > + hi_addr_bits = 0; > + s->addr16 = true; > + break; > + case 128*1024: > + case 256*1024: > + page_size = 64; > + device_bits = 2; > + hi_addr_bits = 0; > + s->addr16 = true; > + break; > + case 512*1024: > + page_size = 128; > + device_bits = 2; > + hi_addr_bits = 0; > + s->addr16 = true; > + break; > + case 1024*1024: > + page_size = 256; > + device_bits = 1; > + hi_addr_bits = 1; > + s->addr16 = true; > + break; Can we data drive this in some way? Pull this out into a table of some description. > + default: > + error_report("invalid EEPROM size, must be 2^(10..17)"); > + return -1; > + } > + s->addr_mask = s->size - 1; > + s->page_mask = ~(page_size - 1); > + s->hi_addr_mask = (1 << hi_addr_bits) - 1; > + > + dev_no = (s->device & ((1 << device_bits) - 1)) << hi_addr_bits; > + i2c->address = AT24_BASE_ADDRESS | dev_no; This kinda breaks the I2C address property. Machine models are able to set I2C addresses and this code here is trampling it. I think it would be cleaner to generalise the I2CSlave::address property to handle the device_bits concept as you have introduced here. Many I2C devices have the device_bits concept not just AT24. E.G. I2C slaves would then have three props num_address_bits (here as device_bits) -> number of high order address bits the machine model can set (in most cases these are physical pins on the device). address -> value of the high order address bits address_mask -> same as implmented in this series num_address_bits defaults to 7 for backwards compatibility of existing I2C devices. Anyone who sets an I2C address without changing num_address_bits just sets a full 7 bit I2C address. Anyone who changes num_address_bits will then interpret the i2c address as these device_bits. > + i2c->address_mask &= ~s->hi_addr_mask; > + Both address and address_mask are private internals to I2C_SLAVE. I think they should be encapsulated by setters. i2c_set_slave_address() already exists. Just need to add its new buddy, i2c_set_slave_mask(). Regards, Peter > + return 0; > +} > + > +static void at24_pre_save(void *opaque) > +{ > + AT24State *s = opaque; > + > + at24_flush_transfer_buffer(s); > +} > + > +static int at24_post_load(void *opaque, int version_id) > +{ > + AT24State *s = opaque; > + > + s->cached_sector = -1; > + return 0; > +} > + > +static const VMStateDescription vmstate_at24 = { > + .name = "at24", > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > + .pre_save = at24_pre_save, > + .post_load = at24_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_UINT32(transfer_state, AT24State), > + VMSTATE_UINT32(transfer_addr, AT24State), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static Property at24_properties[] = { > + DEFINE_BLOCK_PROPERTIES(AT24State, block_conf), > + DEFINE_PROP_UINT8("device", AT24State, device, 0), > + DEFINE_PROP_UINT32("size", AT24State, size, 0), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void at24_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); > + > + sc->init = at24_init; > + sc->event = at24_event; > + sc->recv = at24_rx; > + sc->send = at24_tx; > + dc->desc = "AT24Cxx EEPROM"; > + dc->reset = at24_reset; > + dc->vmsd = &vmstate_at24; > + dc->props = at24_properties; > +} > + > +static TypeInfo at24_info = { > + .name = "at24", > + .parent = TYPE_I2C_SLAVE, > + .instance_size = sizeof(AT24State), > + .class_init = at24_class_init, > +}; > + > +static void at24_register_types(void) > +{ > + type_register_static(&at24_info); > +} > + > +type_init(at24_register_types) > -- > 1.7.3.4 > >
On 2013-02-27 05:44, Peter Crosthwaite wrote: > Hi Jan, > > I actually have my own subset implementation of this currently on > list. Yours is more complete however, So id prefer to drop mine in > favour of yours and reverify Zynq as working with yours. Great! I'm trying to look for a slot to address the review comments and come up with a new version. > > On Sat, Feb 23, 2013 at 6:39 AM, Jan Kiszka <jan.kiszka@siemens.com> wrote: >> This implements I2C EEPROMs of the AT24Cxx series. Sizes from 1Kbit to >> 1024Kbit are supported. Each EEPROM is backed by a block device. Its >> size can be explicitly specified by the "size" property (required for >> sizes < 512, the blockdev sector size) or is derived from the size of >> the backing block device. Device addresses are built from the device >> number property. Write protection can be configured by declaring the >> block device read-only. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> hw/Makefile.objs | 2 +- >> hw/at24.c | 363 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 364 insertions(+), 1 deletions(-) >> create mode 100644 hw/at24.c >> >> diff --git a/hw/Makefile.objs b/hw/Makefile.objs >> index a1f3a80..a64700c 100644 >> --- a/hw/Makefile.objs >> +++ b/hw/Makefile.objs >> @@ -178,7 +178,7 @@ common-obj-$(CONFIG_SSD0323) += ssd0323.o >> common-obj-$(CONFIG_ADS7846) += ads7846.o >> common-obj-$(CONFIG_MAX111X) += max111x.o >> common-obj-$(CONFIG_DS1338) += ds1338.o >> -common-obj-y += i2c.o smbus.o smbus_eeprom.o >> +common-obj-y += i2c.o smbus.o smbus_eeprom.o at24.o >> common-obj-y += eeprom93xx.o >> common-obj-y += scsi-disk.o cdrom.o hd-geometry.o block-common.o >> common-obj-y += scsi-generic.o scsi-bus.o >> diff --git a/hw/at24.c b/hw/at24.c >> new file mode 100644 >> index 0000000..0dfefd4 >> --- /dev/null >> +++ b/hw/at24.c >> @@ -0,0 +1,363 @@ >> +/* >> + * AT24Cxx EEPROM emulation >> + * >> + * Copyright (c) Siemens AG, 2012 >> + * Author: Jan Kiszka >> + * >> + * 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.h" >> +#include "i2c.h" >> +#include "sysemu/blockdev.h" >> +#include "hw/block-common.h" >> + >> +#define AT24_BASE_ADDRESS 0x50 >> +#define AT24_MAX_PAGE_LEN 256 >> + >> +typedef enum AT24TransferState { >> + AT24_IDLE, >> + AT24_RD_ADDR, >> + AT24_WR_ADDR_HI, >> + AT24_WR_ADDR_LO, >> + AT24_RW_DATA0, >> + AT24_RD_DATA, >> + AT24_WR_DATA, >> +} AT24TransferState; >> + >> +typedef struct AT24State { >> + I2CSlave i2c; >> + BlockConf block_conf; >> + BlockDriverState *bs; >> + uint32_t size; >> + bool wp; >> + unsigned int addr_mask; >> + unsigned int page_mask; >> + bool addr16; >> + unsigned int hi_addr_mask; >> + uint8_t device; >> + AT24TransferState transfer_state; >> + uint8_t sector_buffer[BDRV_SECTOR_SIZE]; >> + int cached_sector; >> + bool cache_dirty; >> + uint32_t transfer_addr; >> +} AT24State; >> + >> +static void at24_flush_transfer_buffer(AT24State *s) >> +{ >> + if (s->cached_sector < 0 || !s->cache_dirty) { >> + return; >> + } >> + bdrv_write(s->bs, s->cached_sector, s->sector_buffer, 1); >> + s->cache_dirty = false; >> +} >> + >> +static void at24_event(I2CSlave *i2c, enum i2c_event event, uint8_t param) >> +{ >> + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); >> + >> + switch (event) { >> + case I2C_START_SEND: >> + switch (s->transfer_state) { >> + case AT24_IDLE: >> + if (s->addr16) { >> + s->transfer_addr = (param & s->hi_addr_mask) << 16; >> + s->transfer_state = AT24_WR_ADDR_HI; >> + } else { >> + s->transfer_addr = (param & s->hi_addr_mask) << 8; >> + s->transfer_state = AT24_WR_ADDR_LO; >> + } >> + break; >> + default: > > This looks like a guest error. Could add a > qemu_log_mask(LOG_GUEST_ERROR for completeness. Same for other > default-back-to-idle conditions below. OK. > >> + s->transfer_state = AT24_IDLE; >> + break; >> + } >> + break; >> + case I2C_START_RECV: >> + switch (s->transfer_state) { >> + case AT24_IDLE: >> + s->transfer_state = AT24_RD_ADDR; >> + break; >> + case AT24_RW_DATA0: >> + s->transfer_state = AT24_RD_DATA; >> + break; >> + default: >> + s->transfer_state = AT24_IDLE; >> + break; >> + } >> + break; >> + case I2C_FINISH: >> + switch (s->transfer_state) { >> + case AT24_WR_DATA: >> + at24_flush_transfer_buffer(s); >> + /* fall through */ >> + default: >> + s->transfer_state = AT24_IDLE; >> + break; >> + } >> + break; >> + default: >> + s->transfer_state = AT24_IDLE; >> + break; >> + } >> +} >> + >> +static int at24_cache_sector(AT24State *s, int sector) >> +{ >> + int ret; >> + >> + if (s->cached_sector == sector) { >> + return 0; >> + } >> + ret = bdrv_read(s->bs, sector, s->sector_buffer, 1); >> + if (ret < 0) { >> + s->cached_sector = -1; >> + return ret; >> + } >> + s->cached_sector = sector; >> + s->cache_dirty = false; >> + return 0; >> +} >> + >> +static int at24_tx(I2CSlave *i2c, uint8_t data) >> +{ >> + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); >> + >> + switch (s->transfer_state) { >> + case AT24_WR_ADDR_HI: >> + s->transfer_addr |= (data << 8) & s->addr_mask; >> + s->transfer_state = AT24_WR_ADDR_LO; >> + break; >> + case AT24_WR_ADDR_LO: >> + s->transfer_addr |= data & s->addr_mask; >> + s->transfer_state = AT24_RW_DATA0; >> + break; >> + case AT24_RW_DATA0: >> + s->transfer_state = AT24_WR_DATA; >> + if (at24_cache_sector(s, s->transfer_addr >> BDRV_SECTOR_BITS) < 0) { >> + s->transfer_state = AT24_IDLE; >> + return -1; >> + } > > Minor nitpick - theres an assumption here that page_size < > BDRV_SECTOR_SIZE. The device will bug if larger page devices are added > in the future. Yes, there is an assert (which should better be a BUILD_BUG_ON) below. I don't think it's worth to make the code more clever than that. > >> + /* fall through */ >> + case AT24_WR_DATA: >> + if (!s->wp) { >> + s->sector_buffer[s->transfer_addr & ~BDRV_SECTOR_MASK] = data; >> + s->cache_dirty = true; >> + } >> + s->transfer_addr = (s->transfer_addr & s->page_mask) | >> + ((s->transfer_addr + 1) & ~s->page_mask); >> + break; >> + default: >> + s->transfer_state = AT24_IDLE; >> + return -1; >> + } >> + return 0; >> +} >> + >> +static int at24_rx(I2CSlave *i2c) >> +{ >> + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); >> + unsigned int sector, offset; >> + int result; >> + >> + switch (s->transfer_state) { >> + case AT24_RD_ADDR: >> + s->transfer_state = AT24_IDLE; >> + result = s->transfer_addr; >> + break; >> + case AT24_RD_DATA: >> + sector = s->transfer_addr >> BDRV_SECTOR_BITS; >> + offset = s->transfer_addr & ~BDRV_SECTOR_MASK; >> + s->transfer_addr = (s->transfer_addr + 1) & s->addr_mask; >> + if (at24_cache_sector(s, sector) < 0) { >> + result = 0; >> + break; >> + } >> + result = s->sector_buffer[offset]; >> + break; >> + default: >> + s->transfer_state = AT24_IDLE; >> + result = 0; >> + break; >> + } >> + return result; >> +} >> + >> +static void at24_reset(DeviceState *d) >> +{ >> + AT24State *s = DO_UPCAST(AT24State, i2c.qdev, d); >> + >> + s->transfer_state = AT24_IDLE; >> + s->cached_sector = -1; >> +} >> + >> +static int at24_init(I2CSlave *i2c) >> +{ >> + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); >> + unsigned int page_size; >> + int64_t image_size; >> + int device_bits; >> + int hi_addr_bits; >> + int dev_no; >> + >> + assert(AT24_MAX_PAGE_LEN <= BDRV_SECTOR_SIZE); >> + >> + s->bs = s->block_conf.bs; >> + if (!s->bs) { >> + error_report("drive property not set"); >> + return -1; >> + } >> + >> + s->wp = bdrv_is_read_only(s->bs); >> + >> + image_size = bdrv_getlength(s->bs); >> + if (s->size == 0) { >> + s->size = image_size; >> + } else if (image_size < s->size) { >> + error_report("image file smaller than specified EEPROM size"); >> + return -1; >> + } >> + >> + switch (s->size * 8) { >> + case 1*1024: >> + case 2*1024: >> + page_size = 8; >> + device_bits = 3; >> + hi_addr_bits = 0; >> + break; >> + case 4*1024: >> + page_size = 16; >> + device_bits = 2; >> + hi_addr_bits = 1; >> + break; >> + case 8*1024: >> + page_size = 16; >> + device_bits = 1; >> + hi_addr_bits = 2; >> + break; >> + case 16*1024: >> + page_size = 16; >> + device_bits = 0; >> + hi_addr_bits = 3; >> + break; >> + case 32*1024: >> + case 64*1024: >> + page_size = 32; >> + device_bits = 3; >> + hi_addr_bits = 0; >> + s->addr16 = true; >> + break; >> + case 128*1024: >> + case 256*1024: >> + page_size = 64; >> + device_bits = 2; >> + hi_addr_bits = 0; >> + s->addr16 = true; >> + break; >> + case 512*1024: >> + page_size = 128; >> + device_bits = 2; >> + hi_addr_bits = 0; >> + s->addr16 = true; >> + break; >> + case 1024*1024: >> + page_size = 256; >> + device_bits = 1; >> + hi_addr_bits = 1; >> + s->addr16 = true; >> + break; > > Can we data drive this in some way? Pull this out into a table of some > description. Sure, will play with it. > >> + default: >> + error_report("invalid EEPROM size, must be 2^(10..17)"); >> + return -1; >> + } >> + s->addr_mask = s->size - 1; >> + s->page_mask = ~(page_size - 1); >> + s->hi_addr_mask = (1 << hi_addr_bits) - 1; >> + >> + dev_no = (s->device & ((1 << device_bits) - 1)) << hi_addr_bits; >> + i2c->address = AT24_BASE_ADDRESS | dev_no; > > This kinda breaks the I2C address property. Machine models are able to > set I2C addresses and this code here is trampling it. Well, it's implementing the device spec. > I think it would > be cleaner to generalise the I2CSlave::address property to handle the > device_bits concept as you have introduced here. Many I2C devices have > the device_bits concept not just AT24. E.G. I2C slaves would then have > three props > > num_address_bits (here as device_bits) -> number of high order address > bits the machine model can set (in most cases these are physical pins > on the device). > address -> value of the high order address bits > address_mask -> same as implmented in this series > > num_address_bits defaults to 7 for backwards compatibility of existing > I2C devices. Anyone who sets an I2C address without changing > num_address_bits just sets a full 7 bit I2C address. Anyone who > changes num_address_bits will then interpret the i2c address as these > device_bits. Hmm, will think about this. But I'm not a fan of making something (user-)configurable that isn't - num_address_bits/device_bits are hard-coded values the device defines. > >> + i2c->address_mask &= ~s->hi_addr_mask; >> + > > Both address and address_mask are private internals to I2C_SLAVE. I > think they should be encapsulated by setters. i2c_set_slave_address() > already exists. Just need to add its new buddy, i2c_set_slave_mask(). OK, that's simple. Thanks, Jan
On 2013-02-23 14:40, Andreas Färber wrote: >> +static int at24_init(I2CSlave *i2c) > > hw/i2c.c:i2c_slave_qdev_init() calls I2CSlaveClass::init only, so please > use a realize function instead. Cf. hw/qdev-core.h. > >> +{ >> + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); >> + unsigned int page_size; >> + int64_t image_size; >> + int device_bits; >> + int hi_addr_bits; >> + int dev_no; >> + >> + assert(AT24_MAX_PAGE_LEN <= BDRV_SECTOR_SIZE); > > This should instead do error_setg(errp, "...") then. I just noticed with my latest code that the messages set via error_setg do not make it to the terminal when initializing the device from the command line. Known issue? Jan
diff --git a/hw/Makefile.objs b/hw/Makefile.objs index a1f3a80..a64700c 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -178,7 +178,7 @@ common-obj-$(CONFIG_SSD0323) += ssd0323.o common-obj-$(CONFIG_ADS7846) += ads7846.o common-obj-$(CONFIG_MAX111X) += max111x.o common-obj-$(CONFIG_DS1338) += ds1338.o -common-obj-y += i2c.o smbus.o smbus_eeprom.o +common-obj-y += i2c.o smbus.o smbus_eeprom.o at24.o common-obj-y += eeprom93xx.o common-obj-y += scsi-disk.o cdrom.o hd-geometry.o block-common.o common-obj-y += scsi-generic.o scsi-bus.o diff --git a/hw/at24.c b/hw/at24.c new file mode 100644 index 0000000..0dfefd4 --- /dev/null +++ b/hw/at24.c @@ -0,0 +1,363 @@ +/* + * AT24Cxx EEPROM emulation + * + * Copyright (c) Siemens AG, 2012 + * Author: Jan Kiszka + * + * 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.h" +#include "i2c.h" +#include "sysemu/blockdev.h" +#include "hw/block-common.h" + +#define AT24_BASE_ADDRESS 0x50 +#define AT24_MAX_PAGE_LEN 256 + +typedef enum AT24TransferState { + AT24_IDLE, + AT24_RD_ADDR, + AT24_WR_ADDR_HI, + AT24_WR_ADDR_LO, + AT24_RW_DATA0, + AT24_RD_DATA, + AT24_WR_DATA, +} AT24TransferState; + +typedef struct AT24State { + I2CSlave i2c; + BlockConf block_conf; + BlockDriverState *bs; + uint32_t size; + bool wp; + unsigned int addr_mask; + unsigned int page_mask; + bool addr16; + unsigned int hi_addr_mask; + uint8_t device; + AT24TransferState transfer_state; + uint8_t sector_buffer[BDRV_SECTOR_SIZE]; + int cached_sector; + bool cache_dirty; + uint32_t transfer_addr; +} AT24State; + +static void at24_flush_transfer_buffer(AT24State *s) +{ + if (s->cached_sector < 0 || !s->cache_dirty) { + return; + } + bdrv_write(s->bs, s->cached_sector, s->sector_buffer, 1); + s->cache_dirty = false; +} + +static void at24_event(I2CSlave *i2c, enum i2c_event event, uint8_t param) +{ + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); + + switch (event) { + case I2C_START_SEND: + switch (s->transfer_state) { + case AT24_IDLE: + if (s->addr16) { + s->transfer_addr = (param & s->hi_addr_mask) << 16; + s->transfer_state = AT24_WR_ADDR_HI; + } else { + s->transfer_addr = (param & s->hi_addr_mask) << 8; + s->transfer_state = AT24_WR_ADDR_LO; + } + break; + default: + s->transfer_state = AT24_IDLE; + break; + } + break; + case I2C_START_RECV: + switch (s->transfer_state) { + case AT24_IDLE: + s->transfer_state = AT24_RD_ADDR; + break; + case AT24_RW_DATA0: + s->transfer_state = AT24_RD_DATA; + break; + default: + s->transfer_state = AT24_IDLE; + break; + } + break; + case I2C_FINISH: + switch (s->transfer_state) { + case AT24_WR_DATA: + at24_flush_transfer_buffer(s); + /* fall through */ + default: + s->transfer_state = AT24_IDLE; + break; + } + break; + default: + s->transfer_state = AT24_IDLE; + break; + } +} + +static int at24_cache_sector(AT24State *s, int sector) +{ + int ret; + + if (s->cached_sector == sector) { + return 0; + } + ret = bdrv_read(s->bs, sector, s->sector_buffer, 1); + if (ret < 0) { + s->cached_sector = -1; + return ret; + } + s->cached_sector = sector; + s->cache_dirty = false; + return 0; +} + +static int at24_tx(I2CSlave *i2c, uint8_t data) +{ + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); + + switch (s->transfer_state) { + case AT24_WR_ADDR_HI: + s->transfer_addr |= (data << 8) & s->addr_mask; + s->transfer_state = AT24_WR_ADDR_LO; + break; + case AT24_WR_ADDR_LO: + s->transfer_addr |= data & s->addr_mask; + s->transfer_state = AT24_RW_DATA0; + break; + case AT24_RW_DATA0: + s->transfer_state = AT24_WR_DATA; + if (at24_cache_sector(s, s->transfer_addr >> BDRV_SECTOR_BITS) < 0) { + s->transfer_state = AT24_IDLE; + return -1; + } + /* fall through */ + case AT24_WR_DATA: + if (!s->wp) { + s->sector_buffer[s->transfer_addr & ~BDRV_SECTOR_MASK] = data; + s->cache_dirty = true; + } + s->transfer_addr = (s->transfer_addr & s->page_mask) | + ((s->transfer_addr + 1) & ~s->page_mask); + break; + default: + s->transfer_state = AT24_IDLE; + return -1; + } + return 0; +} + +static int at24_rx(I2CSlave *i2c) +{ + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); + unsigned int sector, offset; + int result; + + switch (s->transfer_state) { + case AT24_RD_ADDR: + s->transfer_state = AT24_IDLE; + result = s->transfer_addr; + break; + case AT24_RD_DATA: + sector = s->transfer_addr >> BDRV_SECTOR_BITS; + offset = s->transfer_addr & ~BDRV_SECTOR_MASK; + s->transfer_addr = (s->transfer_addr + 1) & s->addr_mask; + if (at24_cache_sector(s, sector) < 0) { + result = 0; + break; + } + result = s->sector_buffer[offset]; + break; + default: + s->transfer_state = AT24_IDLE; + result = 0; + break; + } + return result; +} + +static void at24_reset(DeviceState *d) +{ + AT24State *s = DO_UPCAST(AT24State, i2c.qdev, d); + + s->transfer_state = AT24_IDLE; + s->cached_sector = -1; +} + +static int at24_init(I2CSlave *i2c) +{ + AT24State *s = DO_UPCAST(AT24State, i2c, i2c); + unsigned int page_size; + int64_t image_size; + int device_bits; + int hi_addr_bits; + int dev_no; + + assert(AT24_MAX_PAGE_LEN <= BDRV_SECTOR_SIZE); + + s->bs = s->block_conf.bs; + if (!s->bs) { + error_report("drive property not set"); + return -1; + } + + s->wp = bdrv_is_read_only(s->bs); + + image_size = bdrv_getlength(s->bs); + if (s->size == 0) { + s->size = image_size; + } else if (image_size < s->size) { + error_report("image file smaller than specified EEPROM size"); + return -1; + } + + switch (s->size * 8) { + case 1*1024: + case 2*1024: + page_size = 8; + device_bits = 3; + hi_addr_bits = 0; + break; + case 4*1024: + page_size = 16; + device_bits = 2; + hi_addr_bits = 1; + break; + case 8*1024: + page_size = 16; + device_bits = 1; + hi_addr_bits = 2; + break; + case 16*1024: + page_size = 16; + device_bits = 0; + hi_addr_bits = 3; + break; + case 32*1024: + case 64*1024: + page_size = 32; + device_bits = 3; + hi_addr_bits = 0; + s->addr16 = true; + break; + case 128*1024: + case 256*1024: + page_size = 64; + device_bits = 2; + hi_addr_bits = 0; + s->addr16 = true; + break; + case 512*1024: + page_size = 128; + device_bits = 2; + hi_addr_bits = 0; + s->addr16 = true; + break; + case 1024*1024: + page_size = 256; + device_bits = 1; + hi_addr_bits = 1; + s->addr16 = true; + break; + default: + error_report("invalid EEPROM size, must be 2^(10..17)"); + return -1; + } + s->addr_mask = s->size - 1; + s->page_mask = ~(page_size - 1); + s->hi_addr_mask = (1 << hi_addr_bits) - 1; + + dev_no = (s->device & ((1 << device_bits) - 1)) << hi_addr_bits; + i2c->address = AT24_BASE_ADDRESS | dev_no; + i2c->address_mask &= ~s->hi_addr_mask; + + return 0; +} + +static void at24_pre_save(void *opaque) +{ + AT24State *s = opaque; + + at24_flush_transfer_buffer(s); +} + +static int at24_post_load(void *opaque, int version_id) +{ + AT24State *s = opaque; + + s->cached_sector = -1; + return 0; +} + +static const VMStateDescription vmstate_at24 = { + .name = "at24", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .pre_save = at24_pre_save, + .post_load = at24_post_load, + .fields = (VMStateField[]) { + VMSTATE_UINT32(transfer_state, AT24State), + VMSTATE_UINT32(transfer_addr, AT24State), + VMSTATE_END_OF_LIST() + } +}; + +static Property at24_properties[] = { + DEFINE_BLOCK_PROPERTIES(AT24State, block_conf), + DEFINE_PROP_UINT8("device", AT24State, device, 0), + DEFINE_PROP_UINT32("size", AT24State, size, 0), + DEFINE_PROP_END_OF_LIST(), +}; + +static void at24_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + I2CSlaveClass *sc = I2C_SLAVE_CLASS(klass); + + sc->init = at24_init; + sc->event = at24_event; + sc->recv = at24_rx; + sc->send = at24_tx; + dc->desc = "AT24Cxx EEPROM"; + dc->reset = at24_reset; + dc->vmsd = &vmstate_at24; + dc->props = at24_properties; +} + +static TypeInfo at24_info = { + .name = "at24", + .parent = TYPE_I2C_SLAVE, + .instance_size = sizeof(AT24State), + .class_init = at24_class_init, +}; + +static void at24_register_types(void) +{ + type_register_static(&at24_info); +} + +type_init(at24_register_types)
This implements I2C EEPROMs of the AT24Cxx series. Sizes from 1Kbit to 1024Kbit are supported. Each EEPROM is backed by a block device. Its size can be explicitly specified by the "size" property (required for sizes < 512, the blockdev sector size) or is derived from the size of the backing block device. Device addresses are built from the device number property. Write protection can be configured by declaring the block device read-only. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/Makefile.objs | 2 +- hw/at24.c | 363 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 364 insertions(+), 1 deletions(-) create mode 100644 hw/at24.c