Re: [PATCH] LIS3LV02Dx Accelerometer driver (take 4)

[Pavel Machek <pavel@suse.cz>]

Hi!

> > Ok, I guess you have better eyeballs. Yes, the locking was totaly
> > broken (and disabled by initing semaphore to 1 by default).
> To which value the semaphore should have been initialised? I can see
> DECLARE_MUTEX(name) declared as:
> 	struct semaphore name = __SEMAPHORE_INITIALIZER(name, 1)
> 
> (I'm still learning about the locking in the kernel. To me, it looks
> like a maze, and I couldn't find one simple document detailing all the
> mechanisms.)

It is a bit of maze.

You should not really use semaphores. They are old and tricky.

> > This strips some more code, turns semaphore into mutex, and should
> > solve more problems.
> > 
> > I killed the "power down on timer" thingie. I guess we can reintroduce
> > it when we get the basics right (and driver works just fine without
> > that... I did not detect unusual slowness).
> I inserted the timer thingie because I have a program on my laptop which
> reads the position twice per second. Using the joystick interface uses
> two much CPU (it's 20 Hz) and so it's using the sysfs interface.
> However, turning on and off the device twice per second is rather costly
> (especially due to the ACPI access). That's why I made it delayed... and
> I'd like to keep it this way.

Ultimately, we may end up doing that; but for now, I'd like to get
very small and very obvious patch merged...

(Possible workaround: sleep 100 < position should keep the
accelerometer powered up for 100 seconds, right?)

> I'll send this evening a "delayed power down" based on workqueue. Within
> the  work of a workqueue, it's not possible to use a mutex, right? Only
> semaphores are allowed?

Within workqueue, mutex and semaphores are both allowed. What you may
not do is to keep mutex locked and return control.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html