| Message ID | 20220208171643.3486277-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/arm/armv7m: Handle disconnected clock inputs | expand |
On 8/2/22 18:16, Peter Maydell wrote: > In the armv7m object, handle clock inputs that aren't connected. > This is always an error for 'cpuclk'. For 'refclk' it is OK for this > to be disconnected, but we need to handle it by not trying to connect > a sourceless-clock to the systick device. > > This fixes a bug where on the mps2-an521 and similar boards (which > do not have a refclk) the systick device incorrectly reset with > SYST_CSR.CLKSOURCE 0 ("use refclk") rather than 1 ("use CPU clock"). > > Cc: qemu-stable@nongnu.org > Reported-by: Richard Petri <git@rpls.de> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > The other option would be to have clock_has_source() look not > just at clk->source but somehow walk up the clock tree to see > if it can find something that looks like a "root". That seems > overcomplicated... > --- > hw/arm/armv7m.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On 2/9/22 04:16, Peter Maydell wrote: > In the armv7m object, handle clock inputs that aren't connected. > This is always an error for 'cpuclk'. For 'refclk' it is OK for this > to be disconnected, but we need to handle it by not trying to connect > a sourceless-clock to the systick device. > > This fixes a bug where on the mps2-an521 and similar boards (which > do not have a refclk) the systick device incorrectly reset with > SYST_CSR.CLKSOURCE 0 ("use refclk") rather than 1 ("use CPU clock"). > > Cc:qemu-stable@nongnu.org > Reported-by: Richard Petri<git@rpls.de> > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > The other option would be to have clock_has_source() look not > just at clk->source but somehow walk up the clock tree to see > if it can find something that looks like a "root". That seems > overcomplicated... > --- > hw/arm/armv7m.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c index ceb76df3cd4..41cfca0f223 100644 --- a/hw/arm/armv7m.c +++ b/hw/arm/armv7m.c @@ -284,6 +284,12 @@ static void armv7m_realize(DeviceState *dev, Error **errp) return; } + /* cpuclk must be connected; refclk is optional */ + if (!clock_has_source(s->cpuclk)) { + error_setg(errp, "armv7m: cpuclk must be connected"); + return; + } + memory_region_add_subregion_overlap(&s->container, 0, s->board_memory, -1); s->cpu = ARM_CPU(object_new_with_props(s->cpu_type, OBJECT(s), "cpu", @@ -420,8 +426,18 @@ static void armv7m_realize(DeviceState *dev, Error **errp) &s->sysreg_ns_mem); } - /* Create and map the systick devices */ - qdev_connect_clock_in(DEVICE(&s->systick[M_REG_NS]), "refclk", s->refclk); + /* + * Create and map the systick devices. Note that we only connect + * refclk if it has been connected to us; otherwise the systick + * device gets the wrong answer for clock_has_source(refclk), because + * it has an immediate source (the ARMv7M's clock object) but not + * an ultimate source, and then it won't correctly auto-select the + * CPU clock as its only possible clock source. + */ + if (clock_has_source(s->refclk)) { + qdev_connect_clock_in(DEVICE(&s->systick[M_REG_NS]), "refclk", + s->refclk); + } qdev_connect_clock_in(DEVICE(&s->systick[M_REG_NS]), "cpuclk", s->cpuclk); if (!sysbus_realize(SYS_BUS_DEVICE(&s->systick[M_REG_NS]), errp)) { return; @@ -438,8 +454,10 @@ static void armv7m_realize(DeviceState *dev, Error **errp) */ object_initialize_child(OBJECT(dev), "systick-reg-s", &s->systick[M_REG_S], TYPE_SYSTICK); - qdev_connect_clock_in(DEVICE(&s->systick[M_REG_S]), "refclk", - s->refclk); + if (clock_has_source(s->refclk)) { + qdev_connect_clock_in(DEVICE(&s->systick[M_REG_S]), "refclk", + s->refclk); + } qdev_connect_clock_in(DEVICE(&s->systick[M_REG_S]), "cpuclk", s->cpuclk);
In the armv7m object, handle clock inputs that aren't connected. This is always an error for 'cpuclk'. For 'refclk' it is OK for this to be disconnected, but we need to handle it by not trying to connect a sourceless-clock to the systick device. This fixes a bug where on the mps2-an521 and similar boards (which do not have a refclk) the systick device incorrectly reset with SYST_CSR.CLKSOURCE 0 ("use refclk") rather than 1 ("use CPU clock"). Cc: qemu-stable@nongnu.org Reported-by: Richard Petri <git@rpls.de> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- The other option would be to have clock_has_source() look not just at clk->source but somehow walk up the clock tree to see if it can find something that looks like a "root". That seems overcomplicated... --- hw/arm/armv7m.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-)