qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
@ 2020-07-27 16:26 Peter Maydell
  2020-07-27 18:05 ` Philippe Mathieu-Daudé
  2020-07-27 22:20 ` Alistair Francis
  0 siblings, 2 replies; 10+ messages in thread
From: Peter Maydell @ 2020-07-27 16:26 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Alistair Francis

The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
global, which meant that if guest code used the systick timer in "use
the processor clock" mode it would hang because time never advances.

Set the global to match the documented CPU clock speed of these boards.
Judging by the data sheet this is slightly simplistic because the
SoC allows configuration of the SYSCLK source and frequency via the
RCC (reset and clock control) module, but we don't model that.

Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
NB: tested with "make check" only...

 hw/arm/netduino2.c     | 10 ++++++++++
 hw/arm/netduinoplus2.c | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index 79e19392b56..8f103341443 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -30,10 +30,20 @@
 #include "hw/arm/stm32f205_soc.h"
 #include "hw/arm/boot.h"
 
+/* Main SYSCLK frequency in Hz (120MHz) */
+#define SYSCLK_FRQ 120000000ULL
+
 static void netduino2_init(MachineState *machine)
 {
     DeviceState *dev;
 
+    /*
+     * TODO: ideally we would model the SoC RCC and let it handle
+     * system_clock_scale, including its ability to define different
+     * possible SYSCLK sources.
+     */
+    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
+
     dev = qdev_new(TYPE_STM32F205_SOC);
     qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
index 958d21dd9f9..68abd3ec69d 100644
--- a/hw/arm/netduinoplus2.c
+++ b/hw/arm/netduinoplus2.c
@@ -30,10 +30,20 @@
 #include "hw/arm/stm32f405_soc.h"
 #include "hw/arm/boot.h"
 
+/* Main SYSCLK frequency in Hz (168MHz) */
+#define SYSCLK_FRQ 168000000ULL
+
 static void netduinoplus2_init(MachineState *machine)
 {
     DeviceState *dev;
 
+    /*
+     * TODO: ideally we would model the SoC RCC and let it handle
+     * system_clock_scale, including its ability to define different
+     * possible SYSCLK sources.
+     */
+    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
+
     dev = qdev_new(TYPE_STM32F405_SOC);
     qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-07-27 16:26 [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale Peter Maydell
@ 2020-07-27 18:05 ` Philippe Mathieu-Daudé
  2020-07-27 18:07   ` Philippe Mathieu-Daudé
  2020-07-27 22:20 ` Alistair Francis
  1 sibling, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-27 18:05 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis

On 7/27/20 6:26 PM, Peter Maydell wrote:
> The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
> global, which meant that if guest code used the systick timer in "use
> the processor clock" mode it would hang because time never advances.
> 
> Set the global to match the documented CPU clock speed of these boards.
> Judging by the data sheet this is slightly simplistic because the
> SoC allows configuration of the SYSCLK source and frequency via the
> RCC (reset and clock control) module, but we don't model that.
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> NB: tested with "make check" only...

What about adding another patch with:

-- >8 --
--- a/hw/timer/armv7m_systick.c
+++ b/hw/timer/armv7m_systick.c
@@ -17,6 +17,7 @@
 #include "qemu/timer.h"
 #include "qemu/log.h"
 #include "qemu/module.h"
+#include "qapi/error.h"
 #include "trace.h"

 /* qemu timers run at 1GHz.   We want something closer to 1MHz.  */
@@ -221,6 +222,11 @@ static void systick_instance_init(Object *obj)
 static void systick_realize(DeviceState *dev, Error **errp)
 {
     SysTickState *s = SYSTICK(dev);
+
+    if (!system_clock_scale) {
+        error_setg(errp, "can not use systick with 'system_clock_scale
= 0'");
+        return;
+    }
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
 }

---

I'll try to review with the datasheets tomorrow morning in case you want
to get it merged during the day.

> 
>  hw/arm/netduino2.c     | 10 ++++++++++
>  hw/arm/netduinoplus2.c | 10 ++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> index 79e19392b56..8f103341443 100644
> --- a/hw/arm/netduino2.c
> +++ b/hw/arm/netduino2.c
> @@ -30,10 +30,20 @@
>  #include "hw/arm/stm32f205_soc.h"
>  #include "hw/arm/boot.h"
>  
> +/* Main SYSCLK frequency in Hz (120MHz) */
> +#define SYSCLK_FRQ 120000000ULL
> +
>  static void netduino2_init(MachineState *machine)
>  {
>      DeviceState *dev;
>  
> +    /*
> +     * TODO: ideally we would model the SoC RCC and let it handle
> +     * system_clock_scale, including its ability to define different
> +     * possible SYSCLK sources.
> +     */
> +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> +
>      dev = qdev_new(TYPE_STM32F205_SOC);
>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
> index 958d21dd9f9..68abd3ec69d 100644
> --- a/hw/arm/netduinoplus2.c
> +++ b/hw/arm/netduinoplus2.c
> @@ -30,10 +30,20 @@
>  #include "hw/arm/stm32f405_soc.h"
>  #include "hw/arm/boot.h"
>  
> +/* Main SYSCLK frequency in Hz (168MHz) */
> +#define SYSCLK_FRQ 168000000ULL
> +
>  static void netduinoplus2_init(MachineState *machine)
>  {
>      DeviceState *dev;
>  
> +    /*
> +     * TODO: ideally we would model the SoC RCC and let it handle
> +     * system_clock_scale, including its ability to define different
> +     * possible SYSCLK sources.
> +     */
> +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> +
>      dev = qdev_new(TYPE_STM32F405_SOC);
>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-07-27 18:05 ` Philippe Mathieu-Daudé
@ 2020-07-27 18:07   ` Philippe Mathieu-Daudé
  2020-07-27 18:23     ` Peter Maydell
  0 siblings, 1 reply; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-27 18:07 UTC (permalink / raw)
  To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Alistair Francis

On 7/27/20 8:05 PM, Philippe Mathieu-Daudé wrote:
> On 7/27/20 6:26 PM, Peter Maydell wrote:
>> The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
>> global, which meant that if guest code used the systick timer in "use
>> the processor clock" mode it would hang because time never advances.
>>
>> Set the global to match the documented CPU clock speed of these boards.
>> Judging by the data sheet this is slightly simplistic because the
>> SoC allows configuration of the SYSCLK source and frequency via the
>> RCC (reset and clock control) module, but we don't model that.
>>
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> NB: tested with "make check" only...
> 
> What about adding another patch with:
> 
> -- >8 --
> --- a/hw/timer/armv7m_systick.c
> +++ b/hw/timer/armv7m_systick.c
> @@ -17,6 +17,7 @@
>  #include "qemu/timer.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> +#include "qapi/error.h"
>  #include "trace.h"
> 
>  /* qemu timers run at 1GHz.   We want something closer to 1MHz.  */
> @@ -221,6 +222,11 @@ static void systick_instance_init(Object *obj)
>  static void systick_realize(DeviceState *dev, Error **errp)
>  {
>      SysTickState *s = SYSTICK(dev);
> +
> +    if (!system_clock_scale) {
> +        error_setg(errp, "can not use systick with 'system_clock_scale
> = 0'");
> +        return;
> +    }
>      s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
>  }

Hmm testing the microbit:

$ qemu-system-arm -M microbit
qemu-system-arm: can not use systick with 'system_clock_scale = 0'

> 
> ---
> 
> I'll try to review with the datasheets tomorrow morning in case you want
> to get it merged during the day.
> 
>>
>>  hw/arm/netduino2.c     | 10 ++++++++++
>>  hw/arm/netduinoplus2.c | 10 ++++++++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
>> index 79e19392b56..8f103341443 100644
>> --- a/hw/arm/netduino2.c
>> +++ b/hw/arm/netduino2.c
>> @@ -30,10 +30,20 @@
>>  #include "hw/arm/stm32f205_soc.h"
>>  #include "hw/arm/boot.h"
>>  
>> +/* Main SYSCLK frequency in Hz (120MHz) */
>> +#define SYSCLK_FRQ 120000000ULL
>> +
>>  static void netduino2_init(MachineState *machine)
>>  {
>>      DeviceState *dev;
>>  
>> +    /*
>> +     * TODO: ideally we would model the SoC RCC and let it handle
>> +     * system_clock_scale, including its ability to define different
>> +     * possible SYSCLK sources.
>> +     */
>> +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
>> +
>>      dev = qdev_new(TYPE_STM32F205_SOC);
>>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
>>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>> diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
>> index 958d21dd9f9..68abd3ec69d 100644
>> --- a/hw/arm/netduinoplus2.c
>> +++ b/hw/arm/netduinoplus2.c
>> @@ -30,10 +30,20 @@
>>  #include "hw/arm/stm32f405_soc.h"
>>  #include "hw/arm/boot.h"
>>  
>> +/* Main SYSCLK frequency in Hz (168MHz) */
>> +#define SYSCLK_FRQ 168000000ULL
>> +
>>  static void netduinoplus2_init(MachineState *machine)
>>  {
>>      DeviceState *dev;
>>  
>> +    /*
>> +     * TODO: ideally we would model the SoC RCC and let it handle
>> +     * system_clock_scale, including its ability to define different
>> +     * possible SYSCLK sources.
>> +     */
>> +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
>> +
>>      dev = qdev_new(TYPE_STM32F405_SOC);
>>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>>
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-07-27 18:07   ` Philippe Mathieu-Daudé
@ 2020-07-27 18:23     ` Peter Maydell
  0 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2020-07-27 18:23 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-arm, QEMU Developers, Alistair Francis

On Mon, 27 Jul 2020 at 19:07, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> $ qemu-system-arm -M microbit
> qemu-system-arm: can not use systick with 'system_clock_scale = 0'

Yes, that's the other one on my list to do next (need to find
out what frequency it runs at though).

Good idea to have an automatic check for system_clock_scale
being set, but failure to do that is a bug in the board
code, so we might as well assert() it.

-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-07-27 16:26 [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale Peter Maydell
  2020-07-27 18:05 ` Philippe Mathieu-Daudé
@ 2020-07-27 22:20 ` Alistair Francis
  1 sibling, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2020-07-27 22:20 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, qemu-devel@nongnu.org Developers, Alistair Francis

On Mon, Jul 27, 2020 at 9:26 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
> global, which meant that if guest code used the systick timer in "use
> the processor clock" mode it would hang because time never advances.
>
> Set the global to match the documented CPU clock speed of these boards.
> Judging by the data sheet this is slightly simplistic because the
> SoC allows configuration of the SYSCLK source and frequency via the
> RCC (reset and clock control) module, but we don't model that.
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> NB: tested with "make check" only...
>
>  hw/arm/netduino2.c     | 10 ++++++++++
>  hw/arm/netduinoplus2.c | 10 ++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> index 79e19392b56..8f103341443 100644
> --- a/hw/arm/netduino2.c
> +++ b/hw/arm/netduino2.c
> @@ -30,10 +30,20 @@
>  #include "hw/arm/stm32f205_soc.h"
>  #include "hw/arm/boot.h"
>
> +/* Main SYSCLK frequency in Hz (120MHz) */
> +#define SYSCLK_FRQ 120000000ULL
> +
>  static void netduino2_init(MachineState *machine)
>  {
>      DeviceState *dev;
>
> +    /*
> +     * TODO: ideally we would model the SoC RCC and let it handle
> +     * system_clock_scale, including its ability to define different
> +     * possible SYSCLK sources.
> +     */
> +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> +
>      dev = qdev_new(TYPE_STM32F205_SOC);
>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
> index 958d21dd9f9..68abd3ec69d 100644
> --- a/hw/arm/netduinoplus2.c
> +++ b/hw/arm/netduinoplus2.c
> @@ -30,10 +30,20 @@
>  #include "hw/arm/stm32f405_soc.h"
>  #include "hw/arm/boot.h"
>
> +/* Main SYSCLK frequency in Hz (168MHz) */
> +#define SYSCLK_FRQ 168000000ULL
> +
>  static void netduinoplus2_init(MachineState *machine)
>  {
>      DeviceState *dev;
>
> +    /*
> +     * TODO: ideally we would model the SoC RCC and let it handle
> +     * system_clock_scale, including its ability to define different
> +     * possible SYSCLK sources.
> +     */
> +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> +
>      dev = qdev_new(TYPE_STM32F405_SOC);
>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-10-12 15:33 [PATCH 00/10] target/arm: Various v8.1M minor features Peter Maydell
@ 2020-10-12 15:33 ` Peter Maydell
  2020-10-14  0:29   ` Alistair Francis
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2020-10-12 15:33 UTC (permalink / raw)
  To: qemu-arm, qemu-devel; +Cc: Richard Henderson

The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
global, which meant that if guest code used the systick timer in "use
the processor clock" mode it would hang because time never advances.

Set the global to match the documented CPU clock speed of these boards.
Judging by the data sheet this is slightly simplistic because the
SoC allows configuration of the SYSCLK source and frequency via the
RCC (reset and clock control) module, but we don't model that.

Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
NB: tested with "make check" only...

 hw/arm/netduino2.c     | 10 ++++++++++
 hw/arm/netduinoplus2.c | 10 ++++++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index 79e19392b56..8f103341443 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -30,10 +30,20 @@
 #include "hw/arm/stm32f205_soc.h"
 #include "hw/arm/boot.h"
 
+/* Main SYSCLK frequency in Hz (120MHz) */
+#define SYSCLK_FRQ 120000000ULL
+
 static void netduino2_init(MachineState *machine)
 {
     DeviceState *dev;
 
+    /*
+     * TODO: ideally we would model the SoC RCC and let it handle
+     * system_clock_scale, including its ability to define different
+     * possible SYSCLK sources.
+     */
+    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
+
     dev = qdev_new(TYPE_STM32F205_SOC);
     qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
index 958d21dd9f9..68abd3ec69d 100644
--- a/hw/arm/netduinoplus2.c
+++ b/hw/arm/netduinoplus2.c
@@ -30,10 +30,20 @@
 #include "hw/arm/stm32f405_soc.h"
 #include "hw/arm/boot.h"
 
+/* Main SYSCLK frequency in Hz (168MHz) */
+#define SYSCLK_FRQ 168000000ULL
+
 static void netduinoplus2_init(MachineState *machine)
 {
     DeviceState *dev;
 
+    /*
+     * TODO: ideally we would model the SoC RCC and let it handle
+     * system_clock_scale, including its ability to define different
+     * possible SYSCLK sources.
+     */
+    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
+
     dev = qdev_new(TYPE_STM32F405_SOC);
     qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-10-12 15:33 ` [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale Peter Maydell
@ 2020-10-14  0:29   ` Alistair Francis
  2020-10-14  0:31     ` Alistair Francis
  0 siblings, 1 reply; 10+ messages in thread
From: Alistair Francis @ 2020-10-14  0:29 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, Richard Henderson, qemu-devel@nongnu.org Developers

On Mon, Oct 12, 2020 at 8:45 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
> global, which meant that if guest code used the systick timer in "use
> the processor clock" mode it would hang because time never advances.
>
> Set the global to match the documented CPU clock speed of these boards.
> Judging by the data sheet this is slightly simplistic because the
> SoC allows configuration of the SYSCLK source and frequency via the
> RCC (reset and clock control) module, but we don't model that.
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> NB: tested with "make check" only...
>
>  hw/arm/netduino2.c     | 10 ++++++++++
>  hw/arm/netduinoplus2.c | 10 ++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> index 79e19392b56..8f103341443 100644
> --- a/hw/arm/netduino2.c
> +++ b/hw/arm/netduino2.c
> @@ -30,10 +30,20 @@
>  #include "hw/arm/stm32f205_soc.h"
>  #include "hw/arm/boot.h"
>
> +/* Main SYSCLK frequency in Hz (120MHz) */
> +#define SYSCLK_FRQ 120000000ULL
> +
>  static void netduino2_init(MachineState *machine)
>  {
>      DeviceState *dev;
>
> +    /*
> +     * TODO: ideally we would model the SoC RCC and let it handle
> +     * system_clock_scale, including its ability to define different
> +     * possible SYSCLK sources.
> +     */
> +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> +
>      dev = qdev_new(TYPE_STM32F205_SOC);
>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
> index 958d21dd9f9..68abd3ec69d 100644
> --- a/hw/arm/netduinoplus2.c
> +++ b/hw/arm/netduinoplus2.c
> @@ -30,10 +30,20 @@
>  #include "hw/arm/stm32f405_soc.h"
>  #include "hw/arm/boot.h"
>
> +/* Main SYSCLK frequency in Hz (168MHz) */
> +#define SYSCLK_FRQ 168000000ULL
> +
>  static void netduinoplus2_init(MachineState *machine)
>  {
>      DeviceState *dev;
>
> +    /*
> +     * TODO: ideally we would model the SoC RCC and let it handle
> +     * system_clock_scale, including its ability to define different
> +     * possible SYSCLK sources.
> +     */
> +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> +
>      dev = qdev_new(TYPE_STM32F405_SOC);
>      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-10-14  0:29   ` Alistair Francis
@ 2020-10-14  0:31     ` Alistair Francis
  2020-10-14 12:39       ` Peter Maydell
  0 siblings, 1 reply; 10+ messages in thread
From: Alistair Francis @ 2020-10-14  0:31 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, Richard Henderson, qemu-devel@nongnu.org Developers

On Tue, Oct 13, 2020 at 5:29 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, Oct 12, 2020 at 8:45 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
> > global, which meant that if guest code used the systick timer in "use
> > the processor clock" mode it would hang because time never advances.
> >
> > Set the global to match the documented CPU clock speed of these boards.
> > Judging by the data sheet this is slightly simplistic because the
> > SoC allows configuration of the SYSCLK source and frequency via the
> > RCC (reset and clock control) module, but we don't model that.
> >
> > Fixes: https://bugs.launchpad.net/qemu/+bug/1876187

Thanks for fixing this Peter.

Alistair

> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Alistair
>
> > ---
> > NB: tested with "make check" only...
> >
> >  hw/arm/netduino2.c     | 10 ++++++++++
> >  hw/arm/netduinoplus2.c | 10 ++++++++++
> >  2 files changed, 20 insertions(+)
> >
> > diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
> > index 79e19392b56..8f103341443 100644
> > --- a/hw/arm/netduino2.c
> > +++ b/hw/arm/netduino2.c
> > @@ -30,10 +30,20 @@
> >  #include "hw/arm/stm32f205_soc.h"
> >  #include "hw/arm/boot.h"
> >
> > +/* Main SYSCLK frequency in Hz (120MHz) */
> > +#define SYSCLK_FRQ 120000000ULL
> > +
> >  static void netduino2_init(MachineState *machine)
> >  {
> >      DeviceState *dev;
> >
> > +    /*
> > +     * TODO: ideally we would model the SoC RCC and let it handle
> > +     * system_clock_scale, including its ability to define different
> > +     * possible SYSCLK sources.
> > +     */
> > +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> > +
> >      dev = qdev_new(TYPE_STM32F205_SOC);
> >      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3"));
> >      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> > diff --git a/hw/arm/netduinoplus2.c b/hw/arm/netduinoplus2.c
> > index 958d21dd9f9..68abd3ec69d 100644
> > --- a/hw/arm/netduinoplus2.c
> > +++ b/hw/arm/netduinoplus2.c
> > @@ -30,10 +30,20 @@
> >  #include "hw/arm/stm32f405_soc.h"
> >  #include "hw/arm/boot.h"
> >
> > +/* Main SYSCLK frequency in Hz (168MHz) */
> > +#define SYSCLK_FRQ 168000000ULL
> > +
> >  static void netduinoplus2_init(MachineState *machine)
> >  {
> >      DeviceState *dev;
> >
> > +    /*
> > +     * TODO: ideally we would model the SoC RCC and let it handle
> > +     * system_clock_scale, including its ability to define different
> > +     * possible SYSCLK sources.
> > +     */
> > +    system_clock_scale = NANOSECONDS_PER_SECOND / SYSCLK_FRQ;
> > +
> >      dev = qdev_new(TYPE_STM32F405_SOC);
> >      qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m4"));
> >      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
> > --
> > 2.20.1
> >
> >


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-10-14  0:31     ` Alistair Francis
@ 2020-10-14 12:39       ` Peter Maydell
  2020-10-14 14:32         ` Alistair Francis
  0 siblings, 1 reply; 10+ messages in thread
From: Peter Maydell @ 2020-10-14 12:39 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-arm, Richard Henderson, qemu-devel@nongnu.org Developers

On Wed, 14 Oct 2020 at 01:42, Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Oct 13, 2020 at 5:29 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Mon, Oct 12, 2020 at 8:45 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
> > > global, which meant that if guest code used the systick timer in "use
> > > the processor clock" mode it would hang because time never advances.
> > >
> > > Set the global to match the documented CPU clock speed of these boards.
> > > Judging by the data sheet this is slightly simplistic because the
> > > SoC allows configuration of the SYSCLK source and frequency via the
> > > RCC (reset and clock control) module, but we don't model that.
> > >
> > > Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
>
> Thanks for fixing this Peter.

This is already in master (commit e7e5a9595ab) -- this email is
one of a set of stale patchmails I sent out by mistake on Monday
when I mangled a git send-email command. Sorry for the confusion.

-- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale
  2020-10-14 12:39       ` Peter Maydell
@ 2020-10-14 14:32         ` Alistair Francis
  0 siblings, 0 replies; 10+ messages in thread
From: Alistair Francis @ 2020-10-14 14:32 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-arm, Richard Henderson, qemu-devel@nongnu.org Developers

On Wed, Oct 14, 2020 at 5:39 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 14 Oct 2020 at 01:42, Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Oct 13, 2020 at 5:29 PM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Mon, Oct 12, 2020 at 8:45 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> > > >
> > > > The netduino2 and netduinoplus2 boards forgot to set the system_clock_scale
> > > > global, which meant that if guest code used the systick timer in "use
> > > > the processor clock" mode it would hang because time never advances.
> > > >
> > > > Set the global to match the documented CPU clock speed of these boards.
> > > > Judging by the data sheet this is slightly simplistic because the
> > > > SoC allows configuration of the SYSCLK source and frequency via the
> > > > RCC (reset and clock control) module, but we don't model that.
> > > >
> > > > Fixes: https://bugs.launchpad.net/qemu/+bug/1876187
> >
> > Thanks for fixing this Peter.
>
> This is already in master (commit e7e5a9595ab) -- this email is

I thought I saw that in the bug report, but I just assumed it was
pointing to a branch.

> one of a set of stale patchmails I sent out by mistake on Monday
> when I mangled a git send-email command. Sorry for the confusion.

Strange. No worries.

Alistair

>
> -- PMM


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-10-14 14:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-27 16:26 [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale Peter Maydell
2020-07-27 18:05 ` Philippe Mathieu-Daudé
2020-07-27 18:07   ` Philippe Mathieu-Daudé
2020-07-27 18:23     ` Peter Maydell
2020-07-27 22:20 ` Alistair Francis
  -- strict thread matches above, loose matches on Subject: below --
2020-10-12 15:33 [PATCH 00/10] target/arm: Various v8.1M minor features Peter Maydell
2020-10-12 15:33 ` [PATCH for-5.1] hw/arm/netduino2, netduinoplus2: Set system_clock_scale Peter Maydell
2020-10-14  0:29   ` Alistair Francis
2020-10-14  0:31     ` Alistair Francis
2020-10-14 12:39       ` Peter Maydell
2020-10-14 14:32         ` Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).