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

2015-09-10 Thread Peter Crosthwaite 
On Wed, Aug 12, 2015 at 7:24 AM, Guenter Roeck  wrote:
> The Linux kernel only accepts 34 Khz and 67 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
> 21 Khz if ps-clk-frequency was set to  Hz. Change it to
> (ps-clk-frequency * 20 / 2) = 33 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.

Ideally SLCR should follow the TRM reset value by default and this is
really setting up post bootloader state.

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).

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.

> Signed-off-by: Guenter Roeck 
> ---
>  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] = 0x003F;
> @@ -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]   = 0x1843;
>  s->regs[DCI_CLK_CTRL]   = 0x01E03201;
>  s->regs[APER_CLK_CTRL]  = 0x01FFCCCD;
> --
> 2.1.4
>
>

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

2015-09-10 Thread Guenter Roeck 
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  wrote:
> > The Linux kernel only accepts 34 Khz and 67 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
> > 21 Khz if ps-clk-frequency was set to  Hz. Change it to
> > (ps-clk-frequency * 20 / 2) = 33 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 :-(.

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

> 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 ?

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 
> > ---
> >  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] = 0x003F;
> > @@ -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]   = 0x1843;
> >  s->regs[DCI_CLK_CTRL]   = 0x01E03201;
> >  s->regs[APER_CLK_CTRL]  = 0x01FFCCCD;
> > --
> > 2.1.4
> >
> >

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

2015-09-10 Thread Peter Crosthwaite 
On Thu, Sep 10, 2015 at 6:57 PM, Guenter Roeck  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  wrote:
>> > The Linux kernel only accepts 34 Khz and 67 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
>> > 21 Khz if ps-clk-frequency was set to  Hz. Change it to
>> > (ps-clk-frequency * 20 / 2) = 33 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 
>> > ---
>> >  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] = 0x003F;
>> > @@ -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]   = 0x1843;
>> >  s->regs[DCI_CLK_CTRL]   = 0x01E03201;
>> >  s->regs[APER_CLK_CTRL]  = 0x01FFCCCD;
>> > --
>> > 2.1.4
>> >
>> >

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

2015-08-12 Thread Guenter Roeck 
The Linux kernel only accepts 34 Khz and 67 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
21 Khz if ps-clk-frequency was set to  Hz. Change it to
(ps-clk-frequency * 20 / 2) = 33 Khz to make Linux happy.

Signed-off-by: Guenter Roeck li...@roeck-us.net
---
 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] = 0x003F;
@@ -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]   = 0x1843;
 s-regs[DCI_CLK_CTRL]   = 0x01E03201;
 s-regs[APER_CLK_CTRL]  = 0x01FFCCCD;
-- 
2.1.4