On 1/25/21 12:48 PM, Peter Maydell wrote: > On Thu, 21 Jan 2021 at 22:13, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: >> On 1/21/21 8:06 PM, Peter Maydell wrote: >>> Convert the SSYS code in the Stellaris boards (which encapsulates the >>> system registers) to a proper QOM device. This will provide us with >>> somewhere to put the output Clock whose frequency depends on the >>> setting of the PLL configuration registers. >>> >>> This is a migration compatibility break for lm3s811evb, lm3s6965evb. >>> >>> We use 3-phase reset here because the Clock will need to propagate >>> its value in the hold phase. >>> >>> For the moment we reset the device during the board creation so that >>> the system_clock_scale global gets set; this will be removed in a >>> subsequent commit. > >>> + >>> +struct ssys_state { >>> + SysBusDevice parent_obj; >>> + >>> MemoryRegion iomem; >>> uint32_t pborctl; >>> uint32_t ldopctl; >>> @@ -371,11 +376,18 @@ typedef struct { >>> uint32_t dcgc[3]; >>> uint32_t clkvclr; >>> uint32_t ldoarst; >>> + qemu_irq irq; >>> + /* Properties (all read-only registers) */ >>> uint32_t user0; >>> uint32_t user1; >>> - qemu_irq irq; >>> - stellaris_board_info *board; >>> -} ssys_state; >>> + uint32_t did0; >>> + uint32_t did1; >>> + uint32_t dc0; >>> + uint32_t dc1; >>> + uint32_t dc2; >>> + uint32_t dc3; >>> + uint32_t dc4; >> >> Shouldn't these be class properties? > > Could you elaborate on what you think the code ought to look like?
I am thinking something similar how Igor asked me to implement RaspiMachineClass::board_rev in hw/arm/raspi.c, as the did/dc registers are read-only. Anyhow this is 1/ probably not necessary and 2/ out of the scope of this series, this patch is already complex enough, and the work is done. > I just used the usual thing of defining uint32 qdev properties so we > can set these values when we create the device, as a replacement > for the existing code which either reaches directly into the > state struct to set the user0/user1 values or sets the > stellaris_board_info pointer in the state struct. No problem. Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>