| Message ID | 1394888493-20487-6-git-send-email-b.galvani@gmail.com |
|---|---|
| State | New |
| Headers | show |
在 2014-03-15六的 14:01 +0100，Beniamino Galvani写道： > 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.galvani@gmail.com> Reviewed-by: Li Guang <lig.fnst@cn.fujitsu.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); > 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 > + > #define AW_A10_PIT_DEFAULT_CLOCK 0x4 > > typedef struct AwA10PITState AwA10PITState;
On Sat, Mar 15, 2014 at 11:01 PM, Beniamino Galvani <b.galvani@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.galvani@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. > 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. Are you using the clk 32k in your test case at all? Regards, Peter > #define AW_A10_PIT_DEFAULT_CLOCK 0x4 > > typedef struct AwA10PITState AwA10PITState; > -- > 1.7.10.4 > >
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.galvani@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.galvani@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
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); 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 + #define AW_A10_PIT_DEFAULT_CLOCK 0x4 typedef struct AwA10PITState AwA10PITState;
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.galvani@gmail.com> --- hw/timer/allwinner-a10-pit.c | 30 +++++++++++++++++++++++++++++- include/hw/timer/allwinner-a10-pit.h | 8 ++++++++ 2 files changed, 37 insertions(+), 1 deletion(-)