All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] hw/arm/aspeed: allow missing spi_model
@ 2022-03-05  0:06 Patrick Williams
  2022-03-05  0:06 ` [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type Patrick Williams
  2022-03-05  7:53 ` [PATCH 1/2] hw/arm/aspeed: allow missing spi_model Cédric Le Goater
  0 siblings, 2 replies; 8+ messages in thread
From: Patrick Williams @ 2022-03-05  0:06 UTC (permalink / raw)
  Cc: Peter Maydell, Andrew Jeffery, open list:All patches CC here,
	Patrick Williams, open list:ASPEED BMCs, Joel Stanley,
	Cédric Le Goater

Generally all BMCs will use the fmc_model to hold their own flash
and most will have a spi_model to hold the managed system's flash,
but not all systems do.  Add a simple NULL check to allow a system
to set the spi_model as NULL to indicate it should not be instantiated.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
---
 hw/arm/aspeed.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 11558b327b..617a1ecbdc 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -276,7 +276,11 @@ static void aspeed_board_init_flashes(AspeedSMCState *s,
                                       const char *flashtype,
                                       int unit0)
 {
-    int i ;
+    int i;
+
+    if (!flashtype) {
+        return;
+    }
 
     for (i = 0; i < s->num_cs; ++i) {
         DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type
  2022-03-05  0:06 [PATCH 1/2] hw/arm/aspeed: allow missing spi_model Patrick Williams
@ 2022-03-05  0:06 ` Patrick Williams
  2022-03-05  7:57   ` Cédric Le Goater
  2022-03-05  7:53 ` [PATCH 1/2] hw/arm/aspeed: allow missing spi_model Cédric Le Goater
  1 sibling, 1 reply; 8+ messages in thread
From: Patrick Williams @ 2022-03-05  0:06 UTC (permalink / raw)
  Cc: Peter Maydell, Andrew Jeffery, open list:All patches CC here,
	Patrick Williams, open list:ASPEED BMCs, Joel Stanley,
	Cédric Le Goater

Add the 'bletchley-bmc' machine type based on the kernel DTS[1] and
hardware schematics available to me.  The i2c model is as complete as
the current QEMU models support, but in some cases I substituted devices
that are close enough for present functionality.  Strap registers are
kept the same as the AST2600-EVB until I'm able to confirm correct
values with physical hardware.

This has been tested with an openbmc image built from [2] plus a kernel
patch[3] for the SPI flash module.

1. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/aspeed-bmc-facebook-bletchley.dts?id=a8c729e966c4e9d033242d948b0e53c2a62d32e2
2. https://github.com/openbmc/openbmc/commit/b9432b980d7f63f7512ffbcc7124386ba896dfc6
3. https://github.com/openbmc/linux/commit/25b566b9a9d7f5d4f10c1b7304007bdb286eefd7

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
---
 hw/arm/aspeed.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 617a1ecbdc..3dc2ede823 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -167,6 +167,11 @@ struct AspeedMachineState {
 #define FUJI_BMC_HW_STRAP1    0x00000000
 #define FUJI_BMC_HW_STRAP2    0x00000000
 
+/* Bletchley hardware value */
+/* TODO: Leave same as EVB for now. */
+#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
+#define BLETCHLEY_BMC_HW_STRAP2 AST2600_EVB_HW_STRAP2
+
 /*
  * The max ram region is for firmwares that scan the address space
  * with load/store to guess how much RAM the SoC has.
@@ -901,6 +906,54 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
     }
 }
 
+#define TYPE_TMP421 "tmp421"
+
+static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
+{
+    AspeedSoCState *soc = &bmc->soc;
+    I2CBus *i2c[13] = {};
+    for (int i = 0; i < 13; i++) {
+        if ((i == 8) || (i == 11)) {
+            continue;
+        }
+        i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
+    }
+
+    /* Bus 0 - 5 all have the same config. */
+    for (int i = 0; i < 6; i++) {
+        /* Missing model: ti,ina230 @ 0x45 */
+        /* Missing model: mps,mp5023 @ 0x40 */
+        i2c_slave_create_simple(i2c[i], TYPE_TMP421, 0x4f);
+        /* Missing model: nxp,pca9539 @ 0x76, but PCA9552 works enough */
+        i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x76);
+        i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x67);
+        /* Missing model: fsc,fusb302 @ 0x22 */
+    }
+
+    /* Bus 6 */
+    at24c_eeprom_init(i2c[6], 0x56, 65536);
+    /* Missing model: nxp,pcf85263 @ 0x51 , but ds1338 works enough */
+    i2c_slave_create_simple(i2c[6], "ds1338", 0x51);
+
+
+    /* Bus 7 */
+    at24c_eeprom_init(i2c[7], 0x54, 65536);
+
+    /* Bus 9 */
+    i2c_slave_create_simple(i2c[9], TYPE_TMP421, 0x4f);
+
+    /* Bus 10 */
+    i2c_slave_create_simple(i2c[10], TYPE_TMP421, 0x4f);
+    /* Missing model: ti,hdc1080 @ 0x40 */
+    i2c_slave_create_simple(i2c[10], TYPE_PCA9552, 0x67);
+
+    /* Bus 12 */
+    /* Missing model: adi,adm1278 @ 0x11 */
+    i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4c);
+    i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4d);
+    i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
+}
+
 static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
 {
     return ASPEED_MACHINE(obj)->mmio_exec;
@@ -1224,6 +1277,26 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
         aspeed_soc_num_cpus(amc->soc_name);
 };
 
+
+static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
+
+    mc->desc       = "Facebook Bletchley BMC (Cortex-A7)";
+    amc->soc_name  = "ast2600-a3";
+    amc->hw_strap1 = BLETCHLEY_BMC_HW_STRAP1;
+    amc->hw_strap2 = BLETCHLEY_BMC_HW_STRAP2;
+    amc->fmc_model = "w25q01jvq";
+    amc->spi_model = NULL;
+    amc->num_cs    = 1;
+    amc->macs_mask = ASPEED_MAC2_ON;
+    amc->i2c_init  = bletchley_bmc_i2c_init;
+    mc->default_ram_size = 512 * MiB;
+    mc->default_cpus = mc->min_cpus = mc->max_cpus =
+        aspeed_soc_num_cpus(amc->soc_name);
+}
+
 static const TypeInfo aspeed_machine_types[] = {
     {
         .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
@@ -1277,6 +1350,10 @@ static const TypeInfo aspeed_machine_types[] = {
         .name          = MACHINE_TYPE_NAME("fuji-bmc"),
         .parent        = TYPE_ASPEED_MACHINE,
         .class_init    = aspeed_machine_fuji_class_init,
+    }, {
+        .name          = MACHINE_TYPE_NAME("bletchley-bmc"),
+        .parent        = TYPE_ASPEED_MACHINE,
+        .class_init    = aspeed_machine_bletchley_class_init,
     }, {
         .name          = TYPE_ASPEED_MACHINE,
         .parent        = TYPE_MACHINE,
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] hw/arm/aspeed: allow missing spi_model
  2022-03-05  0:06 [PATCH 1/2] hw/arm/aspeed: allow missing spi_model Patrick Williams
  2022-03-05  0:06 ` [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type Patrick Williams
@ 2022-03-05  7:53 ` Cédric Le Goater
  1 sibling, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2022-03-05  7:53 UTC (permalink / raw)
  To: Patrick Williams
  Cc: Andrew Jeffery, Peter Maydell, open list:ASPEED BMCs,
	Joel Stanley, open list:All patches CC here

On 3/5/22 01:06, Patrick Williams wrote:
> Generally all BMCs will use the fmc_model to hold their own flash
> and most will have a spi_model to hold the managed system's flash,
> but not all systems do.  Add a simple NULL check to allow a system
> to set the spi_model as NULL to indicate it should not be instantiated.


OK. Let's do it that way for now but we need to rework 'num_cs' in the
Aspeed SMC model and the Aspeed machines. We started with a simple
requirement (1 FMC and 1 SPI) but since, things have become more complex.

1. we should get rid of AspeedSMCState::num_cs and simply use
    AspeedSMCState::max_peripherals in the SMC model. There is no need to
    restrict the number devices of the controller to match the board layout.

2. aspeed_board_init_flashes() needs a better interface. May be, something
    like :

    static void aspeed_board_init_flashes(AspeedSMCState *s,
                                          const char **flashtype, int count,
                                          int unit0)

and change ->fmc_model and ->spi_model to be arrays. Ideas welcome.

Anyhow,

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

Thanks,

C.

> Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
> ---
>   hw/arm/aspeed.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 11558b327b..617a1ecbdc 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -276,7 +276,11 @@ static void aspeed_board_init_flashes(AspeedSMCState *s,
>                                         const char *flashtype,
>                                         int unit0)
>   {
> -    int i ;
> +    int i;
> +
> +    if (!flashtype) {
> +        return;
> +    }
>   
>       for (i = 0; i < s->num_cs; ++i) {
>           DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i);



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type
  2022-03-05  0:06 ` [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type Patrick Williams
@ 2022-03-05  7:57   ` Cédric Le Goater
  2022-03-07 20:59     ` Patrick Williams
  0 siblings, 1 reply; 8+ messages in thread
From: Cédric Le Goater @ 2022-03-05  7:57 UTC (permalink / raw)
  To: Patrick Williams
  Cc: Andrew Jeffery, Peter Maydell, open list:ASPEED BMCs,
	Joel Stanley, open list:All patches CC here

On 3/5/22 01:06, Patrick Williams wrote:
> Add the 'bletchley-bmc' machine type based on the kernel DTS[1] and
> hardware schematics available to me.  The i2c model is as complete as
> the current QEMU models support, but in some cases I substituted devices
> that are close enough for present functionality.  Strap registers are
> kept the same as the AST2600-EVB until I'm able to confirm correct
> values with physical hardware.
> 
> This has been tested with an openbmc image built from [2] plus a kernel
> patch[3] for the SPI flash module.
> 
> 1. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/aspeed-bmc-facebook-bletchley.dts?id=a8c729e966c4e9d033242d948b0e53c2a62d32e2
> 2. https://github.com/openbmc/openbmc/commit/b9432b980d7f63f7512ffbcc7124386ba896dfc6
> 3. https://github.com/openbmc/linux/commit/25b566b9a9d7f5d4f10c1b7304007bdb286eefd7
> 
> Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
> ---
>   hw/arm/aspeed.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 77 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 617a1ecbdc..3dc2ede823 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -167,6 +167,11 @@ struct AspeedMachineState {
>   #define FUJI_BMC_HW_STRAP1    0x00000000
>   #define FUJI_BMC_HW_STRAP2    0x00000000
>   
> +/* Bletchley hardware value */
> +/* TODO: Leave same as EVB for now. */
> +#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
> +#define BLETCHLEY_BMC_HW_STRAP2 AST2600_EVB_HW_STRAP2
> +
>   /*
>    * The max ram region is for firmwares that scan the address space
>    * with load/store to guess how much RAM the SoC has.
> @@ -901,6 +906,54 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
>       }
>   }
>   
> +#define TYPE_TMP421 "tmp421"
> +
> +static void bletchley_bmc_i2c_init(AspeedMachineState *bmc)
> +{
> +    AspeedSoCState *soc = &bmc->soc;
> +    I2CBus *i2c[13] = {};
> +    for (int i = 0; i < 13; i++) {
> +        if ((i == 8) || (i == 11)) {
> +            continue;
> +        }
> +        i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
> +    }
> +
> +    /* Bus 0 - 5 all have the same config. */
> +    for (int i = 0; i < 6; i++) {
> +        /* Missing model: ti,ina230 @ 0x45 */
> +        /* Missing model: mps,mp5023 @ 0x40 */
> +        i2c_slave_create_simple(i2c[i], TYPE_TMP421, 0x4f);
> +        /* Missing model: nxp,pca9539 @ 0x76, but PCA9552 works enough */
> +        i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x76);
> +        i2c_slave_create_simple(i2c[i], TYPE_PCA9552, 0x67);
> +        /* Missing model: fsc,fusb302 @ 0x22 */
> +    }
> +
> +    /* Bus 6 */
> +    at24c_eeprom_init(i2c[6], 0x56, 65536);
> +    /* Missing model: nxp,pcf85263 @ 0x51 , but ds1338 works enough */
> +    i2c_slave_create_simple(i2c[6], "ds1338", 0x51);
> +
> +
> +    /* Bus 7 */
> +    at24c_eeprom_init(i2c[7], 0x54, 65536);
> +
> +    /* Bus 9 */
> +    i2c_slave_create_simple(i2c[9], TYPE_TMP421, 0x4f);
> +
> +    /* Bus 10 */
> +    i2c_slave_create_simple(i2c[10], TYPE_TMP421, 0x4f);
> +    /* Missing model: ti,hdc1080 @ 0x40 */
> +    i2c_slave_create_simple(i2c[10], TYPE_PCA9552, 0x67);
> +
> +    /* Bus 12 */
> +    /* Missing model: adi,adm1278 @ 0x11 */
> +    i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4c);
> +    i2c_slave_create_simple(i2c[12], TYPE_TMP421, 0x4d);
> +    i2c_slave_create_simple(i2c[12], TYPE_PCA9552, 0x67);
> +}
> +
>   static bool aspeed_get_mmio_exec(Object *obj, Error **errp)
>   {
>       return ASPEED_MACHINE(obj)->mmio_exec;
> @@ -1224,6 +1277,26 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
>           aspeed_soc_num_cpus(amc->soc_name);
>   };
>   
> +
> +static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
> +{
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
> +
> +    mc->desc       = "Facebook Bletchley BMC (Cortex-A7)";
> +    amc->soc_name  = "ast2600-a3";
> +    amc->hw_strap1 = BLETCHLEY_BMC_HW_STRAP1;
> +    amc->hw_strap2 = BLETCHLEY_BMC_HW_STRAP2;
> +    amc->fmc_model = "w25q01jvq";

So we need this patch :

http://patchwork.ozlabs.org/project/qemu-devel/patch/20220304180920.1780992-1-patrick@stwcx.xyz/

May be I can take it through my queue.

> +    amc->spi_model = NULL;
> +    amc->num_cs    = 1;

There are two flash devices on the FMC. I can fix it inline since
it is the only change I would request.

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

Thanks,

C.

> +    amc->macs_mask = ASPEED_MAC2_ON;
> +    amc->i2c_init  = bletchley_bmc_i2c_init;
> +    mc->default_ram_size = 512 * MiB;
> +    mc->default_cpus = mc->min_cpus = mc->max_cpus =
> +        aspeed_soc_num_cpus(amc->soc_name);
> +}
> +
>   static const TypeInfo aspeed_machine_types[] = {
>       {
>           .name          = MACHINE_TYPE_NAME("palmetto-bmc"),
> @@ -1277,6 +1350,10 @@ static const TypeInfo aspeed_machine_types[] = {
>           .name          = MACHINE_TYPE_NAME("fuji-bmc"),
>           .parent        = TYPE_ASPEED_MACHINE,
>           .class_init    = aspeed_machine_fuji_class_init,
> +    }, {
> +        .name          = MACHINE_TYPE_NAME("bletchley-bmc"),
> +        .parent        = TYPE_ASPEED_MACHINE,
> +        .class_init    = aspeed_machine_bletchley_class_init,
>       }, {
>           .name          = TYPE_ASPEED_MACHINE,
>           .parent        = TYPE_MACHINE,



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type
  2022-03-05  7:57   ` Cédric Le Goater
@ 2022-03-07 20:59     ` Patrick Williams
  2022-03-08  8:14       ` Cédric Le Goater
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick Williams @ 2022-03-07 20:59 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Andrew Jeffery, Peter Maydell, open list:ASPEED BMCs,
	Joel Stanley, open list:All patches CC here

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

On Sat, Mar 05, 2022 at 08:57:47AM +0100, Cédric Le Goater wrote:
> On 3/5/22 01:06, Patrick Williams wrote:
> > +    mc->desc       = "Facebook Bletchley BMC (Cortex-A7)";
> > +    amc->soc_name  = "ast2600-a3";
> > +    amc->hw_strap1 = BLETCHLEY_BMC_HW_STRAP1;
> > +    amc->hw_strap2 = BLETCHLEY_BMC_HW_STRAP2;
> > +    amc->fmc_model = "w25q01jvq";
> 
> So we need this patch :
> 
> http://patchwork.ozlabs.org/project/qemu-devel/patch/20220304180920.1780992-1-patrick@stwcx.xyz/
> 
> May be I can take it through my queue.

Yes, it does.  I had sent that one earlier and probably should have been clear
on the dependency.

> > +    amc->spi_model = NULL;
> > +    amc->num_cs    = 1;
> 
> There are two flash devices on the FMC. I can fix it inline since
> it is the only change I would request.

Yes, there are.  I think all of the Facebook systems have dual FMC, for
redundancy in hardware, but we can get by in QEMU with just a single one.

I'll see however you fix it up and see I can update all the other systems as
well.  We have an internal patch to expand the CS on FMC to 2 but we haven't
upstreamed it yet and I'm worried it will break some users w.r.t. the CLI
changing for adding images.  My recollection is that the Romulus CI on OpenBMC
relies on the PNOR being the 2nd argument.

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

Thank you Cedric!

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type
  2022-03-07 20:59     ` Patrick Williams
@ 2022-03-08  8:14       ` Cédric Le Goater
  2022-03-08 17:23         ` Patrick Williams
  0 siblings, 1 reply; 8+ messages in thread
From: Cédric Le Goater @ 2022-03-08  8:14 UTC (permalink / raw)
  To: Patrick Williams
  Cc: Andrew Jeffery, Peter Maydell, open list:ASPEED BMCs,
	Joel Stanley, open list:All patches CC here


>> There are two flash devices on the FMC. I can fix it inline since
>> it is the only change I would request.
> 
> Yes, there are.  I think all of the Facebook systems have dual FMC, for
> redundancy in hardware, but we can get by in QEMU with just a single one.

yes, the kernel will complain though and I don't know how robust
the spi-nor based driver is. I think you sent a patch for a related
issue.

The newer spi-mem driver should be fine.
  
> I'll see however you fix it up and see I can update all the other systems as
> well.  

ok. may be for 7.1 then.

> We have an internal patch to expand the CS on FMC to 2 but we haven't
> upstreamed it yet and I'm worried it will break some users w.r.t. the CLI
> changing for adding images.  

Yes. That's the problem. I am afraid some CI systems will break with
these change in a newer QEMU. The command line options will need to
adapt.

> My recollection is that the Romulus CI on OpenBMC relies on the PNOR 
> being the 2nd argument.

That's the initial assumption made years ago. First mtd device is FMC,
second is the PNOR. It is reaching its limits.

I am looking at improving the command line argument to support:

   -drive file=<file>,format=raw,id=drive0 -device mx66l1g45g,bus=ssi.0,drive=drive0

which we would clearly define the topology. Adding a cs=[0-5] or and
addr=[0-5] is the next step.

Thanks,

C.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type
  2022-03-08  8:14       ` Cédric Le Goater
@ 2022-03-08 17:23         ` Patrick Williams
  2022-03-08 23:07           ` Joel Stanley
  0 siblings, 1 reply; 8+ messages in thread
From: Patrick Williams @ 2022-03-08 17:23 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: Andrew Jeffery, Peter Maydell, open list:ASPEED BMCs,
	Joel Stanley, open list:All patches CC here

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

On Tue, Mar 08, 2022 at 09:14:07AM +0100, Cédric Le Goater wrote:
> 
> >> There are two flash devices on the FMC. I can fix it inline since
> >> it is the only change I would request.
> > 
> > Yes, there are.  I think all of the Facebook systems have dual FMC, for
> > redundancy in hardware, but we can get by in QEMU with just a single one.
> 
> yes, the kernel will complain though and I don't know how robust
> the spi-nor based driver is. I think you sent a patch for a related
> issue.
> 
> The newer spi-mem driver should be fine.

Oh yes.  I already forgot that I'm running with that patch since Joel added it
to our backport 5.15 branch.  One of the reasons I wrote that patch was to make
QEMU not kpanic. :(

>   
> > I'll see however you fix it up and see I can update all the other systems as
> > well.  
> 
> ok. may be for 7.1 then.
> 
> > We have an internal patch to expand the CS on FMC to 2 but we haven't
> > upstreamed it yet and I'm worried it will break some users w.r.t. the CLI
> > changing for adding images.  
> 
> Yes. That's the problem. I am afraid some CI systems will break with
> these change in a newer QEMU. The command line options will need to
> adapt.

My recollection is that the Romulus CI uses a branch of QEMU that at this point
is rather old anyhow.  We should be able to fix up the CI scripts at the same
time we upgrade.

Are you or Andrew J maintaining that branch?

> > My recollection is that the Romulus CI on OpenBMC relies on the PNOR 
> > being the 2nd argument.
> 
> That's the initial assumption made years ago. First mtd device is FMC,
> second is the PNOR. It is reaching its limits.
> 
> I am looking at improving the command line argument to support:
> 
>    -drive file=<file>,format=raw,id=drive0 -device mx66l1g45g,bus=ssi.0,drive=drive0
> 
> which we would clearly define the topology. Adding a cs=[0-5] or and
> addr=[0-5] is the next step.

Seems fine to me.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type
  2022-03-08 17:23         ` Patrick Williams
@ 2022-03-08 23:07           ` Joel Stanley
  0 siblings, 0 replies; 8+ messages in thread
From: Joel Stanley @ 2022-03-08 23:07 UTC (permalink / raw)
  To: Patrick Williams
  Cc: Andrew Jeffery, Peter Maydell, open list:ASPEED BMCs,
	Cédric Le Goater, open list:All patches CC here

On Tue, 8 Mar 2022 at 17:23, Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Tue, Mar 08, 2022 at 09:14:07AM +0100, Cédric Le Goater wrote:
> >
> > >> There are two flash devices on the FMC. I can fix it inline since
> > >> it is the only change I would request.
> > >
> > > Yes, there are.  I think all of the Facebook systems have dual FMC, for
> > > redundancy in hardware, but we can get by in QEMU with just a single one.
> >
> > yes, the kernel will complain though and I don't know how robust
> > the spi-nor based driver is. I think you sent a patch for a related
> > issue.
> >
> > The newer spi-mem driver should be fine.
>
> Oh yes.  I already forgot that I'm running with that patch since Joel added it
> to our backport 5.15 branch.  One of the reasons I wrote that patch was to make
> QEMU not kpanic. :(
>
> >
> > > I'll see however you fix it up and see I can update all the other systems as
> > > well.
> >
> > ok. may be for 7.1 then.
> >
> > > We have an internal patch to expand the CS on FMC to 2 but we haven't
> > > upstreamed it yet and I'm worried it will break some users w.r.t. the CLI
> > > changing for adding images.
> >
> > Yes. That's the problem. I am afraid some CI systems will break with
> > these change in a newer QEMU. The command line options will need to
> > adapt.
>
> My recollection is that the Romulus CI uses a branch of QEMU that at this point
> is rather old anyhow.  We should be able to fix up the CI scripts at the same
> time we upgrade.

It looks like that branch missed the 7.2 boat. Given it's imminent
release, we should update the branch to Cedric's aspeed-7.0 when 7.0
is tagged.

With this branch could do CI on Rainier (or S6Q, or transformers) with
the eMMC image. I think there's value in doing CI on both eMMC and SPI
machines.

I'd like to see a boot test added for all of the machines in CI. Most
(all?) machines will get to bmc standby by booting on a generic Qemu
machine, such as the evb.

The machines that do have more of the system modelled could boot that
instead, and in the future add further tests.

>
> Are you or Andrew J maintaining that branch?
>
> > > My recollection is that the Romulus CI on OpenBMC relies on the PNOR
> > > being the 2nd argument.
> >
> > That's the initial assumption made years ago. First mtd device is FMC,
> > second is the PNOR. It is reaching its limits.
> >
> > I am looking at improving the command line argument to support:
> >
> >    -drive file=<file>,format=raw,id=drive0 -device mx66l1g45g,bus=ssi.0,drive=drive0
> >
> > which we would clearly define the topology. Adding a cs=[0-5] or and
> > addr=[0-5] is the next step.
>
> Seems fine to me.
>
> --
> Patrick Williams


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-03-08 23:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-05  0:06 [PATCH 1/2] hw/arm/aspeed: allow missing spi_model Patrick Williams
2022-03-05  0:06 ` [PATCH 2/2] hw/arm/aspeed: add Bletchley machine type Patrick Williams
2022-03-05  7:57   ` Cédric Le Goater
2022-03-07 20:59     ` Patrick Williams
2022-03-08  8:14       ` Cédric Le Goater
2022-03-08 17:23         ` Patrick Williams
2022-03-08 23:07           ` Joel Stanley
2022-03-05  7:53 ` [PATCH 1/2] hw/arm/aspeed: allow missing spi_model Cédric Le Goater

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.