> -----Original Message-----
> From: Varadarajan, Charulatha
> Sent: Tuesday, December 07, 2010 10:58 AM
> To: DebBarma, Tarun Kanti
> Cc: [email protected]
> Subject: Re: [PATCH v5 10/12] OMAP: dmtimer: switch-over to platform
> device driver
>
> Tarun,
>
> On Tue, Dec 7, 2010 at 05:14, Tarun Kanti DebBarma <[email protected]>
> wrote:
> > switch-over to platform device driver through following changes:
> > (a) call to dmtimer initialization routine from timer-gp.c is
> > removed (b) initiate dmtimer early initialization from
> omap2_init_common_hw
> > in io.c (c) modify plat-omap/dmtimer routines to use new register map
> and
> > platform data.
> >
> > Signed-off-by: Tarun Kanti DebBarma <[email protected]>
> > Reviewed-by: Cousson, Benoit <[email protected]>
> > Reviewed-by: Varadarajan, Charulatha <[email protected]>
> > ---
> > arch/arm/mach-omap2/clock2420_data.c | 2 +-
> > arch/arm/mach-omap2/clock2430_data.c | 2 +-
> > arch/arm/mach-omap2/clock3xxx_data.c | 2 +-
> > arch/arm/mach-omap2/clock44xx_data.c | 2 +-
> > arch/arm/mach-omap2/dmtimer.c | 40 ++++
> > arch/arm/mach-omap2/io.c | 2 +
> > arch/arm/mach-omap2/timer-gp.c | 1 -
> > arch/arm/plat-omap/dmtimer.c | 323 +++++++++-------------
> -------
> > arch/arm/plat-omap/include/plat/dmtimer.h | 2 -
> > 9 files changed, 148 insertions(+), 228 deletions(-)
> >
> <<snip>>
>
> > diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
> > index ddc875b..ac6a498 100644
> > --- a/arch/arm/plat-omap/dmtimer.c
> > +++ b/arch/arm/plat-omap/dmtimer.c
>
> <<snip>>
> > -static struct omap_dm_timer *dm_timers;
> > -static const char **dm_source_names;
> > -static struct clk **dm_source_clocks;
> > -
> > static LIST_HEAD(omap_timer_list);
> > static DEFINE_SPINLOCK(dm_timer_lock);
> >
> > -/*
> > - * Reads timer registers in posted and non-posted mode. The posted mode
> bit
> > - * is encoded in reg. Note that in posted mode write pending bit must
> be
> > - * checked. Otherwise a read of a non completed write will produce an
> error.
> > +/**
> > + * omap_dm_timer_read_reg - read timer registers in posted and non-
> posted mode
> > + * @timer: timer pointer over which read operation to perform
> > + * @reg: lowest byte holds the register offset
> > + *
> > + * The posted mode bit is encoded in reg. Note that in posted mode
> write
> > + * pending bit must be checked. Otherwise a read of a non completed
> write
> > + * will produce an error.
> > */
> > static inline u32 omap_dm_timer_read_reg(struct omap_dm_timer *timer,
> u32 reg)
> > {
> > + struct dmtimer_platform_data *pdata = timer->pdev-
> >dev.platform_data;
>
> Leave one line
Ok.
>
> > + if (reg >= OMAP_TIMER_WAKEUP_EN_REG)
> > + reg += pdata->func_offset;
> > + else if (reg >= OMAP_TIMER_STAT_REG)
> > + reg += pdata->intr_offset;
> > if (timer->posted)
> > while (readl(timer->io_base + (OMAP_TIMER_WRITE_PEND_REG
> & 0xff))
> > & (reg >> WPSHIFT))
> > @@ -290,15 +195,24 @@ static inline u32 omap_dm_timer_read_reg(struct
> omap_dm_timer *timer, u32 reg)
> > return readl(timer->io_base + (reg & 0xff));
> > }
> >
> > -/*
> > - * Writes timer registers in posted and non-posted mode. The posted
> mode bit
> > - * is encoded in reg. Note that in posted mode the write pending bit
> must be
> > - * checked. Otherwise a write on a register which has a pending write
> will be
> > - * lost.
> > +/**
> > + * omap_dm_timer_write_reg - write timer registers in posted and non-
> posted mode
> > + * @timer: timer pointer over which write operation is to perform
> > + * @reg: lowest byte holds the register offset
> > + * @value: data to write into the register
> > + *
> > + * The posted mode bit is encoded in reg. Note that in posted mode the
> write
> > + * pending bit must be checked. Otherwise a write on a register which
> has a
> > + * pending write will be lost.
> > */
> > static void omap_dm_timer_write_reg(struct omap_dm_timer *timer, u32
> reg,
> > u32 value)
> > {
> > + struct dmtimer_platform_data *pdata = timer->pdev-
> >dev.platform_data;
>
> Leave one line
Ok.
>
> > + if (reg >= OMAP_TIMER_WAKEUP_EN_REG)
> > + reg += pdata->func_offset;
> > + else if (reg >= OMAP_TIMER_STAT_REG)
> > + reg += pdata->intr_offset;
> > if (timer->posted)
> > while (readl(timer->io_base + (OMAP_TIMER_WRITE_PEND_REG
> & 0xff))
> > & (reg >> WPSHIFT))
> > @@ -306,6 +220,8 @@ static void omap_dm_timer_write_reg(struct
> omap_dm_timer *timer, u32 reg,
> > writel(value, timer->io_base + (reg & 0xff));
> > }
> >
> > +#if defined(CONFIG_ARCH_OMAP1)
> > +
> > static void omap_dm_timer_wait_for_reset(struct omap_dm_timer *timer)
> > {
> > int c;
> > @@ -324,54 +240,77 @@ static void omap_dm_timer_reset(struct
> omap_dm_timer *timer)
> > {
> > u32 l;
> >
> > - if (!cpu_class_is_omap2() || timer != &dm_timers[0]) {
> > + if (!cpu_class_is_omap2() || timer->pdev->id != 1) {
>
> Should this be '&&' instead of '||' ?
>
> Not required to check the cpu_class as this function is under
> CONFIG_ARCH_OMAP1.
Yes, the check is no longer needed. Thanks.
> Also I think, it is better to use cpu_class is checks than using
> CONFIG_ARCH_OMAP1 checks.
In case this piece of code is kept here I will use cpu_class.
> I guess that this function can be considered to be moved to mach-omap1/*
Sounds good! In this case we have to introduce a function pointer in
Platform data structure guarded by a macro. This can then be called
>From omap_dm_timer_prepare() with cpu_class check. Thanks.
>
> > omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG,
> 0x06);
> > omap_dm_timer_wait_for_reset(timer);
> > }
> > - omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
> >
> > l = omap_dm_timer_read_reg(timer, OMAP_TIMER_OCP_CFG_REG);
> > l |= 0x02 << 3; /* Set to smart-idle mode */
> > l |= 0x2 << 8; /* Set clock activity to perserve f-clock on
> idle */
> >
> > - /*
> > - * Enable wake-up on OMAP2 CPUs.
> > - */
> > - if (cpu_class_is_omap2())
> > - l |= 1 << 2;
> > omap_dm_timer_write_reg(timer, OMAP_TIMER_OCP_CFG_REG, l);
> >
> > - /* Match hardware reset default of posted mode */
> > - omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG,
> > - OMAP_TIMER_CTRL_POSTED);
> > - timer->posted = 1;
> > }
> > +#endif
> >
> > static void omap_dm_timer_prepare(struct omap_dm_timer *timer)
> > {
> > + u32 l;
> > +
> > + timer->fclk = clk_get(&timer->pdev->dev, "fck");
> > + if (IS_ERR_OR_NULL(timer->fclk)) {
> > + pr_info("omap_timer: failed to acquire fclk handle.\n");
> > + WARN_ON(1);
>
> Embed warning message in WARN_ON and remove pr_info usage.
> Give proper warning message in all other places where WARN_ON(1) is being
> used
Ok.
>
> > + return;
> > + }
> > +
> > omap_dm_timer_enable(timer);
> > +
> > +#if defined(CONFIG_ARCH_OMAP1)
> > omap_dm_timer_reset(timer);
> > +#endif
> > +
> > + /*
> > + * Enable wake-up on OMAP2420, OMAP2430 and OMAP3430 CPUs.
> > + * FIXME: SYSC_HAS_ENAWAKEUP flag is already set in hwmod.
> > + * But on OMAP2 code does not work without following
> configuration.
> > + * Need to investigate why this is happening.
> > + */
> > + if (is_omap2430() || is_omap2420() || is_omap3430()) {
>
> Do not use these checks in plat-omap/*.
Yes, there is FIXME message. Need to figure out why this is happening.
> Prefer using cpu_is* checks instead of is_omap2420().
Ok.
>
> > + l = omap_dm_timer_read_reg(timer,
> OMAP_TIMER_OCP_CFG_REG);
> > + l |= 1 << 2;
> > + omap_dm_timer_write_reg(timer, OMAP_TIMER_OCP_CFG_REG,
> l);
> > + }
> > +
> > + omap_dm_timer_set_source(timer, OMAP_TIMER_SRC_32_KHZ);
> > +
> > + /* Match hardware reset default of posted mode */
> > + omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG,
> > + OMAP_TIMER_CTRL_POSTED);
> > + timer->posted = 1;
> > }
>
> <<snip>>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
