Re: [PATCH v2 09/11] aspeed: Introduce a spi_boot region under the SoC

[Cédric Le Goater]

On 3/1/23 18:52, Philippe Mathieu-Daudé wrote:
> On 1/3/23 17:56, Cédric Le Goater wrote:
> > The default boot address of the Aspeed SoCs is 0x0. For this reason,
> > the FMC flash device contents are remapped by HW on the first 256MB of
> > the address space. In QEMU, this is currently done in the machine init
> > with the setup of a region alias.
> >
> > Move this code to the SoC and introduce an extra container to prepare
> > ground for the boot ROM region which will overlap the FMC flash
> > remapping.
> >
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >   include/hw/arm/aspeed_soc.h |  5 +++++
> >   hw/arm/aspeed.c             | 13 +------------
> >   hw/arm/aspeed_ast2600.c     | 13 +++++++++++++
> >   hw/arm/aspeed_soc.c         | 14 ++++++++++++++
>
> Why aspeed2600 duplicates a lot of aspeed_soc_init() /
> aspeed_soc_realize() while inheriting from TYPE_ASPEED_SOC?

We thought, back in 2019 or so, that duplicating the models was a better
approach to keep a clear understanding of the SoCs and their differences.
I agree they are very similar and maybe that was not the best solution.
OTOH, AST2600 has being receiving the most attention since it is the
latest.

The common TYPE_ASPEED_SOC is required to share a few properties at the
board level if I remember well.

Thanks,

C.

> Is that on purpose or because not using device_class_set_parent_realize?
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> >   hw/arm/fby35.c              |  8 +-------
> >   5 files changed, 34 insertions(+), 19 deletions(-)