On Mon, Mar 17, 2014 at 11:27:01AM +1000, Peter Crosthwaite wrote:
> On Sat, Mar 15, 2014 at 11:01 PM, Beniamino Galvani <b.galv...@gmail.com>
> wrote:
> > This implements the prescaler and source fields of the timer control
> > register as described in the A10 user manual.
> >
> > Signed-off-by: Beniamino Galvani <b.galv...@gmail.com>
> > ---
> > hw/timer/allwinner-a10-pit.c | 30 +++++++++++++++++++++++++++++-
> > include/hw/timer/allwinner-a10-pit.h | 8 ++++++++
> > 2 files changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c
> > index f8c9236..a448689 100644
> > --- a/hw/timer/allwinner-a10-pit.c
> > +++ b/hw/timer/allwinner-a10-pit.c
> > @@ -74,6 +74,34 @@ static uint64_t a10_pit_read(void *opaque, hwaddr
> > offset, unsigned size)
> > return 0;
> > }
> >
> > +static void a10_pit_set_freq(AwA10PITState *s, int index)
> > +{
> > + uint32_t prescaler, source;
> > + uint32_t source_freq = AW_A10_PIT_OSC24M_FREQ;
> > +
> > + prescaler = 1 << extract32(s->control[index], 4, 3);
> > + source = extract32(s->control[index], 2, 2);
> > +
> > + switch (source) {
> > + case AW_A10_PIT_SOURCE_LS_OSC:
> > + source_freq = AW_A10_PIT_LS_OSC_FREQ;
> > + break;
> > + case AW_A10_PIT_SOURCE_OSC24M:
> > + source_freq = AW_A10_PIT_OSC24M_FREQ;
> > + break;
> > + case AW_A10_PIT_SOURCE_PLL6:
> > + qemu_log_mask(LOG_UNIMP, "%s: unimplemented clock source %u",
> > __func__,
> > + source);
> > + break;
> > + case AW_A10_PIT_SOURCE_UNDEF:
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid clock source %u",
> > __func__,
> > + source);
> > + break;
> > + }
> > +
> > + ptimer_set_freq(s->timer[index], source_freq / prescaler);
> > +}
> > +
> > static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
> > unsigned size)
> > {
> > @@ -96,6 +124,7 @@ static void a10_pit_write(void *opaque, hwaddr offset,
> > uint64_t value,
> > switch (offset & 0x0f) {
> > case AW_A10_PIT_TIMER_CONTROL:
> > s->control[index] = value;
> > + a10_pit_set_freq(s, index);
>
> Similar to comment in previous patch, I think you need to call this
> from the reset handler. Otherwise you are reliant on a control write
> to set the timer frequency in the first place. Looking at reset
> handler, control is reset to a default value, so reset may cause the
> side effect of a timer frequency change as well.
Hi,
the code calls set_frequency() only when the control register gets
written and the motivation behind this is that the frequency of the
ptimer doesn't matter while the timer is stopped.
After a reset all timers are stopped and to enable them you have to
write to the control register, thus setting the frequency to the right
value.
>
> > if (s->control[index] & AW_A10_PIT_TIMER_RELOAD) {
> > ptimer_set_count(s->timer[index], s->interval[index]);
> > }
> > @@ -239,7 +268,6 @@ static void a10_pit_init(Object *obj)
> > tc->index = i;
> > bh[i] = qemu_bh_new(a10_pit_timer_cb, tc);
> > s->timer[i] = ptimer_init(bh[i]);
> > - ptimer_set_freq(s->timer[i], 240000);
> > }
> > }
> >
> > diff --git a/include/hw/timer/allwinner-a10-pit.h
> > b/include/hw/timer/allwinner-a10-pit.h
> > index a48d3c7..37a2662 100644
> > --- a/include/hw/timer/allwinner-a10-pit.h
> > +++ b/include/hw/timer/allwinner-a10-pit.h
> > @@ -33,6 +33,14 @@
> > #define AW_A10_PIT_TIMER_BASE_END \
> > (AW_A10_PIT_TIMER_BASE * 6 + AW_A10_PIT_TIMER_COUNT)
> >
> > +#define AW_A10_PIT_SOURCE_LS_OSC 0
> > +#define AW_A10_PIT_SOURCE_OSC24M 1
> > +#define AW_A10_PIT_SOURCE_PLL6 2
> > +#define AW_A10_PIT_SOURCE_UNDEF 3
> > +
> > +#define AW_A10_PIT_LS_OSC_FREQ 32768
> > +#define AW_A10_PIT_OSC24M_FREQ 24000000
> > +
>
> So a quick look at a cubieboard schematic:
>
> http://dl.cubieboard.org/hardware/cubieboard_schematic_2012-08-08.pdf
>
> Both of these clocks are pinned out on the A10 SoC top level. (sheet 2
> pins CLK32K_IN / OUT etc). Seems for cubieboard in particular, the 32K
> clock is absent. The pins names are somewhat ugly, one would assume
> you could have a range of crystal frequencies when populating a
> crystal on a board, but naming your pin "CLK24M" suggests otherwise. I
> do think the best org for this is to just define 4 arbitrary
> prameterisable clock inputs and push the policy of whats what up to
> the board level. Its highly likely that that is the design intent of
> the Timer IP.
Ok, seems a good idea.
Aren't 2 properties (one for clk32k and one clk24m) enough?
>
> Are you using the clk 32k in your test case at all?
No, Linux uses clk24m as source for timers.
Beniamino
