[PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order

Jamin Lin via posted 21 patches 6 months, 3 weeks ago
[PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Jamin Lin via 6 months, 3 weeks ago
AST2700 has a single SCU hardware block, memory-mapped at 0x12C02000–0x12C03FFF
from the perspective of the main CA35 processor (PSP). The SSP coprocessor accesses
this same SCU block at a different address: 0x72C02000–0x72C03FFF.

To support this shared SCU model, this commit introduces "ssp.scu_mr_alias",
a "MemoryRegion" alias of the original SCU region ("s->scu.iomem"). The alias is
realized during SSP SoC setup and mapped into the SSP's SoC memory map.

Additionally, because the SCU must be realized before the SSP can create an alias
to it, the device realization order is explicitly managed:
"aspeed_soc_ast2700_ssp_realize()" is invoked after the SCU is initialized.

This ensures that PSP and SSP access a consistent SCU state, as expected by hardware.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/arm/aspeed_soc.h |  1 +
 hw/arm/aspeed_ast27x0-ssp.c |  9 ++-------
 hw/arm/aspeed_ast27x0.c     | 24 ++++++++++++++++++------
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 1e4f8580b1..65a452123b 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -135,6 +135,7 @@ struct Aspeed27x0SSPSoCState {
     UnimplementedDeviceState scuio;
     MemoryRegion memory;
     MemoryRegion sram_mr_alias;
+    MemoryRegion scu_mr_alias;
 
     ARMv7MState armv7m;
 };
diff --git a/hw/arm/aspeed_ast27x0-ssp.c b/hw/arm/aspeed_ast27x0-ssp.c
index b7b886f4bf..0a58b8ea4b 100644
--- a/hw/arm/aspeed_ast27x0-ssp.c
+++ b/hw/arm/aspeed_ast27x0-ssp.c
@@ -135,9 +135,7 @@ static void aspeed_soc_ast27x0ssp_init(Object *obj)
     int i;
 
     object_initialize_child(obj, "armv7m", &a->armv7m, TYPE_ARMV7M);
-    object_initialize_child(obj, "scu", &s->scu, TYPE_ASPEED_2700_SCU);
     s->sysclk = qdev_init_clock_in(DEVICE(s), "sysclk", NULL, NULL, 0);
-    qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev", sc->silicon_rev);
 
     for (i = 0; i < sc->uarts_num; i++) {
         object_initialize_child(obj, "uart[*]", &s->uart[i], TYPE_SERIAL_MM);
@@ -198,10 +196,8 @@ static void aspeed_soc_ast27x0ssp_realize(DeviceState *dev_soc, Error **errp)
                                 &a->sram_mr_alias);
 
     /* SCU */
-    if (!sysbus_realize(SYS_BUS_DEVICE(&s->scu), errp)) {
-        return;
-    }
-    aspeed_mmio_map(s, SYS_BUS_DEVICE(&s->scu), 0, sc->memmap[ASPEED_DEV_SCU]);
+    memory_region_add_subregion(s->memory, sc->memmap[ASPEED_DEV_SCU],
+                                &a->scu_mr_alias);
 
     /* INTC */
     if (!sysbus_realize(SYS_BUS_DEVICE(&a->intc[0]), errp)) {
@@ -273,7 +269,6 @@ static void aspeed_soc_ast27x0ssp_class_init(ObjectClass *klass, const void *dat
     dc->realize = aspeed_soc_ast27x0ssp_realize;
 
     sc->valid_cpu_types = valid_cpu_types;
-    sc->silicon_rev = AST2700_A1_SILICON_REV;
     sc->spis_num = 0;
     sc->ehcis_num = 0;
     sc->wdts_num = 0;
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 8272a28ad5..04b8b340ba 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -641,6 +641,10 @@ static bool aspeed_soc_ast2700_ssp_realize(DeviceState *dev, Error **errp)
     mr = &s->sram;
     memory_region_init_alias(&a->ssp.sram_mr_alias, OBJECT(s), "ssp.sram.alias",
                              mr, 0, memory_region_size(mr));
+
+    mr = &s->scu.iomem;
+    memory_region_init_alias(&a->ssp.scu_mr_alias, OBJECT(s), "ssp.scu.alias",
+                             mr, 0, memory_region_size(mr));
     if (!qdev_realize(DEVICE(&a->ssp), NULL, &error_abort)) {
         return false;
     }
@@ -788,14 +792,22 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
                     sc->memmap[ASPEED_DEV_SCUIO]);
 
     /*
-     * Coprocessors must be realized after the SRAM region.
+     * Coprocessors must be realized after the SRAM and SCU regions.
+     *
+     * The SRAM is used as shared memory between the main CPU (PSP) and the
+     * coprocessors. Coprocessors access this shared SRAM region through a
+     * MemoryRegion alias mapped to a different physical address.
+     *
+     * Similarly, the SCU is a single hardware block shared across all
+     * processors. Coprocessors access it via a MemoryRegion alias that maps
+     * to a different address than the one used by the main CPU.
      *
-     * The SRAM is used for shared memory between the main CPU (PSP) and
-     * coprocessors. The coprocessors accesses this shared SRAM region
-     * through a memory alias mapped to a different physical address.
+     * Therefore, both the SRAM and SCU must be fully initialized before the
+     * coprocessors can create aliases pointing to them.
      *
-     * Therefore, the SRAM must be fully initialized before the coprocessors
-     * can create aliases pointing to it.
+     * To ensure correctness, the device realization order is explicitly
+     * managed:
+     * coprocessors are initialized only after SRAM and SCU are ready.
      */
     if (mc->default_cpus > sc->num_cpus) {
         if (!aspeed_soc_ast2700_ssp_realize(dev, errp)) {
-- 
2.43.0


Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Cédric Le Goater 5 months, 1 week ago
On 7/17/25 05:40, Jamin Lin wrote:
> AST2700 has a single SCU hardware block, memory-mapped at 0x12C02000–0x12C03FFF
> from the perspective of the main CA35 processor (PSP). The SSP coprocessor accesses
> this same SCU block at a different address: 0x72C02000–0x72C03FFF.
> 
> To support this shared SCU model, this commit introduces "ssp.scu_mr_alias",
> a "MemoryRegion" alias of the original SCU region ("s->scu.iomem"). The alias is
> realized during SSP SoC setup and mapped into the SSP's SoC memory map.
> 
> Additionally, because the SCU must be realized before the SSP can create an alias
> to it, the device realization order is explicitly managed:
> "aspeed_soc_ast2700_ssp_realize()" is invoked after the SCU is initialized.
> 
> This ensures that PSP and SSP access a consistent SCU state, as expected by hardware.

The SCU model of the main SoC could be passed as a link to the coprocessor
models, like done for the timer model. But the problem is elsewhere.
I think we need to rework the coprocessor models.

Currently, Aspeed27x0TSPSoCState and Aspeed27x0SSPSoCState inherit from
AspeedSoCState and looking at the aspeed_soc_ast27x0{t,s}sp_init handlers,
it seems clear that AspeedSoCState has too much state. We need a simplified
version of AspeedSoCState for the coprocessors.

Please rethink the proposal with that in mind.

Thanks,

C.



RE: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Jamin Lin 4 months, 2 weeks ago
Hi Cédric

> Subject: Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP
> and ensure correct device realization order
> 
> On 7/17/25 05:40, Jamin Lin wrote:
> > AST2700 has a single SCU hardware block, memory-mapped at
> > 0x12C02000–0x12C03FFF from the perspective of the main CA35 processor
> > (PSP). The SSP coprocessor accesses this same SCU block at a different
> address: 0x72C02000–0x72C03FFF.
> >
> > To support this shared SCU model, this commit introduces
> > "ssp.scu_mr_alias", a "MemoryRegion" alias of the original SCU region
> > ("s->scu.iomem"). The alias is realized during SSP SoC setup and mapped into
> the SSP's SoC memory map.
> >
> > Additionally, because the SCU must be realized before the SSP can
> > create an alias to it, the device realization order is explicitly managed:
> > "aspeed_soc_ast2700_ssp_realize()" is invoked after the SCU is initialized.
> >
> > This ensures that PSP and SSP access a consistent SCU state, as expected by
> hardware.
> 
> The SCU model of the main SoC could be passed as a link to the coprocessor
> models, like done for the timer model. But the problem is elsewhere.
> I think we need to rework the coprocessor models.
> 
> Currently, Aspeed27x0TSPSoCState and Aspeed27x0SSPSoCState inherit from
> AspeedSoCState and looking at the aspeed_soc_ast27x0{t,s}sp_init handlers, it
> seems clear that AspeedSoCState has too much state. We need a simplified
> version of AspeedSoCState for the coprocessors.
> 
> Please rethink the proposal with that in mind.
> 
This rework is quite large. To make review easier and avoid an oversized series, I plan to split it into 3 separate patch series:

Series A
1. Move the boot ROM helper from aspeed.c to aspeed_soc_common.c and declare it in aspeed_soc.h, so all ASPEED boards can reuse it.
2. Support vbootrom with coprocessor.

Series B
3. Migrate all ASPEED coprocessors (e.g. SSP/TSP) to a common AspeedCoprocessorState.

Series C
4. Introduce an FC SoC class.
5. Refactor SSP/TSP to share common controllers (e.g. SRAM/SCU) with PSP.
6. Gate SSP/TSP CPU power via the SCU

Thanks-Jamin

> Thanks,
> 
> C.
> 

Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Cédric Le Goater 4 months, 2 weeks ago
Hello Jamin,

On 9/23/25 10:31, Jamin Lin wrote:
> Hi Cédric
> 
>> Subject: Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP
>> and ensure correct device realization order
>>
>> On 7/17/25 05:40, Jamin Lin wrote:
>>> AST2700 has a single SCU hardware block, memory-mapped at
>>> 0x12C02000–0x12C03FFF from the perspective of the main CA35 processor
>>> (PSP). The SSP coprocessor accesses this same SCU block at a different
>> address: 0x72C02000–0x72C03FFF.
>>>
>>> To support this shared SCU model, this commit introduces
>>> "ssp.scu_mr_alias", a "MemoryRegion" alias of the original SCU region
>>> ("s->scu.iomem"). The alias is realized during SSP SoC setup and mapped into
>> the SSP's SoC memory map.
>>>
>>> Additionally, because the SCU must be realized before the SSP can
>>> create an alias to it, the device realization order is explicitly managed:
>>> "aspeed_soc_ast2700_ssp_realize()" is invoked after the SCU is initialized.
>>>
>>> This ensures that PSP and SSP access a consistent SCU state, as expected by
>> hardware.
>>
>> The SCU model of the main SoC could be passed as a link to the coprocessor
>> models, like done for the timer model. But the problem is elsewhere.
>> I think we need to rework the coprocessor models.
>>
>> Currently, Aspeed27x0TSPSoCState and Aspeed27x0SSPSoCState inherit from
>> AspeedSoCState and looking at the aspeed_soc_ast27x0{t,s}sp_init handlers, it
>> seems clear that AspeedSoCState has too much state. We need a simplified
>> version of AspeedSoCState for the coprocessors.
>>
>> Please rethink the proposal with that in mind.
>>
> This rework is quite large. To make review easier and avoid an oversized series, I plan to split it into 3 separate patch series:
> 
> Series A
> 1. Move the boot ROM helper from aspeed.c to aspeed_soc_common.c and declare it in aspeed_soc.h, so all ASPEED boards can reuse it.
> 2. Support vbootrom with coprocessor.

This should be quickly merged.

> 
> Series B
> 3. Migrate all ASPEED coprocessors (e.g. SSP/TSP) to a common AspeedCoprocessorState.

Is 'AspeedCoprocessorState' a new model structure minimizing the number
of sub controllers ? if so, looks good. Could be merged fairly quickly.

> Series C
> 4. Introduce an FC SoC class.
> 5. Refactor SSP/TSP to share common controllers (e.g. SRAM/SCU) with PSP.
> 6. Gate SSP/TSP CPU power via the SCU

OK That's the long term goal. Let's plan it for QEMU 10.2.

Thanks,

C.


RE: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Jamin Lin 4 months, 2 weeks ago
Hi Cédric

> Subject: Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP
> and ensure correct device realization order
> 
> Hello Jamin,
> 
> On 9/23/25 10:31, Jamin Lin wrote:
> > Hi Cédric
> >
> >> Subject: Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias
> >> for SSP and ensure correct device realization order
> >>
> >> On 7/17/25 05:40, Jamin Lin wrote:
> >>> AST2700 has a single SCU hardware block, memory-mapped at
> >>> 0x12C02000–0x12C03FFF from the perspective of the main CA35
> >>> processor (PSP). The SSP coprocessor accesses this same SCU block at
> >>> a different
> >> address: 0x72C02000–0x72C03FFF.
> >>>
> >>> To support this shared SCU model, this commit introduces
> >>> "ssp.scu_mr_alias", a "MemoryRegion" alias of the original SCU
> >>> region ("s->scu.iomem"). The alias is realized during SSP SoC setup
> >>> and mapped into
> >> the SSP's SoC memory map.
> >>>
> >>> Additionally, because the SCU must be realized before the SSP can
> >>> create an alias to it, the device realization order is explicitly managed:
> >>> "aspeed_soc_ast2700_ssp_realize()" is invoked after the SCU is initialized.
> >>>
> >>> This ensures that PSP and SSP access a consistent SCU state, as
> >>> expected by
> >> hardware.
> >>
> >> The SCU model of the main SoC could be passed as a link to the
> >> coprocessor models, like done for the timer model. But the problem is
> elsewhere.
> >> I think we need to rework the coprocessor models.
> >>
> >> Currently, Aspeed27x0TSPSoCState and Aspeed27x0SSPSoCState inherit
> >> from AspeedSoCState and looking at the aspeed_soc_ast27x0{t,s}sp_init
> >> handlers, it seems clear that AspeedSoCState has too much state. We
> >> need a simplified version of AspeedSoCState for the coprocessors.
> >>
> >> Please rethink the proposal with that in mind.
> >>
> > This rework is quite large. To make review easier and avoid an oversized
> series, I plan to split it into 3 separate patch series:
> >
> > Series A
> > 1. Move the boot ROM helper from aspeed.c to aspeed_soc_common.c and
> declare it in aspeed_soc.h, so all ASPEED boards can reuse it.
> > 2. Support vbootrom with coprocessor.
> 
> This should be quickly merged.
> 
> >
> > Series B
> > 3. Migrate all ASPEED coprocessors (e.g. SSP/TSP) to a common
> AspeedCoprocessorState.
> 
> Is 'AspeedCoprocessorState' a new model structure minimizing the number of
> sub controllers ? if so, looks good. Could be merged fairly quickly.
> 

Yes, I am planning to use the new AspeedCoprocessorState instead of AspeedSoCState for SSP and TSP.
struct Aspeed27x0SSPSoCState {
    AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
    AspeedINTCState intc[2];
    UnimplementedDeviceState ipc[2];
    UnimplementedDeviceState scuio;

    ARMv7MState armv7m;
};

struct Aspeed27x0TSPSoCState {
    AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
    AspeedINTCState intc[2];
    UnimplementedDeviceState ipc[2];
    UnimplementedDeviceState scuio;

    ARMv7MState armv7m;
};
This change consolidates SSP and TSP under a common coprocessor model, 
reducing duplication and aligning them with the new AspeedCoprocessorState abstraction.

Thanks-Jamin

> > Series C
> > 4. Introduce an FC SoC class.
> > 5. Refactor SSP/TSP to share common controllers (e.g. SRAM/SCU) with PSP.
> > 6. Gate SSP/TSP CPU power via the SCU
> 
> OK That's the long term goal. Let's plan it for QEMU 10.2.
> 
> Thanks,
> 
> C.

Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Cédric Le Goater 4 months, 2 weeks ago
Hello Jamin,


>>> 3. Migrate all ASPEED coprocessors (e.g. SSP/TSP) to a common
>> AspeedCoprocessorState.
>>
>> Is 'AspeedCoprocessorState' a new model structure minimizing the number of
>> sub controllers ? if so, looks good. Could be merged fairly quickly.
>>
> 
> Yes, I am planning to use the new AspeedCoprocessorState instead of AspeedSoCState for SSP and TSP.
> struct Aspeed27x0SSPSoCState {
>      AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
>      AspeedINTCState intc[2];
>      UnimplementedDeviceState ipc[2];
>      UnimplementedDeviceState scuio;
> 
>      ARMv7MState armv7m;
> };
> 
> struct Aspeed27x0TSPSoCState {
>      AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
>      AspeedINTCState intc[2];
>      UnimplementedDeviceState ipc[2];
>      UnimplementedDeviceState scuio;
> 
>      ARMv7MState armv7m;
> };
> This change consolidates SSP and TSP under a common coprocessor model,
> reducing duplication and aligning them with the new AspeedCoprocessorState abstraction.

Aspeed27x0TSPSoCState and Aspeed27x0SSPSoCState look similar. Could they
be merged ?

Thanks,

C.
RE: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Jamin Lin 4 months, 2 weeks ago
Hi Cédric

> Subject: Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP
> and ensure correct device realization order
> 
> Hello Jamin,
> 
> 
> >>> 3. Migrate all ASPEED coprocessors (e.g. SSP/TSP) to a common
> >> AspeedCoprocessorState.
> >>
> >> Is 'AspeedCoprocessorState' a new model structure minimizing the
> >> number of sub controllers ? if so, looks good. Could be merged fairly quickly.
> >>
> >
> > Yes, I am planning to use the new AspeedCoprocessorState instead of
> AspeedSoCState for SSP and TSP.
> > struct Aspeed27x0SSPSoCState {
> >      AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
> >      AspeedINTCState intc[2];
> >      UnimplementedDeviceState ipc[2];
> >      UnimplementedDeviceState scuio;
> >
> >      ARMv7MState armv7m;
> > };
> >
> > struct Aspeed27x0TSPSoCState {
> >      AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
> >      AspeedINTCState intc[2];
> >      UnimplementedDeviceState ipc[2];
> >      UnimplementedDeviceState scuio;
> >
> >      ARMv7MState armv7m;
> > };
> > This change consolidates SSP and TSP under a common coprocessor model,
> > reducing duplication and aligning them with the new
> AspeedCoprocessorState abstraction.
> 
> Aspeed27x0TSPSoCState and Aspeed27x0SSPSoCState look similar. Could they
> be merged ?
> 
Thanks for your suggestion.
Will try it.
Jamin
> Thanks,
> 
> C.

RE: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Jamin Lin 4 months, 2 weeks ago
Hi Cédric

> > >>> 3. Migrate all ASPEED coprocessors (e.g. SSP/TSP) to a common
> > >> AspeedCoprocessorState.
> > >>
> > >> Is 'AspeedCoprocessorState' a new model structure minimizing the
> > >> number of sub controllers ? if so, looks good. Could be merged fairly
> quickly.
> > >>
> > >
> > > Yes, I am planning to use the new AspeedCoprocessorState instead of
> > AspeedSoCState for SSP and TSP.
> > > struct Aspeed27x0SSPSoCState {
> > >      AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
> > >      AspeedINTCState intc[2];
> > >      UnimplementedDeviceState ipc[2];
> > >      UnimplementedDeviceState scuio;
> > >
> > >      ARMv7MState armv7m;
> > > };
> > >
> > > struct Aspeed27x0TSPSoCState {
> > >      AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
> > >      AspeedINTCState intc[2];
> > >      UnimplementedDeviceState ipc[2];
> > >      UnimplementedDeviceState scuio;
> > >
> > >      ARMv7MState armv7m;
> > > };
> > > This change consolidates SSP and TSP under a common coprocessor
> > > model, reducing duplication and aligning them with the new
> > AspeedCoprocessorState abstraction.
> >
> > Aspeed27x0TSPSoCState and Aspeed27x0SSPSoCState look similar. Could
> > they be merged ?
> >
> Thanks for your suggestion.
> Will try it.
> Jamin
> > Thanks,

I am considering making the following APIs common so that both the Coprocessor and SoC can use them.

The Coprocessor state is AspeedCoprocessor
The general SoC state is AspeedSoC

A. MMIO Mapping
Currently:

void aspeed_mmio_map(AspeedSoCState *s, SysBusDevice *dev, int n, hwaddr addr)
{
    memory_region_add_subregion(s->memory, addr,
                                sysbus_mmio_get_region(dev, n));
}

void aspeed_mmio_map_unimplemented(AspeedSoCState *s, SysBusDevice *dev,
                                   const char *name, hwaddr addr, uint64_t size)
{
    qdev_prop_set_string(DEVICE(dev), "name", name);
    qdev_prop_set_uint64(DEVICE(dev), "size", size);
    sysbus_realize(dev, &error_abort);

    memory_region_add_subregion_overlap(s->memory, addr,
                                        sysbus_mmio_get_region(dev, 0), -1000);
}

Proposal:
Replace AspeedSoCState *s with MemoryRegion *memory.
This way, the functions can be reused for both AspeedSoC and AspeedCoprocessor.
void aspeed_mmio_map(MemoryRegion *memory, SysBusDevice *dev, int n, hwaddr addr);
void aspeed_mmio_map_unimplemented(MemoryRegion *memory, SysBusDevice *dev,
                                   const char *name, hwaddr addr, uint64_t size);

B. UART Index Helpers
Currently:
static inline int aspeed_uart_first(AspeedSoCClass *sc)
{
    return aspeed_uart_index(sc->uarts_base);
}

static inline int aspeed_uart_last(AspeedSoCClass *sc)
{
    return aspeed_uart_first(sc) + sc->uarts_num - 1;
}

Proposal:
Make them independent of AspeedSoCClass, so they can be reused:

static inline int aspeed_uart_first(int uarts_base);
static inline int aspeed_uart_last(int uarts_base, int uarts_num);

C. UART Realization
This case looks more complex. Possible approaches:
1. Create a new API specifically for the coprocessor.
2. Add extra parameters to the existing APIs instead of relying on AspeedSoCState.

For example, the current SoC UART realization is tightly bound to AspeedSoCState.
Making it generic would require additional parameters (such as the UART array, base index, memmap, and IRQ handler) so it could be reused by both SoC and Coprocessor.
Could you please give me any suggestion?

bool aspeed_soc_uart_realize(AspeedSoCState *s, Error **errp)
{
    AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
    SerialMM *smm;

    for (int i = 0, uart = sc->uarts_base; 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;
        }

        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 uart_first = aspeed_uart_first(sc);
    int uart_index = aspeed_uart_index(dev);
    int i = uart_index - uart_first;

    g_assert(0 <= i && i < ARRAY_SIZE(s->uart) && i < sc->uarts_num);
    qdev_prop_set_chr(DEVICE(&s->uart[i]), "chardev", chr);
}

D. Common APIs in Multiple SoCs

The following APIs are already widely used across AST2400, AST2500, AST2600, AST2700, and AST1030 SoC realizations:
Are you agree if I create a new aspeed_coprocessor_cpu_type and aspeed_coprocessor_get_irq for Coprocessor?
Could you please give me any suggestion?

const char *aspeed_soc_cpu_type(AspeedSoCClass *sc)
{
    assert(sc->valid_cpu_types);
    assert(sc->valid_cpu_types[0]);
    assert(!sc->valid_cpu_types[1]);
    return sc->valid_cpu_types[0];
}

qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev)
{
    return ASPEED_SOC_GET_CLASS(s)->get_irq(s, dev);
}

Thanks-Jamin

> >
> > C.

Re: [SPAM] [PATCH v1 08/21] hw/arm/ast27x0: Add SCU alias for SSP and ensure correct device realization order
Posted by Cédric Le Goater 4 months, 2 weeks ago
Hello Jamin,

On 9/26/25 05:13, Jamin Lin wrote:
> Hi Cédric
> 
>>>>>> 3. Migrate all ASPEED coprocessors (e.g. SSP/TSP) to a common
>>>>> AspeedCoprocessorState.
>>>>>
>>>>> Is 'AspeedCoprocessorState' a new model structure minimizing the
>>>>> number of sub controllers ? if so, looks good. Could be merged fairly
>> quickly.
>>>>>
>>>>
>>>> Yes, I am planning to use the new AspeedCoprocessorState instead of
>>> AspeedSoCState for SSP and TSP.
>>>> struct Aspeed27x0SSPSoCState {
>>>>       AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
>>>>       AspeedINTCState intc[2];
>>>>       UnimplementedDeviceState ipc[2];
>>>>       UnimplementedDeviceState scuio;
>>>>
>>>>       ARMv7MState armv7m;
>>>> };
>>>>
>>>> struct Aspeed27x0TSPSoCState {
>>>>       AspeedSoCState parent;  -------> Change to AspeedCoprocessorState
>>>>       AspeedINTCState intc[2];
>>>>       UnimplementedDeviceState ipc[2];
>>>>       UnimplementedDeviceState scuio;
>>>>
>>>>       ARMv7MState armv7m;
>>>> };
>>>> This change consolidates SSP and TSP under a common coprocessor
>>>> model, reducing duplication and aligning them with the new
>>> AspeedCoprocessorState abstraction.
>>>
>>> Aspeed27x0TSPSoCState and Aspeed27x0SSPSoCState look similar. Could
>>> they be merged ?
>>>
>> Thanks for your suggestion.
>> Will try it.
>> Jamin
>>> Thanks,
> 
> I am considering making the following APIs common so that both the Coprocessor and SoC can use them.
> 
> The Coprocessor state is AspeedCoprocessor
> The general SoC state is AspeedSoC

Yes. If you need a base class to maintain common routines, you could
introduce a parent too.

For rest, please send patches. I rather discuss on a proposal than on
an unrelated email.

Thanks,

C.