Re: [PATCH 2.6.20 1/1] usb/input updated aiptek tablet driver

On Fri, 2007-03-09 at 21:44 -0500, Dmitry Torokhov wrote:
> On Friday 09 March 2007 19:29, Pete Zaitcev wrote:
> > > +/* Activity checking thread. If a sufficient period of inactivity is detected, 
> > > +   the tablet's proximity is reset. */
> > > +static void activity_check(unsigned long data)
> > > +{
> > > +     struct aiptek *aiptek = (struct aiptek *) data;
> > > +
> > > +     /* This timer is set *after* handling input for the table, and
> > > +        cleared *before* handling it. Am guessing that we never run
> > > +        concurrently the module code. */
> > > +
> > > +     /* info("aiptek: timeout proximity\n"); */
> > > +
> > > +     /* apparently over-due. Reset the misc key, no tool, no proximity */
> > > +     input_report_abs(aiptek->inputdev, ABS_MISC, 0);
> > > +     input_sync(aiptek->inputdev);
> > > +}
> > 
> > This concerns me a bit. I am not 100% sure that input_sync and friends
> > are safe to run from several CPUs. Usually, nobody is bitten by this,
> > because they are delivered from a single IRQ. If you start doing this
> > from a timer, it's not longer true. Dmitry must bless this practice
> > (cc-ed). Perhas you want a spinlock here.
> > 
> 
> I want to fix locking in input handlers so concurrent execution of
> ->event() methods is not an issue. However there will be a question
> of correctness of the sequence in what events are delivered so some
> kind of locking in drivers themselves still may make sense. 

The timer is removed before the routine that handles the tablet data
(aiptek_irq) starts sending its own data with input_sync and friends. So
this should not produce any problem. 

I will change the del_timer to del_timer_sync, to remove the race where
the timer´s activity is running while the timer is being removed.

Does that sound reasonable?

René
###########################################

This message has been scanned by F-Secure Anti-Virus for Microsoft Exchange.
For more information, connect to http://www.f-secure.com/

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel