Re: [PATCH 1/4] hw/misc: Create STM32L4x5 SYSCFG clock

[Inès Varhol]

----- Le 7 Mai 24, à 11:50, peter maydell peter.maydell@linaro.org a écrit :

> On Sun, 5 May 2024 at 15:06, Inès Varhol <ines.varhol@telecom-paris.fr> wrote:
>>
>> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr>
> 
> In general you should try to avoid commits with no commit message.
> Sometimes there really isn't anything to say beyond what the
> subject line is, but that should be the exception rather than
> the usual thing.

Hello,

Understood, I'll add messages.

> 
>> ---
>>  include/hw/misc/stm32l4x5_syscfg.h |  1 +
>>  hw/arm/stm32l4x5_soc.c             |  2 ++
>>  hw/misc/stm32l4x5_syscfg.c         | 26 ++++++++++++++++++++++++++
>>  3 files changed, 29 insertions(+)
>>
>> diff --git a/include/hw/misc/stm32l4x5_syscfg.h
>> b/include/hw/misc/stm32l4x5_syscfg.h
>> index 23bb564150..c450df2b9e 100644
>> --- a/include/hw/misc/stm32l4x5_syscfg.h
>> +++ b/include/hw/misc/stm32l4x5_syscfg.h
>> @@ -48,6 +48,7 @@ struct Stm32l4x5SyscfgState {
>>      uint32_t swpr2;
>>
>>      qemu_irq gpio_out[GPIO_NUM_PINS];
>> +    Clock *clk;
>>  };
>>
>>  #endif
>> diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
>> index 38f7a2d5d9..fb2afa6cfe 100644
>> --- a/hw/arm/stm32l4x5_soc.c
>> +++ b/hw/arm/stm32l4x5_soc.c
>> @@ -236,6 +236,8 @@ static void stm32l4x5_soc_realize(DeviceState *dev_soc,
>> Error **errp)
>>
>>      /* System configuration controller */
>>      busdev = SYS_BUS_DEVICE(&s->syscfg);
>> +    qdev_connect_clock_in(DEVICE(&s->syscfg), "clk",
>> +        qdev_get_clock_out(DEVICE(&(s->rcc)), "syscfg-out"));
>>      if (!sysbus_realize(busdev, errp)) {
>>          return;
>>      }
>> diff --git a/hw/misc/stm32l4x5_syscfg.c b/hw/misc/stm32l4x5_syscfg.c
>> index a5a1ce2680..a82864c33d 100644
>> --- a/hw/misc/stm32l4x5_syscfg.c
>> +++ b/hw/misc/stm32l4x5_syscfg.c
>> @@ -26,6 +26,10 @@
>>  #include "trace.h"
>>  #include "hw/irq.h"
>>  #include "migration/vmstate.h"
>> +#include "hw/clock.h"
>> +#include "hw/qdev-clock.h"
>> +#include "qapi/visitor.h"
>> +#include "qapi/error.h"
>>  #include "hw/misc/stm32l4x5_syscfg.h"
>>  #include "hw/gpio/stm32l4x5_gpio.h"
>>
>> @@ -202,6 +206,14 @@ static void stm32l4x5_syscfg_write(void *opaque, hwaddr
>> addr,
>>      }
>>  }
>>
>> +static void clock_freq_get(Object *obj, Visitor *v,
>> +    const char *name, void *opaque, Error **errp)
>> +{
>> +    Stm32l4x5SyscfgState *s = STM32L4X5_SYSCFG(obj);
>> +    uint32_t clock_freq_hz = clock_get_hz(s->clk);
>> +    visit_type_uint32(v, name, &clock_freq_hz, errp);
>> +}
>> +
>>  static const MemoryRegionOps stm32l4x5_syscfg_ops = {
>>      .read = stm32l4x5_syscfg_read,
>>      .write = stm32l4x5_syscfg_write,
>> @@ -225,6 +237,18 @@ static void stm32l4x5_syscfg_init(Object *obj)
>>      qdev_init_gpio_in(DEVICE(obj), stm32l4x5_syscfg_set_irq,
>>                        GPIO_NUM_PINS * NUM_GPIOS);
>>      qdev_init_gpio_out(DEVICE(obj), s->gpio_out, GPIO_NUM_PINS);
>> +    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);
> 
> Why do we need this property? The clock on this device is an input,
> so the device doesn't control its frequency.

Using a QOM property allows to read the clock frequency from a QTest.
(npcm7xx_pwm-test.c does this, I didn't find other examples of reading a
frequency)

Best regards,

Inès Varhol