Re: [Qemu-ppc] [PATCH v2 6/6] PPC 85xx: Find CPU speed on ppce500 from device tree

[Scott Wood]

On Fri, 2014-02-07 at 12:50 +0100, Alexander Graf wrote:
> On 07.02.2014, at 02:33, Scott Wood <address@hidden> wrote:
> 
> > On Thu, 2014-02-06 at 12:11 +0100, Alexander Graf wrote:
> >> On 04.02.2014, at 03:52, Scott Wood <address@hidden> wrote:
> >> 
> >>> On Fri, 2014-01-31 at 12:16 +0100, Alexander Graf wrote:
> >>>> The only thing we know in our PV machine through device tree is the clock
> >>>> speed of the CPUs. Take that as CPU speed, system speed and ddr speed so 
> >>>> that
> >>>> we have some meaningful values there at all.
> >>>> 
> >>>> The CPU speed is important because our timing loops get determined based 
> >>>> on it.
> >>>> 
> >>>> Signed-off-by: Alexander Graf <address@hidden>
> >>>> ---
> >>>> arch/powerpc/cpu/mpc85xx/Makefile           |    2 ++
> >>>> board/freescale/qemu-ppce500/qemu-ppce500.c |   43 
> >>>> +++++++++++++++++++++++++++
> >>>> 2 files changed, 45 insertions(+)
> >>>> 
> >>>> diff --git a/arch/powerpc/cpu/mpc85xx/Makefile 
> >>>> b/arch/powerpc/cpu/mpc85xx/Makefile
> >>>> index ef7637a..4094785 100644
> >>>> --- a/arch/powerpc/cpu/mpc85xx/Makefile
> >>>> +++ b/arch/powerpc/cpu/mpc85xx/Makefile
> >>>> @@ -102,7 +102,9 @@ obj-y        += cpu.o
> >>>> obj-y    += cpu_init.o
> >>>> obj-y    += cpu_init_early.o
> >>>> obj-y    += interrupts.o
> >>>> +ifneq ($(CONFIG_QEMU_E500),y)
> >>>> obj-y    += speed.o
> >>>> +endif
> >>>> obj-y    += tlb.o
> >>>> obj-y    += traps.o
> >>>> 
> >>>> diff --git a/board/freescale/qemu-ppce500/qemu-ppce500.c 
> >>>> b/board/freescale/qemu-ppce500/qemu-ppce500.c
> >>>> index 5d4dd64..9e9d688 100644
> >>>> --- a/board/freescale/qemu-ppce500/qemu-ppce500.c
> >>>> +++ b/board/freescale/qemu-ppce500/qemu-ppce500.c
> >>>> @@ -407,3 +407,46 @@ void init_laws(void)
> >>>> {
> >>>>  /* We don't emulate LAWs yet */
> >>>> }
> >>>> +
> >>>> +static uint32_t get_cpu_freq(void)
> >>>> +{
> >>>> +        const void *fdt = get_fdt();
> >>>> +        int cpus_node = fdt_path_offset(fdt, "/cpus");
> >>>> +        int cpu_node = fdt_first_subnode(fdt, cpus_node);
> >>>> +        return myfdt_one_cell(fdt, cpu_node, "clock-frequency", 0);
> >>>> +}
> >>>> +
> >>>> +void get_sys_info(sys_info_t *sys_info)
> >>>> +{
> >>>> +        int freq = get_cpu_freq();
> >>>> +
> >>>> +        memset(sys_info, 0, sizeof(sys_info_t));
> >>>> +        sys_info->freq_systembus = freq;
> >>>> +        sys_info->freq_ddrbus = freq;
> >>>> +        sys_info->freq_processor[0] = freq;
> >>>> +}
> >>>> +
> >>>> +int get_clocks (void)
> >>>> +{
> >>>> +        sys_info_t sys_info;
> >>>> +
> >>>> +        get_sys_info(&sys_info);
> >>>> +
> >>>> +        gd->cpu_clk = sys_info.freq_processor[0];
> >>>> +        gd->bus_clk = sys_info.freq_systembus;
> >>>> +        gd->mem_clk = sys_info.freq_ddrbus;
> >>>> +        gd->arch.lbc_clk = sys_info.freq_ddrbus;
> >>>> +
> >>>> +        return 0;
> >>>> +}
> >>> 
> >>> This probably decreases the accuracy of the timebase frequency, since
> >>> you'll be basing it on the CPU frequency rather than the bus frequency.
> >>> 
> >>> If you're doing this, why not override get_tbclk() as well?
> >> 
> >> That one only uses variables I already control, no?
> >> 
> >> unsigned long get_tbclk (void)
> >> {
> >>        unsigned long tbclk_div = CONFIG_SYS_FSL_TBCLK_DIV;
> >> 
> >>        return (gd->bus_clk + (tbclk_div >> 1)) / tbclk_div;
> >> }
> > 
> > What did you set CONFIG_SYS_FSL_TBCLK_DIV to?
> 
> Nothing which is why it's defaulting to 8.

Hence why I suggested overriding get_tbclk() rather than trying to fake
up a divider (which could be difficult if the CPU clock is not a
multiple of the bus clock).

-Scott