diff mbox

[U-Boot,v2,1/9] sunxi: initial sun7i clocks and timer support.

Message ID 1395438866-1193-1-git-send-email-ijc@hellion.org.uk
State Changes Requested
Delegated to: Albert ARIBAUD
Headers show

Commit Message

Ian Campbell March 21, 2014, 9:54 p.m. UTC
This has been stripped back for mainlining and supports only sun7i. These
changes are not useful by themselves but are split out to make the patch sizes
more manageable.

As well as the following signed-off-by the sunxi branch shows commits to these
files authored by the following:
  Alejandro Mery
  Carl van Schaik
  Stefan Roese
  Tom Cubie
  yemao

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Henrik Nordstrom <henrik@henriknordstrom.net>
Signed-off-by: Jens Kuske <jenskuske@gmail.com>
Signed-off-by: Luke Leighton <lkcl@lkcl.net>
Signed-off-by: Oliver Schinagl <oliver@schinagl.nl>
Signed-off-by: Ian Campbell <ijc@hellion.org.uk>

---
v2: Based on u-boot-sunxi.git#sunxi d9aa5dd3d15c "sunxi: mmc:
checkpatch whitespace fixes" with v2014.04-rc2 merged in:
    - define magic numbers
    - simplify get_tbclk
    - correct clock_set_pll1 prototype
    - add CONFIG_SUN7I to simplify future SUN?I support.
    - defines for MMC AHB clocks

v1: Based on u-boot-sunxi.git#sunxi commit d854c4de2f57 "arm: Handle .gnu.hash
section in ldscripts" vs v2014.01.
---
 arch/arm/cpu/armv7/sunxi/Makefile           |  11 ++
 arch/arm/cpu/armv7/sunxi/clock.c            | 180 +++++++++++++++++++++
 arch/arm/cpu/armv7/sunxi/timer.c            | 102 ++++++++++++
 arch/arm/include/asm/arch-sunxi/clock.h     | 237 ++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-sunxi/sys_proto.h |  17 ++
 arch/arm/include/asm/arch-sunxi/timer.h     |  88 +++++++++++
 6 files changed, 635 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/sunxi/Makefile
 create mode 100644 arch/arm/cpu/armv7/sunxi/clock.c
 create mode 100644 arch/arm/cpu/armv7/sunxi/timer.c
 create mode 100644 arch/arm/include/asm/arch-sunxi/clock.h
 create mode 100644 arch/arm/include/asm/arch-sunxi/sys_proto.h
 create mode 100644 arch/arm/include/asm/arch-sunxi/timer.h

Comments

Marek Vasut March 24, 2014, 8:52 p.m. UTC | #1
On Friday, March 21, 2014 at 10:54:18 PM, Ian Campbell wrote:
> This has been stripped back for mainlining and supports only sun7i. These
> changes are not useful by themselves but are split out to make the patch
> sizes more manageable.

[...]

> +int clock_init(void)
> +{
> +	struct sunxi_ccm_reg *const ccm =
> +		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +
> +#ifdef CONFIG_SPL_BUILD
> +	clock_init_safe();
> +#endif
> +
> +	/* uart clock source is apb1 */
> +	sr32(&ccm->apb1_clk_div_cfg, 24, 2, APB1_CLK_SRC_OSC24M);
> +	sr32(&ccm->apb1_clk_div_cfg, 16, 2, APB1_FACTOR_N);
> +	sr32(&ccm->apb1_clk_div_cfg, 0, 5, APB1_FACTOR_M);


sr32() is not defined anywhere.

> +	/* open the clock for uart */
> +	sr32(&ccm->apb1_gate, 16 + CONFIG_CONS_INDEX - 1, 1, CLK_GATE_OPEN);
> +
> +	return 0;
> +}
> +
> +/* Return PLL5 frequency in Hz
> + * Note: Assumes PLL5 reference is 24MHz clock
> + */
> +unsigned int clock_get_pll5(void)
> +{
> +	struct sunxi_ccm_reg *const ccm =
> +		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +	uint32_t rval = readl(&ccm->pll5_cfg);
> +	int n = (rval >> 8) & 0x1f;
> +	int k = ((rval >> 4) & 3) + 1;
> +	int p = 1 << ((rval >> 16) & 3);
> +	return 24000000 * n * k / p;

Please fix the magic values here.
[...]

> +#ifdef CONFIG_SPL_BUILD
> +#define PLL1_CFG(N, K, M, P)	(1 << 31 | 0 << 30 | 8 << 26 | 0 << 25 | 
\
> +				 16 << 20 | (P) << 16 | 2 << 13 | (N) << 8 | \
> +				 (K) << 4 | 0 << 3 | 0 << 2 | (M) << 0)

Here is well.

> +#define RDIV(a, b)		((a + (b) - 1) / (b))

This is some kind of DIV_ROUND_UP() from include/common.h ?

[...]

> +	/* Map divisors to register values */
> +	axi = axi - 1;
> +	if (ahb > 4)
> +		ahb = 3;
> +	else if (ahb > 2)
> +		ahb = 2;
> +	else if (ahb > 1)
> +		ahb = 1;
> +	else
> +		ahb = 0;
> +
> +	apb0 = apb0 - 1;
> +
> +	/* Switch to 24MHz clock while changing PLL1 */
> +	writel(AXI_DIV_1 << AXI_DIV_SHIFT |
> +	       AHB_DIV_2 << AHB_DIV_SHIFT |
> +	       APB0_DIV_1 << APB0_DIV_SHIFT |
> +	       CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
> +	       &ccm->cpu_ahb_apb0_cfg);
> +	sdelay(20);

What is sdelay() function all about ?

[...]

> +static struct sunxi_timer *timer_base =
> +	&((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->timer[TIMER_NUM];
> +
> +/* macro to read the 32 bit timer: since it decrements, we invert read
> value */ +#define READ_TIMER() (~readl(&timer_base->val))

This macro has to go, just use ~readl() in place. But still, why do you use that 
negation in "~readl()" anyway ?

[...]
Olliver Schinagl March 24, 2014, 10:42 p.m. UTC | #2
On 03/24/2014 09:52 PM, Marek Vasut wrote:
> On Friday, March 21, 2014 at 10:54:18 PM, Ian Campbell wrote:
>> This has been stripped back for mainlining and supports only sun7i. These
>> changes are not useful by themselves but are split out to make the patch
>> sizes more manageable.
> [...]
>
>> +int clock_init(void)
>> +{
>> +	struct sunxi_ccm_reg *const ccm =
>> +		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
>> +
>> +#ifdef CONFIG_SPL_BUILD
>> +	clock_init_safe();
>> +#endif
>> +
>> +	/* uart clock source is apb1 */
>> +	sr32(&ccm->apb1_clk_div_cfg, 24, 2, APB1_CLK_SRC_OSC24M);
>> +	sr32(&ccm->apb1_clk_div_cfg, 16, 2, APB1_FACTOR_N);
>> +	sr32(&ccm->apb1_clk_div_cfg, 0, 5, APB1_FACTOR_M);
>
> sr32() is not defined anywhere.
it should be defined in
arch/arm/include/asm/arch-sunxi/sys_proto.h
and comes from
arch/arm/cpu/armv7/syslib.c

it was added for the ti omap's

I've got a local cleanup patch set where I fixed this already to 
clrsetbits_le32
>
>> +	/* open the clock for uart */
>> +	sr32(&ccm->apb1_gate, 16 + CONFIG_CONS_INDEX - 1, 1, CLK_GATE_OPEN);
>> +
>> +	return 0;
>> +}
>> +
>> +/* Return PLL5 frequency in Hz
>> + * Note: Assumes PLL5 reference is 24MHz clock
>> + */
>> +unsigned int clock_get_pll5(void)
>> +{
>> +	struct sunxi_ccm_reg *const ccm =
>> +		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
>> +	uint32_t rval = readl(&ccm->pll5_cfg);
>> +	int n = (rval >> 8) & 0x1f;
>> +	int k = ((rval >> 4) & 3) + 1;
>> +	int p = 1 << ((rval >> 16) & 3);
>> +	return 24000000 * n * k / p;
> Please fix the magic values here.
> [...]
Same here, got that in my local tree too
>
>> +#ifdef CONFIG_SPL_BUILD
>> +#define PLL1_CFG(N, K, M, P)	(1 << 31 | 0 << 30 | 8 << 26 | 0 << 25 |
> \
>> +				 16 << 20 | (P) << 16 | 2 << 13 | (N) << 8 | \
>> +				 (K) << 4 | 0 << 3 | 0 << 2 | (M) << 0)
> Here is well.
dito :)
>
>> +#define RDIV(a, b)		((a + (b) - 1) / (b))
> This is some kind of DIV_ROUND_UP() from include/common.h ?
>
> [...]
That one i didn't have;

Ian, I guess you can verify that generic macro works for here?
>
>> +	/* Map divisors to register values */
>> +	axi = axi - 1;
>> +	if (ahb > 4)
>> +		ahb = 3;
>> +	else if (ahb > 2)
>> +		ahb = 2;
>> +	else if (ahb > 1)
>> +		ahb = 1;
>> +	else
>> +		ahb = 0;
>> +
>> +	apb0 = apb0 - 1;
>> +
>> +	/* Switch to 24MHz clock while changing PLL1 */
>> +	writel(AXI_DIV_1 << AXI_DIV_SHIFT |
>> +	       AHB_DIV_2 << AHB_DIV_SHIFT |
>> +	       APB0_DIV_1 << APB0_DIV_SHIFT |
>> +	       CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
>> +	       &ccm->cpu_ahb_apb0_cfg);
>> +	sdelay(20);
> What is sdelay() function all about ?
It also is from
arch/arm/include/asm/arch-sunxi/sys_proto.h
and I thought all where replaced with udelays

With the sr32 and sdelays gone everywhere, sunxi/sys_proto.h can even go!
>
> [...]
>
>> +static struct sunxi_timer *timer_base =
>> +	&((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->timer[TIMER_NUM];
>> +
>> +/* macro to read the 32 bit timer: since it decrements, we invert read
>> value */ +#define READ_TIMER() (~readl(&timer_base->val))
> This macro has to go, just use ~readl() in place. But still, why do you use that
> negation in "~readl()" anyway ?
>
> [...]
Olliver
Marek Vasut March 25, 2014, 12:57 a.m. UTC | #3
On Monday, March 24, 2014 at 11:42:17 PM, Olliver Schinagl wrote:
> On 03/24/2014 09:52 PM, Marek Vasut wrote:
> > On Friday, March 21, 2014 at 10:54:18 PM, Ian Campbell wrote:
> >> This has been stripped back for mainlining and supports only sun7i.
> >> These changes are not useful by themselves but are split out to make
> >> the patch sizes more manageable.
> > 
> > [...]
> > 
> >> +int clock_init(void)
> >> +{
> >> +	struct sunxi_ccm_reg *const ccm =
> >> +		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> >> +
> >> +#ifdef CONFIG_SPL_BUILD
> >> +	clock_init_safe();
> >> +#endif
> >> +
> >> +	/* uart clock source is apb1 */
> >> +	sr32(&ccm->apb1_clk_div_cfg, 24, 2, APB1_CLK_SRC_OSC24M);
> >> +	sr32(&ccm->apb1_clk_div_cfg, 16, 2, APB1_FACTOR_N);
> >> +	sr32(&ccm->apb1_clk_div_cfg, 0, 5, APB1_FACTOR_M);
> > 
> > sr32() is not defined anywhere.
> 
> it should be defined in
> arch/arm/include/asm/arch-sunxi/sys_proto.h
> and comes from
> arch/arm/cpu/armv7/syslib.c
> 
> it was added for the ti omap's
> 
> I've got a local cleanup patch set where I fixed this already to
> clrsetbits_le32

It's not part of this patch, but then, use clrsetbits_le32() instead of course.

> >> +	/* open the clock for uart */
> >> +	sr32(&ccm->apb1_gate, 16 + CONFIG_CONS_INDEX - 1, 1, CLK_GATE_OPEN);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +/* Return PLL5 frequency in Hz
> >> + * Note: Assumes PLL5 reference is 24MHz clock
> >> + */
> >> +unsigned int clock_get_pll5(void)
> >> +{
> >> +	struct sunxi_ccm_reg *const ccm =
> >> +		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> >> +	uint32_t rval = readl(&ccm->pll5_cfg);
> >> +	int n = (rval >> 8) & 0x1f;
> >> +	int k = ((rval >> 4) & 3) + 1;
> >> +	int p = 1 << ((rval >> 16) & 3);
> >> +	return 24000000 * n * k / p;
> > 
> > Please fix the magic values here.
> > [...]
> 
> Same here, got that in my local tree too

Then make it part of the V3 please.

[...]
Wolfgang Denk March 25, 2014, 6:35 a.m. UTC | #4
Dear Olliver Schinagl,

In message <5330B4C9.10302@schinagl.nl> you wrote:
>
> > sr32() is not defined anywhere.
> it should be defined in
> arch/arm/include/asm/arch-sunxi/sys_proto.h
> and comes from
> arch/arm/cpu/armv7/syslib.c

Please avoid using sr32() alltogether.  It is basically only used in a
single file (arch/arm/cpu/armv7/omap3/clock.c) [plus a single call in
board/ti/panda/panda.c] and should be replaced by standard I/O
accessor functions.  I will prepare a patch to remove it from mainline
soon, so better don't start using it.

Thanks.

Wolfgang Denk
Ian Campbell March 26, 2014, 8:23 a.m. UTC | #5
On Mon, 2014-03-24 at 23:42 +0100, Olliver Schinagl wrote:
[...]
> I've got a local cleanup patch set where I fixed this already to 
> clrsetbits_le32
>[...]
> Same here, got that in my local tree too

Could you post what you've got please?

> >> +#ifdef CONFIG_SPL_BUILD
> >> +#define PLL1_CFG(N, K, M, P)	(1 << 31 | 0 << 30 | 8 << 26 | 0 << 25 |
> > \
> >> +				 16 << 20 | (P) << 16 | 2 << 13 | (N) << 8 | \
> >> +				 (K) << 4 | 0 << 3 | 0 << 2 | (M) << 0)
> > Here is well.
> dito :)
> >
> >> +#define RDIV(a, b)		((a + (b) - 1) / (b))
> > This is some kind of DIV_ROUND_UP() from include/common.h ?
> >
> > [...]
> That one i didn't have;
> 
> Ian, I guess you can verify that generic macro works for here?

Yeah, I'll look into that and all the other feedback from Marek.

Ian.
Ian Campbell March 26, 2014, 8:23 a.m. UTC | #6
On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
> On Friday, March 21, 2014 at 10:54:18 PM, Ian Campbell wrote:
> > This has been stripped back for mainlining and supports only sun7i. These
> > changes are not useful by themselves but are split out to make the patch
> > sizes more manageable.
> 
> [...]

Thanks for all your feedback here and on the other patches, I'll sort it
all out.

Ian.
Ian Campbell March 27, 2014, 9:29 p.m. UTC | #7
On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:

> > +static struct sunxi_timer *timer_base =
> > +     &((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->timer[TIMER_NUM];
> > +
> > +/* macro to read the 32 bit timer: since it decrements, we invert read
> > value */ +#define READ_TIMER() (~readl(&timer_base->val))
> 
> This macro has to go, just use ~readl() in place. But still, why do you use that 
> negation in "~readl()" anyway ?

The comment right above it explains why: the timer counts backwards and
inverting it accounts for that.

This is subtle enough that I don't think using ~readl() in place in the
3 callers would be an improvement.

Ian.
Marek Vasut March 27, 2014, 10 p.m. UTC | #8
On Thursday, March 27, 2014 at 10:29:56 PM, Ian Campbell wrote:
> On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
> > > +static struct sunxi_timer *timer_base =
> > > +     &((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->timer[TIMER_NUM];
> > > +
> > > +/* macro to read the 32 bit timer: since it decrements, we invert read
> > > value */ +#define READ_TIMER() (~readl(&timer_base->val))
> > 
> > This macro has to go, just use ~readl() in place. But still, why do you
> > use that negation in "~readl()" anyway ?
> 
> The comment right above it explains why: the timer counts backwards and
> inverting it accounts for that.
> 
> This is subtle enough that I don't think using ~readl() in place in the
> 3 callers would be an improvement.

Please do it, we don't want any implementers down the line using this 
"READ_TIMER()" call and getting hit by "timer_base undefined" . That macro hides 
the dependency on this symbol, while if you expanded it in-place, the dependency 
would be explicit. I really do want to see that macro gone, sorry.

Best regards,
Marek Vasut
Ian Campbell March 27, 2014, 10:12 p.m. UTC | #9
On Thu, 2014-03-27 at 23:00 +0100, Marek Vasut wrote:
> On Thursday, March 27, 2014 at 10:29:56 PM, Ian Campbell wrote:
> > On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
> > > > +static struct sunxi_timer *timer_base =
> > > > +     &((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->timer[TIMER_NUM];
> > > > +
> > > > +/* macro to read the 32 bit timer: since it decrements, we invert read
> > > > value */ +#define READ_TIMER() (~readl(&timer_base->val))
> > > 
> > > This macro has to go, just use ~readl() in place. But still, why do you
> > > use that negation in "~readl()" anyway ?
> > 
> > The comment right above it explains why: the timer counts backwards and
> > inverting it accounts for that.
> > 
> > This is subtle enough that I don't think using ~readl() in place in the
> > 3 callers would be an improvement.
> 
> Please do it, we don't want any implementers down the line using this 
> "READ_TIMER()" call and getting hit by "timer_base undefined" . That macro hides 
> the dependency on this symbol, while if you expanded it in-place, the dependency 
> would be explicit. I really do want to see that macro gone, sorry.

How about a static inline instead of the macro? I'm thinking with a
body:
{
	struct sunxi_timer *timers =
		(struct sunxi_timer_reg *)SUNXI_TIMER_BASE;
	return timers[TIMER_NUM]->val;
}
With something similar in timer_init then both the macro and the static
global timer_base can be dropped.

BTW this macro is in arch/arm/cpu/armv7/sunxi/timer.c not a header, so
I'm not sure which implementers down the line you were worried about
using it in some other context where it breaks.

Ian.
Marek Vasut March 27, 2014, 10:36 p.m. UTC | #10
On Thursday, March 27, 2014 at 11:12:38 PM, Ian Campbell wrote:
> On Thu, 2014-03-27 at 23:00 +0100, Marek Vasut wrote:
> > On Thursday, March 27, 2014 at 10:29:56 PM, Ian Campbell wrote:
> > > On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
> > > > > +static struct sunxi_timer *timer_base =
> > > > > +     &((struct sunxi_timer_reg
> > > > > *)SUNXI_TIMER_BASE)->timer[TIMER_NUM]; +
> > > > > +/* macro to read the 32 bit timer: since it decrements, we invert
> > > > > read value */ +#define READ_TIMER() (~readl(&timer_base->val))
> > > > 
> > > > This macro has to go, just use ~readl() in place. But still, why do
> > > > you use that negation in "~readl()" anyway ?
> > > 
> > > The comment right above it explains why: the timer counts backwards and
> > > inverting it accounts for that.
> > > 
> > > This is subtle enough that I don't think using ~readl() in place in the
> > > 3 callers would be an improvement.
> > 
> > Please do it, we don't want any implementers down the line using this
> > "READ_TIMER()" call and getting hit by "timer_base undefined" . That
> > macro hides the dependency on this symbol, while if you expanded it
> > in-place, the dependency would be explicit. I really do want to see that
> > macro gone, sorry.
> 
> How about a static inline instead of the macro? I'm thinking with a
> body:
> {
> 	struct sunxi_timer *timers =
> 		(struct sunxi_timer_reg *)SUNXI_TIMER_BASE;
> 	return timers[TIMER_NUM]->val;
> }
> With something similar in timer_init then both the macro and the static
> global timer_base can be dropped.

That's just wrapping a readl() into another function, which seems unnecessary 
really.

> BTW this macro is in arch/arm/cpu/armv7/sunxi/timer.c not a header, so
> I'm not sure which implementers down the line you were worried about
> using it in some other context where it breaks.

People plumbing in the timer.c file who are not aware the macro has a dependency 
which is not passed as it's parameter.

Best regards,
Marek Vasut
Ian Campbell March 28, 2014, 8:20 a.m. UTC | #11
On Thu, 2014-03-27 at 23:36 +0100, Marek Vasut wrote:
> On Thursday, March 27, 2014 at 11:12:38 PM, Ian Campbell wrote:
> > On Thu, 2014-03-27 at 23:00 +0100, Marek Vasut wrote:
> > > On Thursday, March 27, 2014 at 10:29:56 PM, Ian Campbell wrote:
> > > > On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
> > > > > > +static struct sunxi_timer *timer_base =
> > > > > > +     &((struct sunxi_timer_reg
> > > > > > *)SUNXI_TIMER_BASE)->timer[TIMER_NUM]; +
> > > > > > +/* macro to read the 32 bit timer: since it decrements, we invert
> > > > > > read value */ +#define READ_TIMER() (~readl(&timer_base->val))
> > > > > 
> > > > > This macro has to go, just use ~readl() in place. But still, why do
> > > > > you use that negation in "~readl()" anyway ?
> > > > 
> > > > The comment right above it explains why: the timer counts backwards and
> > > > inverting it accounts for that.
> > > > 
> > > > This is subtle enough that I don't think using ~readl() in place in the
> > > > 3 callers would be an improvement.
> > > 
> > > Please do it, we don't want any implementers down the line using this
> > > "READ_TIMER()" call and getting hit by "timer_base undefined" . That
> > > macro hides the dependency on this symbol, while if you expanded it
> > > in-place, the dependency would be explicit. I really do want to see that
> > > macro gone, sorry.
> > 
> > How about a static inline instead of the macro? I'm thinking with a
> > body:
> > {
> >       struct sunxi_timer *timers =
> >               (struct sunxi_timer_reg *)SUNXI_TIMER_BASE;
> >       return timers[TIMER_NUM]->val;
> > }
> > With something similar in timer_init then both the macro and the static
> > global timer_base can be dropped.
> 
> That's just wrapping a readl() into another function, which seems unnecessary 
> really.

Sorry, but I think inlining the readl (and along with it the
interesting/subtle) inverting functionality is a terrible idea, it
should be wrapped in some sort of accessor precisely because it is not a
raw readl.

I'm going to make it a function as I suggested.

> > BTW this macro is in arch/arm/cpu/armv7/sunxi/timer.c not a header, so
> > I'm not sure which implementers down the line you were worried about
> > using it in some other context where it breaks.
> 
> People plumbing in the timer.c file who are not aware the macro has a dependency 
> which is not passed as it's parameter.

What people? What plumbing? I've no idea what you are concerned about
here.

Ian.
Marek Vasut March 28, 2014, 8:24 a.m. UTC | #12
On Friday, March 28, 2014 at 09:20:17 AM, Ian Campbell wrote:
> On Thu, 2014-03-27 at 23:36 +0100, Marek Vasut wrote:
> > On Thursday, March 27, 2014 at 11:12:38 PM, Ian Campbell wrote:
> > > On Thu, 2014-03-27 at 23:00 +0100, Marek Vasut wrote:
> > > > On Thursday, March 27, 2014 at 10:29:56 PM, Ian Campbell wrote:
> > > > > On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
> > > > > > > +static struct sunxi_timer *timer_base =
> > > > > > > +     &((struct sunxi_timer_reg
> > > > > > > *)SUNXI_TIMER_BASE)->timer[TIMER_NUM]; +
> > > > > > > +/* macro to read the 32 bit timer: since it decrements, we
> > > > > > > invert read value */ +#define READ_TIMER()
> > > > > > > (~readl(&timer_base->val))
> > > > > > 
> > > > > > This macro has to go, just use ~readl() in place. But still, why
> > > > > > do you use that negation in "~readl()" anyway ?
> > > > > 
> > > > > The comment right above it explains why: the timer counts backwards
> > > > > and inverting it accounts for that.
> > > > > 
> > > > > This is subtle enough that I don't think using ~readl() in place in
> > > > > the 3 callers would be an improvement.
> > > > 
> > > > Please do it, we don't want any implementers down the line using this
> > > > "READ_TIMER()" call and getting hit by "timer_base undefined" . That
> > > > macro hides the dependency on this symbol, while if you expanded it
> > > > in-place, the dependency would be explicit. I really do want to see
> > > > that macro gone, sorry.
> > > 
> > > How about a static inline instead of the macro? I'm thinking with a
> > > body:
> > > {
> > > 
> > >       struct sunxi_timer *timers =
> > >       
> > >               (struct sunxi_timer_reg *)SUNXI_TIMER_BASE;
> > >       
> > >       return timers[TIMER_NUM]->val;
> > > 
> > > }
> > > With something similar in timer_init then both the macro and the static
> > > global timer_base can be dropped.
> > 
> > That's just wrapping a readl() into another function, which seems
> > unnecessary really.
> 
> Sorry, but I think inlining the readl (and along with it the
> interesting/subtle) inverting functionality is a terrible idea, it
> should be wrapped in some sort of accessor precisely because it is not a
> raw readl.
> 
> I'm going to make it a function as I suggested.
> 
> > > BTW this macro is in arch/arm/cpu/armv7/sunxi/timer.c not a header, so
> > > I'm not sure which implementers down the line you were worried about
> > > using it in some other context where it breaks.
> > 
> > People plumbing in the timer.c file who are not aware the macro has a
> > dependency which is not passed as it's parameter.
> 
> What people? What plumbing? I've no idea what you are concerned about
> here.

OK, I will wait for V3 of the patch since this discussion have gone awry . Let's 
talk about V3 , ok ? :)

Best regards,
Marek Vasut
Hans de Goede March 28, 2014, 8:25 a.m. UTC | #13
Hi,

On 03/28/2014 09:20 AM, Ian Campbell wrote:
> On Thu, 2014-03-27 at 23:36 +0100, Marek Vasut wrote:
>> On Thursday, March 27, 2014 at 11:12:38 PM, Ian Campbell wrote:
>>> On Thu, 2014-03-27 at 23:00 +0100, Marek Vasut wrote:
>>>> On Thursday, March 27, 2014 at 10:29:56 PM, Ian Campbell wrote:
>>>>> On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
>>>>>>> +static struct sunxi_timer *timer_base =
>>>>>>> +     &((struct sunxi_timer_reg
>>>>>>> *)SUNXI_TIMER_BASE)->timer[TIMER_NUM]; +
>>>>>>> +/* macro to read the 32 bit timer: since it decrements, we invert
>>>>>>> read value */ +#define READ_TIMER() (~readl(&timer_base->val))
>>>>>>
>>>>>> This macro has to go, just use ~readl() in place. But still, why do
>>>>>> you use that negation in "~readl()" anyway ?
>>>>>
>>>>> The comment right above it explains why: the timer counts backwards and
>>>>> inverting it accounts for that.
>>>>>
>>>>> This is subtle enough that I don't think using ~readl() in place in the
>>>>> 3 callers would be an improvement.
>>>>
>>>> Please do it, we don't want any implementers down the line using this
>>>> "READ_TIMER()" call and getting hit by "timer_base undefined" . That
>>>> macro hides the dependency on this symbol, while if you expanded it
>>>> in-place, the dependency would be explicit. I really do want to see that
>>>> macro gone, sorry.
>>>
>>> How about a static inline instead of the macro? I'm thinking with a
>>> body:
>>> {
>>>       struct sunxi_timer *timers =
>>>               (struct sunxi_timer_reg *)SUNXI_TIMER_BASE;
>>>       return timers[TIMER_NUM]->val;
>>> }
>>> With something similar in timer_init then both the macro and the static
>>> global timer_base can be dropped.
>>
>> That's just wrapping a readl() into another function, which seems unnecessary 
>> really.
> 
> Sorry, but I think inlining the readl (and along with it the
> interesting/subtle) inverting functionality is a terrible idea, it
> should be wrapped in some sort of accessor precisely because it is not a
> raw readl.
> 
> I'm going to make it a function as I suggested.
> 
>>> BTW this macro is in arch/arm/cpu/armv7/sunxi/timer.c not a header, so
>>> I'm not sure which implementers down the line you were worried about
>>> using it in some other context where it breaks.
>>
>> People plumbing in the timer.c file who are not aware the macro has a dependency 
>> which is not passed as it's parameter.
> 
> What people? What plumbing? I've no idea what you are concerned about
> here.

I think what Marek is concerned about is people making some global u-boot change
which for some reason requires fixing up a bunch of platform specific files,
and they end up touching our timer.c without ever test-compiling it.

These kind of things happen in qemu / the kernel too. In this case they could move
a READ_TIMER() somewhere where the timer_base is not defined. Your suggestion of
making it a proper function will fix that though, and I think is the best solution.

Regards,

Hans
Marek Vasut March 28, 2014, 8:39 a.m. UTC | #14
On Friday, March 28, 2014 at 09:25:40 AM, Hans de Goede wrote:
> Hi,
> 
> On 03/28/2014 09:20 AM, Ian Campbell wrote:
> > On Thu, 2014-03-27 at 23:36 +0100, Marek Vasut wrote:
> >> On Thursday, March 27, 2014 at 11:12:38 PM, Ian Campbell wrote:
> >>> On Thu, 2014-03-27 at 23:00 +0100, Marek Vasut wrote:
> >>>> On Thursday, March 27, 2014 at 10:29:56 PM, Ian Campbell wrote:
> >>>>> On Mon, 2014-03-24 at 21:52 +0100, Marek Vasut wrote:
> >>>>>>> +static struct sunxi_timer *timer_base =
> >>>>>>> +     &((struct sunxi_timer_reg
> >>>>>>> *)SUNXI_TIMER_BASE)->timer[TIMER_NUM]; +
> >>>>>>> +/* macro to read the 32 bit timer: since it decrements, we invert
> >>>>>>> read value */ +#define READ_TIMER() (~readl(&timer_base->val))
> >>>>>> 
> >>>>>> This macro has to go, just use ~readl() in place. But still, why do
> >>>>>> you use that negation in "~readl()" anyway ?
> >>>>> 
> >>>>> The comment right above it explains why: the timer counts backwards
> >>>>> and inverting it accounts for that.
> >>>>> 
> >>>>> This is subtle enough that I don't think using ~readl() in place in
> >>>>> the 3 callers would be an improvement.
> >>>> 
> >>>> Please do it, we don't want any implementers down the line using this
> >>>> "READ_TIMER()" call and getting hit by "timer_base undefined" . That
> >>>> macro hides the dependency on this symbol, while if you expanded it
> >>>> in-place, the dependency would be explicit. I really do want to see
> >>>> that macro gone, sorry.
> >>> 
> >>> How about a static inline instead of the macro? I'm thinking with a
> >>> body:
> >>> {
> >>> 
> >>>       struct sunxi_timer *timers =
> >>>       
> >>>               (struct sunxi_timer_reg *)SUNXI_TIMER_BASE;
> >>>       
> >>>       return timers[TIMER_NUM]->val;
> >>> 
> >>> }
> >>> With something similar in timer_init then both the macro and the static
> >>> global timer_base can be dropped.
> >> 
> >> That's just wrapping a readl() into another function, which seems
> >> unnecessary really.
> > 
> > Sorry, but I think inlining the readl (and along with it the
> > interesting/subtle) inverting functionality is a terrible idea, it
> > should be wrapped in some sort of accessor precisely because it is not a
> > raw readl.
> > 
> > I'm going to make it a function as I suggested.
> > 
> >>> BTW this macro is in arch/arm/cpu/armv7/sunxi/timer.c not a header, so
> >>> I'm not sure which implementers down the line you were worried about
> >>> using it in some other context where it breaks.
> >> 
> >> People plumbing in the timer.c file who are not aware the macro has a
> >> dependency which is not passed as it's parameter.
> > 
> > What people? What plumbing? I've no idea what you are concerned about
> > here.
> 
> I think what Marek is concerned about is people making some global u-boot
> change which for some reason requires fixing up a bunch of platform
> specific files, and they end up touching our timer.c without ever
> test-compiling it.

We don't do such changes without test-compiling those (most of us don't and 
those who do ... well ... tsk tsk tsk ! ;-) ).

> These kind of things happen in qemu / the kernel too. In this case they
> could move a READ_TIMER() somewhere where the timer_base is not defined.

Yeah.

> Your suggestion of making it a proper function will fix that though, and I
> think is the best solution.

I think wrapping a plain readl() into a function is not necessary, but I will 
wait for V3 to see it in action and then I will decide .
Ian Campbell April 13, 2014, 7:55 p.m. UTC | #15
On Mon, 2014-03-24 at 23:42 +0100, Olliver Schinagl wrote:
> On 03/24/2014 09:52 PM, Marek Vasut wrote:
> >> +	/* Switch to 24MHz clock while changing PLL1 */
> >> +	writel(AXI_DIV_1 << AXI_DIV_SHIFT |
> >> +	       AHB_DIV_2 << AHB_DIV_SHIFT |
> >> +	       APB0_DIV_1 << APB0_DIV_SHIFT |
> >> +	       CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
> >> +	       &ccm->cpu_ahb_apb0_cfg);
> >> +	sdelay(20);
> > What is sdelay() function all about ?
> It also is from
> arch/arm/include/asm/arch-sunxi/sys_proto.h
> and I thought all where replaced with udelays

Since these sdelays() are used while we are frobbing around with the
clocks I'm not sure that switching to udelay is possible/wise. sdelay is
documented as:
 * sdelay() - simple spin loop.  Will be constant time as
 *  its generally used in bypass conditions only.  This
 *  is necessary until timers are accessible.

IOW it sounds like it is designed to be used in exactly these
circumstances.

Given the lack of documentation for what is actually required by the
hardware when changing these clocks I'm a bit reluctant to go changing
things.

Ian.
Marek Vasut April 13, 2014, 9 p.m. UTC | #16
On Sunday, April 13, 2014 at 09:55:57 PM, Ian Campbell wrote:
> On Mon, 2014-03-24 at 23:42 +0100, Olliver Schinagl wrote:
> > On 03/24/2014 09:52 PM, Marek Vasut wrote:
> > >> +	/* Switch to 24MHz clock while changing PLL1 */
> > >> +	writel(AXI_DIV_1 << AXI_DIV_SHIFT |
> > >> +	       AHB_DIV_2 << AHB_DIV_SHIFT |
> > >> +	       APB0_DIV_1 << APB0_DIV_SHIFT |
> > >> +	       CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
> > >> +	       &ccm->cpu_ahb_apb0_cfg);
> > >> +	sdelay(20);
> > > 
> > > What is sdelay() function all about ?
> > 
> > It also is from
> > arch/arm/include/asm/arch-sunxi/sys_proto.h
> > and I thought all where replaced with udelays
> 
> Since these sdelays() are used while we are frobbing around with the
> clocks I'm not sure that switching to udelay is possible/wise. sdelay is
> documented as:
>  * sdelay() - simple spin loop.  Will be constant time as
>  *  its generally used in bypass conditions only.  This
>  *  is necessary until timers are accessible.
> 
> IOW it sounds like it is designed to be used in exactly these
> circumstances.
> 
> Given the lack of documentation for what is actually required by the
> hardware when changing these clocks I'm a bit reluctant to go changing
> things.

Don't you have an clock-agnostic timer in the chip ? If not, well, can't be 
helped.

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
new file mode 100644
index 0000000..787a127
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/Makefile
@@ -0,0 +1,11 @@ 
+#
+# (C) Copyright 2012 Henrik Nordstrom <henrik@henriknordstrom.net>
+#
+# Based on some other Makefile
+# (C) Copyright 2000-2003
+# Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+obj-y	+= timer.o
+obj-y	+= clock.o
diff --git a/arch/arm/cpu/armv7/sunxi/clock.c b/arch/arm/cpu/armv7/sunxi/clock.c
new file mode 100644
index 0000000..dd01be6
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/clock.c
@@ -0,0 +1,180 @@ 
+/*
+ * (C) Copyright 2007-2012
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * Tom Cubie <tangliang@allwinnertech.com>
+ *
+ * (C) Copyright 2013 Luke Kenneth Casson Leighton <lkcl@lkcl.net>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/gpio.h>
+#include <asm/arch/sys_proto.h>
+
+#ifdef CONFIG_SPL_BUILD
+static void clock_init_safe(void)
+{
+	struct sunxi_ccm_reg * const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+	/* Set safe defaults until PMU is configured */
+	writel(AXI_DIV_1 << AXI_DIV_SHIFT |
+	       AHB_DIV_2 << AHB_DIV_SHIFT |
+	       APB0_DIV_1 << APB0_DIV_SHIFT |
+	       CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
+	       &ccm->cpu_ahb_apb0_cfg);
+	writel(PLL1_CFG_DEFAULT, &ccm->pll1_cfg);
+	sdelay(200);
+	writel(AXI_DIV_1 << AXI_DIV_SHIFT |
+	       AHB_DIV_2 << AHB_DIV_SHIFT |
+	       APB0_DIV_1 << APB0_DIV_SHIFT |
+	       CPU_CLK_SRC_PLL1 << CPU_CLK_SRC_SHIFT,
+	       &ccm->cpu_ahb_apb0_cfg);
+#ifdef CONFIG_SUN7I
+	writel(0x1 << AHB_GATE_OFFSET_DMA | readl(&ccm->ahb_gate0),
+	       &ccm->ahb_gate0);
+	writel(0x1 << PLL6_ENABLE_OFFSET | readl(&ccm->pll6_cfg),
+	       &ccm->pll6_cfg);
+#endif
+}
+#endif
+
+int clock_init(void)
+{
+	struct sunxi_ccm_reg *const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+#ifdef CONFIG_SPL_BUILD
+	clock_init_safe();
+#endif
+
+	/* uart clock source is apb1 */
+	sr32(&ccm->apb1_clk_div_cfg, 24, 2, APB1_CLK_SRC_OSC24M);
+	sr32(&ccm->apb1_clk_div_cfg, 16, 2, APB1_FACTOR_N);
+	sr32(&ccm->apb1_clk_div_cfg, 0, 5, APB1_FACTOR_M);
+
+	/* open the clock for uart */
+	sr32(&ccm->apb1_gate, 16 + CONFIG_CONS_INDEX - 1, 1, CLK_GATE_OPEN);
+
+	return 0;
+}
+
+/* Return PLL5 frequency in Hz
+ * Note: Assumes PLL5 reference is 24MHz clock
+ */
+unsigned int clock_get_pll5(void)
+{
+	struct sunxi_ccm_reg *const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+	uint32_t rval = readl(&ccm->pll5_cfg);
+	int n = (rval >> 8) & 0x1f;
+	int k = ((rval >> 4) & 3) + 1;
+	int p = 1 << ((rval >> 16) & 3);
+	return 24000000 * n * k / p;
+}
+
+int clock_twi_onoff(int port, int state)
+{
+	struct sunxi_ccm_reg *const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+	if (port > 2)
+		return -1;
+
+	/* set the apb1 clock gate for twi */
+	sr32(&ccm->apb1_gate, 0 + port, 1, state);
+
+	return 0;
+}
+
+#ifdef CONFIG_SPL_BUILD
+#define PLL1_CFG(N, K, M, P)	(1 << 31 | 0 << 30 | 8 << 26 | 0 << 25 | \
+				 16 << 20 | (P) << 16 | 2 << 13 | (N) << 8 | \
+				 (K) << 4 | 0 << 3 | 0 << 2 | (M) << 0)
+#define RDIV(a, b)		((a + (b) - 1) / (b))
+
+struct {
+	u32 pll1_cfg;
+	unsigned int freq;
+} pll1_para[] = {
+	{ PLL1_CFG(16, 0, 0, 0), 384000000 },
+	{ PLL1_CFG(16, 1, 0, 0), 768000000 },
+	{ PLL1_CFG(20, 1, 0, 0), 960000000 },
+	{ PLL1_CFG(21, 1, 0, 0), 1008000000},
+	{ PLL1_CFG(22, 1, 0, 0), 1056000000},
+	{ PLL1_CFG(23, 1, 0, 0), 1104000000},
+	{ PLL1_CFG(24, 1, 0, 0), 1152000000},
+	{ PLL1_CFG(25, 1, 0, 0), 1200000000},
+	{ PLL1_CFG(26, 1, 0, 0), 1248000000},
+	{ PLL1_CFG(27, 1, 0, 0), 1296000000},
+	{ PLL1_CFG(28, 1, 0, 0), 1344000000},
+	{ PLL1_CFG(29, 1, 0, 0), 1392000000},
+	{ PLL1_CFG(30, 1, 0, 0), 1440000000},
+	{ PLL1_CFG(31, 1, 0, 0), 1488000000},
+	{ PLL1_CFG(31, 1, 0, 0), ~0},
+};
+
+void clock_set_pll1(int hz)
+{
+	int i = 0;
+	int axi, ahb, apb0;
+	struct sunxi_ccm_reg * const ccm =
+		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+	/* Find target frequency */
+	while (pll1_para[i].freq < hz)
+		i++;
+
+	hz = pll1_para[i].freq;
+
+	/* Calculate system clock divisors */
+	axi = RDIV(hz, 432000000);		/* Max 450MHz */
+	ahb = RDIV(hz/axi, 204000000);		/* Max 250MHz */
+	apb0 = 2;				/* Max 150MHz */
+
+	printf("CPU: %dHz, AXI/AHB/APB: %d/%d/%d\n", hz, axi, ahb, apb0);
+
+	/* Map divisors to register values */
+	axi = axi - 1;
+	if (ahb > 4)
+		ahb = 3;
+	else if (ahb > 2)
+		ahb = 2;
+	else if (ahb > 1)
+		ahb = 1;
+	else
+		ahb = 0;
+
+	apb0 = apb0 - 1;
+
+	/* Switch to 24MHz clock while changing PLL1 */
+	writel(AXI_DIV_1 << AXI_DIV_SHIFT |
+	       AHB_DIV_2 << AHB_DIV_SHIFT |
+	       APB0_DIV_1 << APB0_DIV_SHIFT |
+	       CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
+	       &ccm->cpu_ahb_apb0_cfg);
+	sdelay(20);
+
+	/* Configure sys clock divisors */
+	writel(axi << AXI_DIV_SHIFT |
+	       ahb << AHB_DIV_SHIFT |
+	       apb0 << APB0_DIV_SHIFT |
+	       CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT,
+	       &ccm->cpu_ahb_apb0_cfg);
+
+	/* Configure PLL1 at the desired frequency */
+	writel(pll1_para[i].pll1_cfg, &ccm->pll1_cfg);
+	sdelay(200);
+
+	/* Switch CPU to PLL1 */
+	writel(axi << AXI_DIV_SHIFT |
+	       ahb << AHB_DIV_SHIFT |
+	       apb0 << APB0_DIV_SHIFT |
+	       CPU_CLK_SRC_PLL1 << CPU_CLK_SRC_SHIFT,
+	       &ccm->cpu_ahb_apb0_cfg);
+	sdelay(20);
+}
+#endif
diff --git a/arch/arm/cpu/armv7/sunxi/timer.c b/arch/arm/cpu/armv7/sunxi/timer.c
new file mode 100644
index 0000000..d0d9953
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/timer.c
@@ -0,0 +1,102 @@ 
+/*
+ * (C) Copyright 2007-2011
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * Tom Cubie <tangliang@allwinnertech.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/timer.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define TIMER_MODE   (0x0 << 7)	/* continuous mode */
+#define TIMER_DIV    (0x0 << 4)	/* pre scale 1 */
+#define TIMER_SRC    (0x1 << 2)	/* osc24m */
+#define TIMER_RELOAD (0x1 << 1)	/* reload internal value */
+#define TIMER_EN     (0x1 << 0)	/* enable timer */
+
+#define TIMER_CLOCK		(24 * 1000 * 1000)
+#define COUNT_TO_USEC(x)	((x) / 24)
+#define USEC_TO_COUNT(x)	((x) * 24)
+#define TICKS_PER_HZ		(TIMER_CLOCK / CONFIG_SYS_HZ)
+#define TICKS_TO_HZ(x)		((x) / TICKS_PER_HZ)
+
+#define TIMER_LOAD_VAL		0xffffffff
+
+#define TIMER_NUM		0	/* we use timer 0 */
+
+static struct sunxi_timer *timer_base =
+	&((struct sunxi_timer_reg *)SUNXI_TIMER_BASE)->timer[TIMER_NUM];
+
+/* macro to read the 32 bit timer: since it decrements, we invert read value */
+#define READ_TIMER() (~readl(&timer_base->val))
+
+/* init timer register */
+int timer_init(void)
+{
+	writel(TIMER_LOAD_VAL, &timer_base->inter);
+	writel(TIMER_MODE | TIMER_DIV | TIMER_SRC | TIMER_RELOAD | TIMER_EN,
+	       &timer_base->ctl);
+
+	return 0;
+}
+
+/* timer without interrupts */
+ulong get_timer(ulong base)
+{
+	return get_timer_masked() - base;
+}
+
+ulong get_timer_masked(void)
+{
+	/* current tick value */
+	ulong now = TICKS_TO_HZ(READ_TIMER());
+
+	if (now >= gd->arch.lastinc)	/* normal (non rollover) */
+		gd->arch.tbl += (now - gd->arch.lastinc);
+	else {
+		/* rollover */
+		gd->arch.tbl += (TICKS_TO_HZ(TIMER_LOAD_VAL)
+				- gd->arch.lastinc) + now;
+	}
+	gd->arch.lastinc = now;
+
+	return gd->arch.tbl;
+}
+
+/* delay x useconds */
+void __udelay(unsigned long usec)
+{
+	long tmo = USEC_TO_COUNT(usec);
+	ulong now, last = READ_TIMER();
+
+	while (tmo > 0) {
+		now = READ_TIMER();
+		if (now > last)	/* normal (non rollover) */
+			tmo -= now - last;
+		else		/* rollover */
+			tmo -= TIMER_LOAD_VAL - last + now;
+		last = now;
+	}
+}
+
+/*
+ * This function is derived from PowerPC code (read timebase as long long).
+ * On ARM it just returns the timer value.
+ */
+unsigned long long get_ticks(void)
+{
+	return get_timer(0);
+}
+
+/*
+ * This function is derived from PowerPC code (timebase clock frequency).
+ * On ARM it returns the number of timer ticks per second.
+ */
+ulong get_tbclk(void)
+{
+	return CONFIG_SYS_HZ;
+}
diff --git a/arch/arm/include/asm/arch-sunxi/clock.h b/arch/arm/include/asm/arch-sunxi/clock.h
new file mode 100644
index 0000000..defd229
--- /dev/null
+++ b/arch/arm/include/asm/arch-sunxi/clock.h
@@ -0,0 +1,237 @@ 
+/*
+ * (C) Copyright 2007-2011
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * Tom Cubie <tangliang@allwinnertech.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _SUNXI_CLOCK_H
+#define _SUNXI_CLOCK_H
+
+#include <linux/types.h>
+
+/* clock control module regs definition */
+
+struct sunxi_ccm_reg {
+	u32 pll1_cfg;		/* 0x00 pll1 control */
+	u32 pll1_tun;		/* 0x04 pll1 tuning */
+	u32 pll2_cfg;		/* 0x08 pll2 control */
+	u32 pll2_tun;		/* 0x0c pll2 tuning */
+	u32 pll3_cfg;		/* 0x10 pll3 control */
+	u8 res0[0x4];
+	u32 pll4_cfg;		/* 0x18 pll4 control */
+	u8 res1[0x4];
+	u32 pll5_cfg;		/* 0x20 pll5 control */
+	u32 pll5_tun;		/* 0x24 pll5 tuning */
+	u32 pll6_cfg;		/* 0x28 pll6 control */
+	u32 pll6_tun;		/* 0x2c pll6 tuning */
+	u32 pll7_cfg;		/* 0x30 pll7 control */
+	u32 pll1_tun2;		/* 0x34 pll5 tuning2 */
+	u8 res2[0x4];
+	u32 pll5_tun2;		/* 0x3c pll5 tuning2 */
+	u8 res3[0xc];
+	u32 pll_lock_dbg;	/* 0x4c pll lock time debug */
+	u32 osc24m_cfg;		/* 0x50 osc24m control */
+	u32 cpu_ahb_apb0_cfg;	/* 0x54 cpu,ahb and apb0 divide ratio */
+	u32 apb1_clk_div_cfg;	/* 0x58 apb1 clock dividor */
+	u32 axi_gate;		/* 0x5c axi module clock gating */
+	u32 ahb_gate0;		/* 0x60 ahb module clock gating 0 */
+	u32 ahb_gate1;		/* 0x64 ahb module clock gating 1 */
+	u32 apb0_gate;		/* 0x68 apb0 module clock gating */
+	u32 apb1_gate;		/* 0x6c apb1 module clock gating */
+	u8 res4[0x10];
+	u32 nand_sclk_cfg;	/* 0x80 nand sub clock control */
+	u32 ms_sclk_cfg;	/* 0x84 memory stick sub clock control */
+	u32 sd0_clk_cfg;	/* 0x88 sd0 clock control */
+	u32 sd1_clk_cfg;	/* 0x8c sd1 clock control */
+	u32 sd2_clk_cfg;	/* 0x90 sd2 clock control */
+	u32 sd3_clk_cfg;	/* 0x94 sd3 clock control */
+	u32 ts_clk_cfg;		/* 0x98 transport stream clock control */
+	u32 ss_clk_cfg;		/* 0x9c */
+	u32 spi0_clk_cfg;	/* 0xa0 */
+	u32 spi1_clk_cfg;	/* 0xa4 */
+	u32 spi2_clk_cfg;	/* 0xa8 */
+	u32 pata_clk_cfg;	/* 0xac */
+	u32 ir0_clk_cfg;	/* 0xb0 */
+	u32 ir1_clk_cfg;	/* 0xb4 */
+	u32 iis_clk_cfg;	/* 0xb8 */
+	u32 ac97_clk_cfg;	/* 0xbc */
+	u32 spdif_clk_cfg;	/* 0xc0 */
+	u32 keypad_clk_cfg;	/* 0xc4 */
+	u32 sata_clk_cfg;	/* 0xc8 */
+	u32 usb_clk_cfg;	/* 0xcc */
+	u32 gps_clk_cfg;	/* 0xd0 */
+	u32 spi3_clk_cfg;	/* 0xd4 */
+	u8 res5[0x28];
+	u32 dram_clk_cfg;	/* 0x100 */
+	u32 be0_clk_cfg;	/* 0x104 */
+	u32 be1_clk_cfg;	/* 0x108 */
+	u32 fe0_clk_cfg;	/* 0x10c */
+	u32 fe1_clk_cfg;	/* 0x110 */
+	u32 mp_clk_cfg;		/* 0x114 */
+	u32 lcd0_ch0_clk_cfg;	/* 0x118 */
+	u32 lcd1_ch0_clk_cfg;	/* 0x11c */
+	u32 csi_isp_clk_cfg;	/* 0x120 */
+	u8 res6[0x4];
+	u32 tvd_clk_reg;	/* 0x128 */
+	u32 lcd0_ch1_clk_cfg;	/* 0x12c */
+	u32 lcd1_ch1_clk_cfg;	/* 0x130 */
+	u32 csi0_clk_cfg;	/* 0x134 */
+	u32 csi1_clk_cfg;	/* 0x138 */
+	u32 ve_clk_cfg;		/* 0x13c */
+	u32 audio_codec_clk_cfg;	/* 0x140 */
+	u32 avs_clk_cfg;	/* 0x144 */
+	u32 ace_clk_cfg;	/* 0x148 */
+	u32 lvds_clk_cfg;	/* 0x14c */
+	u32 hdmi_clk_cfg;	/* 0x150 */
+	u32 mali_clk_cfg;	/* 0x154 */
+	u8 res7[0x4];
+	u32 mbus_clk_cfg;	/* 0x15c */
+	u8 res8[0x4];
+	u32 gmac_clk_cfg;	/* 0x164 */
+};
+
+/* apb1 bit field */
+#define APB1_CLK_SRC_OSC24M		0
+#define APB1_FACTOR_M			0
+#define APB1_FACTOR_N			0
+
+/* clock divide */
+#define AXI_DIV_SHIFT		(0)
+#define AXI_DIV_1			0
+#define AXI_DIV_2			1
+#define AXI_DIV_3			2
+#define AXI_DIV_4			3
+#define AHB_DIV_SHIFT		(4)
+#define AHB_DIV_1			0
+#define AHB_DIV_2			1
+#define AHB_DIV_4			2
+#define AHB_DIV_8			3
+#define APB0_DIV_SHIFT		(8)
+#define APB0_DIV_1			0
+#define APB0_DIV_2			1
+#define APB0_DIV_4			2
+#define APB0_DIV_8			3
+#define CPU_CLK_SRC_SHIFT	(16)
+#define CPU_CLK_SRC_OSC24M		1
+#define CPU_CLK_SRC_PLL1		2
+
+#define PLL1_CFG_DEFAULT	0xa1005000
+
+#define PLL6_ENABLE_OFFSET	31
+
+#define CLK_GATE_OPEN			0x1
+#define CLK_GATE_CLOSE			0x0
+
+/* nand clock */
+#define NAND_CLK_SRC_OSC24		0
+#define NAND_CLK_DIV_N			0
+#define NAND_CLK_DIV_M			0
+
+/* gps clock */
+#define GPS_SCLK_GATING_OFF		0
+#define GPS_RESET			0
+
+/* ahb clock gate bit offset */
+#define AHB_GATE_OFFSET_GPS		26
+#define AHB_GATE_OFFSET_SATA		25
+#define AHB_GATE_OFFSET_PATA		24
+#define AHB_GATE_OFFSET_SPI3		23
+#define AHB_GATE_OFFSET_SPI2		22
+#define AHB_GATE_OFFSET_SPI1		21
+#define AHB_GATE_OFFSET_SPI0		20
+#define AHB_GATE_OFFSET_TS0		18
+#define AHB_GATE_OFFSET_EMAC		17
+#define AHB_GATE_OFFSET_ACE		16
+#define AHB_GATE_OFFSET_DLL		15
+#define AHB_GATE_OFFSET_SDRAM		14
+#define AHB_GATE_OFFSET_NAND		13
+#define AHB_GATE_OFFSET_MS		12
+#define AHB_GATE_OFFSET_MMC3		11
+#define AHB_GATE_OFFSET_MMC2		10
+#define AHB_GATE_OFFSET_MMC1		9
+#define AHB_GATE_OFFSET_MMC0		8
+#define AHB_GATE_OFFSET_MMC(n)		(AHB_GATE_OFFSET_MMC0 + (n))
+#define AHB_GATE_OFFSET_BIST		7
+#define AHB_GATE_OFFSET_DMA		6
+#define AHB_GATE_OFFSET_SS		5
+#define AHB_GATE_OFFSET_USB_OHCI1	4
+#define AHB_GATE_OFFSET_USB_EHCI1	3
+#define AHB_GATE_OFFSET_USB_OHCI0	2
+#define AHB_GATE_OFFSET_USB_EHCI0	1
+#define AHB_GATE_OFFSET_USB		0
+
+/* ahb clock gate bit offset (second register) */
+#define AHB_GATE_OFFSET_GMAC		17
+
+#define CCM_AHB_GATE_GPS (0x1 << 26)
+#define CCM_AHB_GATE_SDRAM (0x1 << 14)
+#define CCM_AHB_GATE_DLL (0x1 << 15)
+#define CCM_AHB_GATE_ACE (0x1 << 16)
+
+#define CCM_PLL5_CTRL_M(n) (((n) & 0x3) << 0)
+#define CCM_PLL5_CTRL_M_MASK CCM_PLL5_CTRL_M(0x3)
+#define CCM_PLL5_CTRL_M_X(n) ((n) - 1)
+#define CCM_PLL5_CTRL_M1(n) (((n) & 0x3) << 2)
+#define CCM_PLL5_CTRL_M1_MASK CCM_PLL5_CTRL_M1(0x3)
+#define CCM_PLL5_CTRL_M1_X(n) ((n) - 1)
+#define CCM_PLL5_CTRL_K(n) (((n) & 0x3) << 4)
+#define CCM_PLL5_CTRL_K_MASK CCM_PLL5_CTRL_K(0x3)
+#define CCM_PLL5_CTRL_K_X(n) ((n) - 1)
+#define CCM_PLL5_CTRL_LDO (0x1 << 7)
+#define CCM_PLL5_CTRL_N(n) (((n) & 0x1f) << 8)
+#define CCM_PLL5_CTRL_N_MASK CCM_PLL5_CTRL_N(0x1f)
+#define CCM_PLL5_CTRL_N_X(n) (n)
+#define CCM_PLL5_CTRL_P(n) (((n) & 0x3) << 16)
+#define CCM_PLL5_CTRL_P_MASK CCM_PLL5_CTRL_P(0x3)
+#define CCM_PLL5_CTRL_P_X(n) ((n) - 1)
+#define CCM_PLL5_CTRL_BW (0x1 << 18)
+#define CCM_PLL5_CTRL_VCO_GAIN (0x1 << 19)
+#define CCM_PLL5_CTRL_BIAS(n) (((n) & 0x1f) << 20)
+#define CCM_PLL5_CTRL_BIAS_MASK CCM_PLL5_CTRL_BIAS(0x1f)
+#define CCM_PLL5_CTRL_BIAS_X(n) ((n) - 1)
+#define CCM_PLL5_CTRL_VCO_BIAS (0x1 << 25)
+#define CCM_PLL5_CTRL_DDR_CLK (0x1 << 29)
+#define CCM_PLL5_CTRL_BYPASS (0x1 << 30)
+#define CCM_PLL5_CTRL_EN (0x1 << 31)
+
+#define CCM_GPS_CTRL_RESET (0x1 << 0)
+#define CCM_GPS_CTRL_GATE (0x1 << 1)
+
+#define CCM_DRAM_CTRL_DCLK_OUT (0x1 << 15)
+
+#define CCM_MBUS_CTRL_M(n) (((n) & 0xf) << 0)
+#define CCM_MBUS_CTRL_M_MASK CCM_MBUS_CTRL_M(0xf)
+#define CCM_MBUS_CTRL_M_X(n) ((n) - 1)
+#define CCM_MBUS_CTRL_N(n) (((n) & 0xf) << 16)
+#define CCM_MBUS_CTRL_N_MASK CCM_MBUS_CTRL_N(0xf)
+#define CCM_MBUS_CTRL_N_X(n) (((n) >> 3) ? 3 : (((n) >> 2) ? 2 : (((n) >> 1) ? 1 : 0)))
+#define CCM_MBUS_CTRL_CLK_SRC(n) (((n) & 0x3) << 24)
+#define CCM_MBUS_CTRL_CLK_SRC_MASK CCM_MBUS_CTRL_CLK_SRC(0x3)
+#define CCM_MBUS_CTRL_CLK_SRC_HOSC 0x0
+#define CCM_MBUS_CTRL_CLK_SRC_PLL6 0x1
+#define CCM_MBUS_CTRL_CLK_SRC_PLL5 0x2
+#define CCM_MBUS_CTRL_GATE (0x1 << 31)
+
+#define CCM_MMC_CTRL_OSCM24 (0x0 << 24)
+#define CCM_MMC_CTRL_PLL6   (0x1 << 24)
+#define CCM_MMC_CTRL_PLL5   (0x2 << 24)
+
+#define CCM_MMC_CTRL_ENABLE (0x1 << 31)
+
+#define CCM_GMAC_CTRL_TX_CLK_SRC_MII 0x0
+#define CCM_GMAC_CTRL_TX_CLK_SRC_EXT_RGMII 0x1
+#define CCM_GMAC_CTRL_TX_CLK_SRC_INT_RGMII 0x2
+#define CCM_GMAC_CTRL_GPIT_MII (0x0 << 2)
+#define CCM_GMAC_CTRL_GPIT_RGMII (0x1 << 2)
+
+
+#ifndef __ASSEMBLY__
+int clock_init(void);
+int clock_twi_onoff(int port, int state);
+void clock_set_pll1(int hz);
+unsigned int clock_get_pll5(void);
+#endif
+
+#endif /* _SUNXI_CLOCK_H */
diff --git a/arch/arm/include/asm/arch-sunxi/sys_proto.h b/arch/arm/include/asm/arch-sunxi/sys_proto.h
new file mode 100644
index 0000000..61b29d8
--- /dev/null
+++ b/arch/arm/include/asm/arch-sunxi/sys_proto.h
@@ -0,0 +1,17 @@ 
+/*
+ * (C) Copyright 2007-2012
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * Tom Cubie <tangliang@allwinnertech.com>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _SYS_PROTO_H_
+#define _SYS_PROTO_H_
+
+#include <linux/types.h>
+
+void sr32(u32 *, u32, u32, u32);
+void sdelay(unsigned long);
+
+#endif
diff --git a/arch/arm/include/asm/arch-sunxi/timer.h b/arch/arm/include/asm/arch-sunxi/timer.h
new file mode 100644
index 0000000..6aacfd7
--- /dev/null
+++ b/arch/arm/include/asm/arch-sunxi/timer.h
@@ -0,0 +1,88 @@ 
+/*
+ * (C) Copyright 2007-2011
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * Tom Cubie <tangliang@allwinnertech.com>
+ *
+ * Configuration settings for the Allwinner A10-evb board.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _SUNXI_TIMER_H_
+#define _SUNXI_TIMER_H_
+
+#ifndef __ASSEMBLY__
+
+#include <linux/types.h>
+
+/* General purpose timer */
+struct sunxi_timer {
+	u32 ctl;
+	u32 inter;
+	u32 val;
+	u8 res[4];
+};
+
+/* Audio video sync*/
+struct sunxi_avs {
+	u32 ctl;		/* 0x80 */
+	u32 cnt0;		/* 0x84 */
+	u32 cnt1;		/* 0x88 */
+	u32 div;		/* 0x8c */
+};
+
+/* 64 bit counter */
+struct sunxi_64cnt {
+	u32 ctl;		/* 0xa0 */
+	u32 lo;			/* 0xa4 */
+	u32 hi;			/* 0xa8 */
+};
+
+/* Watchdog */
+struct sunxi_wdog {
+	u32 ctl;		/* 0x90 */
+	u32 mode;		/* 0x94 */
+};
+
+/* Rtc */
+struct sunxi_rtc {
+	u32 ctl;		/* 0x100 */
+	u32 yymmdd;		/* 0x104 */
+	u32 hhmmss;		/* 0x108 */
+};
+
+/* Alarm */
+struct sunxi_alarm {
+	u32 ddhhmmss;		/* 0x10c */
+	u32 hhmmss;		/* 0x110 */
+	u32 en;			/* 0x114 */
+	u32 irqen;		/* 0x118 */
+	u32 irqsta;		/* 0x11c */
+};
+
+/* Timer general purpose register */
+struct sunxi_tgp {
+	u32 tgpd;
+};
+
+struct sunxi_timer_reg {
+	u32 tirqen;		/* 0x00 */
+	u32 tirqsta;		/* 0x04 */
+	u8 res1[8];
+	struct sunxi_timer timer[6];	/* We have 6 timers */
+	u8 res2[16];
+	struct sunxi_avs avs;
+	struct sunxi_wdog wdog;
+	u8 res3[8];
+	struct sunxi_64cnt cnt64;
+	u8 res4[0x58];
+	struct sunxi_rtc rtc;
+	struct sunxi_alarm alarm;
+	struct sunxi_tgp tgp[4];
+	u8 res5[8];
+	u32 cpu_cfg;
+};
+
+#endif /* __ASSEMBLY__ */
+
+#endif