Closed rroohhh closed 5 years ago
Further investigation seems to indicate that this has nothing to do with qemu, but instead with the kernel itself.
The zynq includes two cadence ttc's (triple timer counter's) that can use the cpu_1x
clock as input (this is the cpu frequency clock). This means every cpu frequency has to be a valid input frequency for these ttc's.
When they are disabled (in the devicetree) the error disappears and the cpu frequency can be changed successfully.
However we probably don't want to disable them, as they are used (atleast one of them) as the clocksource for the linux kernel. The only other options for the clocksource are jiffies
and the arm_global_timer
, which both depend on the cpu frequency, which means things like sleep 1
no longer produce the expected result.
As 333334
KHz and 666667
KHz are officially supported frequencies something in the kernel driver for the ttc's is probably wrong, so we should try to fix that.
First guess: The ttc can't handle the full clock speed, but needs a prescaler. This prescaler only support power of two,
The first guess proved to be right, but only because of the combination with qemu.
Qemu start by default with a cpu frequency of 433333
KHz (yes that is not a typo).
When then the frequency is requested to be changed to 666667
KHz or 333334
KHz the possible values for the prescaler (which are only powers of two) only generate frequencies which the kernel deems as to far from the target. (for example when 333334
KHz is choosen the best choice is to leave the prescaler the same as before, but this is 433333 - 333334 = 99999
KHz away from the targeted frequency, which is bigger than 50 KHz
and the kernel rejects the frequency change)
The default frequency of qemu can be changed by a simple patch to 333334
KHz:
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index 7557482..3a387ec 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -310,7 +310,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;
applying this patch removes the warning from qemu and results in successful frequency changes.
However before we apply this patch we should check what is happening on real hardware and if it also uses 433333
KHz as default frequency we should probably adapt our operating-points
in the devicetree. A alternative would be to figure out where the cpu frequency set by the fsbl
is defined and changed that.
@anuejn could you maybe check what is happening on real hardware? I am currently quite busy and would like to fix this bug as quick as possible, as it makes qemu nearly unusable under certain circumstances.
It should just be a matter of checking if something like cpufreq: cpufreq_online: CPU0: Running at unlisted freq: 433333 KHz
occurs in the boot log (possibly with some other frequency).
Ok had some time to check what is happening on real hardware. It seems the fsbl sets it to 666666 KHz. For some reason this is not 666667 KHz as used in the devicetree and setting the devicetree to 666666 KHz and 333333 KHz breaks the frequency scaling on real hardware.
So I guess just using the qemu patch above is a fine fix for now.
However there is still room for improvement. The current kernel code handling the frequency scaling and setting of the ttc
dividers only really works if the cpu is started with the highest ever used frequency, as it sets the divider to the maximum possible value on boot. Maybe looking up the setpoints in the devicetree and figuring out the highest frequency from that is a better way?
using the
qemu-shell
target or booting the image manually in qemu result in the spam ofcpufreq: __target_index: Failed to change cpu frequency: -16
the emulation still works fine, but the spam makes it quite hard to follow what you are typing.
as a temporary fix one can login blindly and execute
echo userspace > /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor
as root to stop the spam