[Qemu-devel] [PATCH v5 0/8] aspeed: add a witherspoon-bmc machine

[Qemu-devel] [PATCH v5 0/8] aspeed: add a witherspoon-bmc machine

[Cédric Le Goater]

Hello,

This series adds a new Aspeed machine to emulate the BMC of a
Witherspoon system. It also extends the other Aspeed machines with I2C
devices and adds a simple model for the pca9552 LED blinker present on
the witherspoon board.

Thanks,

C.

Changes since v4:

 - use a ROM memory region

Changes since v3:

 - introduce a dummy ROM device to catch invalid writes
 - removed 'ignore_memory_transaction_failures' on all Aspeed machines
 - reworked PCA9552 device :
   . simpler I2C handles
   . auto-increment support
   . unit test

Changes since v2:

 - removed comments on the I2C buffer size pf the PCA9552 device.
 - removed 'ignore_memory_transaction_failures' flag on the
   witherspoon machine.
   
Changes since v1:

 - introduced smbus_eeprom_init_one()

Cédric Le Goater (8):
  aspeed: use a ROM memory region to catch invalid writes
  aspeed: remove ignore_memory_transaction_failures all boards
  aspeed: add support for the witherspoon-bmc board
  aspeed: add an I2C RTC device to all machines
  smbus: add a smbus_eeprom_init_one() routine
  aspeed: Add EEPROM I2C devices
  misc: add pca9552 LED blinker model
  aspeed: add the pc9552 chips to the witherspoon machine

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/aspeed.c                 | 110 ++++++++++++++++-
 hw/i2c/smbus_eeprom.c           |  16 ++-
 hw/misc/Makefile.objs           |   1 +
 hw/misc/pca9552.c               | 259 ++++++++++++++++++++++++++++++++++++++++
 include/hw/i2c/smbus.h          |   1 +
 include/hw/misc/pca9552.h       |  33 +++++
 tests/Makefile.include          |   2 +
 tests/pca9552-test.c            | 131 ++++++++++++++++++++
 9 files changed, 544 insertions(+), 10 deletions(-)
 create mode 100644 hw/misc/pca9552.c
 create mode 100644 include/hw/misc/pca9552.h
 create mode 100644 tests/pca9552-test.c

-- 
2.13.6

[Qemu-devel] [PATCH v5 1/8] aspeed: use a ROM memory region to catch invalid writes

[Cédric Le Goater]

Some legacy firmwares access unimplemented addresses on the Aspeed SoC
(old U-Boot code using variables in the bss when it shouldn't do).
Let's use a ROM memory region to catch the invalid writes and support
new boards without using the 'ignore_memory_transaction_failures'
flag.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v4 :

 - use a ROM memory region

 
 hw/arm/aspeed.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index ab895ad490af..d88a8b5120b6 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -166,6 +166,23 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
     }
 }
 
+/*
+ * This is to track invalid writes done in the ROM by some legacy
+ * firmwares
+ */
+static void boot_rom_write(void *opaque, hwaddr offset, uint64_t value,
+                           unsigned size)
+{
+    qemu_log_mask(LOG_GUEST_ERROR,
+                  "%s: 0x%" HWADDR_PRIx " <- 0x%" PRIx64 " [%u]\n",
+                  __func__, offset, value, size);
+}
+
+static const MemoryRegionOps boot_rom_ops = {
+    .write = boot_rom_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 static void aspeed_board_init(MachineState *machine,
                               const AspeedBoardConfig *cfg)
 {
@@ -216,8 +233,9 @@ static void aspeed_board_init(MachineState *machine,
          * SoC and 128MB for the AST2500 SoC, which is twice as big as
          * needed by the flash modules of the Aspeed machines.
          */
-        memory_region_init_rom_nomigrate(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
-                               fl->size, &error_abort);
+        memory_region_init_rom_device(boot_rom, OBJECT(bmc), &boot_rom_ops,
+                                      NULL, "aspeed.boot_rom", fl->size,
+                                      &error_abort);
         memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
                                     boot_rom);
         write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
-- 
2.13.6

Re: [Qemu-devel] [PATCH v5 1/8] aspeed: use a ROM memory region to catch invalid writes

[Peter Maydell]

On 19 October 2017 at 16:12, Cédric Le Goater <clg@kaod.org> wrote:
> Some legacy firmwares access unimplemented addresses on the Aspeed SoC
> (old U-Boot code using variables in the bss when it shouldn't do).
> Let's use a ROM memory region to catch the invalid writes and support
> new boards without using the 'ignore_memory_transaction_failures'
> flag.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
>  Changes since v4 :
>
>  - use a ROM memory region

Probably worth mentioning in the commit message that this
is a migration compatibility break for these boards.

>  hw/arm/aspeed.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index ab895ad490af..d88a8b5120b6 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -166,6 +166,23 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>      }
>  }
>
> +/*
> + * This is to track invalid writes done in the ROM by some legacy
> + * firmwares
> + */
> +static void boot_rom_write(void *opaque, hwaddr offset, uint64_t value,
> +                           unsigned size)
> +{
> +    qemu_log_mask(LOG_GUEST_ERROR,
> +                  "%s: 0x%" HWADDR_PRIx " <- 0x%" PRIx64 " [%u]\n",
> +                  __func__, offset, value, size);
> +}
> +
> +static const MemoryRegionOps boot_rom_ops = {
> +    .write = boot_rom_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
>  static void aspeed_board_init(MachineState *machine,
>                                const AspeedBoardConfig *cfg)
>  {
> @@ -216,8 +233,9 @@ static void aspeed_board_init(MachineState *machine,
>           * SoC and 128MB for the AST2500 SoC, which is twice as big as
>           * needed by the flash modules of the Aspeed machines.
>           */
> -        memory_region_init_rom_nomigrate(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
> -                               fl->size, &error_abort);
> +        memory_region_init_rom_device(boot_rom, OBJECT(bmc), &boot_rom_ops,
> +                                      NULL, "aspeed.boot_rom", fl->size,
> +                                      &error_abort);
>          memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
>                                      boot_rom);
>          write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
> --

Don't you want to create the ROM device even if there happens
not to be any contents specified by the user? The thing is still
there in the SoC even if it's empty...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 1/8] aspeed: use a ROM memory region to catch invalid writes

[Cédric Le Goater]

On 10/19/2017 05:44 PM, Peter Maydell wrote:
> On 19 October 2017 at 16:12, Cédric Le Goater <clg@kaod.org> wrote:
>> Some legacy firmwares access unimplemented addresses on the Aspeed SoC
>> (old U-Boot code using variables in the bss when it shouldn't do).
>> Let's use a ROM memory region to catch the invalid writes and support
>> new boards without using the 'ignore_memory_transaction_failures'
>> flag.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>  Changes since v4 :
>>
>>  - use a ROM memory region
> 
> Probably worth mentioning in the commit message that this
> is a migration compatibility break for these boards.

ah. Indeed :/ 

I tend to forget about migration for these emulated machines 
but people are starting to use these Aspeed guests on large 
Intel hosts as if they were real BMCs. And so being able to 
change host is a valid requirement. I will add a comment.


>>  hw/arm/aspeed.c | 22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index ab895ad490af..d88a8b5120b6 100644
>> --- a/hw/arm/aspeed.c
>> +++ b/hw/arm/aspeed.c
>> @@ -166,6 +166,23 @@ static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>>      }
>>  }
>>
>> +/*
>> + * This is to track invalid writes done in the ROM by some legacy
>> + * firmwares
>> + */
>> +static void boot_rom_write(void *opaque, hwaddr offset, uint64_t value,
>> +                           unsigned size)
>> +{
>> +    qemu_log_mask(LOG_GUEST_ERROR,
>> +                  "%s: 0x%" HWADDR_PRIx " <- 0x%" PRIx64 " [%u]\n",
>> +                  __func__, offset, value, size);
>> +}
>> +
>> +static const MemoryRegionOps boot_rom_ops = {
>> +    .write = boot_rom_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>>  static void aspeed_board_init(MachineState *machine,
>>                                const AspeedBoardConfig *cfg)
>>  {
>> @@ -216,8 +233,9 @@ static void aspeed_board_init(MachineState *machine,
>>           * SoC and 128MB for the AST2500 SoC, which is twickende as big as
>>           * needed by the flash modules of the Aspeed machines.
>>           */
>> -        memory_region_init_rom_nomigrate(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
>> -                               fl->size, &error_abort);
>> +        memory_region_init_rom_device(boot_rom, OBJECT(bmc), &boot_rom_ops,
>> +                                      NULL, "aspeed.boot_rom", fl->size,
>> +                                      &error_abort);
>>          memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
>>                                      boot_rom);
>>          write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
>> --
> 
> Don't you want to create the ROM device even if there happens
> not to be any contents specified by the user? The thing is still
> there in the SoC even if it's empty...

Do you mean that we should create the boot ROM device even if there 
is no file backend ? I think so yes. Let's do a v6 :)

Thanks,

C.

Re: [Qemu-devel] [PATCH v5 1/8] aspeed: use a ROM memory region to catch invalid writes

[Philippe Mathieu-Daudé]

On 10/19/2017 12:44 PM, Peter Maydell wrote:
> On 19 October 2017 at 16:12, Cédric Le Goater <clg@kaod.org> wrote:
>> Some legacy firmwares access unimplemented addresses on the Aspeed SoC
>> (old U-Boot code using variables in the bss when it shouldn't do).
>> Let's use a ROM memory region to catch the invalid writes and support
>> new boards without using the 'ignore_memory_transaction_failures'
>> flag.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>>  Changes since v4 :
>>
>>  - use a ROM memory region
> 
> Probably worth mentioning in the commit message that this
> is a migration compatibility break for these boards.

What about the eeprom_buf from patch 6 "Add EEPROM I2C devices"?

My understanding is a migrated board would resume with a zeroized
eeprom, is this the expected behaviour?

Regards,

Phil.

Re: [Qemu-devel] [PATCH v5 1/8] aspeed: use a ROM memory region to catch invalid writes

[Cédric Le Goater]

On 10/20/2017 05:10 AM, Philippe Mathieu-Daudé wrote:
> On 10/19/2017 12:44 PM, Peter Maydell wrote:
>> On 19 October 2017 at 16:12, Cédric Le Goater <clg@kaod.org> wrote:
>>> Some legacy firmwares access unimplemented addresses on the Aspeed SoC
>>> (old U-Boot code using variables in the bss when it shouldn't do).
>>> Let's use a ROM memory region to catch the invalid writes and support
>>> new boards without using the 'ignore_memory_transaction_failures'
>>> flag.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>
>>>  Changes since v4 :
>>>
>>>  - use a ROM memory region
>>
>> Probably worth mentioning in the commit message that this
>> is a migration compatibility break for these boards.
> 
> What about the eeprom_buf from patch 6 "Add EEPROM I2C devices"?
> 
> My understanding is a migrated board would resume with a zeroized
> eeprom, is this the expected behaviour?

That would be probably the case. yes. As we don't populate the eeprom 
with VPD at machine init time and that migration is clearly not tested, 
I think we will address this issue later.  

Thanks,

C.  

[Qemu-devel] [PATCH v5 2/8] aspeed: remove ignore_memory_transaction_failures all boards

[Cédric Le Goater]

Now that we have a dummy ROM device catching the invalid writes, we
can remove this flag and work on fixing the current firwmares still
accessing unimplemented addresses.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index d88a8b5120b6..91c23968f685 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -288,7 +288,6 @@ static void palmetto_bmc_class_init(ObjectClass *oc, void *data)
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
     mc->no_parallel = 1;
-    mc->ignore_memory_transaction_failures = true;
 }
 
 static const TypeInfo palmetto_bmc_type = {
@@ -321,7 +320,6 @@ static void ast2500_evb_class_init(ObjectClass *oc, void *data)
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
     mc->no_parallel = 1;
-    mc->ignore_memory_transaction_failures = true;
 }
 
 static const TypeInfo ast2500_evb_type = {
@@ -346,7 +344,6 @@ static void romulus_bmc_class_init(ObjectClass *oc, void *data)
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
     mc->no_parallel = 1;
-    mc->ignore_memory_transaction_failures = true;
 }
 
 static const TypeInfo romulus_bmc_type = {
-- 
2.13.6

Re: [Qemu-devel] [PATCH v5 2/8] aspeed: remove ignore_memory_transaction_failures all boards

[Peter Maydell]

On 19 October 2017 at 16:12, Cédric Le Goater <clg@kaod.org> wrote:
> Now that we have a dummy ROM device catching the invalid writes, we
> can remove this flag and work on fixing the current firwmares still
> accessing unimplemented addresses.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v5 3/8] aspeed: add support for the witherspoon-bmc board

[Cédric Le Goater]

The Witherspoon boards are OpenPOWER system hosting POWER9 Processors.
Let's add support for their BMC including a couple of I2C devices as
found on real HW.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---

 Changes since v2:

 - removed 'ignore_memory_transaction_failures' flag on the machine.
 - Added a comment on the I2C device used.

 hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 91c23968f685..f2fc9884dba6 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -46,6 +46,7 @@ enum {
     PALMETTO_BMC,
     AST2500_EVB,
     ROMULUS_BMC,
+    WITHERSPOON_BMC,
 };
 
 /* Palmetto hardware value: 0x120CE416 */
@@ -83,8 +84,12 @@ enum {
         SCU_AST2500_HW_STRAP_ACPI_ENABLE |                              \
         SCU_HW_STRAP_SPI_MODE(SCU_HW_STRAP_SPI_MASTER))
 
+/* Witherspoon hardware value: 0xF10AD216 (but use romulus definition) */
+#define WITHERSPOON_BMC_HW_STRAP1 ROMULUS_BMC_HW_STRAP1
+
 static void palmetto_bmc_i2c_init(AspeedBoardState *bmc);
 static void ast2500_evb_i2c_init(AspeedBoardState *bmc);
+static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc);
 
 static const AspeedBoardConfig aspeed_boards[] = {
     [PALMETTO_BMC] = {
@@ -110,6 +115,14 @@ static const AspeedBoardConfig aspeed_boards[] = {
         .spi_model = "mx66l1g45g",
         .num_cs    = 2,
     },
+    [WITHERSPOON_BMC]  = {
+        .soc_name  = "ast2500-a1",
+        .hw_strap1 = WITHERSPOON_BMC_HW_STRAP1,
+        .fmc_model = "mx25l25635e",
+        .spi_model = "mx66l1g45g",
+        .num_cs    = 2,
+        .i2c_init  = witherspoon_bmc_i2c_init,
+    },
 };
 
 #define FIRMWARE_ADDR 0x0
@@ -352,11 +365,47 @@ static const TypeInfo romulus_bmc_type = {
     .class_init = romulus_bmc_class_init,
 };
 
+static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
+{
+    AspeedSoCState *soc = &bmc->soc;
+
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c);
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c);
+
+    /* The Witherspoon expects a TMP275 but a TMP105 is compatible */
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a);
+}
+
+static void witherspoon_bmc_init(MachineState *machine)
+{
+    aspeed_board_init(machine, &aspeed_boards[WITHERSPOON_BMC]);
+}
+
+static void witherspoon_bmc_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "OpenPOWER Witherspoon BMC (ARM1176)";
+    mc->init = witherspoon_bmc_init;
+    mc->max_cpus = 1;
+    mc->no_sdcard = 1;
+    mc->no_floppy = 1;
+    mc->no_cdrom = 1;
+    mc->no_parallel = 1;
+}
+
+static const TypeInfo witherspoon_bmc_type = {
+    .name = MACHINE_TYPE_NAME("witherspoon-bmc"),
+    .parent = TYPE_MACHINE,
+    .class_init = witherspoon_bmc_class_init,
+};
+
 static void aspeed_machine_init(void)
 {
     type_register_static(&palmetto_bmc_type);
     type_register_static(&ast2500_evb_type);
     type_register_static(&romulus_bmc_type);
+    type_register_static(&witherspoon_bmc_type);
 }
 
 type_init(aspeed_machine_init)
-- 
2.13.6

[Qemu-devel] [PATCH v5 4/8] aspeed: add an I2C RTC device to all machines

[Cédric Le Goater]

The AST2500 EVB does not have an RTC but we can pretend that one is
plugged on the I2C bus header.

The romulus and witherspoon boards expects an Epson RX8900 I2C RTC but
a ds1338 is good enough for the basic features we need.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
 hw/arm/aspeed.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index f2fc9884dba6..f0a440f81fd6 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -89,6 +89,7 @@ enum {
 
 static void palmetto_bmc_i2c_init(AspeedBoardState *bmc);
 static void ast2500_evb_i2c_init(AspeedBoardState *bmc);
+static void romulus_bmc_i2c_init(AspeedBoardState *bmc);
 static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc);
 
 static const AspeedBoardConfig aspeed_boards[] = {
@@ -114,6 +115,7 @@ static const AspeedBoardConfig aspeed_boards[] = {
         .fmc_model = "n25q256a",
         .spi_model = "mx66l1g45g",
         .num_cs    = 2,
+        .i2c_init  = romulus_bmc_i2c_init,
     },
     [WITHERSPOON_BMC]  = {
         .soc_name  = "ast2500-a1",
@@ -315,6 +317,10 @@ static void ast2500_evb_i2c_init(AspeedBoardState *bmc)
 
     /* The AST2500 EVB expects a LM75 but a TMP105 is compatible */
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 7), "tmp105", 0x4d);
+
+    /* The AST2500 EVB does not have an RTC. Let's pretend that one is
+     * plugged on the I2C bus header */
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "ds1338", 0x32);
 }
 
 static void ast2500_evb_init(MachineState *machine)
@@ -341,6 +347,15 @@ static const TypeInfo ast2500_evb_type = {
     .class_init = ast2500_evb_class_init,
 };
 
+static void romulus_bmc_i2c_init(AspeedBoardState *bmc)
+{
+    AspeedSoCState *soc = &bmc->soc;
+
+    /* The romulus board expects Epson RX8900 I2C RTC but a ds1338 is
+     * good enough */
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "ds1338", 0x32);
+}
+
 static void romulus_bmc_init(MachineState *machine)
 {
     aspeed_board_init(machine, &aspeed_boards[ROMULUS_BMC]);
@@ -374,6 +389,10 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
 
     /* The Witherspoon expects a TMP275 but a TMP105 is compatible */
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a);
+
+    /* The witherspoon board expects Epson RX8900 I2C RTC but a ds1338 is
+     * good enough */
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "ds1338", 0x32);
 }
 
 static void witherspoon_bmc_init(MachineState *machine)
-- 
2.13.6

[Qemu-devel] [PATCH v5 5/8] smbus: add a smbus_eeprom_init_one() routine

[Cédric Le Goater]

This is an helper routine to add a single EEPROM on an I2C bus. It can
be directly used by smbus_eeprom_init() which adds a certain number of
EEPROMs on mips and x86 machines.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/i2c/smbus_eeprom.c  | 16 +++++++++++-----
 include/hw/i2c/smbus.h |  1 +
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
index b13ec0fe7a2a..2d24a4cd59bf 100644
--- a/hw/i2c/smbus_eeprom.c
+++ b/hw/i2c/smbus_eeprom.c
@@ -140,6 +140,16 @@ static void smbus_eeprom_register_types(void)
 
 type_init(smbus_eeprom_register_types)
 
+void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf)
+{
+    DeviceState *dev;
+
+    dev = qdev_create((BusState *) smbus, "smbus-eeprom");
+    qdev_prop_set_uint8(dev, "address", address);
+    qdev_prop_set_ptr(dev, "data", eeprom_buf);
+    qdev_init_nofail(dev);
+}
+
 void smbus_eeprom_init(I2CBus *smbus, int nb_eeprom,
                        const uint8_t *eeprom_spd, int eeprom_spd_size)
 {
@@ -150,10 +160,6 @@ void smbus_eeprom_init(I2CBus *smbus, int nb_eeprom,
     }
 
     for (i = 0; i < nb_eeprom; i++) {
-        DeviceState *eeprom;
-        eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");
-        qdev_prop_set_uint8(eeprom, "address", 0x50 + i);
-        qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256));
-        qdev_init_nofail(eeprom);
+        smbus_eeprom_init_one(smbus, 0x50 + i, eeprom_buf + (i * 256));
     }
 }
diff --git a/include/hw/i2c/smbus.h b/include/hw/i2c/smbus.h
index 544bbc19574f..666cdeb04c07 100644
--- a/include/hw/i2c/smbus.h
+++ b/include/hw/i2c/smbus.h
@@ -77,6 +77,7 @@ int smbus_read_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data);
 int smbus_write_block(I2CBus *bus, uint8_t addr, uint8_t command, uint8_t *data,
                       int 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);
 
-- 
2.13.6

[Qemu-devel] [PATCH v5 6/8] aspeed: Add EEPROM I2C devices

[Cédric Le Goater]

The Aspeed boards have at least one EEPROM to hold the Vital Product
Data (VPD).

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
 hw/arm/aspeed.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index f0a440f81fd6..246bf3ff67e6 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -17,6 +17,7 @@
 #include "hw/arm/arm.h"
 #include "hw/arm/aspeed_soc.h"
 #include "hw/boards.h"
+#include "hw/i2c/smbus.h"
 #include "qemu/log.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
@@ -273,11 +274,15 @@ static void palmetto_bmc_i2c_init(AspeedBoardState *bmc)
 {
     AspeedSoCState *soc = &bmc->soc;
     DeviceState *dev;
+    uint8_t *eeprom_buf = g_malloc0(32 * 1024);
 
     /* The palmetto platform expects a ds3231 RTC but a ds1338 is
      * enough to provide basic RTC features. Alarms will be missing */
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 0), "ds1338", 0x68);
 
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 0), 0x50,
+                          eeprom_buf);
+
     /* add a TMP423 temperature sensor */
     dev = i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 2),
                            "tmp423", 0x4c);
@@ -314,6 +319,10 @@ static const TypeInfo palmetto_bmc_type = {
 static void ast2500_evb_i2c_init(AspeedBoardState *bmc)
 {
     AspeedSoCState *soc = &bmc->soc;
+    uint8_t *eeprom_buf = g_malloc0(8 * 1024);
+
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3), 0x50,
+                          eeprom_buf);
 
     /* The AST2500 EVB expects a LM75 but a TMP105 is compatible */
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 7), "tmp105", 0x4d);
@@ -383,6 +392,7 @@ static const TypeInfo romulus_bmc_type = {
 static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
 {
     AspeedSoCState *soc = &bmc->soc;
+    uint8_t *eeprom_buf = g_malloc0(8 * 1024);
 
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c);
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c);
@@ -393,6 +403,9 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
     /* The witherspoon board expects Epson RX8900 I2C RTC but a ds1338 is
      * good enough */
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "ds1338", 0x32);
+
+    smbus_eeprom_init_one(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), 0x51,
+                          eeprom_buf);
 }
 
 static void witherspoon_bmc_init(MachineState *machine)
-- 
2.13.6

[Qemu-devel] [PATCH v5 7/8] misc: add pca9552 LED blinker model

[Cédric Le Goater]

Specs are available here :

    https://www.nxp.com/docs/en/application-note/AN264.pdf

This is a simple model supporting the basic registers for led and GPIO
mode. The device also supports two blinking rates but not the model
yet.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 Changes since v3:

 - introduced auto-increment support
 - removed the buffer collecting bytes on the bus
 - improved reset
 - used extract32
 - added a unit test

 Changes since v2:

 - removed comments on the I2C buffer size, but kept the array. I did
   not want to rewrite the buffer handling

 default-configs/arm-softmmu.mak |   1 +
 hw/misc/Makefile.objs           |   1 +
 hw/misc/pca9552.c               | 259 ++++++++++++++++++++++++++++++++++++++++
 include/hw/misc/pca9552.h       |  33 +++++
 tests/Makefile.include          |   2 +
 tests/pca9552-test.c            | 131 ++++++++++++++++++++
 6 files changed, 427 insertions(+)
 create mode 100644 hw/misc/pca9552.c
 create mode 100644 include/hw/misc/pca9552.h
 create mode 100644 tests/pca9552-test.c

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 5059d134c816..d868d1095a6c 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -16,6 +16,7 @@ CONFIG_TSC2005=y
 CONFIG_LM832X=y
 CONFIG_TMP105=y
 CONFIG_TMP421=y
+CONFIG_PCA9552=y
 CONFIG_STELLARIS=y
 CONFIG_STELLARIS_INPUT=y
 CONFIG_STELLARIS_ENET=y
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index e8f0a02f35af..e4e22880dbbc 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -7,6 +7,7 @@ common-obj-$(CONFIG_SGA) += sga.o
 common-obj-$(CONFIG_ISA_TESTDEV) += pc-testdev.o
 common-obj-$(CONFIG_PCI_TESTDEV) += pci-testdev.o
 common-obj-$(CONFIG_EDU) += edu.o
+common-obj-$(CONFIG_PCA9552) += pca9552.o
 
 common-obj-y += unimp.o
 
diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
new file mode 100644
index 000000000000..70ce6f038da2
--- /dev/null
+++ b/hw/misc/pca9552.c
@@ -0,0 +1,259 @@
+/*
+ * PCA9552 I2C LED blinker
+ *
+ *     https://www.nxp.com/docs/en/application-note/AN264.pdf
+ *
+ * Copyright (c) 2017, IBM Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/hw.h"
+#include "hw/misc/pca9552.h"
+
+/*
+ * Bits [0:3] are used to address a specific register.
+ */
+#define PCA9552_INPUT0   0 /* read only input register 0 */
+#define PCA9552_INPUT1   1 /* read only input register 1  */
+#define PCA9552_PSC0     2 /* read/write frequency prescaler 0 */
+#define PCA9552_PWM0     3 /* read/write PWM register 0 */
+#define PCA9552_PSC1     4 /* read/write frequency prescaler 1 */
+#define PCA9552_PWM1     5 /* read/write PWM register 1 */
+#define PCA9552_LS0      6 /* read/write LED0 to LED3 selector */
+#define PCA9552_LS1      7 /* read/write LED4 to LED7 selector */
+#define PCA9552_LS2      8 /* read/write LED8 to LED11 selector */
+#define PCA9552_LS3      9 /* read/write LED12 to LED15 selector */
+
+/*
+ * Bit [4] is used to activate the Auto-Increment option of the
+ * register address
+ */
+#define PCA9552_AUTOINC  (1 << 4)
+
+#define PCA9552_LED_ON   0x0
+#define PCA9552_LED_OFF  0x1
+#define PCA9552_LED_PWM0 0x2
+#define PCA9552_LED_PWM1 0x3
+
+static uint8_t pca9552_pin_get_config(PCA9552State *s, int pin)
+{
+    uint8_t reg   = PCA9552_LS0 + (pin / 4);
+    uint8_t shift = (pin % 4) << 1;
+
+    return extract32(s->regs[reg], shift, 2);
+}
+
+static void pca9552_update_pin_input(PCA9552State *s)
+{
+    int i;
+
+    for (i = 0; i < s->nr_leds; i++) {
+        uint8_t input_reg = PCA9552_INPUT0 + (i / 8);
+        uint8_t input_shift = (i % 8);
+        uint8_t config = pca9552_pin_get_config(s, i);
+
+        switch (config) {
+        case PCA9552_LED_ON:
+            s->regs[input_reg] |= 1 << input_shift;
+            break;
+        case PCA9552_LED_OFF:
+            s->regs[input_reg] &= ~(1 << input_shift);
+            break;
+        case PCA9552_LED_PWM0:
+        case PCA9552_LED_PWM1:
+            /* TODO */
+        default:
+            break;
+        }
+    }
+}
+
+static uint8_t pca9552_read(PCA9552State *s, uint8_t reg)
+{
+    switch (reg) {
+    case PCA9552_INPUT0:
+    case PCA9552_INPUT1:
+    case PCA9552_PSC0:
+    case PCA9552_PWM0:
+    case PCA9552_PSC1:
+    case PCA9552_PWM1:
+    case PCA9552_LS0:
+    case PCA9552_LS1:
+    case PCA9552_LS2:
+    case PCA9552_LS3:
+        return s->regs[reg];
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected read to register %d\n",
+                      __func__, reg);
+        return 0xFF;
+    }
+}
+
+static void pca9552_write(PCA9552State *s, uint8_t reg, uint8_t data)
+{
+    switch (reg) {
+    case PCA9552_PSC0:
+    case PCA9552_PWM0:
+    case PCA9552_PSC1:
+    case PCA9552_PWM1:
+        s->regs[reg] = data;
+        break;
+
+    case PCA9552_LS0:
+    case PCA9552_LS1:
+    case PCA9552_LS2:
+    case PCA9552_LS3:
+        s->regs[reg] = data;
+        pca9552_update_pin_input(s);
+        break;
+
+    case PCA9552_INPUT0:
+    case PCA9552_INPUT1:
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected write to register %d\n",
+                      __func__, reg);
+    }
+}
+
+/*
+ * When Auto-Increment is on, the register address is incremented
+ * after each byte is sent to or received by the device. The index
+ * rollovers to 0 when the maximum register address is reached.
+ */
+static void pca9552_autoinc(PCA9552State *s)
+{
+    if (s->pointer != 0xFF && s->pointer & PCA9552_AUTOINC) {
+        uint8_t reg = s->pointer & 0xf;
+
+        reg = (reg + 1) % (s->max_reg + 1);
+        s->pointer = reg | PCA9552_AUTOINC;
+    }
+}
+
+static int pca9552_recv(I2CSlave *i2c)
+{
+    PCA9552State *s = PCA9552(i2c);
+    uint8_t ret;
+
+    ret = pca9552_read(s, s->pointer & 0xf);
+
+    /*
+     * From the Specs:
+     *
+     *     Important Note: When a Read sequence is initiated and the
+     *     AI bit is set to Logic Level 1, the Read Sequence MUST
+     *     start by a register different from 0.
+     *
+     * I don't know what should be done in this case, so throw an
+     * error.
+     */
+    if (s->pointer == PCA9552_AUTOINC) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Autoincrement read starting with register 0\n",
+                      __func__);
+    }
+
+    pca9552_autoinc(s);
+
+    return ret;
+}
+
+static int pca9552_send(I2CSlave *i2c, uint8_t data)
+{
+    PCA9552State *s = PCA9552(i2c);
+
+    /* First byte sent by is the register address */
+    if (s->len == 0) {
+        s->pointer = data;
+        s->len++;
+    } else {
+        pca9552_write(s, s->pointer & 0xf, data);
+
+        pca9552_autoinc(s);
+    }
+
+    return 0;
+}
+
+static int pca9552_event(I2CSlave *i2c, enum i2c_event event)
+{
+    PCA9552State *s = PCA9552(i2c);
+
+    s->len = 0;
+    return 0;
+}
+
+static const VMStateDescription pca9552_vmstate = {
+    .name = "PCA9552",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT8(len, PCA9552State),
+        VMSTATE_UINT8(pointer, PCA9552State),
+        VMSTATE_UINT8_ARRAY(regs, PCA9552State, PCA9552_NR_REGS),
+        VMSTATE_I2C_SLAVE(i2c, PCA9552State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void pca9552_reset(DeviceState *dev)
+{
+    PCA9552State *s = PCA9552(dev);
+
+    s->regs[PCA9552_PSC0] = 0xFF;
+    s->regs[PCA9552_PWM0] = 0x80;
+    s->regs[PCA9552_PSC1] = 0xFF;
+    s->regs[PCA9552_PWM1] = 0x80;
+    s->regs[PCA9552_LS0] = 0x55; /* all OFF */
+    s->regs[PCA9552_LS1] = 0x55;
+    s->regs[PCA9552_LS2] = 0x55;
+    s->regs[PCA9552_LS3] = 0x55;
+
+    pca9552_update_pin_input(s);
+
+    s->pointer = 0xFF;
+    s->len = 0;
+}
+
+static void pca9552_initfn(Object *obj)
+{
+    PCA9552State *s = PCA9552(obj);
+
+    /* If support for the other PCA955X devices are implemented, these
+     * constant values might be part of class structure describing the
+     * PCA955X device
+     */
+    s->max_reg = PCA9552_LS3;
+    s->nr_leds = 16;
+}
+
+static void pca9552_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    I2CSlaveClass *k = I2C_SLAVE_CLASS(klass);
+
+    k->event = pca9552_event;
+    k->recv = pca9552_recv;
+    k->send = pca9552_send;
+    dc->reset = pca9552_reset;
+    dc->vmsd = &pca9552_vmstate;
+}
+
+static const TypeInfo pca9552_info = {
+    .name          = TYPE_PCA9552,
+    .parent        = TYPE_I2C_SLAVE,
+    .instance_init = pca9552_initfn,
+    .instance_size = sizeof(PCA9552State),
+    .class_init    = pca9552_class_init,
+};
+
+static void pca9552_register_types(void)
+{
+    type_register_static(&pca9552_info);
+}
+
+type_init(pca9552_register_types)
diff --git a/include/hw/misc/pca9552.h b/include/hw/misc/pca9552.h
new file mode 100644
index 000000000000..5df8d6407ca7
--- /dev/null
+++ b/include/hw/misc/pca9552.h
@@ -0,0 +1,33 @@
+/*
+ * PCA9552 I2C LED blinker
+ *
+ * Copyright (c) 2017, IBM Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+#ifndef PCA9552_H
+#define PCA9552_H
+
+#include "hw/i2c/i2c.h"
+
+#define TYPE_PCA9552 "pca9552"
+#define PCA9552(obj) OBJECT_CHECK(PCA9552State, (obj), TYPE_PCA9552)
+
+
+#define PCA9552_NR_REGS 10
+
+typedef struct PCA9552State {
+    /*< private >*/
+    I2CSlave i2c;
+    /*< public >*/
+
+    uint8_t len;
+    uint8_t pointer;
+
+    uint8_t regs[PCA9552_NR_REGS];
+    uint8_t max_reg;
+    uint8_t nr_leds;
+} PCA9552State;
+
+#endif
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 4ca15e681703..8ac2595c8512 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -347,6 +347,7 @@ check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
 check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
 
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
+check-qtest-arm-y += tests/pca9552-test$(EXESUF)
 check-qtest-arm-y += tests/ds1338-test$(EXESUF)
 check-qtest-arm-y += tests/m25p80-test$(EXESUF)
 gcov-files-arm-y += hw/misc/tmp105.c
@@ -739,6 +740,7 @@ tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
 	tests/boot-sector.o tests/acpi-utils.o $(libqos-obj-y)
 tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
 tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
+tests/pca9552-test$(EXESUF): tests/pca9552-test.o $(libqos-omap-obj-y)
 tests/ds1338-test$(EXESUF): tests/ds1338-test.o $(libqos-imx-obj-y)
 tests/m25p80-test$(EXESUF): tests/m25p80-test.o
 tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
diff --git a/tests/pca9552-test.c b/tests/pca9552-test.c
new file mode 100644
index 000000000000..53414452a12e
--- /dev/null
+++ b/tests/pca9552-test.c
@@ -0,0 +1,131 @@
+/*
+ * QTest testcase for the PCA9552 LED blinker
+ *
+ * Copyright (c) 2017, IBM Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "libqtest.h"
+#include "libqos/i2c.h"
+
+#define PCA9552_INPUT0   0 /* read only input register 0 */
+#define PCA9552_INPUT1   1 /* read only input register 1  */
+#define PCA9552_PSC0     2 /* read/write frequency prescaler 0 */
+#define PCA9552_PWM0     3 /* read/write PWM register 0 */
+#define PCA9552_PSC1     4 /* read/write frequency prescaler 1 */
+#define PCA9552_PWM1     5 /* read/write PWM register 1 */
+#define PCA9552_LS0      6 /* read/write LED0 to LED3 selector */
+#define PCA9552_LS1      7 /* read/write LED4 to LED7 selector */
+#define PCA9552_LS2      8 /* read/write LED8 to LED11 selector */
+#define PCA9552_LS3      9 /* read/write LED12 to LED15 selector */
+
+#define PCA9552_AUTOINC  (1 << 4)
+
+
+#define OMAP2_I2C_1_BASE 0x48070000
+
+#define PCA9552_TEST_ID   "pca9552-test"
+#define PCA9552_TEST_ADDR 0x60
+
+static I2CAdapter *i2c;
+
+static uint8_t pca9552_get8(I2CAdapter *i2c, uint8_t addr, uint8_t reg)
+{
+    uint8_t resp[1];
+    i2c_send(i2c, addr, &reg, 1);
+    i2c_recv(i2c, addr, resp, 1);
+    return resp[0];
+}
+
+static void pca9552_set8(I2CAdapter *i2c, uint8_t addr, uint8_t reg,
+                         uint8_t value)
+{
+    uint8_t cmd[2];
+    uint8_t resp[1];
+
+    cmd[0] = reg;
+    cmd[1] = value;
+    i2c_send(i2c, addr, cmd, 2);
+    i2c_recv(i2c, addr, resp, 1);
+    g_assert_cmphex(resp[0], ==, cmd[1]);
+}
+
+static void receive_autoinc(void)
+{
+    uint8_t resp;
+    uint8_t reg = PCA9552_LS0 | PCA9552_AUTOINC;
+
+    i2c_send(i2c, PCA9552_TEST_ADDR, &reg, 1);
+
+    /* PCA9552_LS0 */
+    i2c_recv(i2c, PCA9552_TEST_ADDR, &resp, 1);
+    g_assert_cmphex(resp, ==, 0x54);
+
+    /* PCA9552_LS1 */
+    i2c_recv(i2c, PCA9552_TEST_ADDR, &resp, 1);
+    g_assert_cmphex(resp, ==, 0x55);
+
+    /* PCA9552_LS2 */
+    i2c_recv(i2c, PCA9552_TEST_ADDR, &resp, 1);
+    g_assert_cmphex(resp, ==, 0x55);
+
+    /* PCA9552_LS3 */
+    i2c_recv(i2c, PCA9552_TEST_ADDR, &resp, 1);
+    g_assert_cmphex(resp, ==, 0x54);
+}
+
+static void send_and_receive(void)
+{
+    uint8_t value;
+
+    value = pca9552_get8(i2c, PCA9552_TEST_ADDR, PCA9552_LS0);
+    g_assert_cmphex(value, ==, 0x55);
+
+    value = pca9552_get8(i2c, PCA9552_TEST_ADDR, PCA9552_INPUT0);
+    g_assert_cmphex(value, ==, 0x0);
+
+    /* Switch on LED 0 */
+    pca9552_set8(i2c, PCA9552_TEST_ADDR, PCA9552_LS0, 0x54);
+    value = pca9552_get8(i2c, PCA9552_TEST_ADDR, PCA9552_LS0);
+    g_assert_cmphex(value, ==, 0x54);
+
+    value = pca9552_get8(i2c, PCA9552_TEST_ADDR, PCA9552_INPUT0);
+    g_assert_cmphex(value, ==, 0x01);
+
+    /* Switch on LED 12 */
+    pca9552_set8(i2c, PCA9552_TEST_ADDR, PCA9552_LS3, 0x54);
+    value = pca9552_get8(i2c, PCA9552_TEST_ADDR, PCA9552_LS3);
+    g_assert_cmphex(value, ==, 0x54);
+
+    value = pca9552_get8(i2c, PCA9552_TEST_ADDR, PCA9552_INPUT1);
+    g_assert_cmphex(value, ==, 0x10);
+}
+
+int main(int argc, char **argv)
+{
+    QTestState *s = NULL;
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    s = qtest_start("-machine n800 "
+                    "-device pca9552,bus=i2c-bus.0,id=" PCA9552_TEST_ID
+                    ",address=0x60");
+    i2c = omap_i2c_create(OMAP2_I2C_1_BASE);
+
+    qtest_add_func("/pca9552/tx-rx", send_and_receive);
+    qtest_add_func("/pca9552/rx-autoinc", receive_autoinc);
+
+    ret = g_test_run();
+
+    if (s) {
+        qtest_quit(s);
+    }
+    g_free(i2c);
+
+    return ret;
+}
-- 
2.13.6

Re: [Qemu-devel] [PATCH v5 7/8] misc: add pca9552 LED blinker model

[Peter Maydell]

On 19 October 2017 at 16:12, Cédric Le Goater <clg@kaod.org> wrote:
> Specs are available here :
>
>     https://www.nxp.com/docs/en/application-note/AN264.pdf
>
> This is a simple model supporting the basic registers for led and GPIO
> mode. The device also supports two blinking rates but not the model
> yet.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
>  Changes since v3:
>
>  - introduced auto-increment support
>  - removed the buffer collecting bytes on the bus
>  - improved reset
>  - used extract32
>  - added a unit test
>
>  Changes since v2:
>
>  - removed comments on the I2C buffer size, but kept the array. I did
>    not want to rewrite the buffer handling
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[Qemu-devel] [PATCH v5 8/8] aspeed: add the pc9552 chips to the witherspoon machine

[Cédric Le Goater]

The pca9552 LED blinkers on the Witherspoon machine are used for leds
but also as GPIOs to control fans and GPUs.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
---
 hw/arm/aspeed.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 246bf3ff67e6..f06ce68624c7 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -394,6 +394,8 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
     AspeedSoCState *soc = &bmc->soc;
     uint8_t *eeprom_buf = g_malloc0(8 * 1024);
 
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 3), "pca9552", 0x60);
+
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 4), "tmp423", 0x4c);
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c);
 
@@ -406,6 +408,8 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
 
     smbus_eeprom_init_one(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), 0x51,
                           eeprom_buf);
+    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "pca9552",
+                     0x60);
 }
 
 static void witherspoon_bmc_init(MachineState *machine)
-- 
2.13.6

Re: [Qemu-devel] [PATCH v5 0/8] aspeed: add a witherspoon-bmc machine

[Cédric Le Goater]

On 10/19/2017 05:12 PM, Cédric Le Goater wrote:
> Hello,
> 
> This series adds a new Aspeed machine to emulate the BMC of a
> Witherspoon system. It also extends the other Aspeed machines with I2C
> devices and adds a simple model for the pca9552 LED blinker present on
> the witherspoon board.
> 
> Thanks,
> 
> C.
> 
> Changes since v4:
> 
>  - use a ROM memory region
> 
> Changes since v3:
> 
>  - introduce a dummy ROM device to catch invalid writes
>  - removed 'ignore_memory_transaction_failures' on all Aspeed machines

So that was a bad idea because old firwmares run load/store loops
beyond the address space to guess how much RAM the SoC has.  

I am wondering if it is possible to model the same behavior with
a MMIO region (of a correct size depending on the max ram size of 
the SoC) and mapped beyond the RAM region to catch such accesses 
and keep 'ignore_memory_transaction_failures' to false. 
  
Thanks,

C.


>  - reworked PCA9552 device :
>    . simpler I2C handles
>    . auto-increment support
>    . unit test
> 
> Changes since v2:
> 
>  - removed comments on the I2C buffer size pf the PCA9552 device.
>  - removed 'ignore_memory_transaction_failures' flag on the
>    witherspoon machine.
>    
> Changes since v1:
> 
>  - introduced smbus_eeprom_init_one()
> 
> Cédric Le Goater (8):
>   aspeed: use a ROM memory region to catch invalid writes
>   aspeed: remove ignore_memory_transaction_failures all boards
>   aspeed: add support for the witherspoon-bmc board
>   aspeed: add an I2C RTC device to all machines
>   smbus: add a smbus_eeprom_init_one() routine
>   aspeed: Add EEPROM I2C devices
>   misc: add pca9552 LED blinker model
>   aspeed: add the pc9552 chips to the witherspoon machine
> 
>  default-configs/arm-softmmu.mak |   1 +
>  hw/arm/aspeed.c                 | 110 ++++++++++++++++-
>  hw/i2c/smbus_eeprom.c           |  16 ++-
>  hw/misc/Makefile.objs           |   1 +
>  hw/misc/pca9552.c               | 259 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/i2c/smbus.h          |   1 +
>  include/hw/misc/pca9552.h       |  33 +++++
>  tests/Makefile.include          |   2 +
>  tests/pca9552-test.c            | 131 ++++++++++++++++++++
>  9 files changed, 544 insertions(+), 10 deletions(-)
>  create mode 100644 hw/misc/pca9552.c
>  create mode 100644 include/hw/misc/pca9552.h
>  create mode 100644 tests/pca9552-test.c
> 

Re: [Qemu-devel] [PATCH v5 0/8] aspeed: add a witherspoon-bmc machine

[Peter Maydell]

On 29 November 2017 at 09:58, Cédric Le Goater <clg@kaod.org> wrote:
> On 10/19/2017 05:12 PM, Cédric Le Goater wrote:
>>  - introduce a dummy ROM device to catch invalid writes
>>  - removed 'ignore_memory_transaction_failures' on all Aspeed machines
>
> So that was a bad idea because old firwmares run load/store loops
> beyond the address space to guess how much RAM the SoC has.
>
> I am wondering if it is possible to model the same behavior with
> a MMIO region (of a correct size depending on the max ram size of
> the SoC) and mapped beyond the RAM region to catch such accesses
> and keep 'ignore_memory_transaction_failures' to false.

Yeah, if the hardware genuinely ignores accesses to a particular
region of the address space then you can model this with an
appropriate background memory region. You just need to figure
out what range of the address space it should apply to...

thanks
-- PMM