[PATCH v2 1/9] hw/i2c/pca954x: Add method to get channels

[PATCH v2 1/9] hw/i2c/pca954x: Add method to get channels

[Peter Delevoryas]

I added this helper in the Aspeed machine file a while ago to help
initialize fuji-bmc i2c devices. This moves it to the official pca954x
file so that other files can use it.

This does something very similar to pca954x_i2c_get_bus, but I think
this is useful when you have a very complicated dts with a lot of
switches, like the fuji dts.

This convenience method lets you write code that produces a flat array
of I2C buses that matches the naming in the dts. After that you can just
add individual sensors using the flat array of I2C buses.

See fuji_bmc_i2c_init to understand this point further.

The fuji dts is here for reference:

https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 hw/arm/aspeed.c                  | 29 +++++++++--------------------
 hw/i2c/i2c_mux_pca954x.c         | 10 ++++++++++
 include/hw/i2c/i2c_mux_pca954x.h | 13 +++++++++++++
 3 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6fe9b13548..bee8a748ec 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -793,15 +793,6 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
     create_pca9552(soc, 15, 0x60);
 }
 
-static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr,
-                                 I2CBus **channels)
-{
-    I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr);
-    for (int i = 0; i < 8; i++) {
-        channels[i] = pca954x_i2c_get_bus(mux, i);
-    }
-}
-
 #define TYPE_LM75 TYPE_TMP105
 #define TYPE_TMP75 TYPE_TMP105
 #define TYPE_TMP422 "tmp422"
@@ -814,20 +805,18 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
     for (int i = 0; i < 16; i++) {
         i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
     }
-    I2CBus *i2c180 = i2c[2];
-    I2CBus *i2c480 = i2c[8];
-    I2CBus *i2c600 = i2c[11];
 
-    get_pca9548_channels(i2c180, 0x70, &i2c[16]);
-    get_pca9548_channels(i2c480, 0x70, &i2c[24]);
+    pca954x_i2c_get_channels(i2c[2], 0x70, "pca9548", &i2c[16]);
+    pca954x_i2c_get_channels(i2c[8], 0x70, "pca9548", &i2c[24]);
     /* NOTE: The device tree skips [32, 40) in the alias numbering */
-    get_pca9548_channels(i2c600, 0x77, &i2c[40]);
-    get_pca9548_channels(i2c[24], 0x71, &i2c[48]);
-    get_pca9548_channels(i2c[25], 0x72, &i2c[56]);
-    get_pca9548_channels(i2c[26], 0x76, &i2c[64]);
-    get_pca9548_channels(i2c[27], 0x76, &i2c[72]);
+    pca954x_i2c_get_channels(i2c[11], 0x77, "pca9548", &i2c[40]);
+    pca954x_i2c_get_channels(i2c[24], 0x71, "pca9548", &i2c[48]);
+    pca954x_i2c_get_channels(i2c[25], 0x72, "pca9548", &i2c[56]);
+    pca954x_i2c_get_channels(i2c[26], 0x76, "pca9548", &i2c[64]);
+    pca954x_i2c_get_channels(i2c[27], 0x76, "pca9548", &i2c[72]);
     for (int i = 0; i < 8; i++) {
-        get_pca9548_channels(i2c[40 + i], 0x76, &i2c[80 + i * 8]);
+        pca954x_i2c_get_channels(i2c[40 + i], 0x76, "pca9548",
+                                 &i2c[80 + i * 8]);
     }
 
     i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c);
diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
index 3945de795c..6b07804546 100644
--- a/hw/i2c/i2c_mux_pca954x.c
+++ b/hw/i2c/i2c_mux_pca954x.c
@@ -169,6 +169,16 @@ I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
     return pca954x->bus[channel];
 }
 
+void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
+                              const char *type_name, I2CBus **channels)
+{
+    I2CSlave *mux = i2c_slave_create_simple(bus, type_name, address);
+    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
+    Pca954xState *pca954x = PCA954X(mux);
+
+    memcpy(channels, pca954x->bus, pc->nchans * sizeof(channels[0]));
+}
+
 static void pca9546_class_init(ObjectClass *klass, void *data)
 {
     Pca954xClass *s = PCA954X_CLASS(klass);
diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
index 3dd25ec983..3a676a30a9 100644
--- a/include/hw/i2c/i2c_mux_pca954x.h
+++ b/include/hw/i2c/i2c_mux_pca954x.h
@@ -16,4 +16,17 @@
  */
 I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
 
+/**
+ * Creates an i2c mux and retrieves all of the channels associated with it.
+ *
+ * @bus: the i2c bus where the i2c mux resides.
+ * @address: the address of the i2c mux on the aforementioned i2c bus.
+ * @type_name: name of the i2c mux type to create.
+ * @channels: an output parameter specifying where to return the channels.
+ *
+ * Returns: None
+ */
+void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
+                              const char *type_name, I2CBus **channels);
+
 #endif
-- 
2.37.0

Re: [PATCH v2 1/9] hw/i2c/pca954x: Add method to get channels

[Corey Minyard]

On Tue, Jul 05, 2022 at 12:13:52PM -0700, Peter Delevoryas wrote:
> I added this helper in the Aspeed machine file a while ago to help
> initialize fuji-bmc i2c devices. This moves it to the official pca954x
> file so that other files can use it.
> 
> This does something very similar to pca954x_i2c_get_bus, but I think
> this is useful when you have a very complicated dts with a lot of
> switches, like the fuji dts.
> 
> This convenience method lets you write code that produces a flat array
> of I2C buses that matches the naming in the dts. After that you can just
> add individual sensors using the flat array of I2C buses.

This is an improvment, I think.  But it really needs to be two patches,
one with the I2C portion, and one with the aspeed portion.

Also, the name is a little misleading, you might want to name it 
pca954x_i2c_create_get_channels

-corey

> 
> See fuji_bmc_i2c_init to understand this point further.
> 
> The fuji dts is here for reference:
> 
> https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts
> 
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> ---
>  hw/arm/aspeed.c                  | 29 +++++++++--------------------
>  hw/i2c/i2c_mux_pca954x.c         | 10 ++++++++++
>  include/hw/i2c/i2c_mux_pca954x.h | 13 +++++++++++++
>  3 files changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 6fe9b13548..bee8a748ec 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -793,15 +793,6 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>      create_pca9552(soc, 15, 0x60);
>  }
>  
> -static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr,
> -                                 I2CBus **channels)
> -{
> -    I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr);
> -    for (int i = 0; i < 8; i++) {
> -        channels[i] = pca954x_i2c_get_bus(mux, i);
> -    }
> -}
> -
>  #define TYPE_LM75 TYPE_TMP105
>  #define TYPE_TMP75 TYPE_TMP105
>  #define TYPE_TMP422 "tmp422"
> @@ -814,20 +805,18 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
>      for (int i = 0; i < 16; i++) {
>          i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
>      }
> -    I2CBus *i2c180 = i2c[2];
> -    I2CBus *i2c480 = i2c[8];
> -    I2CBus *i2c600 = i2c[11];
>  
> -    get_pca9548_channels(i2c180, 0x70, &i2c[16]);
> -    get_pca9548_channels(i2c480, 0x70, &i2c[24]);
> +    pca954x_i2c_get_channels(i2c[2], 0x70, "pca9548", &i2c[16]);
> +    pca954x_i2c_get_channels(i2c[8], 0x70, "pca9548", &i2c[24]);
>      /* NOTE: The device tree skips [32, 40) in the alias numbering */
> -    get_pca9548_channels(i2c600, 0x77, &i2c[40]);
> -    get_pca9548_channels(i2c[24], 0x71, &i2c[48]);
> -    get_pca9548_channels(i2c[25], 0x72, &i2c[56]);
> -    get_pca9548_channels(i2c[26], 0x76, &i2c[64]);
> -    get_pca9548_channels(i2c[27], 0x76, &i2c[72]);
> +    pca954x_i2c_get_channels(i2c[11], 0x77, "pca9548", &i2c[40]);
> +    pca954x_i2c_get_channels(i2c[24], 0x71, "pca9548", &i2c[48]);
> +    pca954x_i2c_get_channels(i2c[25], 0x72, "pca9548", &i2c[56]);
> +    pca954x_i2c_get_channels(i2c[26], 0x76, "pca9548", &i2c[64]);
> +    pca954x_i2c_get_channels(i2c[27], 0x76, "pca9548", &i2c[72]);
>      for (int i = 0; i < 8; i++) {
> -        get_pca9548_channels(i2c[40 + i], 0x76, &i2c[80 + i * 8]);
> +        pca954x_i2c_get_channels(i2c[40 + i], 0x76, "pca9548",
> +                                 &i2c[80 + i * 8]);
>      }
>  
>      i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c);
> diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> index 3945de795c..6b07804546 100644
> --- a/hw/i2c/i2c_mux_pca954x.c
> +++ b/hw/i2c/i2c_mux_pca954x.c
> @@ -169,6 +169,16 @@ I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
>      return pca954x->bus[channel];
>  }
>  
> +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
> +                              const char *type_name, I2CBus **channels)
> +{
> +    I2CSlave *mux = i2c_slave_create_simple(bus, type_name, address);
> +    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
> +    Pca954xState *pca954x = PCA954X(mux);
> +
> +    memcpy(channels, pca954x->bus, pc->nchans * sizeof(channels[0]));
> +}
> +
>  static void pca9546_class_init(ObjectClass *klass, void *data)
>  {
>      Pca954xClass *s = PCA954X_CLASS(klass);
> diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
> index 3dd25ec983..3a676a30a9 100644
> --- a/include/hw/i2c/i2c_mux_pca954x.h
> +++ b/include/hw/i2c/i2c_mux_pca954x.h
> @@ -16,4 +16,17 @@
>   */
>  I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
>  
> +/**
> + * Creates an i2c mux and retrieves all of the channels associated with it.
> + *
> + * @bus: the i2c bus where the i2c mux resides.
> + * @address: the address of the i2c mux on the aforementioned i2c bus.
> + * @type_name: name of the i2c mux type to create.
> + * @channels: an output parameter specifying where to return the channels.
> + *
> + * Returns: None
> + */
> +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
> +                              const char *type_name, I2CBus **channels);
> +
>  #endif
> -- 
> 2.37.0
> 
> 

Re: [PATCH v2 1/9] hw/i2c/pca954x: Add method to get channels

[Peter Delevoryas]

On Tue, Jul 05, 2022 at 03:06:24PM -0500, Corey Minyard wrote:
> On Tue, Jul 05, 2022 at 12:13:52PM -0700, Peter Delevoryas wrote:
> > I added this helper in the Aspeed machine file a while ago to help
> > initialize fuji-bmc i2c devices. This moves it to the official pca954x
> > file so that other files can use it.
> > 
> > This does something very similar to pca954x_i2c_get_bus, but I think
> > this is useful when you have a very complicated dts with a lot of
> > switches, like the fuji dts.
> > 
> > This convenience method lets you write code that produces a flat array
> > of I2C buses that matches the naming in the dts. After that you can just
> > add individual sensors using the flat array of I2C buses.
> 
> This is an improvment, I think.  But it really needs to be two patches,
> one with the I2C portion, and one with the aspeed portion.
> 
> Also, the name is a little misleading, you might want to name it 
> pca954x_i2c_create_get_channels

You're right: Cedric, you can just ignore the pca954x patch then if you'd like,
I can resubmit it with the future I2C series that uses it. I probably shouldn't
have submitted it quite yet.

I can also resubmit the series with that patch removed, not sure if that's
necessary or not.

> 
> -corey
> 
> > 
> > See fuji_bmc_i2c_init to understand this point further.
> > 
> > The fuji dts is here for reference:
> > 
> > https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts
> > 
> > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > ---
> >  hw/arm/aspeed.c                  | 29 +++++++++--------------------
> >  hw/i2c/i2c_mux_pca954x.c         | 10 ++++++++++
> >  include/hw/i2c/i2c_mux_pca954x.h | 13 +++++++++++++
> >  3 files changed, 32 insertions(+), 20 deletions(-)
> > 
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > index 6fe9b13548..bee8a748ec 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -793,15 +793,6 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
> >      create_pca9552(soc, 15, 0x60);
> >  }
> >  
> > -static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr,
> > -                                 I2CBus **channels)
> > -{
> > -    I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr);
> > -    for (int i = 0; i < 8; i++) {
> > -        channels[i] = pca954x_i2c_get_bus(mux, i);
> > -    }
> > -}
> > -
> >  #define TYPE_LM75 TYPE_TMP105
> >  #define TYPE_TMP75 TYPE_TMP105
> >  #define TYPE_TMP422 "tmp422"
> > @@ -814,20 +805,18 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
> >      for (int i = 0; i < 16; i++) {
> >          i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
> >      }
> > -    I2CBus *i2c180 = i2c[2];
> > -    I2CBus *i2c480 = i2c[8];
> > -    I2CBus *i2c600 = i2c[11];
> >  
> > -    get_pca9548_channels(i2c180, 0x70, &i2c[16]);
> > -    get_pca9548_channels(i2c480, 0x70, &i2c[24]);
> > +    pca954x_i2c_get_channels(i2c[2], 0x70, "pca9548", &i2c[16]);
> > +    pca954x_i2c_get_channels(i2c[8], 0x70, "pca9548", &i2c[24]);
> >      /* NOTE: The device tree skips [32, 40) in the alias numbering */
> > -    get_pca9548_channels(i2c600, 0x77, &i2c[40]);
> > -    get_pca9548_channels(i2c[24], 0x71, &i2c[48]);
> > -    get_pca9548_channels(i2c[25], 0x72, &i2c[56]);
> > -    get_pca9548_channels(i2c[26], 0x76, &i2c[64]);
> > -    get_pca9548_channels(i2c[27], 0x76, &i2c[72]);
> > +    pca954x_i2c_get_channels(i2c[11], 0x77, "pca9548", &i2c[40]);
> > +    pca954x_i2c_get_channels(i2c[24], 0x71, "pca9548", &i2c[48]);
> > +    pca954x_i2c_get_channels(i2c[25], 0x72, "pca9548", &i2c[56]);
> > +    pca954x_i2c_get_channels(i2c[26], 0x76, "pca9548", &i2c[64]);
> > +    pca954x_i2c_get_channels(i2c[27], 0x76, "pca9548", &i2c[72]);
> >      for (int i = 0; i < 8; i++) {
> > -        get_pca9548_channels(i2c[40 + i], 0x76, &i2c[80 + i * 8]);
> > +        pca954x_i2c_get_channels(i2c[40 + i], 0x76, "pca9548",
> > +                                 &i2c[80 + i * 8]);
> >      }
> >  
> >      i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c);
> > diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> > index 3945de795c..6b07804546 100644
> > --- a/hw/i2c/i2c_mux_pca954x.c
> > +++ b/hw/i2c/i2c_mux_pca954x.c
> > @@ -169,6 +169,16 @@ I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
> >      return pca954x->bus[channel];
> >  }
> >  
> > +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
> > +                              const char *type_name, I2CBus **channels)
> > +{
> > +    I2CSlave *mux = i2c_slave_create_simple(bus, type_name, address);
> > +    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
> > +    Pca954xState *pca954x = PCA954X(mux);
> > +
> > +    memcpy(channels, pca954x->bus, pc->nchans * sizeof(channels[0]));
> > +}
> > +
> >  static void pca9546_class_init(ObjectClass *klass, void *data)
> >  {
> >      Pca954xClass *s = PCA954X_CLASS(klass);
> > diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
> > index 3dd25ec983..3a676a30a9 100644
> > --- a/include/hw/i2c/i2c_mux_pca954x.h
> > +++ b/include/hw/i2c/i2c_mux_pca954x.h
> > @@ -16,4 +16,17 @@
> >   */
> >  I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
> >  
> > +/**
> > + * Creates an i2c mux and retrieves all of the channels associated with it.
> > + *
> > + * @bus: the i2c bus where the i2c mux resides.
> > + * @address: the address of the i2c mux on the aforementioned i2c bus.
> > + * @type_name: name of the i2c mux type to create.
> > + * @channels: an output parameter specifying where to return the channels.
> > + *
> > + * Returns: None
> > + */
> > +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
> > +                              const char *type_name, I2CBus **channels);
> > +
> >  #endif
> > -- 
> > 2.37.0
> > 
> > 

Re: [PATCH v2 1/9] hw/i2c/pca954x: Add method to get channels

[Peter Delevoryas]

On Tue, Jul 05, 2022 at 02:40:32PM -0700, Peter Delevoryas wrote:
> On Tue, Jul 05, 2022 at 03:06:24PM -0500, Corey Minyard wrote:
> > On Tue, Jul 05, 2022 at 12:13:52PM -0700, Peter Delevoryas wrote:
> > > I added this helper in the Aspeed machine file a while ago to help
> > > initialize fuji-bmc i2c devices. This moves it to the official pca954x
> > > file so that other files can use it.
> > > 
> > > This does something very similar to pca954x_i2c_get_bus, but I think
> > > this is useful when you have a very complicated dts with a lot of
> > > switches, like the fuji dts.
> > > 
> > > This convenience method lets you write code that produces a flat array
> > > of I2C buses that matches the naming in the dts. After that you can just
> > > add individual sensors using the flat array of I2C buses.
> > 
> > This is an improvment, I think.  But it really needs to be two patches,
> > one with the I2C portion, and one with the aspeed portion.
> > 
> > Also, the name is a little misleading, you might want to name it 
> > pca954x_i2c_create_get_channels
> 
> You're right: Cedric, you can just ignore the pca954x patch then if you'd like,
> I can resubmit it with the future I2C series that uses it. I probably shouldn't
> have submitted it quite yet.
> 
> I can also resubmit the series with that patch removed, not sure if that's
> necessary or not.

This was hastily written, what I meant to say was:

Cedric, feel free to remove this patch from the series. If you'd like, I can
also resubmit this series as v3 with the patch removed.

> 
> > 
> > -corey
> > 
> > > 
> > > See fuji_bmc_i2c_init to understand this point further.
> > > 
> > > The fuji dts is here for reference:
> > > 
> > > https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts
> > > 
> > > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > > ---
> > >  hw/arm/aspeed.c                  | 29 +++++++++--------------------
> > >  hw/i2c/i2c_mux_pca954x.c         | 10 ++++++++++
> > >  include/hw/i2c/i2c_mux_pca954x.h | 13 +++++++++++++
> > >  3 files changed, 32 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > > index 6fe9b13548..bee8a748ec 100644
> > > --- a/hw/arm/aspeed.c
> > > +++ b/hw/arm/aspeed.c
> > > @@ -793,15 +793,6 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
> > >      create_pca9552(soc, 15, 0x60);
> > >  }
> > >  
> > > -static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr,
> > > -                                 I2CBus **channels)
> > > -{
> > > -    I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr);
> > > -    for (int i = 0; i < 8; i++) {
> > > -        channels[i] = pca954x_i2c_get_bus(mux, i);
> > > -    }
> > > -}
> > > -
> > >  #define TYPE_LM75 TYPE_TMP105
> > >  #define TYPE_TMP75 TYPE_TMP105
> > >  #define TYPE_TMP422 "tmp422"
> > > @@ -814,20 +805,18 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
> > >      for (int i = 0; i < 16; i++) {
> > >          i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
> > >      }
> > > -    I2CBus *i2c180 = i2c[2];
> > > -    I2CBus *i2c480 = i2c[8];
> > > -    I2CBus *i2c600 = i2c[11];
> > >  
> > > -    get_pca9548_channels(i2c180, 0x70, &i2c[16]);
> > > -    get_pca9548_channels(i2c480, 0x70, &i2c[24]);
> > > +    pca954x_i2c_get_channels(i2c[2], 0x70, "pca9548", &i2c[16]);
> > > +    pca954x_i2c_get_channels(i2c[8], 0x70, "pca9548", &i2c[24]);
> > >      /* NOTE: The device tree skips [32, 40) in the alias numbering */
> > > -    get_pca9548_channels(i2c600, 0x77, &i2c[40]);
> > > -    get_pca9548_channels(i2c[24], 0x71, &i2c[48]);
> > > -    get_pca9548_channels(i2c[25], 0x72, &i2c[56]);
> > > -    get_pca9548_channels(i2c[26], 0x76, &i2c[64]);
> > > -    get_pca9548_channels(i2c[27], 0x76, &i2c[72]);
> > > +    pca954x_i2c_get_channels(i2c[11], 0x77, "pca9548", &i2c[40]);
> > > +    pca954x_i2c_get_channels(i2c[24], 0x71, "pca9548", &i2c[48]);
> > > +    pca954x_i2c_get_channels(i2c[25], 0x72, "pca9548", &i2c[56]);
> > > +    pca954x_i2c_get_channels(i2c[26], 0x76, "pca9548", &i2c[64]);
> > > +    pca954x_i2c_get_channels(i2c[27], 0x76, "pca9548", &i2c[72]);
> > >      for (int i = 0; i < 8; i++) {
> > > -        get_pca9548_channels(i2c[40 + i], 0x76, &i2c[80 + i * 8]);
> > > +        pca954x_i2c_get_channels(i2c[40 + i], 0x76, "pca9548",
> > > +                                 &i2c[80 + i * 8]);
> > >      }
> > >  
> > >      i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c);
> > > diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> > > index 3945de795c..6b07804546 100644
> > > --- a/hw/i2c/i2c_mux_pca954x.c
> > > +++ b/hw/i2c/i2c_mux_pca954x.c
> > > @@ -169,6 +169,16 @@ I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
> > >      return pca954x->bus[channel];
> > >  }
> > >  
> > > +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
> > > +                              const char *type_name, I2CBus **channels)
> > > +{
> > > +    I2CSlave *mux = i2c_slave_create_simple(bus, type_name, address);
> > > +    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
> > > +    Pca954xState *pca954x = PCA954X(mux);
> > > +
> > > +    memcpy(channels, pca954x->bus, pc->nchans * sizeof(channels[0]));
> > > +}
> > > +
> > >  static void pca9546_class_init(ObjectClass *klass, void *data)
> > >  {
> > >      Pca954xClass *s = PCA954X_CLASS(klass);
> > > diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
> > > index 3dd25ec983..3a676a30a9 100644
> > > --- a/include/hw/i2c/i2c_mux_pca954x.h
> > > +++ b/include/hw/i2c/i2c_mux_pca954x.h
> > > @@ -16,4 +16,17 @@
> > >   */
> > >  I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
> > >  
> > > +/**
> > > + * Creates an i2c mux and retrieves all of the channels associated with it.
> > > + *
> > > + * @bus: the i2c bus where the i2c mux resides.
> > > + * @address: the address of the i2c mux on the aforementioned i2c bus.
> > > + * @type_name: name of the i2c mux type to create.
> > > + * @channels: an output parameter specifying where to return the channels.
> > > + *
> > > + * Returns: None
> > > + */
> > > +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
> > > +                              const char *type_name, I2CBus **channels);
> > > +
> > >  #endif
> > > -- 
> > > 2.37.0
> > > 
> > > 
> 

Re: [PATCH v2 1/9] hw/i2c/pca954x: Add method to get channels

[Cédric Le Goater]

On 7/5/22 23:44, Peter Delevoryas wrote:
> On Tue, Jul 05, 2022 at 02:40:32PM -0700, Peter Delevoryas wrote:
>> On Tue, Jul 05, 2022 at 03:06:24PM -0500, Corey Minyard wrote:
>>> On Tue, Jul 05, 2022 at 12:13:52PM -0700, Peter Delevoryas wrote:
>>>> I added this helper in the Aspeed machine file a while ago to help
>>>> initialize fuji-bmc i2c devices. This moves it to the official pca954x
>>>> file so that other files can use it.
>>>>
>>>> This does something very similar to pca954x_i2c_get_bus, but I think
>>>> this is useful when you have a very complicated dts with a lot of
>>>> switches, like the fuji dts.
>>>>
>>>> This convenience method lets you write code that produces a flat array
>>>> of I2C buses that matches the naming in the dts. After that you can just
>>>> add individual sensors using the flat array of I2C buses.
>>>
>>> This is an improvment, I think.  But it really needs to be two patches,
>>> one with the I2C portion, and one with the aspeed portion.
>>>
>>> Also, the name is a little misleading, you might want to name it
>>> pca954x_i2c_create_get_channels
>>
>> You're right: Cedric, you can just ignore the pca954x patch then if you'd like,
>> I can resubmit it with the future I2C series that uses it. I probably shouldn't
>> have submitted it quite yet.
>>
>> I can also resubmit the series with that patch removed, not sure if that's
>> necessary or not.
> 
> This was hastily written, what I meant to say was:
> 
> Cedric, feel free to remove this patch from the series. If you'd like, I can
> also resubmit this series as v3 with the patch removed.


I moved it at the end of the series to come just before the other patches
needing it, the last three patches of :

   http://patchwork.ozlabs.org/project/qemu-devel/list/?series=307305

You can resend all of them when fixed.


I think we are done with the initial fby35.

Next time, please add a cover letter and keep the Reviewed-by tags
of the previous version. It helps the reviewers. I re-added them
manually for this spin.

Thanks,

C.

> 
>>
>>>
>>> -corey
>>>
>>>>
>>>> See fuji_bmc_i2c_init to understand this point further.
>>>>
>>>> The fuji dts is here for reference:
>>>>
>>>> https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts
>>>>
>>>> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
>>>> ---
>>>>   hw/arm/aspeed.c                  | 29 +++++++++--------------------
>>>>   hw/i2c/i2c_mux_pca954x.c         | 10 ++++++++++
>>>>   include/hw/i2c/i2c_mux_pca954x.h | 13 +++++++++++++
>>>>   3 files changed, 32 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>>> index 6fe9b13548..bee8a748ec 100644
>>>> --- a/hw/arm/aspeed.c
>>>> +++ b/hw/arm/aspeed.c
>>>> @@ -793,15 +793,6 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
>>>>       create_pca9552(soc, 15, 0x60);
>>>>   }
>>>>   
>>>> -static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr,
>>>> -                                 I2CBus **channels)
>>>> -{
>>>> -    I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr);
>>>> -    for (int i = 0; i < 8; i++) {
>>>> -        channels[i] = pca954x_i2c_get_bus(mux, i);
>>>> -    }
>>>> -}
>>>> -
>>>>   #define TYPE_LM75 TYPE_TMP105
>>>>   #define TYPE_TMP75 TYPE_TMP105
>>>>   #define TYPE_TMP422 "tmp422"
>>>> @@ -814,20 +805,18 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
>>>>       for (int i = 0; i < 16; i++) {
>>>>           i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
>>>>       }
>>>> -    I2CBus *i2c180 = i2c[2];
>>>> -    I2CBus *i2c480 = i2c[8];
>>>> -    I2CBus *i2c600 = i2c[11];
>>>>   
>>>> -    get_pca9548_channels(i2c180, 0x70, &i2c[16]);
>>>> -    get_pca9548_channels(i2c480, 0x70, &i2c[24]);
>>>> +    pca954x_i2c_get_channels(i2c[2], 0x70, "pca9548", &i2c[16]);
>>>> +    pca954x_i2c_get_channels(i2c[8], 0x70, "pca9548", &i2c[24]);
>>>>       /* NOTE: The device tree skips [32, 40) in the alias numbering */
>>>> -    get_pca9548_channels(i2c600, 0x77, &i2c[40]);
>>>> -    get_pca9548_channels(i2c[24], 0x71, &i2c[48]);
>>>> -    get_pca9548_channels(i2c[25], 0x72, &i2c[56]);
>>>> -    get_pca9548_channels(i2c[26], 0x76, &i2c[64]);
>>>> -    get_pca9548_channels(i2c[27], 0x76, &i2c[72]);
>>>> +    pca954x_i2c_get_channels(i2c[11], 0x77, "pca9548", &i2c[40]);
>>>> +    pca954x_i2c_get_channels(i2c[24], 0x71, "pca9548", &i2c[48]);
>>>> +    pca954x_i2c_get_channels(i2c[25], 0x72, "pca9548", &i2c[56]);
>>>> +    pca954x_i2c_get_channels(i2c[26], 0x76, "pca9548", &i2c[64]);
>>>> +    pca954x_i2c_get_channels(i2c[27], 0x76, "pca9548", &i2c[72]);
>>>>       for (int i = 0; i < 8; i++) {
>>>> -        get_pca9548_channels(i2c[40 + i], 0x76, &i2c[80 + i * 8]);
>>>> +        pca954x_i2c_get_channels(i2c[40 + i], 0x76, "pca9548",
>>>> +                                 &i2c[80 + i * 8]);
>>>>       }
>>>>   
>>>>       i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c);
>>>> diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
>>>> index 3945de795c..6b07804546 100644
>>>> --- a/hw/i2c/i2c_mux_pca954x.c
>>>> +++ b/hw/i2c/i2c_mux_pca954x.c
>>>> @@ -169,6 +169,16 @@ I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
>>>>       return pca954x->bus[channel];
>>>>   }
>>>>   
>>>> +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
>>>> +                              const char *type_name, I2CBus **channels)
>>>> +{
>>>> +    I2CSlave *mux = i2c_slave_create_simple(bus, type_name, address);
>>>> +    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
>>>> +    Pca954xState *pca954x = PCA954X(mux);
>>>> +
>>>> +    memcpy(channels, pca954x->bus, pc->nchans * sizeof(channels[0]));
>>>> +}
>>>> +
>>>>   static void pca9546_class_init(ObjectClass *klass, void *data)
>>>>   {
>>>>       Pca954xClass *s = PCA954X_CLASS(klass);
>>>> diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
>>>> index 3dd25ec983..3a676a30a9 100644
>>>> --- a/include/hw/i2c/i2c_mux_pca954x.h
>>>> +++ b/include/hw/i2c/i2c_mux_pca954x.h
>>>> @@ -16,4 +16,17 @@
>>>>    */
>>>>   I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
>>>>   
>>>> +/**
>>>> + * Creates an i2c mux and retrieves all of the channels associated with it.
>>>> + *
>>>> + * @bus: the i2c bus where the i2c mux resides.
>>>> + * @address: the address of the i2c mux on the aforementioned i2c bus.
>>>> + * @type_name: name of the i2c mux type to create.
>>>> + * @channels: an output parameter specifying where to return the channels.
>>>> + *
>>>> + * Returns: None
>>>> + */
>>>> +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
>>>> +                              const char *type_name, I2CBus **channels);
>>>> +
>>>>   #endif
>>>> -- 
>>>> 2.37.0
>>>>
>>>>
>>

Re: [PATCH v2 1/9] hw/i2c/pca954x: Add method to get channels

[Peter Delevoryas]

On Wed, Jul 06, 2022 at 08:06:34AM +0200, Cédric Le Goater wrote:
> On 7/5/22 23:44, Peter Delevoryas wrote:
> > On Tue, Jul 05, 2022 at 02:40:32PM -0700, Peter Delevoryas wrote:
> > > On Tue, Jul 05, 2022 at 03:06:24PM -0500, Corey Minyard wrote:
> > > > On Tue, Jul 05, 2022 at 12:13:52PM -0700, Peter Delevoryas wrote:
> > > > > I added this helper in the Aspeed machine file a while ago to help
> > > > > initialize fuji-bmc i2c devices. This moves it to the official pca954x
> > > > > file so that other files can use it.
> > > > > 
> > > > > This does something very similar to pca954x_i2c_get_bus, but I think
> > > > > this is useful when you have a very complicated dts with a lot of
> > > > > switches, like the fuji dts.
> > > > > 
> > > > > This convenience method lets you write code that produces a flat array
> > > > > of I2C buses that matches the naming in the dts. After that you can just
> > > > > add individual sensors using the flat array of I2C buses.
> > > > 
> > > > This is an improvment, I think.  But it really needs to be two patches,
> > > > one with the I2C portion, and one with the aspeed portion.
> > > > 
> > > > Also, the name is a little misleading, you might want to name it
> > > > pca954x_i2c_create_get_channels
> > > 
> > > You're right: Cedric, you can just ignore the pca954x patch then if you'd like,
> > > I can resubmit it with the future I2C series that uses it. I probably shouldn't
> > > have submitted it quite yet.
> > > 
> > > I can also resubmit the series with that patch removed, not sure if that's
> > > necessary or not.
> > 
> > This was hastily written, what I meant to say was:
> > 
> > Cedric, feel free to remove this patch from the series. If you'd like, I can
> > also resubmit this series as v3 with the patch removed.
> 
> 
> I moved it at the end of the series to come just before the other patches
> needing it, the last three patches of :
> 
>   http://patchwork.ozlabs.org/project/qemu-devel/list/?series=307305
> 
> You can resend all of them when fixed.

That sounds good, thanks.

> 
> 
> I think we are done with the initial fby35.
> 
> Next time, please add a cover letter and keep the Reviewed-by tags
> of the previous version. It helps the reviewers. I re-added them
> manually for this spin.

Yeah that's my bad again, I need to get in a better habit of adding those.
Thanks for reminding me again.

> Thanks,
> 
> C.
> 
> > 
> > > 
> > > > 
> > > > -corey
> > > > 
> > > > > 
> > > > > See fuji_bmc_i2c_init to understand this point further.
> > > > > 
> > > > > The fuji dts is here for reference:
> > > > > 
> > > > > https://github.com/torvalds/linux/blob/40cb6373b46/arch/arm/boot/dts/aspeed-bmc-facebook-fuji.dts
> > > > > 
> > > > > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > > > > ---
> > > > >   hw/arm/aspeed.c                  | 29 +++++++++--------------------
> > > > >   hw/i2c/i2c_mux_pca954x.c         | 10 ++++++++++
> > > > >   include/hw/i2c/i2c_mux_pca954x.h | 13 +++++++++++++
> > > > >   3 files changed, 32 insertions(+), 20 deletions(-)
> > > > > 
> > > > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > > > > index 6fe9b13548..bee8a748ec 100644
> > > > > --- a/hw/arm/aspeed.c
> > > > > +++ b/hw/arm/aspeed.c
> > > > > @@ -793,15 +793,6 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc)
> > > > >       create_pca9552(soc, 15, 0x60);
> > > > >   }
> > > > > -static void get_pca9548_channels(I2CBus *bus, uint8_t mux_addr,
> > > > > -                                 I2CBus **channels)
> > > > > -{
> > > > > -    I2CSlave *mux = i2c_slave_create_simple(bus, "pca9548", mux_addr);
> > > > > -    for (int i = 0; i < 8; i++) {
> > > > > -        channels[i] = pca954x_i2c_get_bus(mux, i);
> > > > > -    }
> > > > > -}
> > > > > -
> > > > >   #define TYPE_LM75 TYPE_TMP105
> > > > >   #define TYPE_TMP75 TYPE_TMP105
> > > > >   #define TYPE_TMP422 "tmp422"
> > > > > @@ -814,20 +805,18 @@ static void fuji_bmc_i2c_init(AspeedMachineState *bmc)
> > > > >       for (int i = 0; i < 16; i++) {
> > > > >           i2c[i] = aspeed_i2c_get_bus(&soc->i2c, i);
> > > > >       }
> > > > > -    I2CBus *i2c180 = i2c[2];
> > > > > -    I2CBus *i2c480 = i2c[8];
> > > > > -    I2CBus *i2c600 = i2c[11];
> > > > > -    get_pca9548_channels(i2c180, 0x70, &i2c[16]);
> > > > > -    get_pca9548_channels(i2c480, 0x70, &i2c[24]);
> > > > > +    pca954x_i2c_get_channels(i2c[2], 0x70, "pca9548", &i2c[16]);
> > > > > +    pca954x_i2c_get_channels(i2c[8], 0x70, "pca9548", &i2c[24]);
> > > > >       /* NOTE: The device tree skips [32, 40) in the alias numbering */
> > > > > -    get_pca9548_channels(i2c600, 0x77, &i2c[40]);
> > > > > -    get_pca9548_channels(i2c[24], 0x71, &i2c[48]);
> > > > > -    get_pca9548_channels(i2c[25], 0x72, &i2c[56]);
> > > > > -    get_pca9548_channels(i2c[26], 0x76, &i2c[64]);
> > > > > -    get_pca9548_channels(i2c[27], 0x76, &i2c[72]);
> > > > > +    pca954x_i2c_get_channels(i2c[11], 0x77, "pca9548", &i2c[40]);
> > > > > +    pca954x_i2c_get_channels(i2c[24], 0x71, "pca9548", &i2c[48]);
> > > > > +    pca954x_i2c_get_channels(i2c[25], 0x72, "pca9548", &i2c[56]);
> > > > > +    pca954x_i2c_get_channels(i2c[26], 0x76, "pca9548", &i2c[64]);
> > > > > +    pca954x_i2c_get_channels(i2c[27], 0x76, "pca9548", &i2c[72]);
> > > > >       for (int i = 0; i < 8; i++) {
> > > > > -        get_pca9548_channels(i2c[40 + i], 0x76, &i2c[80 + i * 8]);
> > > > > +        pca954x_i2c_get_channels(i2c[40 + i], 0x76, "pca9548",
> > > > > +                                 &i2c[80 + i * 8]);
> > > > >       }
> > > > >       i2c_slave_create_simple(i2c[17], TYPE_LM75, 0x4c);
> > > > > diff --git a/hw/i2c/i2c_mux_pca954x.c b/hw/i2c/i2c_mux_pca954x.c
> > > > > index 3945de795c..6b07804546 100644
> > > > > --- a/hw/i2c/i2c_mux_pca954x.c
> > > > > +++ b/hw/i2c/i2c_mux_pca954x.c
> > > > > @@ -169,6 +169,16 @@ I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel)
> > > > >       return pca954x->bus[channel];
> > > > >   }
> > > > > +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
> > > > > +                              const char *type_name, I2CBus **channels)
> > > > > +{
> > > > > +    I2CSlave *mux = i2c_slave_create_simple(bus, type_name, address);
> > > > > +    Pca954xClass *pc = PCA954X_GET_CLASS(mux);
> > > > > +    Pca954xState *pca954x = PCA954X(mux);
> > > > > +
> > > > > +    memcpy(channels, pca954x->bus, pc->nchans * sizeof(channels[0]));
> > > > > +}
> > > > > +
> > > > >   static void pca9546_class_init(ObjectClass *klass, void *data)
> > > > >   {
> > > > >       Pca954xClass *s = PCA954X_CLASS(klass);
> > > > > diff --git a/include/hw/i2c/i2c_mux_pca954x.h b/include/hw/i2c/i2c_mux_pca954x.h
> > > > > index 3dd25ec983..3a676a30a9 100644
> > > > > --- a/include/hw/i2c/i2c_mux_pca954x.h
> > > > > +++ b/include/hw/i2c/i2c_mux_pca954x.h
> > > > > @@ -16,4 +16,17 @@
> > > > >    */
> > > > >   I2CBus *pca954x_i2c_get_bus(I2CSlave *mux, uint8_t channel);
> > > > > +/**
> > > > > + * Creates an i2c mux and retrieves all of the channels associated with it.
> > > > > + *
> > > > > + * @bus: the i2c bus where the i2c mux resides.
> > > > > + * @address: the address of the i2c mux on the aforementioned i2c bus.
> > > > > + * @type_name: name of the i2c mux type to create.
> > > > > + * @channels: an output parameter specifying where to return the channels.
> > > > > + *
> > > > > + * Returns: None
> > > > > + */
> > > > > +void pca954x_i2c_get_channels(I2CBus *bus, uint8_t address,
> > > > > +                              const char *type_name, I2CBus **channels);
> > > > > +
> > > > >   #endif
> > > > > -- 
> > > > > 2.37.0
> > > > > 
> > > > > 
> > > 
> 

[PATCH v2 2/9] aspeed: Create SRAM name from first CPU index

[Peter Delevoryas]

To support multiple SoC's running simultaneously, we need a unique name for
each RAM region. DRAM is created by the machine, but SRAM is created by the
SoC, since in hardware it is part of the SoC's internals.

We need a way to uniquely identify each SRAM region though, for VM
migration. Since each of the SoC's CPU's has an index which identifies it
uniquely from other CPU's in the machine, we can use the index of any of the
CPU's in the SoC to uniquely identify differentiate the SRAM name from other
SoC SRAM's. In this change, I just elected to use the index of the first CPU
in each SoC.

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 hw/arm/aspeed_ast10x0.c | 5 ++++-
 hw/arm/aspeed_ast2600.c | 5 +++--
 hw/arm/aspeed_soc.c     | 5 +++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 33ef331771..677699e54c 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -159,6 +159,7 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     DeviceState *armv7m;
     Error *err = NULL;
     int i;
+    g_autofree char *sram_name = NULL;
 
     if (!clock_has_source(s->sysclk)) {
         error_setg(errp, "sysclk clock must be wired up by the board code");
@@ -183,7 +184,9 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
     sysbus_realize(SYS_BUS_DEVICE(&s->armv7m), &error_abort);
 
     /* Internal SRAM */
-    memory_region_init_ram(&s->sram, NULL, "aspeed.sram", sc->sram_size, &err);
+    sram_name = g_strdup_printf("aspeed.sram.%d",
+                                CPU(s->armv7m.cpu)->cpu_index);
+    memory_region_init_ram(&s->sram, OBJECT(s), sram_name, sc->sram_size, &err);
     if (err != NULL) {
         error_propagate(errp, err);
         return;
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 3f0611ac11..64eb5a7b26 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -276,6 +276,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     Error *err = NULL;
     qemu_irq irq;
+    g_autofree char *sram_name = NULL;
 
     /* IO space */
     aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
@@ -335,8 +336,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     }
 
     /* SRAM */
-    memory_region_init_ram(&s->sram, OBJECT(dev), "aspeed.sram",
-                           sc->sram_size, &err);
+    sram_name = g_strdup_printf("aspeed.sram.%d", CPU(&s->cpu[0])->cpu_index);
+    memory_region_init_ram(&s->sram, OBJECT(s), sram_name, sc->sram_size, &err);
     if (err) {
         error_propagate(errp, err);
         return;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 0f675e7fcd..0bb6a2f092 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -239,6 +239,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     AspeedSoCState *s = ASPEED_SOC(dev);
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
     Error *err = NULL;
+    g_autofree char *sram_name = NULL;
 
     /* IO space */
     aspeed_mmio_map_unimplemented(s, SYS_BUS_DEVICE(&s->iomem), "aspeed.io",
@@ -259,8 +260,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     }
 
     /* SRAM */
-    memory_region_init_ram(&s->sram, OBJECT(dev), "aspeed.sram",
-                           sc->sram_size, &err);
+    sram_name = g_strdup_printf("aspeed.sram.%d", CPU(&s->cpu[0])->cpu_index);
+    memory_region_init_ram(&s->sram, OBJECT(s), sram_name, sc->sram_size, &err);
     if (err) {
         error_propagate(errp, err);
         return;
-- 
2.37.0

[PATCH v2 3/9] aspeed: Refactor UART init for multi-SoC machines

[Peter Delevoryas]

This change moves the code that connects the SoC UART's to serial_hd's
to the machine.

It makes each UART a proper child member of the SoC, and then allows the
machine to selectively initialize the chardev for each UART with a
serial_hd.

This should preserve backwards compatibility, but also allow multi-SoC
boards to completely change the wiring of serial devices from the
command line to specific SoC UART's.

This also removes the uart-default property from the SoC, since the SoC
doesn't need to know what UART is the "default" on the machine anymore.

I tested this using the images and commands from the previous
refactoring, and another test image for the ast1030:

    wget https://github.com/facebook/openbmc/releases/download/v2021.49.0/fuji.mtd
    wget https://github.com/facebook/openbmc/releases/download/v2021.49.0/wedge100.mtd
    wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf

Fuji uses UART1:

    qemu-system-arm -machine fuji-bmc \
        -drive file=fuji.mtd,format=raw,if=mtd \
        -nographic

ast2600-evb uses uart-default=UART5:

    qemu-system-arm -machine ast2600-evb \
        -drive file=fuji.mtd,format=raw,if=mtd \
        -serial null -serial mon:stdio -display none

Wedge100 uses UART3:

    qemu-system-arm -machine palmetto-bmc \
        -drive file=wedge100.mtd,format=raw,if=mtd \
        -serial null -serial null -serial null \
        -serial mon:stdio -display none

AST1030 EVB uses UART5:

    qemu-system-arm -machine ast1030-evb \
        -kernel Y35BCL.elf -nographic

Fixes: 6827ff20b2975 ("hw: aspeed: Init all UART's with serial devices")
Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 hw/arm/aspeed.c             | 22 +++++++++++++---
 hw/arm/aspeed_ast10x0.c     |  8 +++++-
 hw/arm/aspeed_ast2600.c     |  8 +++++-
 hw/arm/aspeed_soc.c         | 50 +++++++++++++++++++++++++------------
 include/hw/arm/aspeed_soc.h |  7 ++++--
 5 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index bee8a748ec..cc395f988c 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -26,6 +26,7 @@
 #include "qemu/error-report.h"
 #include "qemu/units.h"
 #include "hw/qdev-clock.h"
+#include "sysemu/sysemu.h"
 
 static struct arm_boot_info aspeed_board_binfo = {
     .board_id = -1, /* device-tree-only board */
@@ -301,6 +302,21 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
                                &error_fatal);
 }
 
+static void connect_serial_hds_to_uarts(AspeedMachineState *bmc)
+{
+    AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc);
+    AspeedSoCState *s = &bmc->soc;
+    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+
+    aspeed_soc_uart_set_chr(s, amc->uart_default, serial_hd(0));
+    for (int i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) {
+        if (uart == amc->uart_default) {
+            continue;
+        }
+        aspeed_soc_uart_set_chr(s, uart, serial_hd(i));
+    }
+}
+
 static void aspeed_machine_init(MachineState *machine)
 {
     AspeedMachineState *bmc = ASPEED_MACHINE(machine);
@@ -346,8 +362,7 @@ static void aspeed_machine_init(MachineState *machine)
         object_property_set_int(OBJECT(&bmc->soc), "hw-prot-key",
                                 ASPEED_SCU_PROT_KEY, &error_abort);
     }
-    qdev_prop_set_uint32(DEVICE(&bmc->soc), "uart-default",
-                         amc->uart_default);
+    connect_serial_hds_to_uarts(bmc);
     qdev_realize(DEVICE(&bmc->soc), NULL, &error_abort);
 
     aspeed_board_init_flashes(&bmc->soc.fmc,
@@ -1372,8 +1387,7 @@ static void aspeed_minibmc_machine_init(MachineState *machine)
 
     object_property_set_link(OBJECT(&bmc->soc), "memory",
                              OBJECT(get_system_memory()), &error_abort);
-    qdev_prop_set_uint32(DEVICE(&bmc->soc), "uart-default",
-                         amc->uart_default);
+    connect_serial_hds_to_uarts(bmc);
     qdev_realize(DEVICE(&bmc->soc), NULL, &error_abort);
 
     aspeed_board_init_flashes(&bmc->soc.fmc,
diff --git a/hw/arm/aspeed_ast10x0.c b/hw/arm/aspeed_ast10x0.c
index 677699e54c..4d0b9b115f 100644
--- a/hw/arm/aspeed_ast10x0.c
+++ b/hw/arm/aspeed_ast10x0.c
@@ -144,6 +144,10 @@ static void aspeed_soc_ast1030_init(Object *obj)
         object_initialize_child(obj, "wdt[*]", &s->wdt[i], typename);
     }
 
+    for (i = 0; i < sc->uarts_num; i++) {
+        object_initialize_child(obj, "uart[*]", &s->uart[i], TYPE_SERIAL_MM);
+    }
+
     snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
     object_initialize_child(obj, "gpio", &s->gpio, typename);
 
@@ -255,7 +259,9 @@ static void aspeed_soc_ast1030_realize(DeviceState *dev_soc, Error **errp)
                                 sc->irqmap[ASPEED_DEV_KCS] + aspeed_lpc_kcs_4));
 
     /* UART */
-    aspeed_soc_uart_init(s);
+    if (!aspeed_soc_uart_realize(s, errp)) {
+        return;
+    }
 
     /* Timer */
     object_property_set_link(OBJECT(&s->timerctrl), "scu", OBJECT(&s->scu),
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 64eb5a7b26..aa2cd90bec 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -214,6 +214,10 @@ static void aspeed_soc_ast2600_init(Object *obj)
         object_initialize_child(obj, "mii[*]", &s->mii[i], TYPE_ASPEED_MII);
     }
 
+    for (i = 0; i < sc->uarts_num; i++) {
+        object_initialize_child(obj, "uart[*]", &s->uart[i], TYPE_SERIAL_MM);
+    }
+
     snprintf(typename, sizeof(typename), TYPE_ASPEED_XDMA "-%s", socname);
     object_initialize_child(obj, "xdma", &s->xdma, typename);
 
@@ -386,7 +390,9 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
                        aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
 
     /* UART */
-    aspeed_soc_uart_init(s);
+    if (!aspeed_soc_uart_realize(s, errp)) {
+        return;
+    }
 
     /* I2C */
     object_property_set_link(OBJECT(&s->i2c), "dram", OBJECT(s->dram_mr),
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 0bb6a2f092..b05b9dd416 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -208,6 +208,10 @@ static void aspeed_soc_init(Object *obj)
                                 TYPE_FTGMAC100);
     }
 
+    for (i = 0; i < sc->uarts_num; i++) {
+        object_initialize_child(obj, "uart[*]", &s->uart[i], TYPE_SERIAL_MM);
+    }
+
     snprintf(typename, sizeof(typename), TYPE_ASPEED_XDMA "-%s", socname);
     object_initialize_child(obj, "xdma", &s->xdma, typename);
 
@@ -315,7 +319,9 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
                        aspeed_soc_get_irq(s, ASPEED_DEV_ADC));
 
     /* UART */
-    aspeed_soc_uart_init(s);
+    if (!aspeed_soc_uart_realize(s, errp)) {
+        return;
+    }
 
     /* I2C */
     object_property_set_link(OBJECT(&s->i2c), "dram", OBJECT(s->dram_mr),
@@ -482,8 +488,6 @@ static Property aspeed_soc_properties[] = {
                      MemoryRegion *),
     DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
                      MemoryRegion *),
-    DEFINE_PROP_UINT32("uart-default", AspeedSoCState, uart_default,
-                       ASPEED_DEV_UART5),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -573,23 +577,37 @@ qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev)
     return ASPEED_SOC_GET_CLASS(s)->get_irq(s, dev);
 }
 
-void aspeed_soc_uart_init(AspeedSoCState *s)
+bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp)
 {
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
-    int i, uart;
-
-    /* Attach an 8250 to the IO space as our UART */
-    serial_mm_init(s->memory, sc->memmap[s->uart_default], 2,
-                   aspeed_soc_get_irq(s, s->uart_default), 38400,
-                   serial_hd(0), DEVICE_LITTLE_ENDIAN);
-    for (i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) {
-        if (uart == s->uart_default) {
-            uart++;
+    SerialMM *smm;
+
+    for (int i = 0, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) {
+        smm = &s->uart[i];
+
+        /* Chardev property is set by the machine. */
+        qdev_prop_set_uint8(DEVICE(smm), "regshift", 2);
+        qdev_prop_set_uint32(DEVICE(smm), "baudbase", 38400);
+        qdev_set_legacy_instance_id(DEVICE(smm), sc->memmap[uart], 2);
+        qdev_prop_set_uint8(DEVICE(smm), "endianness", DEVICE_LITTLE_ENDIAN);
+        if (!sysbus_realize(SYS_BUS_DEVICE(smm), errp)) {
+            return false;
         }
-        serial_mm_init(s->memory, sc->memmap[uart], 2,
-                       aspeed_soc_get_irq(s, uart), 38400,
-                       serial_hd(i), DEVICE_LITTLE_ENDIAN);
+
+        sysbus_connect_irq(SYS_BUS_DEVICE(smm), 0, aspeed_soc_get_irq(s, uart));
+        aspeed_mmio_map(s, SYS_BUS_DEVICE(smm), 0, sc->memmap[uart]);
     }
+
+    return true;
+}
+
+void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr)
+{
+    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+    int i = dev - ASPEED_DEV_UART1;
+
+    g_assert(0 <= i && i < ARRAY_SIZE(s->uart) && i < sc->uarts_num);
+    qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", chr);
 }
 
 /*
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index e65926a667..68e907cd64 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -36,12 +36,14 @@
 #include "hw/misc/aspeed_lpc.h"
 #include "hw/misc/unimp.h"
 #include "hw/misc/aspeed_peci.h"
+#include "hw/char/serial.h"
 
 #define ASPEED_SPIS_NUM  2
 #define ASPEED_EHCIS_NUM 2
 #define ASPEED_WDTS_NUM  4
 #define ASPEED_CPUS_NUM  2
 #define ASPEED_MACS_NUM  4
+#define ASPEED_UARTS_NUM 13
 
 struct AspeedSoCState {
     /*< private >*/
@@ -79,7 +81,7 @@ struct AspeedSoCState {
     AspeedSDHCIState emmc;
     AspeedLPCState lpc;
     AspeedPECIState peci;
-    uint32_t uart_default;
+    SerialMM uart[ASPEED_UARTS_NUM];
     Clock *sysclk;
     UnimplementedDeviceState iomem;
     UnimplementedDeviceState video;
@@ -175,7 +177,8 @@ enum {
 };
 
 qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
-void aspeed_soc_uart_init(AspeedSoCState *s);
+bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp);
+void aspeed_soc_uart_set_chr(AspeedSoCState *s, int dev, Chardev *chr);
 bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp);
 void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr);
 void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
-- 
2.37.0

[PATCH v2 4/9] aspeed: Make aspeed_board_init_flashes public

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 hw/arm/aspeed.c             | 2 +-
 include/hw/arm/aspeed_soc.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index cc395f988c..b2486a9e78 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -262,7 +262,7 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
     rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
 }
 
-static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
+void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
                                       unsigned int count, int unit0)
 {
     int i;
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 68e907cd64..8389200b2d 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -184,5 +184,7 @@ void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr);
 void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
                                    const char *name, hwaddr addr,
                                    uint64_t size);
+void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
+                               unsigned int count, int unit0);
 
 #endif /* ASPEED_SOC_H */
-- 
2.37.0

[PATCH v2 5/9] aspeed: Add fby35 skeleton

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
 MAINTAINERS        |  1 +
 hw/arm/fby35.c     | 39 +++++++++++++++++++++++++++++++++++++++
 hw/arm/meson.build |  3 ++-
 3 files changed, 42 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/fby35.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 450abd0252..fce6161ce5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1065,6 +1065,7 @@ F: hw/net/ftgmac100.c
 F: include/hw/net/ftgmac100.h
 F: docs/system/arm/aspeed.rst
 F: tests/qtest/*aspeed*
+F: hw/arm/fby35.c
 
 NRF51
 M: Joel Stanley <joel@jms.id.au>
diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
new file mode 100644
index 0000000000..03b458584c
--- /dev/null
+++ b/hw/arm/fby35.c
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates. (http://www.meta.com)
+ *
+ * This code is licensed under the GPL version 2 or later. See the COPYING
+ * file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+
+#define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
+OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
+
+struct Fby35State {
+    MachineState parent_obj;
+};
+
+static void fby35_init(MachineState *machine)
+{
+}
+
+static void fby35_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->desc = "Meta Platforms fby35";
+    mc->init = fby35_init;
+}
+
+static const TypeInfo fby35_types[] = {
+    {
+        .name = MACHINE_TYPE_NAME("fby35"),
+        .parent = TYPE_MACHINE,
+        .class_init = fby35_class_init,
+        .instance_size = sizeof(Fby35State),
+    },
+};
+
+DEFINE_TYPES(fby35_types);
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 2d8381339c..92f9f6e000 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -51,7 +51,8 @@ arm_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files(
   'aspeed_soc.c',
   'aspeed.c',
   'aspeed_ast2600.c',
-  'aspeed_ast10x0.c'))
+  'aspeed_ast10x0.c',
+  'fby35.c'))
 arm_ss.add(when: 'CONFIG_MPS2', if_true: files('mps2.c'))
 arm_ss.add(when: 'CONFIG_MPS2', if_true: files('mps2-tz.c'))
 arm_ss.add(when: 'CONFIG_MSF2', if_true: files('msf2-soc.c'))
-- 
2.37.0

[PATCH v2 6/9] aspeed: Add AST2600 (BMC) to fby35

[Peter Delevoryas]

You can test booting the BMC with both '-device loader' and '-drive
file'. This is necessary because of how the fb-openbmc boot sequence
works (jump to 0x20000000 after U-Boot SPL).

    wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
    qemu-system-arm -machine fby35 -nographic \
        -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 hw/arm/fby35.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index 03b458584c..5c5224d374 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -6,17 +6,55 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "sysemu/sysemu.h"
 #include "hw/boards.h"
+#include "hw/arm/aspeed_soc.h"
 
 #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
 OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
 
 struct Fby35State {
     MachineState parent_obj;
+
+    MemoryRegion bmc_memory;
+    MemoryRegion bmc_dram;
+    MemoryRegion bmc_boot_rom;
+
+    AspeedSoCState bmc;
 };
 
+#define FBY35_BMC_RAM_SIZE (2 * GiB)
+
+static void fby35_bmc_init(Fby35State *s)
+{
+    memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
+    memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram",
+                           FBY35_BMC_RAM_SIZE, &error_abort);
+
+    object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3");
+    object_property_set_int(OBJECT(&s->bmc), "ram-size", FBY35_BMC_RAM_SIZE,
+                            &error_abort);
+    object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory),
+                             &error_abort);
+    object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram),
+                             &error_abort);
+    object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0,
+                            &error_abort);
+    object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003,
+                            &error_abort);
+    aspeed_soc_uart_set_chr(&s->bmc, ASPEED_DEV_UART5, serial_hd(0));
+    qdev_realize(DEVICE(&s->bmc), NULL, &error_abort);
+
+    aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0);
+}
+
 static void fby35_init(MachineState *machine)
 {
+    Fby35State *s = FBY35(machine);
+
+    fby35_bmc_init(s);
 }
 
 static void fby35_class_init(ObjectClass *oc, void *data)
@@ -25,6 +63,9 @@ static void fby35_class_init(ObjectClass *oc, void *data)
 
     mc->desc = "Meta Platforms fby35";
     mc->init = fby35_init;
+    mc->no_floppy = 1;
+    mc->no_cdrom = 1;
+    mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
 }
 
 static const TypeInfo fby35_types[] = {
-- 
2.37.0

Re: [PATCH v2 6/9] aspeed: Add AST2600 (BMC) to fby35

[Cédric Le Goater]

On 7/5/22 21:13, Peter Delevoryas wrote:
> You can test booting the BMC with both '-device loader' and '-drive
> file'. This is necessary because of how the fb-openbmc boot sequence
> works (jump to 0x20000000 after U-Boot SPL).
> 
>      wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
>      qemu-system-arm -machine fby35 -nographic \
>          -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd
> 
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> ---
>   hw/arm/fby35.c | 41 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
> 
> diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
> index 03b458584c..5c5224d374 100644
> --- a/hw/arm/fby35.c
> +++ b/hw/arm/fby35.c
> @@ -6,17 +6,55 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "qapi/error.h"
> +#include "sysemu/sysemu.h"
>   #include "hw/boards.h"
> +#include "hw/arm/aspeed_soc.h"
>   
>   #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
>   OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
>   
>   struct Fby35State {
>       MachineState parent_obj;
> +
> +    MemoryRegion bmc_memory;
> +    MemoryRegion bmc_dram;
> +    MemoryRegion bmc_boot_rom;
> +
> +    AspeedSoCState bmc;
>   };
>   
> +#define FBY35_BMC_RAM_SIZE (2 * GiB)
> +
> +static void fby35_bmc_init(Fby35State *s)
> +{
> +    memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
> +    memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram",
> +                           FBY35_BMC_RAM_SIZE, &error_abort);

A MachineState object is used as a owner of the RAM region and this
should assert in memory_region_init_ram() :

     /* This will assert if owner is neither NULL nor a DeviceState.
      * We only want the owner here for the purposes of defining a
      * unique name for migration. TODO: Ideally we should implement
      * a naming scheme for Objects which are not DeviceStates, in
      * which case we can relax this restriction.
      */
     owner_dev = DEVICE(owner);

It went unnoticed until I started experimenting with some MachineState
modifications. CONFIG_QOM_CAST_DEBUG needs to be defined to catch the
error. I would have thought that CI was doing this check. It seems not,
which is surprising.

Anyhow, this needs a fix for 7.1 and I will work on it.

C.

> +
> +    object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3");
> +    object_property_set_int(OBJECT(&s->bmc), "ram-size", FBY35_BMC_RAM_SIZE,
> +                            &error_abort);
> +    object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory),
> +                             &error_abort);
> +    object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram),
> +                             &error_abort);
> +    object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0,
> +                            &error_abort);
> +    object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003,
> +                            &error_abort);
> +    aspeed_soc_uart_set_chr(&s->bmc, ASPEED_DEV_UART5, serial_hd(0));
> +    qdev_realize(DEVICE(&s->bmc), NULL, &error_abort);
> +
> +    aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0);
> +}
> +
>   static void fby35_init(MachineState *machine)
>   {
> +    Fby35State *s = FBY35(machine);
> +
> +    fby35_bmc_init(s);
>   }
>   
>   static void fby35_class_init(ObjectClass *oc, void *data)
> @@ -25,6 +63,9 @@ static void fby35_class_init(ObjectClass *oc, void *data)
>   
>       mc->desc = "Meta Platforms fby35";
>       mc->init = fby35_init;
> +    mc->no_floppy = 1;
> +    mc->no_cdrom = 1;
> +    mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
>   }
>   
>   static const TypeInfo fby35_types[] = {

Re: [PATCH v2 6/9] aspeed: Add AST2600 (BMC) to fby35

[Peter Delevoryas]

On Wed, Jul 27, 2022 at 12:05:58PM +0200, Cédric Le Goater wrote:
> On 7/5/22 21:13, Peter Delevoryas wrote:
> > You can test booting the BMC with both '-device loader' and '-drive
> > file'. This is necessary because of how the fb-openbmc boot sequence
> > works (jump to 0x20000000 after U-Boot SPL).
> > 
> >      wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> >      qemu-system-arm -machine fby35 -nographic \
> >          -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd
> > 
> > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > ---
> >   hw/arm/fby35.c | 41 +++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 41 insertions(+)
> > 
> > diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
> > index 03b458584c..5c5224d374 100644
> > --- a/hw/arm/fby35.c
> > +++ b/hw/arm/fby35.c
> > @@ -6,17 +6,55 @@
> >    */
> >   #include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > +#include "qapi/error.h"
> > +#include "sysemu/sysemu.h"
> >   #include "hw/boards.h"
> > +#include "hw/arm/aspeed_soc.h"
> >   #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
> >   OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
> >   struct Fby35State {
> >       MachineState parent_obj;
> > +
> > +    MemoryRegion bmc_memory;
> > +    MemoryRegion bmc_dram;
> > +    MemoryRegion bmc_boot_rom;
> > +
> > +    AspeedSoCState bmc;
> >   };
> > +#define FBY35_BMC_RAM_SIZE (2 * GiB)
> > +
> > +static void fby35_bmc_init(Fby35State *s)
> > +{
> > +    memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
> > +    memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram",
> > +                           FBY35_BMC_RAM_SIZE, &error_abort);
> 
> A MachineState object is used as a owner of the RAM region and this
> should assert in memory_region_init_ram() :
> 
>     /* This will assert if owner is neither NULL nor a DeviceState.
>      * We only want the owner here for the purposes of defining a
>      * unique name for migration. TODO: Ideally we should implement
>      * a naming scheme for Objects which are not DeviceStates, in
>      * which case we can relax this restriction.
>      */
>     owner_dev = DEVICE(owner);
> 
> It went unnoticed until I started experimenting with some MachineState
> modifications. CONFIG_QOM_CAST_DEBUG needs to be defined to catch the
> error. I would have thought that CI was doing this check. It seems not,
> which is surprising.

Hmmm! I see, didn't realize this was a requirement. Thanks for catching it!

> 
> Anyhow, this needs a fix for 7.1 and I will work on it.

I see, yes, thanks!!

> 
> C.
> 
> > +
> > +    object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3");
> > +    object_property_set_int(OBJECT(&s->bmc), "ram-size", FBY35_BMC_RAM_SIZE,
> > +                            &error_abort);
> > +    object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory),
> > +                             &error_abort);
> > +    object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram),
> > +                             &error_abort);
> > +    object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0,
> > +                            &error_abort);
> > +    object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003,
> > +                            &error_abort);
> > +    aspeed_soc_uart_set_chr(&s->bmc, ASPEED_DEV_UART5, serial_hd(0));
> > +    qdev_realize(DEVICE(&s->bmc), NULL, &error_abort);
> > +
> > +    aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0);
> > +}
> > +
> >   static void fby35_init(MachineState *machine)
> >   {
> > +    Fby35State *s = FBY35(machine);
> > +
> > +    fby35_bmc_init(s);
> >   }
> >   static void fby35_class_init(ObjectClass *oc, void *data)
> > @@ -25,6 +63,9 @@ static void fby35_class_init(ObjectClass *oc, void *data)
> >       mc->desc = "Meta Platforms fby35";
> >       mc->init = fby35_init;
> > +    mc->no_floppy = 1;
> > +    mc->no_cdrom = 1;
> > +    mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
> >   }
> >   static const TypeInfo fby35_types[] = {
> 

[PATCH v2 7/9] aspeed: fby35: Add a bootrom for the BMC

[Peter Delevoryas]

From: Cédric Le Goater <clg@kaod.org>

The BMC boots from the first flash device by fetching instructions
from the flash contents. Add an alias region on 0x0 for this
purpose. There are currently performance issues with this method (TBs
being flushed too often), so as a faster alternative, install the
flash contents as a ROM in the BMC memory space.

See commit 1a15311a12fa ("hw/arm/aspeed: add a 'execute-in-place'
property to boot directly from CE0")

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 hw/arm/fby35.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index 5c5224d374..d3edfa3b10 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -9,6 +9,7 @@
 #include "qemu/units.h"
 #include "qapi/error.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/block-backend.h"
 #include "hw/boards.h"
 #include "hw/arm/aspeed_soc.h"
 
@@ -23,12 +24,49 @@ struct Fby35State {
     MemoryRegion bmc_boot_rom;
 
     AspeedSoCState bmc;
+
+    bool mmio_exec;
 };
 
 #define FBY35_BMC_RAM_SIZE (2 * GiB)
+#define FBY35_BMC_FIRMWARE_ADDR 0x0
+
+static void fby35_bmc_write_boot_rom(DriveInfo *dinfo, MemoryRegion *mr,
+                                     hwaddr offset, size_t rom_size,
+                                     Error **errp)
+{
+    BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
+    g_autofree void *storage = NULL;
+    int64_t size;
+
+    /*
+     * The block backend size should have already been 'validated' by
+     * the creation of the m25p80 object.
+     */
+    size = blk_getlength(blk);
+    if (size <= 0) {
+        error_setg(errp, "failed to get flash size");
+        return;
+    }
+
+    if (rom_size > size) {
+        rom_size = size;
+    }
+
+    storage = g_malloc0(rom_size);
+    if (blk_pread(blk, 0, storage, rom_size) < 0) {
+        error_setg(errp, "failed to read the initial flash content");
+        return;
+    }
+
+    /* TODO: find a better way to install the ROM */
+    memcpy(memory_region_get_ram_ptr(mr) + offset, storage, rom_size);
+}
 
 static void fby35_bmc_init(Fby35State *s)
 {
+    DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
+
     memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX);
     memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram",
                            FBY35_BMC_RAM_SIZE, &error_abort);
@@ -48,6 +86,28 @@ static void fby35_bmc_init(Fby35State *s)
     qdev_realize(DEVICE(&s->bmc), NULL, &error_abort);
 
     aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0);
+
+    /* Install first FMC flash content as a boot rom. */
+    if (drive0) {
+        AspeedSMCFlash *fl = &s->bmc.fmc.flashes[0];
+        MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
+        uint64_t size = memory_region_size(&fl->mmio);
+
+        if (s->mmio_exec) {
+            memory_region_init_alias(boot_rom, NULL, "aspeed.boot_rom",
+                                     &fl->mmio, 0, size);
+            memory_region_add_subregion(&s->bmc_memory, FBY35_BMC_FIRMWARE_ADDR,
+                                        boot_rom);
+        } else {
+
+            memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom",
+                                   size, &error_abort);
+            memory_region_add_subregion(&s->bmc_memory, FBY35_BMC_FIRMWARE_ADDR,
+                                        boot_rom);
+            fby35_bmc_write_boot_rom(drive0, boot_rom, FBY35_BMC_FIRMWARE_ADDR,
+                                     size, &error_abort);
+        }
+    }
 }
 
 static void fby35_init(MachineState *machine)
@@ -57,6 +117,22 @@ static void fby35_init(MachineState *machine)
     fby35_bmc_init(s);
 }
 
+
+static bool fby35_get_mmio_exec(Object *obj, Error **errp)
+{
+    return FBY35(obj)->mmio_exec;
+}
+
+static void fby35_set_mmio_exec(Object *obj, bool value, Error **errp)
+{
+    FBY35(obj)->mmio_exec = value;
+}
+
+static void fby35_instance_init(Object *obj)
+{
+    FBY35(obj)->mmio_exec = false;
+}
+
 static void fby35_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
@@ -66,6 +142,12 @@ static void fby35_class_init(ObjectClass *oc, void *data)
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
     mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
+
+    object_class_property_add_bool(oc, "execute-in-place",
+                                   fby35_get_mmio_exec,
+                                   fby35_set_mmio_exec);
+    object_class_property_set_description(oc, "execute-in-place",
+                           "boot directly from CE0 flash device");
 }
 
 static const TypeInfo fby35_types[] = {
@@ -74,6 +156,7 @@ static const TypeInfo fby35_types[] = {
         .parent = TYPE_MACHINE,
         .class_init = fby35_class_init,
         .instance_size = sizeof(Fby35State),
+        .instance_init = fby35_instance_init,
     },
 };
 
-- 
2.37.0

[PATCH v2 8/9] aspeed: Add AST1030 (BIC) to fby35

[Peter Delevoryas]

With the BIC, the easiest way to run everything is to create two pty's
for each SoC and reserve stdin/stdout for the monitor:

    wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
    wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf
    qemu-system-arm -machine fby35 \
        -drive file=fby35.mtd,format=raw,if=mtd \
        -device loader,file=fby35.mtd,addr=0,cpu-num=0 \
        -serial pty -serial pty -serial mon:stdio -display none -S

    screen /dev/ttys0
    screen /dev/ttys1
    (qemu) c

This commit only adds the the first server board's Bridge IC, but in the
future we'll try to include the other three server board Bridge IC's
too.

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 hw/arm/fby35.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c
index d3edfa3b10..031602800f 100644
--- a/hw/arm/fby35.c
+++ b/hw/arm/fby35.c
@@ -11,7 +11,9 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/block-backend.h"
 #include "hw/boards.h"
+#include "hw/qdev-clock.h"
 #include "hw/arm/aspeed_soc.h"
+#include "hw/arm/boot.h"
 
 #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35")
 OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35);
@@ -22,8 +24,11 @@ struct Fby35State {
     MemoryRegion bmc_memory;
     MemoryRegion bmc_dram;
     MemoryRegion bmc_boot_rom;
+    MemoryRegion bic_memory;
+    Clock *bic_sysclk;
 
     AspeedSoCState bmc;
+    AspeedSoCState bic;
 
     bool mmio_exec;
 };
@@ -110,11 +115,31 @@ static void fby35_bmc_init(Fby35State *s)
     }
 }
 
+static void fby35_bic_init(Fby35State *s)
+{
+    s->bic_sysclk = clock_new(OBJECT(s), "SYSCLK");
+    clock_set_hz(s->bic_sysclk, 200000000ULL);
+
+    memory_region_init(&s->bic_memory, OBJECT(s), "bic-memory", UINT64_MAX);
+
+    object_initialize_child(OBJECT(s), "bic", &s->bic, "ast1030-a1");
+    qdev_connect_clock_in(DEVICE(&s->bic), "sysclk", s->bic_sysclk);
+    object_property_set_link(OBJECT(&s->bic), "memory", OBJECT(&s->bic_memory),
+                             &error_abort);
+    aspeed_soc_uart_set_chr(&s->bic, ASPEED_DEV_UART5, serial_hd(1));
+    qdev_realize(DEVICE(&s->bic), NULL, &error_abort);
+
+    aspeed_board_init_flashes(&s->bic.fmc, "sst25vf032b", 2, 2);
+    aspeed_board_init_flashes(&s->bic.spi[0], "sst25vf032b", 2, 4);
+    aspeed_board_init_flashes(&s->bic.spi[1], "sst25vf032b", 2, 6);
+}
+
 static void fby35_init(MachineState *machine)
 {
     Fby35State *s = FBY35(machine);
 
     fby35_bmc_init(s);
+    fby35_bic_init(s);
 }
 
 
@@ -141,7 +166,7 @@ static void fby35_class_init(ObjectClass *oc, void *data)
     mc->init = fby35_init;
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
-    mc->min_cpus = mc->max_cpus = mc->default_cpus = 2;
+    mc->min_cpus = mc->max_cpus = mc->default_cpus = 3;
 
     object_class_property_add_bool(oc, "execute-in-place",
                                    fby35_get_mmio_exec,
-- 
2.37.0

[PATCH v2 9/9] docs: aspeed: Add fby35 multi-SoC machine section

[Peter Delevoryas]

Signed-off-by: Peter Delevoryas <peter@pjd.dev>
---
 docs/system/arm/aspeed.rst | 48 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
index 5d0a7865d3..b233191b67 100644
--- a/docs/system/arm/aspeed.rst
+++ b/docs/system/arm/aspeed.rst
@@ -136,6 +136,54 @@ AST1030 SoC based machines :
 
 - ``ast1030-evb``          Aspeed AST1030 Evaluation board (Cortex-M4F)
 
+Facebook Yosemite v3.5 Platform and CraterLake Server (``fby35``)
+==================================================================
+
+Facebook has a series of multi-node compute server designs named
+Yosemite. The most recent version released was
+`Yosemite v3 <https://www.opencompute.org/documents/ocp-yosemite-v3-platform-design-specification-1v16-pdf>`.
+
+Yosemite v3.5 is an iteration on this design, and is very similar: there's a
+baseboard with a BMC, and 4 server slots. The new server board design termed
+"CraterLake" includes a Bridge IC (BIC), with room for expansion boards to
+include various compute accelerators (video, inferencing, etc). At the moment,
+only the first server slot's BIC is included.
+
+Yosemite v3.5 is itself a sled which fits into a 40U chassis, and 3 sleds
+can be fit into a chassis. See `here <https://www.opencompute.org/products/423/wiwynn-yosemite-v3-server>`
+for an example.
+
+In this generation, the BMC is an AST2600 and each BIC is an AST1030. The BMC
+runs `OpenBMC <https://github.com/facebook/openbmc>`, and the BIC runs
+`OpenBIC <https://github.com/facebook/openbic>`.
+
+Firmware images can be retrieved from the Github releases or built from the
+source code, see the README's for instructions on that. This image uses the
+"fby35" machine recipe from OpenBMC, and the "yv35-cl" target from OpenBIC.
+Some reference images can also be found here:
+
+.. code-block:: bash
+
+    $ wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
+    $ wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf
+
+Since this machine has multiple SoC's, each with their own serial console, the
+recommended way to run it is to allocate a pseudoterminal for each serial
+console and let the monitor use stdio. Also, starting in a paused state is
+useful because it allows you to attach to the pseudoterminals before the boot
+process starts.
+
+.. code-block:: bash
+
+    $ qemu-system-arm -machine fby35 \
+        -drive file=fby35.mtd,format=raw,if=mtd \
+        -device loader,file=Y35BCL.elf,addr=0,cpu-num=2 \
+        -serial pty -serial pty -serial mon:stdio \
+        -display none -S
+    $ screen /dev/tty0 # In a separate TMUX pane, terminal window, etc.
+    $ screen /dev/tty1
+    $ (qemu) c		   # Start the boot process once screen is setup.
+
 Supported devices
 -----------------
 
-- 
2.37.0

Re: [PATCH v2 9/9] docs: aspeed: Add fby35 multi-SoC machine section

[Cédric Le Goater]

On 7/5/22 21:14, Peter Delevoryas wrote:
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>

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

I fixed inline the URL links and moved the section at the end of the file.

Thanks,

C.


> ---
>   docs/system/arm/aspeed.rst | 48 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
> 
> diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> index 5d0a7865d3..b233191b67 100644
> --- a/docs/system/arm/aspeed.rst
> +++ b/docs/system/arm/aspeed.rst
> @@ -136,6 +136,54 @@ AST1030 SoC based machines :
>   
>   - ``ast1030-evb``          Aspeed AST1030 Evaluation board (Cortex-M4F)
>   
> +Facebook Yosemite v3.5 Platform and CraterLake Server (``fby35``)
> +==================================================================
> +
> +Facebook has a series of multi-node compute server designs named
> +Yosemite. The most recent version released was
> +`Yosemite v3 <https://www.opencompute.org/documents/ocp-yosemite-v3-platform-design-specification-1v16-pdf>`.
> +
> +Yosemite v3.5 is an iteration on this design, and is very similar: there's a
> +baseboard with a BMC, and 4 server slots. The new server board design termed
> +"CraterLake" includes a Bridge IC (BIC), with room for expansion boards to
> +include various compute accelerators (video, inferencing, etc). At the moment,
> +only the first server slot's BIC is included.
> +
> +Yosemite v3.5 is itself a sled which fits into a 40U chassis, and 3 sleds
> +can be fit into a chassis. See `here <https://www.opencompute.org/products/423/wiwynn-yosemite-v3-server>`
> +for an example.
> +
> +In this generation, the BMC is an AST2600 and each BIC is an AST1030. The BMC
> +runs `OpenBMC <https://github.com/facebook/openbmc>`, and the BIC runs
> +`OpenBIC <https://github.com/facebook/openbic>`.
> +
> +Firmware images can be retrieved from the Github releases or built from the
> +source code, see the README's for instructions on that. This image uses the
> +"fby35" machine recipe from OpenBMC, and the "yv35-cl" target from OpenBIC.
> +Some reference images can also be found here:
> +
> +.. code-block:: bash
> +
> +    $ wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> +    $ wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf
> +
> +Since this machine has multiple SoC's, each with their own serial console, the
> +recommended way to run it is to allocate a pseudoterminal for each serial
> +console and let the monitor use stdio. Also, starting in a paused state is
> +useful because it allows you to attach to the pseudoterminals before the boot
> +process starts.
> +
> +.. code-block:: bash
> +
> +    $ qemu-system-arm -machine fby35 \
> +        -drive file=fby35.mtd,format=raw,if=mtd \
> +        -device loader,file=Y35BCL.elf,addr=0,cpu-num=2 \
> +        -serial pty -serial pty -serial mon:stdio \
> +        -display none -S
> +    $ screen /dev/tty0 # In a separate TMUX pane, terminal window, etc.
> +    $ screen /dev/tty1
> +    $ (qemu) c		   # Start the boot process once screen is setup.
> +
>   Supported devices
>   -----------------
>   

Re: [PATCH v2 9/9] docs: aspeed: Add fby35 multi-SoC machine section

[Peter Delevoryas]

On Wed, Jul 06, 2022 at 07:58:44AM +0200, Cédric Le Goater wrote:
> On 7/5/22 21:14, Peter Delevoryas wrote:
> > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> 
> I fixed inline the URL links and moved the section at the end of the file.
> 
> Thanks,
> 
> C.

Thanks for that!

> 
> > ---
> >   docs/system/arm/aspeed.rst | 48 ++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 48 insertions(+)
> > 
> > diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> > index 5d0a7865d3..b233191b67 100644
> > --- a/docs/system/arm/aspeed.rst
> > +++ b/docs/system/arm/aspeed.rst
> > @@ -136,6 +136,54 @@ AST1030 SoC based machines :
> >   - ``ast1030-evb``          Aspeed AST1030 Evaluation board (Cortex-M4F)
> > +Facebook Yosemite v3.5 Platform and CraterLake Server (``fby35``)
> > +==================================================================
> > +
> > +Facebook has a series of multi-node compute server designs named
> > +Yosemite. The most recent version released was
> > +`Yosemite v3 <https://www.opencompute.org/documents/ocp-yosemite-v3-platform-design-specification-1v16-pdf>`.
> > +
> > +Yosemite v3.5 is an iteration on this design, and is very similar: there's a
> > +baseboard with a BMC, and 4 server slots. The new server board design termed
> > +"CraterLake" includes a Bridge IC (BIC), with room for expansion boards to
> > +include various compute accelerators (video, inferencing, etc). At the moment,
> > +only the first server slot's BIC is included.
> > +
> > +Yosemite v3.5 is itself a sled which fits into a 40U chassis, and 3 sleds
> > +can be fit into a chassis. See `here <https://www.opencompute.org/products/423/wiwynn-yosemite-v3-server>`
> > +for an example.
> > +
> > +In this generation, the BMC is an AST2600 and each BIC is an AST1030. The BMC
> > +runs `OpenBMC <https://github.com/facebook/openbmc>`, and the BIC runs
> > +`OpenBIC <https://github.com/facebook/openbic>`.
> > +
> > +Firmware images can be retrieved from the Github releases or built from the
> > +source code, see the README's for instructions on that. This image uses the
> > +"fby35" machine recipe from OpenBMC, and the "yv35-cl" target from OpenBIC.
> > +Some reference images can also be found here:
> > +
> > +.. code-block:: bash
> > +
> > +    $ wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> > +    $ wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf
> > +
> > +Since this machine has multiple SoC's, each with their own serial console, the
> > +recommended way to run it is to allocate a pseudoterminal for each serial
> > +console and let the monitor use stdio. Also, starting in a paused state is
> > +useful because it allows you to attach to the pseudoterminals before the boot
> > +process starts.
> > +
> > +.. code-block:: bash
> > +
> > +    $ qemu-system-arm -machine fby35 \
> > +        -drive file=fby35.mtd,format=raw,if=mtd \
> > +        -device loader,file=Y35BCL.elf,addr=0,cpu-num=2 \
> > +        -serial pty -serial pty -serial mon:stdio \
> > +        -display none -S
> > +    $ screen /dev/tty0 # In a separate TMUX pane, terminal window, etc.
> > +    $ screen /dev/tty1
> > +    $ (qemu) c		   # Start the boot process once screen is setup.
> > +
> >   Supported devices
> >   -----------------
> 

Re: [PATCH v2 9/9] docs: aspeed: Add fby35 multi-SoC machine section

[Joel Stanley]

On Tue, 5 Jul 2022 at 19:14, Peter Delevoryas <peter@pjd.dev> wrote:
>
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>

Reviewed-by: Joel Stanley <joel@jms.id.au>

> ---
>  docs/system/arm/aspeed.rst | 48 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> index 5d0a7865d3..b233191b67 100644
> --- a/docs/system/arm/aspeed.rst
> +++ b/docs/system/arm/aspeed.rst
> @@ -136,6 +136,54 @@ AST1030 SoC based machines :
>
>  - ``ast1030-evb``          Aspeed AST1030 Evaluation board (Cortex-M4F)
>
> +Facebook Yosemite v3.5 Platform and CraterLake Server (``fby35``)
> +==================================================================
> +
> +Facebook has a series of multi-node compute server designs named
> +Yosemite. The most recent version released was
> +`Yosemite v3 <https://www.opencompute.org/documents/ocp-yosemite-v3-platform-design-specification-1v16-pdf>`.
> +
> +Yosemite v3.5 is an iteration on this design, and is very similar: there's a
> +baseboard with a BMC, and 4 server slots. The new server board design termed
> +"CraterLake" includes a Bridge IC (BIC), with room for expansion boards to
> +include various compute accelerators (video, inferencing, etc). At the moment,
> +only the first server slot's BIC is included.
> +
> +Yosemite v3.5 is itself a sled which fits into a 40U chassis, and 3 sleds
> +can be fit into a chassis. See `here <https://www.opencompute.org/products/423/wiwynn-yosemite-v3-server>`
> +for an example.
> +
> +In this generation, the BMC is an AST2600 and each BIC is an AST1030. The BMC
> +runs `OpenBMC <https://github.com/facebook/openbmc>`, and the BIC runs
> +`OpenBIC <https://github.com/facebook/openbic>`.
> +
> +Firmware images can be retrieved from the Github releases or built from the
> +source code, see the README's for instructions on that. This image uses the
> +"fby35" machine recipe from OpenBMC, and the "yv35-cl" target from OpenBIC.
> +Some reference images can also be found here:
> +
> +.. code-block:: bash
> +
> +    $ wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> +    $ wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf
> +
> +Since this machine has multiple SoC's, each with their own serial console, the
> +recommended way to run it is to allocate a pseudoterminal for each serial
> +console and let the monitor use stdio. Also, starting in a paused state is
> +useful because it allows you to attach to the pseudoterminals before the boot
> +process starts.
> +
> +.. code-block:: bash
> +
> +    $ qemu-system-arm -machine fby35 \
> +        -drive file=fby35.mtd,format=raw,if=mtd \
> +        -device loader,file=Y35BCL.elf,addr=0,cpu-num=2 \

I came across a quirk of the qemu commandline when testing.

-drive knows how to expand ~ in a path, but -device loader does not.
Something for someone to look into on a rainy day!

eg:

$ build/qemu-system-arm -M fby35 -drive
file=~/tmp/fby35.mtd,format=raw,if=mtd -device
loader,file=~/tmp/Y35BCL.elf,addr=0,cpu-num=2 -serial pty -serial pty
-serial mon:stdio -display none -S
char device redirected to /dev/pts/3 (label serial0)
char device redirected to /dev/pts/5 (label serial1)
qemu-system-arm: warning: Aspeed iBT has no chardev backend
~/tmp/Y35BCL.elf: No such file or directory
qemu-system-arm: -device
loader,file=~/tmp/Y35BCL.elf,addr=0,cpu-num=2: Cannot load specified
image ~/tmp/Y35BCL.elf

loader uses open(2) in load_elf_ram_sym.

The call stack for -drive looks like this (using a bad path to make it
easier to identify what's going on):

#0  __libc_open64 (file=file@entry=0xaaaaac009250
"/home/joel/tmp/fby35.mtda", oflag=oflag@entry=524288) at
../sysdeps/unix/sysv/linux/open64.c:37
#1  0x0000aaaaab3b18bc in open64 (__oflag=524288,
__path=0xaaaaac009250 "/home/joel/tmp/fby35.mtda") at
/usr/include/aarch64-linux-gnu/bits/fcntl2.h:59
#2  qemu_open_cloexec (mode=0, flags=0, name=0xaaaaac009250
"/home/joel/tmp/fby35.mtda") at ../util/osdep.c:286
#3  qemu_open_internal (name=name@entry=0xaaaaac009250
"/home/joel/tmp/fby35.mtda", flags=flags@entry=0, mode=mode@entry=0,
errp=errp@entry=0xffffffffeb70) at ../util/osdep.c:330
#4  0x0000aaaaab3b1f30 in qemu_open (name=name@entry=0xaaaaac009250
"/home/joel/tmp/fby35.mtda", flags=flags@entry=0,
errp=errp@entry=0xffffffffeb70) at ../util/osdep.c:360
#5  0x0000aaaaab30d9d8 in raw_open_common (bs=0xaaaaac002c40,
options=<optimized out>, bdrv_flags=155650, open_flags=<optimized
out>, device=<optimized out>, errp=0xffffffffeb70)
    at ../block/file-posix.c:680
#6  0x0000aaaaab2a53d0 in bdrv_open_driver
(bs=bs@entry=0xaaaaac002c40, drv=drv@entry=0xaaaaabc13250 <bdrv_file>,
node_name=<optimized out>, options=options@entry=0xaaaaac008230,
    open_flags=open_flags@entry=155650,
errp=errp@entry=0xffffffffec18) at ../block.c:1625
#7  0x0000aaaaab2a9454 in bdrv_open_common (errp=0xffffffffec18,
options=0xaaaaac008230, file=0x0, bs=0xaaaaac002c40) at
../block.c:1922
#8  bdrv_open_inherit (filename=<optimized out>,
filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
reference=reference@entry=0x0, options=0xaaaaac008230,
flags=<optimized out>,
    flags@entry=0, parent=parent@entry=0xaaaaabffb580,
child_class=child_class@entry=0xaaaaab864498 <child_of_bds>,
child_role=child_role@entry=19, errp=errp@entry=0xffffffffed48)
    at ../block.c:3991
#9  0x0000aaaaab2aa35c in bdrv_open_child_bs
(filename=filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
options=options@entry=0xaaaaac000a50,
    bdref_key=bdref_key@entry=0xaaaaab573010 "file",
parent=parent@entry=0xaaaaabffb580,
child_class=child_class@entry=0xaaaaab864498 <child_of_bds>,
child_role=child_role@entry=19,
    allow_none=allow_none@entry=true, errp=errp@entry=0xffffffffed48)
at ../block.c:3624
#10 0x0000aaaaab2a98e4 in bdrv_open_inherit
(filename=filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
reference=reference@entry=0x0, options=0xaaaaac000a50,
    options@entry=0xaaaaabff8b70, flags=<optimized out>,
parent=parent@entry=0x0, child_class=child_class@entry=0x0,
child_role=child_role@entry=0,
    errp=errp@entry=0xaaaaabc62368 <error_fatal>) at ../block.c:3938
#11 0x0000aaaaab2aa758 in bdrv_open
(filename=filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
reference=reference@entry=0x0, options=options@entry=0xaaaaabff8b70,
    flags=<optimized out>, errp=errp@entry=0xaaaaabc62368
<error_fatal>) at ../block.c:4086
#12 0x0000aaaaab2c59c8 in blk_new_open
(filename=filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
reference=reference@entry=0x0, options=options@entry=0xaaaaabff8b70,
    flags=<optimized out>, errp=errp@entry=0xaaaaabc62368
<error_fatal>) at ../block/block-backend.c:454
#13 0x0000aaaaab2956c8 in blockdev_init
(file=file@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
bs_opts=bs_opts@entry=0xaaaaabff8b70, errp=errp@entry=0xaaaaabc62368
<error_fatal>)
    at ../blockdev.c:592
#14 0x0000aaaaab296884 in drive_new (all_opts=0xaaaaabd1a3b0,
block_default_type=<optimized out>, errp=0xaaaaabc62368 <error_fatal>)
at ../blockdev.c:981
#15 0x0000aaaaaafea1e8 in drive_init_func (opaque=<optimized out>,
opts=<optimized out>, errp=<optimized out>) at ../softmmu/vl.c:648
#16 0x0000aaaaab3c0a74 in qemu_opts_foreach (list=<optimized out>,
func=func@entry=0xaaaaaafea1d0 <drive_init_func>,
opaque=opaque@entry=0xaaaaabf3eb58,
    errp=errp@entry=0xaaaaabc62368 <error_fatal>) at ../util/qemu-option.c:1135
#17 0x0000aaaaaafee9d8 in configure_blockdev (bdo_queue=0xaaaaabb866c0
<bdo_queue>, snapshot=0, machine_class=0xaaaaabf3eab0) at
../softmmu/vl.c:707
#18 qemu_create_early_backends () at ../softmmu/vl.c:1882
#19 qemu_init (argc=<optimized out>, argv=0xfffffffff358,
envp=envp@entry=0x0) at ../softmmu/vl.c:3502
#20 0x0000aaaaaadb4300 in qemu_main (envp=0x0, argv=<optimized out>,
argc=<optimized out>) at ../softmmu/main.c:35
#21 main (argc=<optimized out>, argv=<optimized out>) at ../softmmu/main.c:45

and that was where I lost interest.


> +        -serial pty -serial pty -serial mon:stdio \
> +        -display none -S
> +    $ screen /dev/tty0 # In a separate TMUX pane, terminal window, etc.
> +    $ screen /dev/tty1
> +    $ (qemu) c            # Start the boot process once screen is setup.
> +
>  Supported devices
>  -----------------
>
> --
> 2.37.0
>

Re: [PATCH v2 9/9] docs: aspeed: Add fby35 multi-SoC machine section

[Peter Delevoryas]

On Wed, Jul 06, 2022 at 08:21:12AM +0000, Joel Stanley wrote:
> On Tue, 5 Jul 2022 at 19:14, Peter Delevoryas <peter@pjd.dev> wrote:
> >
> > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> 
> > ---
> >  docs/system/arm/aspeed.rst | 48 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >
> > diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst
> > index 5d0a7865d3..b233191b67 100644
> > --- a/docs/system/arm/aspeed.rst
> > +++ b/docs/system/arm/aspeed.rst
> > @@ -136,6 +136,54 @@ AST1030 SoC based machines :
> >
> >  - ``ast1030-evb``          Aspeed AST1030 Evaluation board (Cortex-M4F)
> >
> > +Facebook Yosemite v3.5 Platform and CraterLake Server (``fby35``)
> > +==================================================================
> > +
> > +Facebook has a series of multi-node compute server designs named
> > +Yosemite. The most recent version released was
> > +`Yosemite v3 <https://www.opencompute.org/documents/ocp-yosemite-v3-platform-design-specification-1v16-pdf>`.
> > +
> > +Yosemite v3.5 is an iteration on this design, and is very similar: there's a
> > +baseboard with a BMC, and 4 server slots. The new server board design termed
> > +"CraterLake" includes a Bridge IC (BIC), with room for expansion boards to
> > +include various compute accelerators (video, inferencing, etc). At the moment,
> > +only the first server slot's BIC is included.
> > +
> > +Yosemite v3.5 is itself a sled which fits into a 40U chassis, and 3 sleds
> > +can be fit into a chassis. See `here <https://www.opencompute.org/products/423/wiwynn-yosemite-v3-server>`
> > +for an example.
> > +
> > +In this generation, the BMC is an AST2600 and each BIC is an AST1030. The BMC
> > +runs `OpenBMC <https://github.com/facebook/openbmc>`, and the BIC runs
> > +`OpenBIC <https://github.com/facebook/openbic>`.
> > +
> > +Firmware images can be retrieved from the Github releases or built from the
> > +source code, see the README's for instructions on that. This image uses the
> > +"fby35" machine recipe from OpenBMC, and the "yv35-cl" target from OpenBIC.
> > +Some reference images can also be found here:
> > +
> > +.. code-block:: bash
> > +
> > +    $ wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd
> > +    $ wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf
> > +
> > +Since this machine has multiple SoC's, each with their own serial console, the
> > +recommended way to run it is to allocate a pseudoterminal for each serial
> > +console and let the monitor use stdio. Also, starting in a paused state is
> > +useful because it allows you to attach to the pseudoterminals before the boot
> > +process starts.
> > +
> > +.. code-block:: bash
> > +
> > +    $ qemu-system-arm -machine fby35 \
> > +        -drive file=fby35.mtd,format=raw,if=mtd \
> > +        -device loader,file=Y35BCL.elf,addr=0,cpu-num=2 \
> 
> I came across a quirk of the qemu commandline when testing.
> 
> -drive knows how to expand ~ in a path, but -device loader does not.
> Something for someone to look into on a rainy day!
> 
> eg:
> 
> $ build/qemu-system-arm -M fby35 -drive
> file=~/tmp/fby35.mtd,format=raw,if=mtd -device
> loader,file=~/tmp/Y35BCL.elf,addr=0,cpu-num=2 -serial pty -serial pty
> -serial mon:stdio -display none -S
> char device redirected to /dev/pts/3 (label serial0)
> char device redirected to /dev/pts/5 (label serial1)
> qemu-system-arm: warning: Aspeed iBT has no chardev backend
> ~/tmp/Y35BCL.elf: No such file or directory
> qemu-system-arm: -device
> loader,file=~/tmp/Y35BCL.elf,addr=0,cpu-num=2: Cannot load specified
> image ~/tmp/Y35BCL.elf
> 
> loader uses open(2) in load_elf_ram_sym.
> 
> The call stack for -drive looks like this (using a bad path to make it
> easier to identify what's going on):
> 
> #0  __libc_open64 (file=file@entry=0xaaaaac009250
> "/home/joel/tmp/fby35.mtda", oflag=oflag@entry=524288) at
> ../sysdeps/unix/sysv/linux/open64.c:37
> #1  0x0000aaaaab3b18bc in open64 (__oflag=524288,
> __path=0xaaaaac009250 "/home/joel/tmp/fby35.mtda") at
> /usr/include/aarch64-linux-gnu/bits/fcntl2.h:59
> #2  qemu_open_cloexec (mode=0, flags=0, name=0xaaaaac009250
> "/home/joel/tmp/fby35.mtda") at ../util/osdep.c:286
> #3  qemu_open_internal (name=name@entry=0xaaaaac009250
> "/home/joel/tmp/fby35.mtda", flags=flags@entry=0, mode=mode@entry=0,
> errp=errp@entry=0xffffffffeb70) at ../util/osdep.c:330
> #4  0x0000aaaaab3b1f30 in qemu_open (name=name@entry=0xaaaaac009250
> "/home/joel/tmp/fby35.mtda", flags=flags@entry=0,
> errp=errp@entry=0xffffffffeb70) at ../util/osdep.c:360
> #5  0x0000aaaaab30d9d8 in raw_open_common (bs=0xaaaaac002c40,
> options=<optimized out>, bdrv_flags=155650, open_flags=<optimized
> out>, device=<optimized out>, errp=0xffffffffeb70)
>     at ../block/file-posix.c:680
> #6  0x0000aaaaab2a53d0 in bdrv_open_driver
> (bs=bs@entry=0xaaaaac002c40, drv=drv@entry=0xaaaaabc13250 <bdrv_file>,
> node_name=<optimized out>, options=options@entry=0xaaaaac008230,
>     open_flags=open_flags@entry=155650,
> errp=errp@entry=0xffffffffec18) at ../block.c:1625
> #7  0x0000aaaaab2a9454 in bdrv_open_common (errp=0xffffffffec18,
> options=0xaaaaac008230, file=0x0, bs=0xaaaaac002c40) at
> ../block.c:1922
> #8  bdrv_open_inherit (filename=<optimized out>,
> filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
> reference=reference@entry=0x0, options=0xaaaaac008230,
> flags=<optimized out>,
>     flags@entry=0, parent=parent@entry=0xaaaaabffb580,
> child_class=child_class@entry=0xaaaaab864498 <child_of_bds>,
> child_role=child_role@entry=19, errp=errp@entry=0xffffffffed48)
>     at ../block.c:3991
> #9  0x0000aaaaab2aa35c in bdrv_open_child_bs
> (filename=filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
> options=options@entry=0xaaaaac000a50,
>     bdref_key=bdref_key@entry=0xaaaaab573010 "file",
> parent=parent@entry=0xaaaaabffb580,
> child_class=child_class@entry=0xaaaaab864498 <child_of_bds>,
> child_role=child_role@entry=19,
>     allow_none=allow_none@entry=true, errp=errp@entry=0xffffffffed48)
> at ../block.c:3624
> #10 0x0000aaaaab2a98e4 in bdrv_open_inherit
> (filename=filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
> reference=reference@entry=0x0, options=0xaaaaac000a50,
>     options@entry=0xaaaaabff8b70, flags=<optimized out>,
> parent=parent@entry=0x0, child_class=child_class@entry=0x0,
> child_role=child_role@entry=0,
>     errp=errp@entry=0xaaaaabc62368 <error_fatal>) at ../block.c:3938
> #11 0x0000aaaaab2aa758 in bdrv_open
> (filename=filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
> reference=reference@entry=0x0, options=options@entry=0xaaaaabff8b70,
>     flags=<optimized out>, errp=errp@entry=0xaaaaabc62368
> <error_fatal>) at ../block.c:4086
> #12 0x0000aaaaab2c59c8 in blk_new_open
> (filename=filename@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
> reference=reference@entry=0x0, options=options@entry=0xaaaaabff8b70,
>     flags=<optimized out>, errp=errp@entry=0xaaaaabc62368
> <error_fatal>) at ../block/block-backend.c:454
> #13 0x0000aaaaab2956c8 in blockdev_init
> (file=file@entry=0xaaaaabff9cd0 "/home/joel/tmp/fby35.mtda",
> bs_opts=bs_opts@entry=0xaaaaabff8b70, errp=errp@entry=0xaaaaabc62368
> <error_fatal>)
>     at ../blockdev.c:592
> #14 0x0000aaaaab296884 in drive_new (all_opts=0xaaaaabd1a3b0,
> block_default_type=<optimized out>, errp=0xaaaaabc62368 <error_fatal>)
> at ../blockdev.c:981
> #15 0x0000aaaaaafea1e8 in drive_init_func (opaque=<optimized out>,
> opts=<optimized out>, errp=<optimized out>) at ../softmmu/vl.c:648
> #16 0x0000aaaaab3c0a74 in qemu_opts_foreach (list=<optimized out>,
> func=func@entry=0xaaaaaafea1d0 <drive_init_func>,
> opaque=opaque@entry=0xaaaaabf3eb58,
>     errp=errp@entry=0xaaaaabc62368 <error_fatal>) at ../util/qemu-option.c:1135
> #17 0x0000aaaaaafee9d8 in configure_blockdev (bdo_queue=0xaaaaabb866c0
> <bdo_queue>, snapshot=0, machine_class=0xaaaaabf3eab0) at
> ../softmmu/vl.c:707
> #18 qemu_create_early_backends () at ../softmmu/vl.c:1882
> #19 qemu_init (argc=<optimized out>, argv=0xfffffffff358,
> envp=envp@entry=0x0) at ../softmmu/vl.c:3502
> #20 0x0000aaaaaadb4300 in qemu_main (envp=0x0, argv=<optimized out>,
> argc=<optimized out>) at ../softmmu/main.c:35
> #21 main (argc=<optimized out>, argv=<optimized out>) at ../softmmu/main.c:45
> 
> and that was where I lost interest.

Oh that's odd. I can go look into this, I'd be curious to find out what's
happening.

Thanks for reviewing everything and trying it out!
Peter

> 
> 
> > +        -serial pty -serial pty -serial mon:stdio \
> > +        -display none -S
> > +    $ screen /dev/tty0 # In a separate TMUX pane, terminal window, etc.
> > +    $ screen /dev/tty1
> > +    $ (qemu) c            # Start the boot process once screen is setup.
> > +
> >  Supported devices
> >  -----------------
> >
> > --
> > 2.37.0
> >