On Mon, Mar 03, 2014 at 08:57:33PM +1000, Peter Crosthwaite wrote:
> On Mon, Mar 3, 2014 at 12:06 AM, 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 | 19 ++++++++++++++++++-
> > 1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c
> > index 4723b25..f2f2567 100644
> > --- a/hw/timer/allwinner-a10-pit.c
> > +++ b/hw/timer/allwinner-a10-pit.c
> > @@ -79,6 +79,23 @@ 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_freqs[] = {32768, 24000000};
> > +
>
> These smell like external oscillator frequencies which is board level
> information. Ideally, you should propertyify the input frequencies and
> pass them in on the board, so others can make different boards with
> with different crystals.
According to the A10 datasheet, the possible values for the field are:
00: Low speed OSC
01: OSC24M
10: PLL6/6
11: /
OSC24M is a fixed 24MHz oscillator; I suppose that "Low speed OSC" is
the 32768Hz oscillator described in section 6. So both frequencies
seem to be fixed in this case.
> > + prescaler = 1 << extract32(s->control[index], 4, 3);
> > + source = extract32(s->control[index], 2, 2);
> > +
> > + if (source > 1) {
> > + qemu_log_mask(LOG_UNIMP, "%s: unimplemented clock source %d",
>
> LOG_UNIMP is usually used for something that does exist but QEMU
> chooses knowingly not to implement. Are there extra clock wires from
> other sources not handled or are encodings 1 & 2 just unpopulated on
> this board? (In which case it should be a GUEST_ERROR).
Encoding 2 is associated to PLL6/6 and should give a UNIMPL; encoding
3 is not defined and should give a GUEST_ERROR, right?
Thanks for the reviews,
Beniamino
>
> Regards,
> Peter
>
> > + __func__, source);
> > + source = 0;
> > + }
> > +
> > + ptimer_set_freq(s->timer[index], source_freqs[source] / prescaler);
> > +}
> > +
> > static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
> > unsigned size)
> > {
> > @@ -101,6 +118,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);
> > if (s->control[index] & AW_A10_PIT_TIMER_RELOAD) {
> > ptimer_set_count(s->timer[index], s->interval[index]);
> > }
> > @@ -244,7 +262,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);
> > }
> > }
> >
> > --
> > 1.7.10.4
> >
> >
