lkml.org 
[lkml]   [2009]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC][PATCH 03/10] arm: mxc: changes to common plat-mxc code to add support for i.MX5
Some comments below:

On 09 Dec 04, Sascha Hauer wrote:
> On Thu, Dec 03, 2009 at 08:12:58PM -0700, Herring Robert-RA7055 wrote:
> > Amit,
> >
> > I would suggest you refactor the timer code into version 1 and version 2
> > either as 2 separate files or with a timer_is_v2() function rather than
> > the mess of cpu_is_X macros it currently has. Essentially there are 2
> > versions of the timer hardware. Version 1 is found on MX1/MXL and MX21.
> > Version 2 is found on MX25, MX27, MX31, MX35, MX37, MX51, and future
> > parts. I will send you what we have done in our tree.
> Like this:
>
>
> commit ed6bbc59e3f6b33b48a0d5ac053220cd318ceee7
> Author: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Tue Nov 17 16:31:13 2009 +0100
>
> mxc timer: Add mx51 support
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> diff --git a/arch/arm/plat-mxc/time.c b/arch/arm/plat-mxc/time.c
> index 844567e..0c45509 100644
> --- a/arch/arm/plat-mxc/time.c
> +++ b/arch/arm/plat-mxc/time.c
> @@ -57,6 +57,9 @@
> #define MX3_TCN 0x24
> #define MX3_TCMP 0x10
>
> +#define timer_is_v1() (cpu_is_mx1() || cpu_is_mx27())
^^^^
Shouldn't this be mx21 according to
Rob's comment?

> +#define timer_is_v2() (cpu_is_mx3() || cpu_is_mx25() || cpu_is_mx51())

And add a cpu_is_mx27() here?

> static struct clock_event_device clockevent_mxc;
> static enum clock_event_mode clockevent_mode = CLOCK_EVT_MODE_UNUSED;
>
> @@ -66,7 +69,7 @@ static inline void gpt_irq_disable(void)
> {
> unsigned int tmp;
>
> - if (cpu_is_mx3() || cpu_is_mx25())
> + if (timer_is_v2())
> __raw_writel(0, timer_base + MX3_IR);
> else {
> tmp = __raw_readl(timer_base + MXC_TCTL);
> @@ -76,7 +79,7 @@ static inline void gpt_irq_disable(void)
>
> static inline void gpt_irq_enable(void)
> {
> - if (cpu_is_mx3() || cpu_is_mx25())
> + if (timer_is_v2())
> __raw_writel(1<<0, timer_base + MX3_IR);
> else {
> __raw_writel(__raw_readl(timer_base + MXC_TCTL) | MX1_2_TCTL_IRQEN,
> @@ -90,7 +93,7 @@ static void gpt_irq_acknowledge(void)
> __raw_writel(0, timer_base + MX1_2_TSTAT);
> if (cpu_is_mx2())
> __raw_writel(MX2_TSTAT_CAPT | MX2_TSTAT_COMP, timer_base + MX1_2_TSTAT);
> - if (cpu_is_mx3() || cpu_is_mx25())
> + if (timer_is_v2())
> __raw_writel(MX3_TSTAT_OF1, timer_base + MX3_TSTAT);
> }
>
> @@ -117,7 +120,7 @@ static int __init mxc_clocksource_init(struct clk *timer_clk)
> {
> unsigned int c = clk_get_rate(timer_clk);
>
> - if (cpu_is_mx3() || cpu_is_mx25())
> + if (timer_is_v2())
> clocksource_mxc.read = mx3_get_cycles;
>
> clocksource_mxc.mult = clocksource_hz2mult(c,
> @@ -180,7 +183,7 @@ static void mxc_set_mode(enum clock_event_mode mode,
>
> if (mode != clockevent_mode) {
> /* Set event time into far-far future */
> - if (cpu_is_mx3() || cpu_is_mx25())
> + if (timer_is_v2())
> __raw_writel(__raw_readl(timer_base + MX3_TCN) - 3,
> timer_base + MX3_TCMP);
> else
> @@ -233,7 +236,7 @@ static irqreturn_t mxc_timer_interrupt(int irq, void *dev_id)
> struct clock_event_device *evt = &clockevent_mxc;
> uint32_t tstat;
>
> - if (cpu_is_mx3() || cpu_is_mx25())
> + if (timer_is_v2())
> tstat = __raw_readl(timer_base + MX3_TSTAT);
> else
> tstat = __raw_readl(timer_base + MX1_2_TSTAT);
> @@ -264,7 +267,7 @@ static int __init mxc_clockevent_init(struct clk *timer_clk)
> {
> unsigned int c = clk_get_rate(timer_clk);
>
> - if (cpu_is_mx3() || cpu_is_mx25())
> + if (timer_is_v2())
> clockevent_mxc.set_next_event = mx3_set_next_event;
>
> clockevent_mxc.mult = div_sc(c, NSEC_PER_SEC,
> @@ -296,7 +299,7 @@ void __init mxc_timer_init(struct clk *timer_clk, void __iomem *base, int irq)
> __raw_writel(0, timer_base + MXC_TCTL);
> __raw_writel(0, timer_base + MXC_TPRER); /* see datasheet note */
>
> - if (cpu_is_mx3() || cpu_is_mx25())
> + if (timer_is_v2())
> tctl_val = MX3_TCTL_CLK_IPG | MX3_TCTL_FRR | MX3_TCTL_WAITEN | MXC_TCTL_TEN;
> else
> tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN;
>

Otherwise these changes look good and make the code more readable.

/Amit
--
------------------------------------------------------------
Amit Kucheria, Finland
------------------------------------------------------------


\
 
 \ /
  Last update: 2009-12-04 11:33    [W:0.053 / U:1.940 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site