Re: [Qemu-devel] [PATCH] hw/misc/zynq_slcr: Change CPU clock rate

[Peter Crosthwaite]

On Thu, Sep 10, 2015 at 6:57 PM, Guenter Roeck <address@hidden> wrote:
> Hi Peter,
>
> On Thu, Sep 10, 2015 at 04:26:13PM -0700, Peter Crosthwaite wrote:
>> On Wed, Aug 12, 2015 at 7:24 AM, Guenter Roeck <address@hidden> wrote:
>> > The Linux kernel only accepts 333334 Khz and 666667 Khz clock rates, and
>> > may crash if the actual clock rate is too low. The clock rate used to be
>> > (ps-clk-frequency * 26 / 4), which resulted in a CPU frequency of
>> > 216666 Khz if ps-clk-frequency was set to 33333333 Hz. Change it to
>> > (ps-clk-frequency * 20 / 2) = 333333 Khz to make Linux happy.
>> >
>>
>> This is a long known problem that is solved via patch in the yocto
>> meta-layer via a similar hack to slcr.
>>
> Doesn't sound like a solution to me :-(.
>

It's not :)

>> Ideally SLCR should follow the TRM reset value by default and this is
>> really setting up post bootloader state.
>>
> Which sclr ? The Linux driver ?
>

No I mean this QEMU device model. It would be nice if the Kernel was
able to deal with absent clocking (perhaps configured via DTS?) but in
reality I think we have to work around this from the QEMU side. We
just need to make your changes in this patch Linux specific rather
than absolute.

>> But PMM recently upstreamed the needed core patches to have a device
>> register Linux specific setup:
>>
>> Search for:
>>
>> "hw/arm: new interface for devices which need to behave differently
>> for kernel boot"
>>
>> and following patches that show how we can achieve best of both words,
>> with Linux specific device setup as well keep the ability to boot raw
>> (for testing firmware such as FSBL itself).
>>
> So what would be the correct solution here ? Implement the arm_linux_init
> callback for this driver, which would set the register values below, or
> some other acceptable register values ?
>

Yes I think so.

Regards,
Peter

> Or something else ?
>
>> Regards,
>> Peter
>>
>> P.S. Sorry about the delay (and to PMM on missed review of that
>> series). I dropped off the radar for a month or so July->Aug.
>>
> No worries.
>
> Thanks,
> Guenter
>
>> > Signed-off-by: Guenter Roeck <address@hidden>
>> > ---
>> >  hw/misc/zynq_slcr.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
>> > index 964f253..d3e1ce0 100644
>> > --- a/hw/misc/zynq_slcr.c
>> > +++ b/hw/misc/zynq_slcr.c
>> > @@ -189,7 +189,7 @@ static void zynq_slcr_reset(DeviceState *d)
>> >
>> >      s->regs[LOCKSTA] = 1;
>> >      /* 0x100 - 0x11C */
>> > -    s->regs[ARM_PLL_CTRL]   = 0x0001A008;
>> > +    s->regs[ARM_PLL_CTRL]   = 0x00014008;
>> >      s->regs[DDR_PLL_CTRL]   = 0x0001A008;
>> >      s->regs[IO_PLL_CTRL]    = 0x0001A008;
>> >      s->regs[PLL_STATUS]     = 0x0000003F;
>> > @@ -198,7 +198,7 @@ static void zynq_slcr_reset(DeviceState *d)
>> >      s->regs[IO_PLL_CFG]     = 0x00014000;
>> >
>> >      /* 0x120 - 0x16C */
>> > -    s->regs[ARM_CLK_CTRL]   = 0x1F000400;
>> > +    s->regs[ARM_CLK_CTRL]   = 0x1F000200;
>> >      s->regs[DDR_CLK_CTRL]   = 0x18400003;
>> >      s->regs[DCI_CLK_CTRL]   = 0x01E03201;
>> >      s->regs[APER_CLK_CTRL]  = 0x01FFCCCD;
>> > --
>> > 2.1.4
>> >
>> >