Re: [Qemu-devel] [PATCH 2/2] hw/armv7m_nvic: Add global variable for SysTick external reference clock

[Andre Beckus]

On Wed, 2012-10-10 at 13:03 +0100, Peter Maydell wrote:
> On 9 October 2012 23:29, Andre Beckus <address@hidden> wrote:
> > Adds a new external reference clock scale variable to complement the 
> > existing
> > system_clock_scale variable.  Previously, the value was hardcoded to 1000
> > when calculating the SysTick scale.  The new variable defaults to 1000 to
> > maintain backward compatibility.
> >
> > Signed-off-by: Andre Beckus <address@hidden>
> 
> Hi. Do you have a use planned for this (new board maybe, or some
> change to the stellaris board)? I generally prefer to put this kind
> of patch in at the same time as the patches which make use of the
> new feature, rather than putting them into the codebase without
> any immediate users.

No, this is for an STM32 implementation I wrote.  For the time being, I
am keeping it in a forked repository, and do not plan on submitting it -
the machine I made is based on a bare-bones dev board which has no
screen, etc..  I figured I would submit it if I ever add a more capable
machine that could, for example, run Linux and that might be of more
interest.

I had second thoughts after submitting this patch, and agree that it
should not be included.

> In this particular case, the system_clock_scale variable is a nasty
> hack which I would like to get rid of. I think the right way to
> implement the systick scaling/calibration is that instead of a
> random global int, the NVIC exposes a GPIO input which corresponds
> to the hardware's STCALIB input lines (which set the TENMS field
> in SYST_CALIB), and the stellaris hardware sets that line via
> qemu_set_irq().
> 
> I think that if the board provides an external reference clock,
> then the scale should (as well as being used in the calculations,
> as your patch does) appear in the return value form SYST_CALIB
> (currently we return a hardcoded 10000 value.)

I agree that GPIO pins would be a natural way to set the SYST_CALIB
register value.

> We should check what the stellaris code is doing when it sets
> system_clock_scale -- I guess that's the actual system clock,
> not the reference clock (if the stellaris even provides one).
> The hardware doesn't have an explicit signal input to provide
> that but we should probably fake one up to avoid the global.

I took a brief look at some Stellaris documentation, and it says the
Stellaris family does not include an external reference clock.
Furthermore, for at least some of the chips (and probably all of them),
the SYST_CALIB register is not implemented.

As a case study, the STM32 does have a reference clock.  It is simply
the system clock divided by 8 (maybe not ARM's intention for it to be
tied so closely to the system clock).  The documentation says the TENMS
field is hardwired to 9000, which corresponds to a 1 ms period when the
external reference clock is selected and the system clock is running at
72 Mhz.  So, the TENMS field will not be accurate if the system clock is
running at a different frequency (the SKEW bit is hardwired to 1).

Even if the SYST_CALIB were driven by GPIO lines, I think there would
still need to be a way of setting the frequency/scale of the two
sources, since they are not necessarily tied to the calibration value.  

Looking at the big picture, it seems that QEMU could benefit from a new
"clock line" type for handling clock signals.  They could be exposed by
devices in a similar manner to GPIO lines (there would be both input and
output clock lines).  I could see them being useful (at least in the
microcontroller world) for passing clock signals back and forth between
peripherals, interfacing timer peripherals to machines, setting
oscillator frequencies, and serving as the plumbing for clock trees.  I
know I had to do a lot of hacking with the STM32 implementation to
propagate the clock controller's signals to the other peripherals.

When I searched on the topic, I saw that you discussed/requested a
common clock framework back in July (in regards to the an exynos4210
patch).  Do you know if any progress was made?  As a side note, I had
made an attempt at a simple, generic clock tree interface for the STM32
implementation (it doesn't include clock lines that I mentioned above,
though).  It is not QOM - just a collection of C functions.  I've
included links to the code here:
https://github.com/beckus/qemu_stm32/blob/stm32/hw/clktree.h
https://github.com/beckus/qemu_stm32/blob/stm32/hw/clktree.c

Thank you,
Andre Beckus