* [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
@ 2015-09-12 21:06 Guenter Roeck
2015-09-13 20:22 ` Peter Crosthwaite
2015-09-27 5:50 ` Peter Crosthwaite
0 siblings, 2 replies; 11+ messages in thread
From: Guenter Roeck @ 2015-09-12 21:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Crosthwaite, Guenter Roeck, Alistair Francis
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 for to make Linux happy.
Limit the change to Linux boots only.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Limit scope of change to Linux boots.
hw/misc/zynq_slcr.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index 964f253..ed510fb 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -14,6 +14,7 @@
* with this program; if not, see <http://www.gnu.org/licenses/>.
*/
+#include "hw/arm/linux-boot-if.h"
#include "hw/hw.h"
#include "qemu/timer.h"
#include "hw/sysbus.h"
@@ -177,6 +178,8 @@ typedef struct ZynqSLCRState {
MemoryRegion iomem;
+ bool is_linux;
+
uint32_t regs[ZYNQ_SLCR_NUM_REGS];
} ZynqSLCRState;
@@ -189,7 +192,11 @@ static void zynq_slcr_reset(DeviceState *d)
s->regs[LOCKSTA] = 1;
/* 0x100 - 0x11C */
- s->regs[ARM_PLL_CTRL] = 0x0001A008;
+ if (!s->is_linux) {
+ s->regs[ARM_PLL_CTRL] = 0x0001A008;
+ } else {
+ 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 +205,11 @@ static void zynq_slcr_reset(DeviceState *d)
s->regs[IO_PLL_CFG] = 0x00014000;
/* 0x120 - 0x16C */
- s->regs[ARM_CLK_CTRL] = 0x1F000400;
+ if (!s->is_linux) {
+ s->regs[ARM_CLK_CTRL] = 0x1F000400;
+ } else {
+ s->regs[ARM_CLK_CTRL] = 0x1F000200;
+ }
s->regs[DDR_CLK_CTRL] = 0x18400003;
s->regs[DCI_CLK_CTRL] = 0x01E03201;
s->regs[APER_CLK_CTRL] = 0x01FFCCCD;
@@ -429,17 +440,27 @@ static const VMStateDescription vmstate_zynq_slcr = {
.version_id = 2,
.minimum_version_id = 2,
.fields = (VMStateField[]) {
+ VMSTATE_BOOL(is_linux, ZynqSLCRState),
VMSTATE_UINT32_ARRAY(regs, ZynqSLCRState, ZYNQ_SLCR_NUM_REGS),
VMSTATE_END_OF_LIST()
}
};
+static void zynq_sclr_linux_init(ARMLinuxBootIf *obj, bool secure_boot)
+{
+ ZynqSLCRState *s = ZYNQ_SLCR(obj);
+
+ s->is_linux = true;
+}
+
static void zynq_slcr_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+ ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_CLASS(klass);
dc->vmsd = &vmstate_zynq_slcr;
dc->reset = zynq_slcr_reset;
+ albifc->arm_linux_init = zynq_sclr_linux_init;
}
static const TypeInfo zynq_slcr_info = {
@@ -448,6 +469,10 @@ static const TypeInfo zynq_slcr_info = {
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(ZynqSLCRState),
.instance_init = zynq_slcr_init,
+ .interfaces = (InterfaceInfo []) {
+ { TYPE_ARM_LINUX_BOOT_IF },
+ { },
+ },
};
static void zynq_slcr_register_types(void)
--
2.1.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-12 21:06 [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots Guenter Roeck
@ 2015-09-13 20:22 ` Peter Crosthwaite
2015-09-13 20:37 ` Guenter Roeck
2015-09-13 20:47 ` Peter Maydell
2015-09-27 5:50 ` Peter Crosthwaite
1 sibling, 2 replies; 11+ messages in thread
From: Peter Crosthwaite @ 2015-09-13 20:22 UTC (permalink / raw)
To: Guenter Roeck, Peter Maydell, Nathan Rossi, Alistair Francis
Cc: qemu-devel@nongnu.org Developers, Peter Crosthwaite
On Sat, Sep 12, 2015 at 2:06 PM, Guenter Roeck <linux@roeck-us.net> 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 for to make Linux happy.
> Limit the change to Linux boots only.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
Can this go via target-arm? (cc PMM).
There may be more changes worth making on is_linux. I don't have the
patch with the full list of FSBL-related SLCR changes handy and can't
seem to find it in any modern Yocto trees. Wondering if Yocto still
supports booting Zynq without FSBL (Nathan/Alistair may know more)?
Regards,
Peter
> ---
> v2: Limit scope of change to Linux boots.
>
> hw/misc/zynq_slcr.c | 29 +++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index 964f253..ed510fb 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -14,6 +14,7 @@
> * with this program; if not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include "hw/arm/linux-boot-if.h"
> #include "hw/hw.h"
> #include "qemu/timer.h"
> #include "hw/sysbus.h"
> @@ -177,6 +178,8 @@ typedef struct ZynqSLCRState {
>
> MemoryRegion iomem;
>
> + bool is_linux;
> +
> uint32_t regs[ZYNQ_SLCR_NUM_REGS];
> } ZynqSLCRState;
>
> @@ -189,7 +192,11 @@ static void zynq_slcr_reset(DeviceState *d)
>
> s->regs[LOCKSTA] = 1;
> /* 0x100 - 0x11C */
> - s->regs[ARM_PLL_CTRL] = 0x0001A008;
> + if (!s->is_linux) {
> + s->regs[ARM_PLL_CTRL] = 0x0001A008;
> + } else {
> + 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 +205,11 @@ static void zynq_slcr_reset(DeviceState *d)
> s->regs[IO_PLL_CFG] = 0x00014000;
>
> /* 0x120 - 0x16C */
> - s->regs[ARM_CLK_CTRL] = 0x1F000400;
> + if (!s->is_linux) {
> + s->regs[ARM_CLK_CTRL] = 0x1F000400;
> + } else {
> + s->regs[ARM_CLK_CTRL] = 0x1F000200;
> + }
> s->regs[DDR_CLK_CTRL] = 0x18400003;
> s->regs[DCI_CLK_CTRL] = 0x01E03201;
> s->regs[APER_CLK_CTRL] = 0x01FFCCCD;
> @@ -429,17 +440,27 @@ static const VMStateDescription vmstate_zynq_slcr = {
> .version_id = 2,
> .minimum_version_id = 2,
> .fields = (VMStateField[]) {
> + VMSTATE_BOOL(is_linux, ZynqSLCRState),
> VMSTATE_UINT32_ARRAY(regs, ZynqSLCRState, ZYNQ_SLCR_NUM_REGS),
> VMSTATE_END_OF_LIST()
> }
> };
>
> +static void zynq_sclr_linux_init(ARMLinuxBootIf *obj, bool secure_boot)
> +{
> + ZynqSLCRState *s = ZYNQ_SLCR(obj);
> +
> + s->is_linux = true;
> +}
> +
> static void zynq_slcr_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> + ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_CLASS(klass);
>
> dc->vmsd = &vmstate_zynq_slcr;
> dc->reset = zynq_slcr_reset;
> + albifc->arm_linux_init = zynq_sclr_linux_init;
> }
>
> static const TypeInfo zynq_slcr_info = {
> @@ -448,6 +469,10 @@ static const TypeInfo zynq_slcr_info = {
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(ZynqSLCRState),
> .instance_init = zynq_slcr_init,
> + .interfaces = (InterfaceInfo []) {
> + { TYPE_ARM_LINUX_BOOT_IF },
> + { },
> + },
> };
>
> static void zynq_slcr_register_types(void)
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-13 20:22 ` Peter Crosthwaite
@ 2015-09-13 20:37 ` Guenter Roeck
2015-09-13 20:47 ` Peter Maydell
1 sibling, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2015-09-13 20:37 UTC (permalink / raw)
To: Peter Crosthwaite, Peter Maydell, Nathan Rossi, Alistair Francis
Cc: qemu-devel@nongnu.org Developers, Peter Crosthwaite
On 09/13/2015 01:22 PM, Peter Crosthwaite wrote:
> On Sat, Sep 12, 2015 at 2:06 PM, Guenter Roeck <linux@roeck-us.net> 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 for to make Linux happy.
>> Limit the change to Linux boots only.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>
>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>
> Can this go via target-arm? (cc PMM).
>
> There may be more changes worth making on is_linux. I don't have the
> patch with the full list of FSBL-related SLCR changes handy and can't
> seem to find it in any modern Yocto trees. Wondering if Yocto still
> supports booting Zynq without FSBL (Nathan/Alistair may know more)?
>
Good question. I didn't find any related patches in Yocto 1.8,
but on the other side I wasn't sure if I was looking in the right place.
Guenter
> Regards,
> Peter
>
>> ---
>> v2: Limit scope of change to Linux boots.
>>
>> hw/misc/zynq_slcr.c | 29 +++++++++++++++++++++++++++--
>> 1 file changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
>> index 964f253..ed510fb 100644
>> --- a/hw/misc/zynq_slcr.c
>> +++ b/hw/misc/zynq_slcr.c
>> @@ -14,6 +14,7 @@
>> * with this program; if not, see <http://www.gnu.org/licenses/>.
>> */
>>
>> +#include "hw/arm/linux-boot-if.h"
>> #include "hw/hw.h"
>> #include "qemu/timer.h"
>> #include "hw/sysbus.h"
>> @@ -177,6 +178,8 @@ typedef struct ZynqSLCRState {
>>
>> MemoryRegion iomem;
>>
>> + bool is_linux;
>> +
>> uint32_t regs[ZYNQ_SLCR_NUM_REGS];
>> } ZynqSLCRState;
>>
>> @@ -189,7 +192,11 @@ static void zynq_slcr_reset(DeviceState *d)
>>
>> s->regs[LOCKSTA] = 1;
>> /* 0x100 - 0x11C */
>> - s->regs[ARM_PLL_CTRL] = 0x0001A008;
>> + if (!s->is_linux) {
>> + s->regs[ARM_PLL_CTRL] = 0x0001A008;
>> + } else {
>> + 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 +205,11 @@ static void zynq_slcr_reset(DeviceState *d)
>> s->regs[IO_PLL_CFG] = 0x00014000;
>>
>> /* 0x120 - 0x16C */
>> - s->regs[ARM_CLK_CTRL] = 0x1F000400;
>> + if (!s->is_linux) {
>> + s->regs[ARM_CLK_CTRL] = 0x1F000400;
>> + } else {
>> + s->regs[ARM_CLK_CTRL] = 0x1F000200;
>> + }
>> s->regs[DDR_CLK_CTRL] = 0x18400003;
>> s->regs[DCI_CLK_CTRL] = 0x01E03201;
>> s->regs[APER_CLK_CTRL] = 0x01FFCCCD;
>> @@ -429,17 +440,27 @@ static const VMStateDescription vmstate_zynq_slcr = {
>> .version_id = 2,
>> .minimum_version_id = 2,
>> .fields = (VMStateField[]) {
>> + VMSTATE_BOOL(is_linux, ZynqSLCRState),
>> VMSTATE_UINT32_ARRAY(regs, ZynqSLCRState, ZYNQ_SLCR_NUM_REGS),
>> VMSTATE_END_OF_LIST()
>> }
>> };
>>
>> +static void zynq_sclr_linux_init(ARMLinuxBootIf *obj, bool secure_boot)
>> +{
>> + ZynqSLCRState *s = ZYNQ_SLCR(obj);
>> +
>> + s->is_linux = true;
>> +}
>> +
>> static void zynq_slcr_class_init(ObjectClass *klass, void *data)
>> {
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> + ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_CLASS(klass);
>>
>> dc->vmsd = &vmstate_zynq_slcr;
>> dc->reset = zynq_slcr_reset;
>> + albifc->arm_linux_init = zynq_sclr_linux_init;
>> }
>>
>> static const TypeInfo zynq_slcr_info = {
>> @@ -448,6 +469,10 @@ static const TypeInfo zynq_slcr_info = {
>> .parent = TYPE_SYS_BUS_DEVICE,
>> .instance_size = sizeof(ZynqSLCRState),
>> .instance_init = zynq_slcr_init,
>> + .interfaces = (InterfaceInfo []) {
>> + { TYPE_ARM_LINUX_BOOT_IF },
>> + { },
>> + },
>> };
>>
>> static void zynq_slcr_register_types(void)
>> --
>> 2.1.4
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-13 20:22 ` Peter Crosthwaite
2015-09-13 20:37 ` Guenter Roeck
@ 2015-09-13 20:47 ` Peter Maydell
2015-09-13 21:51 ` Guenter Roeck
2015-09-13 22:42 ` Peter Crosthwaite
1 sibling, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2015-09-13 20:47 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Crosthwaite, Alistair Francis,
qemu-devel@nongnu.org Developers, Guenter Roeck, Nathan Rossi
On 13 September 2015 at 21:22, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sat, Sep 12, 2015 at 2:06 PM, Guenter Roeck <linux@roeck-us.net> 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 for to make Linux happy.
>> Limit the change to Linux boots only.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>
>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>
> Can this go via target-arm? (cc PMM).
>
> There may be more changes worth making on is_linux. I don't have the
> patch with the full list of FSBL-related SLCR changes handy and can't
> seem to find it in any modern Yocto trees. Wondering if Yocto still
> supports booting Zynq without FSBL (Nathan/Alistair may know more)?
I'd prefer us not to propagate lots of "only if Linux boot"
changes into devices. The GIC *must* have these because the
kernel can't configure it otherwise from non-secure mode.
I'm not sure that applies here.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-13 20:47 ` Peter Maydell
@ 2015-09-13 21:51 ` Guenter Roeck
2015-09-13 22:42 ` Peter Crosthwaite
1 sibling, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2015-09-13 21:51 UTC (permalink / raw)
To: Peter Maydell, Peter Crosthwaite
Cc: Peter Crosthwaite, Alistair Francis,
qemu-devel@nongnu.org Developers, Nathan Rossi
Peter,
On 09/13/2015 01:47 PM, Peter Maydell wrote:
> On 13 September 2015 at 21:22, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sat, Sep 12, 2015 at 2:06 PM, Guenter Roeck <linux@roeck-us.net> 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 for to make Linux happy.
>>> Limit the change to Linux boots only.
>>>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>
>>
>> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>>
>> Can this go via target-arm? (cc PMM).
>>
>> There may be more changes worth making on is_linux. I don't have the
>> patch with the full list of FSBL-related SLCR changes handy and can't
>> seem to find it in any modern Yocto trees. Wondering if Yocto still
>> supports booting Zynq without FSBL (Nathan/Alistair may know more)?
>
> I'd prefer us not to propagate lots of "only if Linux boot"
> changes into devices. The GIC *must* have these because the
> kernel can't configure it otherwise from non-secure mode.
> I'm not sure that applies here.
>
Not sure I understand. Is this a NACK ?
Thanks,
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-13 20:47 ` Peter Maydell
2015-09-13 21:51 ` Guenter Roeck
@ 2015-09-13 22:42 ` Peter Crosthwaite
2015-09-14 12:07 ` Peter Maydell
1 sibling, 1 reply; 11+ messages in thread
From: Peter Crosthwaite @ 2015-09-13 22:42 UTC (permalink / raw)
To: Peter Maydell, Michal Simek
Cc: Peter Crosthwaite, Alistair Francis,
qemu-devel@nongnu.org Developers, Guenter Roeck, Nathan Rossi
On Sun, Sep 13, 2015 at 1:47 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 13 September 2015 at 21:22, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sat, Sep 12, 2015 at 2:06 PM, Guenter Roeck <linux@roeck-us.net> 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 for to make Linux happy.
>>> Limit the change to Linux boots only.
>>>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>
>>
>> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
>>
>> Can this go via target-arm? (cc PMM).
>>
>> There may be more changes worth making on is_linux. I don't have the
>> patch with the full list of FSBL-related SLCR changes handy and can't
>> seem to find it in any modern Yocto trees. Wondering if Yocto still
>> supports booting Zynq without FSBL (Nathan/Alistair may know more)?
>
> I'd prefer us not to propagate lots of "only if Linux boot"
> changes into devices. The GIC *must* have these because the
> kernel can't configure it otherwise from non-secure mode.
> I'm not sure that applies here.
>
At least this change is a must. I have had this discussion with kernel
people before and they insist that initing the PLLs and clocks to
desired values is the job of the bootloader and the kernel reads back
the values from this core. It is same philosophy at the GIC init,
which is at the end of the day, done by some pre-boot software. The
same bootloader (FSBL) makes other changes that kernels past present
and future may rely on and it would be good to have those.
Regards,
Peter
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-13 22:42 ` Peter Crosthwaite
@ 2015-09-14 12:07 ` Peter Maydell
2015-09-14 13:17 ` Nathan Rossi
0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2015-09-14 12:07 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Crosthwaite, qemu-devel@nongnu.org Developers,
Alistair Francis, Nathan Rossi, Michal Simek, Guenter Roeck
On 13 September 2015 at 23:42, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sun, Sep 13, 2015 at 1:47 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 13 September 2015 at 21:22, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> There may be more changes worth making on is_linux. I don't have the
>>> patch with the full list of FSBL-related SLCR changes handy and can't
>>> seem to find it in any modern Yocto trees. Wondering if Yocto still
>>> supports booting Zynq without FSBL (Nathan/Alistair may know more)?
>>
>> I'd prefer us not to propagate lots of "only if Linux boot"
>> changes into devices. The GIC *must* have these because the
>> kernel can't configure it otherwise from non-secure mode.
>> I'm not sure that applies here.
>>
>
> At least this change is a must. I have had this discussion with kernel
> people before and they insist that initing the PLLs and clocks to
> desired values is the job of the bootloader and the kernel reads back
> the values from this core. It is same philosophy at the GIC init,
> which is at the end of the day, done by some pre-boot software. The
> same bootloader (FSBL) makes other changes that kernels past present
> and future may rely on and it would be good to have those.
The thing is that if we go down this path we end up incorporating
most of a boot firmware into QEMU, scattered across different
devices (and what do we do if we find that two boards want a
single device set up differently?). The current in-QEMU ARM
bootloader basically assumes a traditional 32-bit ARM setup,
where the kernel didn't really trust the firmware or bootloader
and did a lot of hardware setup itself. This model is starting
to break down as modern kernels assume more that the firmware
has done certain setup, but it's what QEMU's design here is based
on.
The other approach would be to actually run some firmware
blob at startup, and let that do the setup.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-14 12:07 ` Peter Maydell
@ 2015-09-14 13:17 ` Nathan Rossi
2015-09-14 14:05 ` Guenter Roeck
2015-09-27 5:44 ` Peter Crosthwaite
0 siblings, 2 replies; 11+ messages in thread
From: Nathan Rossi @ 2015-09-14 13:17 UTC (permalink / raw)
To: Peter Maydell
Cc: Peter Crosthwaite, qemu-devel@nongnu.org Developers, Michal Simek,
Peter Crosthwaite, Alistair Francis, Guenter Roeck
On Mon, Sep 14, 2015 at 10:07 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 13 September 2015 at 23:42, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sun, Sep 13, 2015 at 1:47 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 13 September 2015 at 21:22, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> There may be more changes worth making on is_linux. I don't have the
>>>> patch with the full list of FSBL-related SLCR changes handy and can't
>>>> seem to find it in any modern Yocto trees. Wondering if Yocto still
>>>> supports booting Zynq without FSBL (Nathan/Alistair may know more)?
I have been running QEMU without any patches for zynq in Yocto for
some time now. I have not experienced the bug mentioned, at least not
with the kernels I have been testing with. Is this something that is
more apparent in a 4.2 or newer kernel?
The only thing that was causing problems was the Ethernet kernel
driver demanding a valid 125mhz clock, that was solved by putting a
fixed clock in the device tree
(http://git.yoctoproject.org/cgit/cgit.cgi/meta-xilinx/tree/conf/machine/boards/qemu/qemuzynq-base.dtsi#n12).
>>>
>>> I'd prefer us not to propagate lots of "only if Linux boot"
>>> changes into devices. The GIC *must* have these because the
>>> kernel can't configure it otherwise from non-secure mode.
>>> I'm not sure that applies here.
>>>
>>
>> At least this change is a must. I have had this discussion with kernel
>> people before and they insist that initing the PLLs and clocks to
>> desired values is the job of the bootloader and the kernel reads back
>> the values from this core. It is same philosophy at the GIC init,
>> which is at the end of the day, done by some pre-boot software. The
>> same bootloader (FSBL) makes other changes that kernels past present
>> and future may rely on and it would be good to have those.
>
> The thing is that if we go down this path we end up incorporating
> most of a boot firmware into QEMU, scattered across different
> devices (and what do we do if we find that two boards want a
> single device set up differently?). The current in-QEMU ARM
> bootloader basically assumes a traditional 32-bit ARM setup,
> where the kernel didn't really trust the firmware or bootloader
> and did a lot of hardware setup itself. This model is starting
> to break down as modern kernels assume more that the firmware
> has done certain setup, but it's what QEMU's design here is based
> on.
>
> The other approach would be to actually run some firmware
> blob at startup, and let that do the setup.
For reference newer u-boot versions (mainline, not Xilinx) have
definitions for some Zynq boards and with the complete ps7_init.c
register configuration setup. So building a u-boot-spl firmware to do
this seems like an alternate solution. Although I am not exactly sure
how booting this setup would work, and I assume a custom ps7_init.c
might need to be created for QEMU.
Regards,
Nathan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-14 13:17 ` Nathan Rossi
@ 2015-09-14 14:05 ` Guenter Roeck
2015-09-27 5:44 ` Peter Crosthwaite
1 sibling, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2015-09-14 14:05 UTC (permalink / raw)
To: Nathan Rossi, Peter Maydell
Cc: qemu-devel@nongnu.org Developers, Peter Crosthwaite,
Peter Crosthwaite, Michal Simek, Alistair Francis
On 09/14/2015 06:17 AM, Nathan Rossi wrote:
> On Mon, Sep 14, 2015 at 10:07 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 13 September 2015 at 23:42, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> On Sun, Sep 13, 2015 at 1:47 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> On 13 September 2015 at 21:22, Peter Crosthwaite
>>>> <crosthwaitepeter@gmail.com> wrote:
>>>>> There may be more changes worth making on is_linux. I don't have the
>>>>> patch with the full list of FSBL-related SLCR changes handy and can't
>>>>> seem to find it in any modern Yocto trees. Wondering if Yocto still
>>>>> supports booting Zynq without FSBL (Nathan/Alistair may know more)?
>
> I have been running QEMU without any patches for zynq in Yocto for
> some time now. I have not experienced the bug mentioned, at least not
> with the kernels I have been testing with. Is this something that is
> more apparent in a 4.2 or newer kernel?
>
With mainline kernel v4.3-rc1 and qemu v2.4.0-417-g30c38c9, using multi_v7_defconfig,
I get
[ 0.478288] cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 216666 KHz
[ 0.479378] cpu cpu0: failed to set clock rate: -16
[ 0.479515] cpufreq: __target_index: Failed to change cpu frequency: -16
[ 0.480107] ------------[ cut here ]------------
[ 0.480222] kernel BUG at drivers/cpufreq/cpufreq.c:1287!
with all the upstream devicetree files I am testing with. The crash happens all
the way back to 3.18, so it isn't new either.
Also, with kernel 3.18+, the same version of qemu, and the clock patch applied,
the kernel hangs because it expects the ADC to be supported by the hardware.
Effectively this suggests that you are using a different kernel configuration,
a different devicetree file, and/or an older kernel. I can not do this since the
required configuration and devicetree files change from kernel version to kernel
version, and I test a large range of kernel versions (currently 3.18, 4.1, 4.2,
and mainline, for this platform).
Thanks,
Guenter
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-14 13:17 ` Nathan Rossi
2015-09-14 14:05 ` Guenter Roeck
@ 2015-09-27 5:44 ` Peter Crosthwaite
1 sibling, 0 replies; 11+ messages in thread
From: Peter Crosthwaite @ 2015-09-27 5:44 UTC (permalink / raw)
To: Nathan Rossi
Cc: Peter Maydell, Peter Crosthwaite,
qemu-devel@nongnu.org Developers, Michal Simek, Alistair Francis,
Guenter Roeck
On Mon, Sep 14, 2015 at 6:17 AM, Nathan Rossi <nathan@nathanrossi.com> wrote:
> On Mon, Sep 14, 2015 at 10:07 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 13 September 2015 at 23:42, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> On Sun, Sep 13, 2015 at 1:47 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> On 13 September 2015 at 21:22, Peter Crosthwaite
>>>> <crosthwaitepeter@gmail.com> wrote:
>>>>> There may be more changes worth making on is_linux. I don't have the
>>>>> patch with the full list of FSBL-related SLCR changes handy and can't
>>>>> seem to find it in any modern Yocto trees. Wondering if Yocto still
>>>>> supports booting Zynq without FSBL (Nathan/Alistair may know more)?
>
> I have been running QEMU without any patches for zynq in Yocto for
> some time now. I have not experienced the bug mentioned, at least not
> with the kernels I have been testing with. Is this something that is
> more apparent in a 4.2 or newer kernel?
>
> The only thing that was causing problems was the Ethernet kernel
> driver demanding a valid 125mhz clock, that was solved by putting a
> fixed clock in the device tree
> (http://git.yoctoproject.org/cgit/cgit.cgi/meta-xilinx/tree/conf/machine/boards/qemu/qemuzynq-base.dtsi#n12).
>
I think Guenters patch (just the first hunk) fixes this. I'm running
yocto Zynq without this workaround now. We should look to getting
something like Guenters patch in to allow the defconfig to work as
well as revert out that workaround from the Yocto dtsi.
Regards,
Peter
>>>>
>>>> I'd prefer us not to propagate lots of "only if Linux boot"
>>>> changes into devices. The GIC *must* have these because the
>>>> kernel can't configure it otherwise from non-secure mode.
>>>> I'm not sure that applies here.
>>>>
>>>
>>> At least this change is a must. I have had this discussion with kernel
>>> people before and they insist that initing the PLLs and clocks to
>>> desired values is the job of the bootloader and the kernel reads back
>>> the values from this core. It is same philosophy at the GIC init,
>>> which is at the end of the day, done by some pre-boot software. The
>>> same bootloader (FSBL) makes other changes that kernels past present
>>> and future may rely on and it would be good to have those.
>>
>> The thing is that if we go down this path we end up incorporating
>> most of a boot firmware into QEMU, scattered across different
>> devices (and what do we do if we find that two boards want a
>> single device set up differently?). The current in-QEMU ARM
>> bootloader basically assumes a traditional 32-bit ARM setup,
>> where the kernel didn't really trust the firmware or bootloader
>> and did a lot of hardware setup itself. This model is starting
>> to break down as modern kernels assume more that the firmware
>> has done certain setup, but it's what QEMU's design here is based
>> on.
>>
>> The other approach would be to actually run some firmware
>> blob at startup, and let that do the setup.
>
> For reference newer u-boot versions (mainline, not Xilinx) have
> definitions for some Zynq boards and with the complete ps7_init.c
> register configuration setup. So building a u-boot-spl firmware to do
> this seems like an alternate solution. Although I am not exactly sure
> how booting this setup would work, and I assume a custom ps7_init.c
> might need to be created for QEMU.
>
> Regards,
> Nathan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots
2015-09-12 21:06 [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots Guenter Roeck
2015-09-13 20:22 ` Peter Crosthwaite
@ 2015-09-27 5:50 ` Peter Crosthwaite
1 sibling, 0 replies; 11+ messages in thread
From: Peter Crosthwaite @ 2015-09-27 5:50 UTC (permalink / raw)
To: Guenter Roeck
Cc: Peter Maydell, Peter Crosthwaite,
qemu-devel@nongnu.org Developers, Alistair Francis
On Sat, Sep 12, 2015 at 2:06 PM, Guenter Roeck <linux@roeck-us.net> 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 for to make Linux happy.
> Limit the change to Linux boots only.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> ---
> v2: Limit scope of change to Linux boots.
>
> hw/misc/zynq_slcr.c | 29 +++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
> index 964f253..ed510fb 100644
> --- a/hw/misc/zynq_slcr.c
> +++ b/hw/misc/zynq_slcr.c
> @@ -14,6 +14,7 @@
> * with this program; if not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include "hw/arm/linux-boot-if.h"
> #include "hw/hw.h"
> #include "qemu/timer.h"
> #include "hw/sysbus.h"
> @@ -177,6 +178,8 @@ typedef struct ZynqSLCRState {
>
> MemoryRegion iomem;
>
> + bool is_linux;
> +
> uint32_t regs[ZYNQ_SLCR_NUM_REGS];
> } ZynqSLCRState;
>
> @@ -189,7 +192,11 @@ static void zynq_slcr_reset(DeviceState *d)
>
> s->regs[LOCKSTA] = 1;
> /* 0x100 - 0x11C */
> - s->regs[ARM_PLL_CTRL] = 0x0001A008;
> + if (!s->is_linux) {
> + s->regs[ARM_PLL_CTRL] = 0x0001A008;
> + } else {
> + 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 +205,11 @@ static void zynq_slcr_reset(DeviceState *d)
> s->regs[IO_PLL_CFG] = 0x00014000;
>
> /* 0x120 - 0x16C */
> - s->regs[ARM_CLK_CTRL] = 0x1F000400;
> + if (!s->is_linux) {
> + s->regs[ARM_CLK_CTRL] = 0x1F000400;
> + } else {
> + s->regs[ARM_CLK_CTRL] = 0x1F000200;
> + }
This hunk is not needed. The CLK_CTRL is the property of the Kernel so
cpufreq drivers should be able to set this to whatever it wants.
Your change is actually compensating for an existing bug. Heres the fix:
@@ -393,12 +393,12 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
return;
}
- if (!s->regs[LOCKSTA]) {
- s->regs[offset / 4] = val;
- } else {
- DB_PRINT("SCLR registers are locked. Unlock them first\n");
+ if (s->regs[LOCKSTA]) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "SCLR registers are locked. Unlock them first\n");
return;
}
+ s->regs[offset] = val;
There's a little bit of extra cleanup there while I was touching the
code, but the issue is that /4 in original code is bogus. This means
SLCR writes have not worked for sometime (doh). I'll spin this as a
patch soon.
I have replicated this (as well as the ADC hang) and am working on a
machine-level firmwareish version of your PLL change. I'll keep
everyone posted.
Regards,
Peter
> s->regs[DDR_CLK_CTRL] = 0x18400003;
> s->regs[DCI_CLK_CTRL] = 0x01E03201;
> s->regs[APER_CLK_CTRL] = 0x01FFCCCD;
> @@ -429,17 +440,27 @@ static const VMStateDescription vmstate_zynq_slcr = {
> .version_id = 2,
> .minimum_version_id = 2,
> .fields = (VMStateField[]) {
> + VMSTATE_BOOL(is_linux, ZynqSLCRState),
> VMSTATE_UINT32_ARRAY(regs, ZynqSLCRState, ZYNQ_SLCR_NUM_REGS),
> VMSTATE_END_OF_LIST()
> }
> };
>
> +static void zynq_sclr_linux_init(ARMLinuxBootIf *obj, bool secure_boot)
> +{
> + ZynqSLCRState *s = ZYNQ_SLCR(obj);
> +
> + s->is_linux = true;
> +}
> +
> static void zynq_slcr_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> + ARMLinuxBootIfClass *albifc = ARM_LINUX_BOOT_IF_CLASS(klass);
>
> dc->vmsd = &vmstate_zynq_slcr;
> dc->reset = zynq_slcr_reset;
> + albifc->arm_linux_init = zynq_sclr_linux_init;
> }
>
> static const TypeInfo zynq_slcr_info = {
> @@ -448,6 +469,10 @@ static const TypeInfo zynq_slcr_info = {
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(ZynqSLCRState),
> .instance_init = zynq_slcr_init,
> + .interfaces = (InterfaceInfo []) {
> + { TYPE_ARM_LINUX_BOOT_IF },
> + { },
> + },
> };
>
> static void zynq_slcr_register_types(void)
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-09-27 5:50 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-12 21:06 [Qemu-devel] [PATCH v2] hw/misc/zynq_slcr: Change CPU clock rate for Linux boots Guenter Roeck
2015-09-13 20:22 ` Peter Crosthwaite
2015-09-13 20:37 ` Guenter Roeck
2015-09-13 20:47 ` Peter Maydell
2015-09-13 21:51 ` Guenter Roeck
2015-09-13 22:42 ` Peter Crosthwaite
2015-09-14 12:07 ` Peter Maydell
2015-09-14 13:17 ` Nathan Rossi
2015-09-14 14:05 ` Guenter Roeck
2015-09-27 5:44 ` Peter Crosthwaite
2015-09-27 5:50 ` Peter Crosthwaite
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.