On 31 May 2011 15:16, Vasily Khoruzhick <anars...@gmail.com> wrote:
> +static uint32_t zipit_lcd_transfer(SSISlave *dev, uint32_t value)
> +{
> + ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev);
> + if (z->enabled) {
> + z->buf[z->pos] = value & 0xff;
> + z->pos++;
> + }
> + if (z->pos == 3) {
> + switch (z->buf[0]) {
> + case 0x74:
> + printf("%s: reg: 0x%.2x\n", __func__, z->buf[2]);
> + break;
> + case 0x76:
> + printf("%s: value: 0x%.4x\n", __func__, z->buf[1] << 8 |
> z->buf[2]);
> + break;
> + default:
> + printf("%s: unknown command!\n", __func__);
> + break;
> + }
> + z->pos = 0;
> + }
> + return 0;
> +}
Presumably this is a stub for later functionality?
I don't think we should be printing the debug tracing by default.
> +static SSISlaveInfo zipit_lcd_info = {
> + .qdev.name = "zipit-lcd",
> + .qdev.size = sizeof(ZipitLCD),
> + .init = zipit_lcd_init,
> + .transfer = zipit_lcd_transfer
> +};
Not that the device does much, but it ought to have a
VMStateDescription structure to save/load its state.
> + case I2C_START_RECV:
> + if (s->len != 1)
> + printf("%s: short message!?\n", __func__);
QEMU coding style demands braces here. Running through
scripts/checkpatch.pl should catch this kind of thing.
> +static int aer915_recv(i2c_slave *slave)
> +{
> + int retval = 0x00;
> + AER915State *s = FROM_I2C_SLAVE(AER915State, slave);
> +
> + /* Hardcoded value */
This comment isn't telling us anything we can't see from
the code. More interesting would be what the hardcoded
value actually means...
> +static I2CSlaveInfo aer915_info = {
> + .qdev.name = "aer915",
> + .qdev.size = sizeof(AER915State),
> + .init = aer915_init,
> + .event = aer915_event,
> + .recv = aer915_recv,
> + .send = aer915_send
> +};
Missing save/load support again.
> + bus = pxa2xx_i2c_bus(cpu->i2c[0]);
> + i2c_create_slave(bus, "aer915", 0x55);
> + wm = i2c_create_slave(bus, "wm8750", 0x1b);
> + /* .. and to the sound interface. */
This comment has come from spitz.c, and it doesn't make
much sense here since you've not copied the preceding
comment which it is a continuation of:
/* Attach a WM8750 to the bus */
-- PMM
