[Qemu-devel] [PATCH v2 0/6] aspeed: add a witherspoon-bmc machine

[Qemu-devel] [PATCH v2 0/6] 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 v1:

 - introduced smbus_eeprom_init_one()

Cédric Le Goater (6):
  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                 |  85 ++++++++++++++++
 hw/i2c/smbus_eeprom.c           |  16 ++-
 hw/misc/Makefile.objs           |   1 +
 hw/misc/pca9552.c               | 212 ++++++++++++++++++++++++++++++++++++++++
 include/hw/i2c/smbus.h          |   1 +
 include/hw/misc/pca9552.h       |  32 ++++++
 7 files changed, 343 insertions(+), 5 deletions(-)
 create mode 100644 hw/misc/pca9552.c
 create mode 100644 include/hw/misc/pca9552.h

-- 
2.13.5

[Qemu-devel] [PATCH v2 1/6] 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>
---
 hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index ab895ad490af..81f522f711ae 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
@@ -337,11 +350,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);
+
+    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;
+    mc->ignore_memory_transaction_failures = true;
+}
+
+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.5

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Peter Maydell]

On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> wrote:
> 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>
> ---
>  hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index ab895ad490af..81f522f711ae 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
> @@ -337,11 +350,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);
> +
> +    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;
> +    mc->ignore_memory_transaction_failures = true;

Please don't set this flag for new board models, it is only
for our legacy existing ones. Instead implement any devices
that you need for guest code to boot (stub them out with
create_unimplemented_device() if you like).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Cédric Le Goater]

On 10/06/2017 05:10 PM, Peter Maydell wrote:
> On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> wrote:
>> 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>
>> ---
>>  hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index ab895ad490af..81f522f711ae 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
>> @@ -337,11 +350,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);
>> +
>> +    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;
>> +    mc->ignore_memory_transaction_failures = true;
> 
> Please don't set this flag for new board models, it is only
> for our legacy existing ones. Instead implement any devices
> that you need for guest code to boot (stub them out with
> create_unimplemented_device() if you like).

OK. I am discovering this. I will take a look for the next
round.

C. 

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Cédric Le Goater]

On 10/06/2017 05:10 PM, Peter Maydell wrote:
> On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> wrote:
>> 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>
>> ---
>>  hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index ab895ad490af..81f522f711ae 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
>> @@ -337,11 +350,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);
>> +
>> +    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;
>> +    mc->ignore_memory_transaction_failures = true;
> 
> Please don't set this flag for new board models, it is only
> for our legacy existing ones. Instead implement any devices
> that you need for guest code to boot (stub them out with
> create_unimplemented_device() if you like).

I have dug into this a little more and it seems that it is 
required for the Aspeed bootloader (a modified u-boot) which 
uses static variables in early init phases. So legacy firmwares 
won't work in QEMU but will on real HW. 

It's fine with me but what is the goal of the approach ? Force 
SoC providers into fixing their FW when they use QEMU ?

Thanks,

C. 

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Peter Maydell]

On 10 October 2017 at 10:19, Cédric Le Goater <clg@kaod.org> wrote:
> On 10/06/2017 05:10 PM, Peter Maydell wrote:
>>> +    mc->ignore_memory_transaction_failures = true;
>>
>> Please don't set this flag for new board models, it is only
>> for our legacy existing ones. Instead implement any devices
>> that you need for guest code to boot (stub them out with
>> create_unimplemented_device() if you like).
>
> I have dug into this a little more and it seems that it is
> required for the Aspeed bootloader (a modified u-boot) which
> uses static variables in early init phases. So legacy firmwares
> won't work in QEMU but will on real HW.
>
> It's fine with me but what is the goal of the approach ? Force
> SoC providers into fixing their FW when they use QEMU ?

The goal is to model hardware correctly. Hardware gives
aborts if you touch a physical address with no device there,
and so QEMU's model should do the same. If you have guest
code that touches a physical address and blows up because
of an abort (but doesn't when run on h/w) then either:
 * it is trying to probe a device that exists in real h/w:
   you need to provide a stub implementation in QEMU
 * the SoC's bus fabric really doesn't pass aborts back
   to the CPU; I think this is unlikely, but you can model
   it at the SoC level with a suitable default memory region

The purpose of the flag is purely for existing board models,
where it is impossible to enable the correct (abort)
behaviour without possibly breaking guest code images that
work for people using released QEMU code. On a new board
model we don't have that problem and we can get it right
from the start. If we don't get it right from the start
then we will never have a chance to fix it in future.

Our mismodelling of this (not turning accesses to invalid
addresses into CPU aborts) meant that in the past it was
possible to be lazy when implementing a board model and
just not model half the hardware at all. Now it isn't,
but I don't think that adding a set of calls to
create_unimplemented_device() is a significant imposition.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Cédric Le Goater]

On 10/10/2017 11:54 AM, Peter Maydell wrote:
> On 10 October 2017 at 10:19, Cédric Le Goater <clg@kaod.org> wrote:
>> On 10/06/2017 05:10 PM, Peter Maydell wrote:
>>>> +    mc->ignore_memory_transaction_failures = true;
>>>
>>> Please don't set this flag for new board models, it is only
>>> for our legacy existing ones. Instead implement any devices
>>> that you need for guest code to boot (stub them out with
>>> create_unimplemented_device() if you like).
>>
>> I have dug into this a little more and it seems that it is
>> required for the Aspeed bootloader (a modified u-boot) which
>> uses static variables in early init phases. So legacy firmwares
>> won't work in QEMU but will on real HW.
>>
>> It's fine with me but what is the goal of the approach ? Force
>> SoC providers into fixing their FW when they use QEMU ?
> 
> The goal is to model hardware correctly. Hardware gives
> aborts if you touch a physical address with no device there,
> and so QEMU's model should do the same. If you have guest
> code that touches a physical address and blows up because
> of an abort (but doesn't when run on h/w) then either:
>  * it is trying to probe a device that exists in real h/w:
>    you need to provide a stub implementation in QEMU
>  * the SoC's bus fabric really doesn't pass aborts back
>    to the CPU; I think this is unlikely, but you can model
>    it at the SoC level with a suitable default memory region

well, that is case it seems. 

Anyhow, I found the required fixes in u-boot, so I will go with 
ignore_memory_transaction_failures=false with this new machine.

Thanks,

C. 
 
> The purpose of the flag is purely for existing board models,
> where it is impossible to enable the correct (abort)
> behaviour without possibly breaking guest code images that
> work for people using released QEMU code. On a new board
> model we don't have that problem and we can get it right
> from the start. If we don't get it right from the start
> then we will never have a chance to fix it in future.
> 
> Our mismodelling of this (not turning accesses to invalid
> addresses into CPU aborts) meant that in the past it was
> possible to be lazy when implementing a board model and
> just not model half the hardware at all. Now it isn't,
> but I don't think that adding a set of calls to
> create_unimplemented_device() is a significant imposition.
> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Peter Maydell]

On 10 October 2017 at 14:21, Cédric Le Goater <clg@kaod.org> wrote:
> On 10/10/2017 11:54 AM, Peter Maydell wrote:
>> The goal is to model hardware correctly. Hardware gives
>> aborts if you touch a physical address with no device there,
>> and so QEMU's model should do the same. If you have guest
>> code that touches a physical address and blows up because
>> of an abort (but doesn't when run on h/w) then either:
>>  * it is trying to probe a device that exists in real h/w:
>>    you need to provide a stub implementation in QEMU
>>  * the SoC's bus fabric really doesn't pass aborts back
>>    to the CPU; I think this is unlikely, but you can model
>>    it at the SoC level with a suitable default memory region
>
> well, that is case it seems.

If it is, then we should model the SoC that way, ie find
out from the hardware docs what part of the bus fabric
ignores decode errors and use memory regions with the
right default behaviour to cover the relevant address
ranges.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Cédric Le Goater]

On 10/10/2017 03:24 PM, Peter Maydell wrote:
> On 10 October 2017 at 14:21, Cédric Le Goater <clg@kaod.org> wrote:
>> On 10/10/2017 11:54 AM, Peter Maydell wrote:
>>> The goal is to model hardware correctly. Hardware gives
>>> aborts if you touch a physical address with no device there,
>>> and so QEMU's model should do the same. If you have guest
>>> code that touches a physical address and blows up because
>>> of an abort (but doesn't when run on h/w) then either:
>>>  * it is trying to probe a device that exists in real h/w:
>>>    you need to provide a stub implementation in QEMU
>>>  * the SoC's bus fabric really doesn't pass aborts back
>>>    to the CPU; I think this is unlikely, but you can model
>>>    it at the SoC level with a suitable default memory region
>>
>> well, that is case it seems.
> 
> If it is, then we should model the SoC that way, ie find
> out from the hardware docs what part of the bus fabric
> ignores decode errors and use memory regions with the
> right default behaviour to cover the relevant address
> ranges.

The addresses generating memory fault errors are all in 
the region where the BMC SPI Flash Memory is mapped : 
[ 20000000-2FFFFFFF ]

but we should not be doing any writes there. I will make
some inquiries.

Thanks,

C. 

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Peter Maydell]

On 10 October 2017 at 16:38, Cédric Le Goater <clg@kaod.org> wrote:
> On 10/10/2017 03:24 PM, Peter Maydell wrote:
>> On 10 October 2017 at 14:21, Cédric Le Goater <clg@kaod.org> wrote:
>>> On 10/10/2017 11:54 AM, Peter Maydell wrote:
>>>> The goal is to model hardware correctly. Hardware gives
>>>> aborts if you touch a physical address with no device there,
>>>> and so QEMU's model should do the same. If you have guest
>>>> code that touches a physical address and blows up because
>>>> of an abort (but doesn't when run on h/w) then either:
>>>>  * it is trying to probe a device that exists in real h/w:
>>>>    you need to provide a stub implementation in QEMU
>>>>  * the SoC's bus fabric really doesn't pass aborts back
>>>>    to the CPU; I think this is unlikely, but you can model
>>>>    it at the SoC level with a suitable default memory region
>>>
>>> well, that is case it seems.
>>
>> If it is, then we should model the SoC that way, ie find
>> out from the hardware docs what part of the bus fabric
>> ignores decode errors and use memory regions with the
>> right default behaviour to cover the relevant address
>> ranges.
>
> The addresses generating memory fault errors are all in
> the region where the BMC SPI Flash Memory is mapped :
> [ 20000000-2FFFFFFF ]

If there's an actual flash device there then this sounds
like my first case above, where we just need to stub out
that range of addresses until we get round to really
implementing the flash device.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Cédric Le Goater]

On 10/10/2017 05:45 PM, Peter Maydell wrote:
> On 10 October 2017 at 16:38, Cédric Le Goater <clg@kaod.org> wrote:
>> On 10/10/2017 03:24 PM, Peter Maydell wrote:
>>> On 10 October 2017 at 14:21, Cédric Le Goater <clg@kaod.org> wrote:
>>>> On 10/10/2017 11:54 AM, Peter Maydell wrote:
>>>>> The goal is to model hardware correctly. Hardware gives
>>>>> aborts if you touch a physical address with no device there,
>>>>> and so QEMU's model should do the same. If you have guest
>>>>> code that touches a physical address and blows up because
>>>>> of an abort (but doesn't when run on h/w) then either:
>>>>>  * it is trying to probe a device that exists in real h/w:
>>>>>    you need to provide a stub implementation in QEMU
>>>>>  * the SoC's bus fabric really doesn't pass aborts back
>>>>>    to the CPU; I think this is unlikely, but you can model
>>>>>    it at the SoC level with a suitable default memory region
>>>>
>>>> well, that is case it seems.
>>>
>>> If it is, then we should model the SoC that way, ie find
>>> out from the hardware docs what part of the bus fabric
>>> ignores decode errors and use memory regions with the
>>> right default behaviour to cover the relevant address
>>> ranges.
>>
>> The addresses generating memory fault errors are all in
>> the region where the BMC SPI Flash Memory is mapped :
>> [ 20000000-2FFFFFFF ]
> 
> If there's an actual flash device there then this sounds
> like my first case above, where we just need to stub out
> that range of addresses until we get round to really
> implementing the flash device.

but it is implemented ! and the region available.

C.

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 3849 bytes --]

On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
> 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>
> ---
>  hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index ab895ad490af..81f522f711ae 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
> @@ -337,11 +350,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);
> +
> +    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a);

Looks like I need to track down newer versions of the schematics I have.

> +}
> +
> +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;
> +    mc->ignore_memory_transaction_failures = true;

Aside from the issue with the above as pointed out by Peter,

Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

> +}
> +
> +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)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Cédric Le Goater]

On 10/09/2017 02:04 AM, Andrew Jeffery wrote:
> On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
>> 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>
>> ---
>>  hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>  
>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>> index ab895ad490af..81f522f711ae 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
>> @@ -337,11 +350,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);
>> +
>> +    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a);
> 
> Looks like I need to track down newer versions of the schematics I have.

the device on the board is a tmp275 but the tmp105 model is compatible. 


>> +}
>> +
>> +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;
>> +    mc->ignore_memory_transaction_failures = true;
> 
> Aside from the issue with the above as pointed out by Peter,
> 
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>

Thanks,

C. 

> 
>> +}
>> +
>> +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)

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Peter Maydell]

On 10 October 2017 at 14:30, Cédric Le Goater <clg@kaod.org> wrote:
> On 10/09/2017 02:04 AM, Andrew Jeffery wrote:
>> On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
>>> +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);
>>> +
>>> +    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a);
>>
>> Looks like I need to track down newer versions of the schematics I have.
>
> the device on the board is a tmp275 but the tmp105 model is compatible.

This kind of deviation from the real hardware is worth having
a comment to document, I think.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 3095 bytes --]

On Tue, 2017-10-10 at 15:30 +0200, Cédric Le Goater wrote:
> On 10/09/2017 02:04 AM, Andrew Jeffery wrote:
> > On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
> > > 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>
> > > ---
> > >  hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 49 insertions(+)
> > >  
> > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > > index ab895ad490af..81f522f711ae 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
> > > @@ -337,11 +350,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);
> > > +
> > > +    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a);
> > 
> > Looks like I need to track down newer versions of the schematics I have.
> 
> the device on the board is a tmp275 but the tmp105 model is compatible.

It neither device is listed in the version I have :)

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Cédric Le Goater]

On 10/11/2017 05:49 AM, Andrew Jeffery wrote:
> On Tue, 2017-10-10 at 15:30 +0200, Cédric Le Goater wrote:
>> On 10/09/2017 02:04 AM, Andrew Jeffery wrote:
>>> On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
>>>> 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>
>>>> ---
>>>>  hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 49 insertions(+)
>>>>  
>>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>>> index ab895ad490af..81f522f711ae 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
>>>> @@ -337,11 +350,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);
>>>> +
>>>> +    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a);
>>>
>>> Looks like I need to track down newer versions of the schematics I have.
>>
>> the device on the board is a tmp275 but the tmp105 model is compatible.
> 
> It neither device is listed in the version I have :)

Here is my source :

	https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts#L504

C. 

Re: [Qemu-devel] [PATCH v2 1/6] aspeed: add support for the witherspoon-bmc board

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 3731 bytes --]

On Wed, 2017-10-11 at 09:28 +0200, Cédric Le Goater wrote:
> On 10/11/2017 05:49 AM, Andrew Jeffery wrote:
> > On Tue, 2017-10-10 at 15:30 +0200, Cédric Le Goater wrote:
> > > On 10/09/2017 02:04 AM, Andrew Jeffery wrote:
> > > > On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
> > > > > 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>
> > > > > 
> > > > > ---
> > > > >  hw/arm/aspeed.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 49 insertions(+)
> > > > >  
> > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > > > > index ab895ad490af..81f522f711ae 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
> > > > > @@ -337,11 +350,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);
> > > > > +
> > > > > +    i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 9), "tmp105", 0x4a);
> > > > 
> > > > Looks like I need to track down newer versions of the schematics I have.
> > > 
> > > the device on the board is a tmp275 but the tmp105 model is compatible.
> > 
> > It neither device is listed in the version I have :)
> 
> Here is my source :
> 
> 	https://github.com/openbmc/linux/blob/dev-4.10/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts#L504
> 

Yeah, I ended up jumping on a machine and verifying the device was on
the bus.

Cheers,

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[Qemu-devel] [PATCH v2 2/6] 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>
---
 hw/arm/aspeed.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 81f522f711ae..362b683e9021 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",
@@ -298,6 +300,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)
@@ -325,6 +331,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]);
@@ -358,6 +373,10 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
     i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c);
 
     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.5

Re: [Qemu-devel] [PATCH v2 2/6] aspeed: add an I2C RTC device to all machines

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 3050 bytes --]

On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
> 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 81f522f711ae..362b683e9021 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",
> @@ -298,6 +300,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)
> @@ -325,6 +331,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]);
> @@ -358,6 +373,10 @@ static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
>      i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 5), "tmp423", 0x4c);
>  
>      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)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[Qemu-devel] [PATCH v2 3/6] 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>
---
 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.5

Re: [Qemu-devel] [PATCH v2 3/6] smbus: add a smbus_eeprom_init_one() routine

[Philippe Mathieu-Daudé]

On 09/20/2017 04:01 AM, Cédric Le Goater wrote:
> 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);
>  
> 

[Qemu-devel] [PATCH v2 4/6] 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>
---

Changes since v1:

 - fixed palmetto EEPROM size
 - used smbus_eeprom_init_one()

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

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 362b683e9021..462f8008cff5 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"
@@ -255,11 +256,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);
@@ -297,6 +302,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);
@@ -368,6 +377,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);
@@ -377,6 +387,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.5

Re: [Qemu-devel] [PATCH v2 4/6] aspeed: Add EEPROM I2C devices

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 3181 bytes --]

On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
> 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>

> ---
> 
> Changes since v1:
> 
>  - fixed palmetto EEPROM size
>  - used smbus_eeprom_init_one()
> 
> hw/arm/aspeed.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 362b683e9021..462f8008cff5 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"
> @@ -255,11 +256,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);
> @@ -297,6 +302,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);
> @@ -368,6 +377,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);
> @@ -377,6 +387,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)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[Qemu-devel] [PATCH v2 5/6] misc: add pca9552 LED blinker model

[Cédric Le Goater]

Specs are available here :

  https://www.nxp.com/docs/en/data-sheet/PCA9552.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>
---
 default-configs/arm-softmmu.mak |   1 +
 hw/misc/Makefile.objs           |   1 +
 hw/misc/pca9552.c               | 212 ++++++++++++++++++++++++++++++++++++++++
 include/hw/misc/pca9552.h       |  32 ++++++
 4 files changed, 246 insertions(+)
 create mode 100644 hw/misc/pca9552.c
 create mode 100644 include/hw/misc/pca9552.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index bbdd3c1d8b7f..17b8fc323f27 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 29fb922cefc6..33ba1d6642f9 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..22460f4c14fe
--- /dev/null
+++ b/hw/misc/pca9552.c
@@ -0,0 +1,212 @@
+/*
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/hw.h"
+#include "hw/misc/pca9552.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_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 (s->regs[reg] >> shift) & 0x3;
+}
+
+static void pca9552_update_pin_input(PCA9552State *s)
+{
+    int i;
+
+    for (i = 0; i < 16; 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:
+            /* ??? */
+        default:
+            break;
+        }
+    }
+}
+
+static void pca9552_read(PCA9552State *s)
+{
+    uint8_t reg = s->pointer & 0xf;
+
+    s->len = 0;
+
+    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:
+        s->buf[s->len++] = s->regs[reg];
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected read to register %d\n",
+                      __func__, reg);
+    }
+}
+
+static void pca9552_write(PCA9552State *s)
+{
+    uint8_t reg = s->pointer & 0xf;
+
+    switch (reg) {
+    case PCA9552_PSC0:
+    case PCA9552_PWM0:
+    case PCA9552_PSC1:
+    case PCA9552_PWM1:
+        s->regs[reg] = s->buf[0];
+        break;
+
+    case PCA9552_LS0:
+    case PCA9552_LS1:
+    case PCA9552_LS2:
+    case PCA9552_LS3:
+        s->regs[reg] = s->buf[0];
+        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);
+    }
+}
+
+static int pca9552_recv(I2CSlave *i2c)
+{
+    PCA9552State *s = PCA9552(i2c);
+
+    if (s->len < sizeof(s->buf)) {
+        return s->buf[s->len++];
+    } else {
+        return 0xff;
+    }
+}
+
+static int pca9552_send(I2CSlave *i2c, uint8_t data)
+{
+    PCA9552State *s = PCA9552(i2c);
+
+    if (s->len == 0) {
+        s->pointer = data;
+        s->len++;
+    } else {
+        if (s->len <= sizeof(s->buf)) {
+            s->buf[s->len - 1] = data;
+        }
+        s->len++;
+        pca9552_write(s);
+    }
+
+    return 0;
+}
+
+static int pca9552_event(I2CSlave *i2c, enum i2c_event event)
+{
+    PCA9552State *s = PCA9552(i2c);
+
+    if (event == I2C_START_RECV) {
+        pca9552_read(s);
+    }
+
+    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(buf, PCA9552State, 1),
+        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);
+}
+
+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_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..875467725526
--- /dev/null
+++ b/include/hw/misc/pca9552.h
@@ -0,0 +1,32 @@
+/*
+ * 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 buf[1]; /* just to remember how to handle a larger buffer */
+
+    uint8_t regs[PCA9552_NR_REGS];
+} PCA9552State;
+
+#endif
-- 
2.13.5

Re: [Qemu-devel] [PATCH v2 5/6] misc: add pca9552 LED blinker model

[Peter Maydell]

On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> wrote:
> Specs are available here :
>
>   https://www.nxp.com/docs/en/data-sheet/PCA9552.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>


> --- /dev/null
> +++ b/include/hw/misc/pca9552.h
> @@ -0,0 +1,32 @@
> +/*
> + * 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 buf[1]; /* just to remember how to handle a larger buffer */

Changing this later is going to be a migration compatibility break
(or at least a bit painful to keep compat).
Do we know how big the buffer is supposed to be? If so it would
be best to make it the correct size to start with.

> +
> +    uint8_t regs[PCA9552_NR_REGS];
> +} PCA9552State;

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 5/6] misc: add pca9552 LED blinker model

[Cédric Le Goater]

On 10/06/2017 05:12 PM, Peter Maydell wrote:
> On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> wrote:
>> Specs are available here :
>>
>>   https://www.nxp.com/docs/en/data-sheet/PCA9552.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>
> 
> 
>> --- /dev/null
>> +++ b/include/hw/misc/pca9552.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * 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 buf[1]; /* just to remember how to handle a larger buffer */
> 
> Changing this later is going to be a migration compatibility break
> (or at least a bit painful to keep compat).
> Do we know how big the buffer is supposed to be? 

one. 

> If so it would be best to make it the correct size to start with.

yes I will change that.

Thanks,

C.

> 
>> +
>> +    uint8_t regs[PCA9552_NR_REGS];
>> +} PCA9552State;
> 
> thanks
> -- PMM
> 

[Qemu-devel] [PATCH v2 6/6] 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>
---
 hw/arm/aspeed.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 462f8008cff5..2ae46d71bff7 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -379,6 +379,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);
 
@@ -390,6 +392,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.5

Re: [Qemu-devel] [PATCH v2 6/6] aspeed: add the pc9552 chips to the witherspoon machine

[Andrew Jeffery]

[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]

On Wed, 2017-09-20 at 09:01 +0200, Cédric Le Goater wrote:
> 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 462f8008cff5..2ae46d71bff7 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -379,6 +379,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);
>  
> @@ -390,6 +392,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)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

[Peter Maydell]

On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> 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 v1:
>
>  - introduced smbus_eeprom_init_one()
>
> Cédric Le Goater (6):
>   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

Hi -- I've made a couple of review comments and nothing else
leapt out as obviously wrong, but I'm hoping that somebody more
familiar with the aspeed devices will do review...

thanks
-- PMM

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

[Cédric Le Goater]

On 10/06/2017 05:13 PM, Peter Maydell wrote:
> On 20 September 2017 at 08:01, Cédric Le Goater <clg@kaod.org> 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 v1:
>>
>>  - introduced smbus_eeprom_init_one()
>>
>> Cédric Le Goater (6):
>>   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
> 
> Hi -- I've made a couple of review comments and nothing else
> leapt out as obviously wrong, but I'm hoping that somebody more
> familiar with the aspeed devices will do review...

Joel, 

Now that we have updated the DTs, could you please take a look 
to make sure that the I2C busses in the models have devices 
which makes sense with the real boards.

Thanks,

C.