[PATCH] cs5535-mfgpt: reuse timers that have never been set up

Jens Rottmann
2010-07-08T15:19:05Z

cs5535-mfgpt: reuse timers that have never been set up

The MFGPT hardware may be set up only once, therefore cs5535_mfgpt_free_timer()
didn't re-set the timer's "avail" bit. However if a timer is freed before it
has actually been in use then it may be made available again.

Signed-off-by: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>
---

Hi Andres,

Jens wrote:
> Andres wrote:
>> Jens wrote:
>>> cs5535_mfgpt_init() doesn't free up the timer in the error path
>>
>> Yeah, we can't really free the timer, unfortunately.
>> It actually might not be a bad idea to reverse the code so that the
>> IRQ allocation happens first, since we can clean that up if mfgpt
>> allocation fails.
>
> [...] I guess for reversing the order we'd have to split both
> cs5535_mfgpt_alloc_timer() and cs5535_mfgpt_setup_irq() into two parts [...]
> Maybe cs5535_mfgpt_free_timer() can be made more intelligent to set
> mfgpt->avail again if the hardware isn't actually in a non-reversible state
> yet [...]

As you can see I did just that. What do you think?

Cheers,
Jens

--- linux-2.6.35-rc4/drivers/misc/cs5535-mfgpt.c
+++ mfgpt_reuse_timers/drivers/misc/cs5535-mfgpt.c
@@ -211,6 +211,17 @@
*/
void cs5535_mfgpt_free_timer(struct cs5535_mfgpt_timer *timer)
{
+ unsigned long flags;
+ uint16_t val;
+
+ /* timer can be made available again only if never set up */
+ val = cs5535_mfgpt_read(timer, MFGPT_REG_SETUP);
+ if (!(val & MFGPT_SETUP_SETUP)) {
+ spin_lock_irqsave(&timer->chip->lock, flags);
+ __set_bit(timer->nr, timer->chip->avail);
+ spin_unlock_irqrestore(&timer->chip->lock, flags);
+ }
+
kfree(timer);
}
EXPORT_SYMBOL_GPL(cs5535_mfgpt_free_timer);
_