Re: [PATCH v3 12/20] hw/mips/r4k: Explicit CPU frequency is 200 MHz

[Philippe Mathieu-Daudé]

Hi Huacai,

On 10/11/20 5:52 AM, chen huacai wrote:
> Hi, Philippe,
>
> On Sun, Oct 11, 2020 at 4:43 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > Since its introduction in commit 6af0bf9c7c3,
> > the 'r4k' machine runs at 200 MHz.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> >   hw/mips/r4k.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/mips/r4k.c b/hw/mips/r4k.c
> > index 3487013a4a1..e64687b505a 100644
> > --- a/hw/mips/r4k.c
> > +++ b/hw/mips/r4k.c
> > @@ -37,6 +37,7 @@
> >   #include "sysemu/reset.h"
> >   #include "sysemu/runstate.h"
> >   #include "qemu/error-report.h"
> > +#include "hw/qdev-clock.h"
> >
> >   #define MAX_IDE_BUS 2
> >
> > @@ -184,6 +185,7 @@ void mips_r4k_init(MachineState *machine)
> >       int bios_size;
> >       MIPSCPU *cpu;
> >       CPUMIPSState *env;
> > +    Clock *cpuclk;
> >       ResetData *reset_info;
> >       int i;
> >       qemu_irq *i8259;
> > @@ -193,7 +195,11 @@ void mips_r4k_init(MachineState *machine)
> >       int be;
> >
> >       /* init CPUs */
> > -    cpu = MIPS_CPU(cpu_create(machine->cpu_type));
> > +    cpu = MIPS_CPU(object_new(machine->cpu_type));
> > +    cpuclk = clock_new(OBJECT(machine), "cpu-refclk");
> > +    clock_set_hz(cpuclk, 200000000); /* 200 MHz */
> > +    qdev_connect_clock_in(DEVICE(cpu), "clk-in", cpuclk);
> > +    qdev_realize(DEVICE(cpu), NULL, &error_abort);
>
> Can we add a new parameter to cpu_create() and set the freq in the core code?

Adding a new parameter seems a good idea.

Both maintainers of the core code are reluctant to add
a CPU clock to the core code, see:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg747589.html
https://www.mail-archive.com/qemu-devel@nongnu.org/msg747612.html
Which is why I restricted that to the MIPS CPUs.

On ARM, Damien started to use clocks on the Zynq SoC (merged):
https://www.mail-archive.com/qemu-devel@nongnu.org/msg694604.html
Luc is working on adding a clock manager to the Broadcom SoC:
https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg02840.html

I also started converting one UART devices:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg727972.html

So IMO the core code will soon require this. Maybe we will add it
during the next development window.

Meanwhile I can add a mips_cpu_create_with_clock() in preparation
of cpu_create_with_clock().

> Huacai
> >       env = &cpu->env;
> >
> >       reset_info = g_malloc0(sizeof(ResetData));
> > --
> > 2.26.2