Hi all,

I am considering refactoring the ASPEED INTC design.

According to the previous discussion, one suggestion was to create separate 
memory regions for different register groups in order to reduce code size.
The total register space of INTC is 0x4000 (16 KB), and the register space of 
INTCIO is 0x400.
Based on this idea, I created a GIC memory region with size 0xB08 and mapped it 
at offset 0x1000.

Relevant links:

https://github.com/qemu/qemu/commit/7ffee511fcf1487e016ae1d11c5e191557a8b804
https://patchwork.kernel.org/project/qemu-devel/patch/[email protected]/

As mentioned in the discussion:

```````
However, we could "optimize" a bit the MMIO apertures to avoid mapping
huge unused gaps and only map the useful set of registers :

   INTC Registers      [ 0x1000 - 0x1B04 ]
   SSP INTC Registers  [ 0x2000 - 0x2B04 ]
   INTCIO Registers    [ 0x0100 - 0x0154 ]

Each set would be associated with a subregion which would be mapped at
the right offset in the region container.

This is just a suggestion.


Thanks,

C.
```````

However, after implementing this approach, I see several issues that make it 
difficult to maintain in the long term:

  1.  Register offsets no longer match the datasheet
For example:
REG32(GICINT128_EN,         0x000)  -> 0x1000 in the datasheet.
REG32(GICINT128_STATUS,     0x004)  -> 0x1004 in the datasheet.
This mismatch makes the code harder to understand and more error-prone.


  1.  Sharing INTC / INTCIO between PSP, TSP, and SSP becomes complex
To allow TSP and SSP to access the same INTC and INTCIO registers, we would 
need to create many memory aliases, which significantly increases complexity.


  1.  Poor scalability for future SoCs
Currently, we create separate SSP INTC, SSP INTCIO, TSP INTC, and TSP INTCIO 
memory regions only for AST2700.
Supporting future SoCs would require adding even more memory regions, which 
does not scale well.


  1.  Total mapped size exceeds the original INTC size
The size of SSP INTC is 0x2B08 and the size of TSP INTC is 0x3B08. The combined 
size
(0x2B08 + 0x3B08 + 0x0B08) is larger than the original INTC size of 0x4000, 
which is conceptually incorrect.

Because of these concerns, I would like to change the design to create only one 
INTC and one INTCIO to improve maintainability and correctness.
In this model, coprocessors would only need to create a single memory alias to 
share the INTC between PSP and SSP.

If you have any suggestions or alternative approaches, please let me know.

Thanks-Jamin


Reply via email to