RE: [PATCHv4 7/14] OMAP: dmtimer: use list instead of static array

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Cousson, Benoit
> Sent: Tuesday, November 23, 2010 8:52 PM
> To: DebBarma, Tarun Kanti
> Cc: linux-omap@xxxxxxxxxxxxxxx
> Subject: Re: [PATCHv4 7/14] OMAP: dmtimer: use list instead of static
> array
> 
> On 11/20/2010 3:39 AM, Tarun Kanti DebBarma wrote:
> > Convert dmtimers static array in functions into list structure.
> > Please note that the static arrays will be completely removed
> > in subsequent patches when dmtimer is converted to platform driver.
> 
> Why do you still need to keep your own list of timers?
> The driver already contains a list of devices.
> You even have some iterator available in the kernel:
> driver_for_each_device or driver_find_device.
> 
> You could probably get rid of all that stuff for my point of view.
Ok, this partially answers my previous email.
As I said this would involve change of exported APIs.
I will try making the change to avoid this if possible.
Thanks.

--
Tarun

> 
> >
> > Signed-off-by: Tarun Kanti DebBarma<tarun.kanti@xxxxxx>
> > ---
> >   arch/arm/plat-omap/dmtimer.c |   67 +++++++++++++++++++++++-----------
> --------
> >   1 files changed, 37 insertions(+), 30 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
> > index 10daa9d..124fd21 100644
> > --- a/arch/arm/plat-omap/dmtimer.c
> > +++ b/arch/arm/plat-omap/dmtimer.c
> > @@ -168,6 +168,7 @@ struct omap_dm_timer {
> >   	unsigned enabled:1;
> >   	unsigned posted:1;
> >   	struct platform_device *pdev;
> > +	struct list_head node;
> >   };
> >
> >   static int dm_timer_count;
> > @@ -290,7 +291,8 @@ static struct omap_dm_timer *dm_timers;
> >   static const char **dm_source_names;
> >   static struct clk **dm_source_clocks;
> >
> > -static spinlock_t dm_timer_lock;
> > +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
> > @@ -340,7 +342,7 @@ 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->id != 1) {
> >   		omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG, 0x06);
> >   		omap_dm_timer_wait_for_reset(timer);
> >   	}
> > @@ -371,23 +373,24 @@ static void omap_dm_timer_prepare(struct
> omap_dm_timer *timer)
> >
> >   struct omap_dm_timer *omap_dm_timer_request(void)
> >   {
> > -	struct omap_dm_timer *timer = NULL;
> > +	struct omap_dm_timer *timer = NULL, *t;
> >   	unsigned long flags;
> > -	int i;
> >
> >   	spin_lock_irqsave(&dm_timer_lock, flags);
> > -	for (i = 0; i<  dm_timer_count; i++) {
> > -		if (dm_timers[i].reserved)
> > +	list_for_each_entry(t,&omap_timer_list, node) {
> > +		if (t->reserved)
> >   			continue;
> >
> > -		timer =&dm_timers[i];
> > +		timer = t;
> >   		timer->reserved = 1;
> >   		break;
> >   	}
> >   	spin_unlock_irqrestore(&dm_timer_lock, flags);
> >
> > -	if (timer != NULL)
> > +	if (timer)
> >   		omap_dm_timer_prepare(timer);
> > +	else
> > +		pr_debug("%s: free timer not available.\n", __func__);
> >
> >   	return timer;
> >   }
> > @@ -395,23 +398,23 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_request);
> >
> >   struct omap_dm_timer *omap_dm_timer_request_specific(int id)
> >   {
> > -	struct omap_dm_timer *timer;
> > +	struct omap_dm_timer *timer = NULL, *t;
> >   	unsigned long flags;
> >
> >   	spin_lock_irqsave(&dm_timer_lock, flags);
> > -	if (id<= 0 || id>  dm_timer_count || dm_timers[id-1].reserved) {
> > -		spin_unlock_irqrestore(&dm_timer_lock, flags);
> > -		printk("BUG: warning at %s:%d/%s(): unable to get timer %d\n",
> > -		       __FILE__, __LINE__, __func__, id);
> > -		dump_stack();
> > -		return NULL;
> > +	list_for_each_entry(t,&omap_timer_list, node) {
> > +		if (t->id == id&&  !t->reserved) {
> > +			timer = t;
> > +			timer->reserved = 1;
> > +			break;
> > +		}
> >   	}
> > -
> > -	timer =&dm_timers[id-1];
> > -	timer->reserved = 1;
> >   	spin_unlock_irqrestore(&dm_timer_lock, flags);
> >
> > -	omap_dm_timer_prepare(timer);
> > +	if (timer)
> > +		omap_dm_timer_prepare(timer);
> > +	else
> > +		pr_debug("%s: timer%d not available.\n", __func__, id);
> >
> >   	return timer;
> >   }
> > @@ -474,24 +477,29 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_get_irq);
> >    */
> >   __u32 omap_dm_timer_modify_idlect_mask(__u32 inputmask)
> >   {
> > -	int i;
> > +	int i = 0;
> > +	struct omap_dm_timer *timer = NULL;
> > +	unsigned long flags;
> >
> >   	/* If ARMXOR cannot be idled this function call is unnecessary */
> >   	if (!(inputmask&  (1<<  1)))
> >   		return inputmask;
> >
> >   	/* If any active timer is using ARMXOR return modified mask */
> > -	for (i = 0; i<  dm_timer_count; i++) {
> > +	spin_lock_irqsave(&dm_timer_lock, flags);
> > +	list_for_each_entry(timer,&omap_timer_list, node) {
> >   		u32 l;
> >
> > -		l = omap_dm_timer_read_reg(&dm_timers[i], OMAP_TIMER_CTRL_REG);
> > +		l = omap_dm_timer_read_reg(timer, OMAP_TIMER_CTRL_REG);
> >   		if (l&  OMAP_TIMER_CTRL_ST) {
> >   			if (((omap_readl(MOD_CONF_CTRL_1)>>  (i * 2))&  0x03) == 0)
> >   				inputmask&= ~(1<<  1);
> >   			else
> >   				inputmask&= ~(1<<  2);
> >   		}
> > +		i++;
> >   	}
> > +	spin_unlock_irqrestore(&dm_timer_lock, flags);
> >
> >   	return inputmask;
> >   }
> > @@ -722,13 +730,9 @@ EXPORT_SYMBOL_GPL(omap_dm_timer_write_counter);
> >
> >   int omap_dm_timers_active(void)
> >   {
> > -	int i;
> > -
> > -	for (i = 0; i<  dm_timer_count; i++) {
> > -		struct omap_dm_timer *timer;
> > -
> > -		timer =&dm_timers[i];
> > +	struct omap_dm_timer *timer;
> >
> > +	list_for_each_entry(timer,&omap_timer_list, node) {
> >   		if (!timer->enabled)
> >   			continue;
> >
> > @@ -743,14 +747,13 @@ EXPORT_SYMBOL_GPL(omap_dm_timers_active);
> >
> >   int __init omap_dm_timer_init(void)
> >   {
> > +	unsigned long flags;
> >   	struct omap_dm_timer *timer;
> >   	int i, map_size = SZ_8K;	/* Module 4KB + L4 4KB except on omap1 */
> >
> >   	if (!(cpu_is_omap16xx() || cpu_class_is_omap2()))
> >   		return -ENODEV;
> >
> > -	spin_lock_init(&dm_timer_lock);
> > -
> >   	if (cpu_class_is_omap1()) {
> >   		dm_timers = omap1_dm_timers;
> >   		dm_timer_count = omap1_dm_timer_count;
> > @@ -795,6 +798,10 @@ int __init omap_dm_timer_init(void)
> >   			timer->fclk = clk_get(NULL, clk_name);
> >   		}
> >   #endif
> > +		timer->id = i + 1; /* id starts from 1*/
> > +		spin_lock_irqsave(&dm_timer_lock, flags);
> > +		list_add_tail(&timer->node,&omap_timer_list);
> > +		spin_unlock_irqrestore(&dm_timer_lock, flags);
> >   	}
> >
> >   	return 0;

--
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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux