On Mon, Nov 22, 2010 at 14:44, DebBarma, Tarun Kanti <tarun.kanti@xxxxxx> wrote:
>> -----Original Message-----
>> From: Varadarajan, Charulatha
>> Sent: Monday, November 22, 2010 12:15 PM
>> To: DebBarma, Tarun Kanti
>> Cc: linux-omap@xxxxxxxxxxxxxxx
>> Subject: Re: [PATCHv4 12/14] OMAP: dmtimer: switch-over to platform device
>> driver
>>
>> <<snip>>
>>
>> > -/*
>> > - * 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;
>> > +
>> > + if (reg >= OMAP_TIMER_WAKEUP_EN_REG)
>> > + reg += pdata->func_offst;
>> > +
>> > if (timer->posted)
>> > - while (readl(timer->io_base + (OMAP_TIMER_WRITE_PEND_REG
>> & 0xff))
>> > - & (reg >> WPSHIFT))
>> > + while (readl(timer->io_base +
>> > + ((OMAP_TIMER_WRITE_PEND_REG + pdata->func_offst)
>> > + & 0xff)) & (reg >> WPSHIFT))
>>
>> You may add a timeout in this.
> There are few reasons why I have not done:
> (1) This was in this way from the beginning and I thought some analysis
> Was done already for not adding timeout and so did not wish to alter that.
> (2) wanted to keep low level functions as simple as possible by avoiding
> As many checks as possible unless really needed.
> (3) The client driver can take care of timeout. Even if timeout is
> Introduced in this api, the client driver anyways have to check for the
> Timeout.
I don't think that client driver should take care of this. The client
driver should
check only for the return value. If timeout, return error.
In your case, one of the calls to omap_dm_timer_write_reg() is as given below:
omap2_gp_timer_init()
-> omap2_gp_clockevent_init()
-> omap_dm_timer_request_specific()
-> omap_dm_timer_prepare()
-> omap_dm_timer_write_reg()
Here timeout is not taken care.
>
> --
> Tarun
>>
>> > cpu_relax();
>> > 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;
>> > +
>> > + if (reg >= OMAP_TIMER_WAKEUP_EN_REG)
>> > + reg += pdata->func_offst;
>> > +
>> > if (timer->posted)
>> > - while (readl(timer->io_base + (OMAP_TIMER_WRITE_PEND_REG
>> & 0xff))
>> > - & (reg >> WPSHIFT))
>> > + while (readl(timer->io_base +
>> > + ((OMAP_TIMER_WRITE_PEND_REG + pdata->func_offst)
>> > + & 0xff)) & (reg >> WPSHIFT))
>>
>> Ditto.
>>
>> > cpu_relax();
>> > writel(value, timer->io_base + (reg & 0xff));
>> > }
>>
>>
>> <<snip>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
