Re: [RFC PATCH 20/23] hw/ssi: add support for flexspi

[Philippe Mathieu-Daudé]

On 8/8/24 23:31, Octavian Purdila wrote:

> > > +static Property flexspi_properties[] = {
> > > +    DEFINE_PROP_UINT32("mmap_base", FlexSpiState, mmap_base, 0),
> > > +    DEFINE_PROP_UINT32("mmap_size", FlexSpiState, mmap_size, 0),

Preferably simply 'size'.

> > > +    DEFINE_PROP_END_OF_LIST(),
> > > +};
> > > +
> > > +static void flexspi_init(Object *obj)
> > > +{
> > > +    FlexSpiState *s = FLEXSPI(obj);
> > > +
> > > +    memory_region_init_io(&s->mmio, obj, &flexspi_ops, s, TYPE_FLEXSPI,
> > > +                          sizeof(FLEXSPI_Type));
> > > +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
> > > +}
> > > +
> > > +static void flexspi_realize(DeviceState *dev, Error **errp)
> > > +{
> > > +    FlexSpiState *s = FLEXSPI(dev);
> > > +
> > > +    if (s->mmap_size) {
> > > +        memory_region_init_ram(&s->mem, OBJECT(s), DEVICE(s)->id, s->mmap_size,
> > > +                               NULL);
> > > +        memory_region_add_subregion(get_system_memory(), s->mmap_base, 
> > > &s->mem);
> >
> > Where is this region used?
>
> These regions are enabled in rt500.c when instantiating the flexspi
> peripherals. As implemented now they are backed by RAM, the full
> implementation should translate accesses to spi commands to FLASH or
> PSRAM devices.
>
> We need the memory regions because even the simplest NXP SDK examples
> are using the memory mapped flexspi0 region.

Devices shouldn't access get_system_memory() directly (it is
documented as kind of deprecated =) ). Since you implement a
sysbus device, you need to export the region with sysbus_init_mmio()
then the upper layer (SoC) instantiating it gets the regions calling
sysbus_mmio_get_region() and maps it. Then you don't need the
'mmap_base' property.