Re: [PATCH 3/4] hw/char: Add QOM property for STM32L4x5 USART clock frequency

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi,

On 5/5/24 16:05, Inès Varhol wrote:
> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> ---
>   hw/char/stm32l4x5_usart.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/hw/char/stm32l4x5_usart.c b/hw/char/stm32l4x5_usart.c
> index fc5dcac0c4..ee7727481c 100644
> --- a/hw/char/stm32l4x5_usart.c
> +++ b/hw/char/stm32l4x5_usart.c
> @@ -26,6 +26,7 @@
>   #include "hw/clock.h"
>   #include "hw/irq.h"
>   #include "hw/qdev-clock.h"
> +#include "qapi/visitor.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/qdev-properties-system.h"
>   #include "hw/registerfields.h"
> @@ -523,6 +524,14 @@ static Property stm32l4x5_usart_base_properties[] = {
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   
> +static void clock_freq_get(Object *obj, Visitor *v,
> +    const char *name, void *opaque, Error **errp)
> +{
> +    Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
> +    uint32_t clock_freq_hz = clock_get_hz(s->clk);
> +    visit_type_uint32(v, name, &clock_freq_hz, errp);
> +}
> +
>   static void stm32l4x5_usart_base_init(Object *obj)
>   {
>       Stm32l4x5UsartBaseState *s = STM32L4X5_USART_BASE(obj);
> @@ -534,6 +543,9 @@ static void stm32l4x5_usart_base_init(Object *obj)
>       sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
>   
>       s->clk = qdev_init_clock_in(DEVICE(s), "clk", NULL, s, 0);
> +
> +    object_property_add(obj, "clock-freq-hz", "uint32",
> +                        clock_freq_get, NULL, NULL, NULL);

Patch LGTM, but I wonder if registering QOM getter without setter
is recommended. Perhaps we should encourage parity? In normal HW
emulation we shouldn't update this clock externally, but thinking
about testing, this could be interesting to introduce jitter.

Any opinion on this?

>   }
>   
>   static int stm32l4x5_usart_base_post_load(void *opaque, int version_id)