1. Patchew
  2. QEMU
  3. [PATCH 0/4] Add support for gb200-bmc machine
  4. View patch

[PATCH 2/4] hw/arm/aspeed: Add second SPI chip to Aspeed model

Ed Tanous posted 4 patches 4 months, 2 weeks ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH 2/4] hw/arm/aspeed: Add second SPI chip to Aspeed model
Posted by Ed Tanous 4 months, 2 weeks ago 
Aspeed2600 has two spi lanes;  Add a new struct that can mount the
second SPI.

Signed-off-by: Ed Tanous <etanous@nvidia.com>
---
 hw/arm/aspeed.c         | 2 ++
 include/hw/arm/aspeed.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index d0b333646e..3ef7f6c5b2 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -465,6 +465,8 @@ static void aspeed_machine_init(MachineState *machine)
         aspeed_board_init_flashes(&bmc->soc->spi[0],
                               bmc->spi_model ? bmc->spi_model : amc->spi_model,
                               1, amc->num_cs);
+        aspeed_board_init_flashes(&bmc->soc->spi[1],
+                                  amc->spi2_model, 1, amc->num_cs2);
     }
 
     if (machine->kernel_filename && sc->num_cpus > 1) {
diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 973277bea6..6c36455656 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -35,7 +35,9 @@ struct AspeedMachineClass {
     uint32_t hw_strap2;
     const char *fmc_model;
     const char *spi_model;
+    const char *spi2_model;
     uint32_t num_cs;
+    uint32_t num_cs2;
     uint32_t macs_mask;
     void (*i2c_init)(AspeedMachineState *bmc);
     uint32_t uart_default;
-- 
2.43.0
Re: [PATCH 2/4] hw/arm/aspeed: Add second SPI chip to Aspeed model
Posted by Cédric Le Goater 4 months, 2 weeks ago 
On 7/1/25 22:33, Ed Tanous wrote:
> Aspeed2600 has two spi lanes;  Add a new struct that can mount the
> second SPI.
> 
> Signed-off-by: Ed Tanous <etanous@nvidia.com>
> ---
>   hw/arm/aspeed.c         | 2 ++
>   include/hw/arm/aspeed.h | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index d0b333646e..3ef7f6c5b2 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -465,6 +465,8 @@ static void aspeed_machine_init(MachineState *machine)
>           aspeed_board_init_flashes(&bmc->soc->spi[0],
>                                 bmc->spi_model ? bmc->spi_model : amc->spi_model,
>                                 1, amc->num_cs);
> +        aspeed_board_init_flashes(&bmc->soc->spi[1],
> +                                  amc->spi2_model, 1, amc->num_cs2);
>       }
>   
>       if (machine->kernel_filename && sc->num_cpus > 1) {
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index 973277bea6..6c36455656 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -35,7 +35,9 @@ struct AspeedMachineClass {
>       uint32_t hw_strap2;
>       const char *fmc_model;
>       const char *spi_model;
> +    const char *spi2_model;
>       uint32_t num_cs;
> +    uint32_t num_cs2;
>       uint32_t macs_mask;
>       void (*i2c_init)(AspeedMachineState *bmc);
>       uint32_t uart_default;


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.
Re: [PATCH 2/4] hw/arm/aspeed: Add second SPI chip to Aspeed model
Posted by Cédric Le Goater 4 months, 2 weeks ago 
On 7/1/25 22:33, Ed Tanous wrote:
> Aspeed2600 has two spi lanes;  Add a new struct that can mount the
> second SPI.
> 
> Signed-off-by: Ed Tanous <etanous@nvidia.com>
> ---
>   hw/arm/aspeed.c         | 2 ++
>   include/hw/arm/aspeed.h | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index d0b333646e..3ef7f6c5b2 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -465,6 +465,8 @@ static void aspeed_machine_init(MachineState *machine)
>           aspeed_board_init_flashes(&bmc->soc->spi[0],
>                                 bmc->spi_model ? bmc->spi_model : amc->spi_model,
>                                 1, amc->num_cs);
> +        aspeed_board_init_flashes(&bmc->soc->spi[1],
> +                                  amc->spi2_model, 1, amc->num_cs2);
>       }
>   
>       if (machine->kernel_filename && sc->num_cpus > 1) {
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index 973277bea6..6c36455656 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -35,7 +35,9 @@ struct AspeedMachineClass {
>       uint32_t hw_strap2;
>       const char *fmc_model;
>       const char *spi_model;
> +    const char *spi2_model;
>       uint32_t num_cs;
> +    uint32_t num_cs2;
>       uint32_t macs_mask;
>       void (*i2c_init)(AspeedMachineState *bmc);
>       uint32_t uart_default;

Another way specifying backends for all SPI devices is to use -blockdev :

   $ qemu-system-arm -M ast2600-evb \
       -blockdev node-name=fmc0,driver=file,filename=/path/to/fmc0.img \
       -device mx66u51235f,bus=ssi.0,cs=0x0,drive=fmc0 \
       -blockdev node-name=fmc1,driver=file,filename=/path/to/fmc1.img \
       -device mx66u51235f,bus=ssi.0,cs=0x1,drive=fmc1 \
       -blockdev node-name=spi1,driver=file,filename=/path/to/spi1.img \
       -device mx66u51235f,cs=0x0,bus=ssi.1,drive=spi1 \
       -nographic -nodefaults

See https://www.qemu.org/docs/master/system/arm/aspeed.html.

Have you tried it ?


Thanks,

C.
Re: [PATCH 2/4] hw/arm/aspeed: Add second SPI chip to Aspeed model
Posted by etanous via 4 months, 2 weeks ago 
On Wed, Jul 02, 2025 at 09:00:20AM +0200, Cédric Le Goater wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 7/1/25 22:33, Ed Tanous wrote:
> > Aspeed2600 has two spi lanes;  Add a new struct that can mount the
> > second SPI.
> > 
> > Signed-off-by: Ed Tanous <etanous@nvidia.com>
> > ---
> >   hw/arm/aspeed.c         | 2 ++
> >   include/hw/arm/aspeed.h | 2 ++
> >   2 files changed, 4 insertions(+)
> > 
> > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> > index d0b333646e..3ef7f6c5b2 100644
> > --- a/hw/arm/aspeed.c
> > +++ b/hw/arm/aspeed.c
> > @@ -465,6 +465,8 @@ static void aspeed_machine_init(MachineState *machine)
> >           aspeed_board_init_flashes(&bmc->soc->spi[0],
> >                                 bmc->spi_model ? bmc->spi_model : amc->spi_model,
> >                                 1, amc->num_cs);
> > +        aspeed_board_init_flashes(&bmc->soc->spi[1],
> > +                                  amc->spi2_model, 1, amc->num_cs2);
> >       }
> > 
> >       if (machine->kernel_filename && sc->num_cpus > 1) {
> > diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> > index 973277bea6..6c36455656 100644
> > --- a/include/hw/arm/aspeed.h
> > +++ b/include/hw/arm/aspeed.h
> > @@ -35,7 +35,9 @@ struct AspeedMachineClass {
> >       uint32_t hw_strap2;
> >       const char *fmc_model;
> >       const char *spi_model;
> > +    const char *spi2_model;
> >       uint32_t num_cs;
> > +    uint32_t num_cs2;
> >       uint32_t macs_mask;
> >       void (*i2c_init)(AspeedMachineState *bmc);
> >       uint32_t uart_default;
> 
> Another way specifying backends for all SPI devices is to use -blockdev :
> 
>   $ qemu-system-arm -M ast2600-evb \
>       -blockdev node-name=fmc0,driver=file,filename=/path/to/fmc0.img \
>       -device mx66u51235f,bus=ssi.0,cs=0x0,drive=fmc0 \
>       -blockdev node-name=fmc1,driver=file,filename=/path/to/fmc1.img \
>       -device mx66u51235f,bus=ssi.0,cs=0x1,drive=fmc1 \
>       -blockdev node-name=spi1,driver=file,filename=/path/to/spi1.img \
>       -device mx66u51235f,cs=0x0,bus=ssi.1,drive=spi1 \
>       -nographic -nodefaults
> 

I had attempted that at one point.  The second SPI flash is expected to
be empty on first boot, so building up an empty file with dd seemed like
a waste, and pushed more details on the user calling the machine to
know the machine configuration.  FWIW, yoctos 'runqemu' helper script is
also very useful, but getting it to spit out non-standard args and files
isn't the easiest.  If what's in this patch is ok, I'd like to stick
with it.  If not, I can dig deeper into trying to do this on command
line.

> See <LINK SCRUBBED>
> 
> Have you tried it ?
> 
> 
> Thanks,
> 
> C.
> 
>
Re: [PATCH 2/4] hw/arm/aspeed: Add second SPI chip to Aspeed model
Posted by Cédric Le Goater 4 months, 2 weeks ago 
On 7/2/25 21:27, etanous wrote:
> On Wed, Jul 02, 2025 at 09:00:20AM +0200, Cédric Le Goater wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 7/1/25 22:33, Ed Tanous wrote:
>>> Aspeed2600 has two spi lanes;  Add a new struct that can mount the
>>> second SPI.
>>>
>>> Signed-off-by: Ed Tanous <etanous@nvidia.com>
>>> ---
>>>    hw/arm/aspeed.c         | 2 ++
>>>    include/hw/arm/aspeed.h | 2 ++
>>>    2 files changed, 4 insertions(+)
>>>
>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>> index d0b333646e..3ef7f6c5b2 100644
>>> --- a/hw/arm/aspeed.c
>>> +++ b/hw/arm/aspeed.c
>>> @@ -465,6 +465,8 @@ static void aspeed_machine_init(MachineState *machine)
>>>            aspeed_board_init_flashes(&bmc->soc->spi[0],
>>>                                  bmc->spi_model ? bmc->spi_model : amc->spi_model,
>>>                                  1, amc->num_cs);
>>> +        aspeed_board_init_flashes(&bmc->soc->spi[1],
>>> +                                  amc->spi2_model, 1, amc->num_cs2);
>>>        }
>>>
>>>        if (machine->kernel_filename && sc->num_cpus > 1) {
>>> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
>>> index 973277bea6..6c36455656 100644
>>> --- a/include/hw/arm/aspeed.h
>>> +++ b/include/hw/arm/aspeed.h
>>> @@ -35,7 +35,9 @@ struct AspeedMachineClass {
>>>        uint32_t hw_strap2;
>>>        const char *fmc_model;
>>>        const char *spi_model;
>>> +    const char *spi2_model;
>>>        uint32_t num_cs;
>>> +    uint32_t num_cs2;
>>>        uint32_t macs_mask;
>>>        void (*i2c_init)(AspeedMachineState *bmc);
>>>        uint32_t uart_default;
>>
>> Another way specifying backends for all SPI devices is to use -blockdev :
>>
>>    $ qemu-system-arm -M ast2600-evb \
>>        -blockdev node-name=fmc0,driver=file,filename=/path/to/fmc0.img \
>>        -device mx66u51235f,bus=ssi.0,cs=0x0,drive=fmc0 \
>>        -blockdev node-name=fmc1,driver=file,filename=/path/to/fmc1.img \
>>        -device mx66u51235f,bus=ssi.0,cs=0x1,drive=fmc1 \
>>        -blockdev node-name=spi1,driver=file,filename=/path/to/spi1.img \
>>        -device mx66u51235f,cs=0x0,bus=ssi.1,drive=spi1 \
>>        -nographic -nodefaults
>>
> 
> I had attempted that at one point.  The second SPI flash is expected to
> be empty on first boot, so building up an empty file with dd seemed like
> a waste, and pushed more details on the user calling the machine to
> know the machine configuration.  FWIW, yoctos 'runqemu' helper script is
> also very useful, but getting it to spit out non-standard args and files
> isn't the easiest.  If what's in this patch is ok, I'd like to stick
> with it.  If not, I can dig deeper into trying to do this on command
> line.

It's fine. we will merge it. I am trying to promote the -blockdev interface
to get rid of -drive but this seems to be wishful thinking.


Thanks,

C.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.