gilles grimaud <[email protected]> writes:

> Hi Alex,
>
> Thanks for the review. I have looked at the impact of these comments, and
> for v3 I propose to:
>
> * split the current patch into two focused patches: one for SYSINFO and one
>   for SYSCFG;

Yes

> * remove the powered-off memory overlays. They were intended to make CPU
>   and DMA accesses fail after setting MEMPOWERDOWN, but this goes beyond the
>   documented register behavior;
> * retain the documented MEMPOWERDOWN register state, reset value, and write
>   mask, without claiming to model physical ROM or SRAM power gating;

So is this modelling individual cores power states? I'm still a bit
confused as to what in the model will be accessing them when the cores
are down.

> * route PROC0_NMI_MASK and PROC1_NMI_MASK independently to the corresponding
>   core;
> * remove the MemoryRegionOps.with_attrs callbacks, since they are only
>   needed by the powered-off memory overlays;

My preference would be the other way. The with_attr callbacks allow for
richer memory modelling. Indeed if you really wanted to model powerdown
state you could keep the same accessor and return MEMTX_ACCESS in the
main implementation without needing an overlay.

> * remove the per-subsystem RP2040 Kconfig symbols and build the internal
>   devices under the single CONFIG_RP2040 symbol.
>
> I will add focused qtests to the separate SYSINFO and SYSCFG patches.
>
> Does this sound in the right direction?
>
> Thanks,

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Reply via email to