Thanks Peter for the review,
----- Original Message -----
> From: "Peter Maydell"
> To: "Arnaud Minier"
> Cc: "qemu-devel" , "Thomas Huth" , "Laurent Vivier" , "Inès
> Varhol" , "Samuel Tardieu" , "qemu-arm"
> , "Alistair Francis" , "Paolo Bonzini" , "Alistair
> Francis"
> Sent: Friday, February 23, 2024 3:26:46 PM
> Subject: Re: [PATCH v5 1/8] Implement STM32L4x5_RCC skeleton
> On Mon, 19 Feb 2024 at 20:11, Arnaud Minier
> wrote:
>>
>> Add the necessary files to add a simple RCC implementation with just
>> reads from and writes to registers. Also instanciate the RCC in the
>
> "instantiate"
Fixed.
>
>> STM32L4x5_SoC. It is needed for accurate emulation of all the SoC
>> clocks and timers.
>>
>> Signed-off-by: Arnaud Minier
>> Signed-off-by: Inès Varhol
>> Acked-by: Alistair Francis
>> ---
>
>
>
>> +static const MemoryRegionOps stm32l4x5_rcc_ops = {
>> + .read = stm32l4x5_rcc_read,
>> + .write = stm32l4x5_rcc_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> + .valid = {
>> + .max_access_size = 4,
>> + .unaligned = false
>> + },
>> +};
>
> What's the .valid.min_access_size ?
> Do we need to set the .impl max/min access size here too ?
I honestly don't really understand the differences between .valid and .impl.
However, since all the code assumes that 4-byte accesses are made,
I think we can set all these values to 4 for now.
>
>
>> +
>> +static const ClockPortInitArray stm32l4x5_rcc_clocks = {
>> + QDEV_CLOCK_IN(Stm32l4x5RccState, hsi16_rc, NULL, 0),
>> + QDEV_CLOCK_IN(Stm32l4x5RccState, msi_rc, NULL, 0),
>> + QDEV_CLOCK_IN(Stm32l4x5RccState, hse, NULL, 0),
>> + QDEV_CLOCK_IN(Stm32l4x5RccState, lsi_rc, NULL, 0),
>> + QDEV_CLOCK_IN(Stm32l4x5RccState, lse_crystal, NULL, 0),
>> + QDEV_CLOCK_IN(Stm32l4x5RccState, sai1_extclk, NULL, 0),
>> + QDEV_CLOCK_IN(Stm32l4x5RccState, sai2_extclk, NULL, 0),
>> + QDEV_CLOCK_END
>> +};
>
> These are input clocks, so they each need a VMSTATE_CLOCK()
> line in the VMStateDescription. (I think only input clocks
> need to be migrated.)
Sure, will add these in the VMStateDescription.
>
>> +
>> +
>> +static void stm32l4x5_rcc_init(Object *obj)
>> +{
>> + Stm32l4x5RccState *s = STM32L4X5_RCC(obj);
>> +
>> + sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
>> +
>> + memory_region_init_io(&s->mmio, obj, &stm32l4x5_rcc_ops, s,
>> + TYPE_STM32L4X5_RCC, 0x400);
>> + sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
>> +
>> + qdev_init_clocks(DEVICE(s), stm32l4x5_rcc_clocks);
>> +
>> + s->gnd = clock_new(obj, "gnd");
>> +}
>
> Otherwise
> Reviewed-by: Peter Maydell
>
> thanks
> -- PMM
