| Message ID | 1442091963-10658-1-git-send-email-linux@roeck-us.net |
|---|---|
| State | New |
| Headers | show |
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 >
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 >> >
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
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
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
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
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
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
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
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 >
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)
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(-)