[PATCH] kill mutex in lis

[Sean McNeil]

Hi Simon,

Simon Kagstrom wrote:
> On Fri, 10 Oct 2008 14:57:47 -0200
> Werner Almesberger <werner at openmoko.org> wrote:
>
>   
>> How bad was the workqueue really ? It's certainly wasteful,
>> but if we consider that all the data goes up to userspace
>> eagerly consuming every event anyway, it may not matter all
>> that much in the big picture.
>>     
>
> I did some measurements after I posted my patch to do this (not
> realising that this is how it was done earlier). And there was
> significant difference from just reading the accelerometers. Workqueue
> numbers first:
>
>   
>>   Cpu(s): 29.1%us, 24.5%sy,  0.0%ni, 46.2%id,  0.0%wa,  0.0%hi,  0.3%si,  0.0%st
>>    2239 root      20   0  5168 2952 1748 S 18.1  2.4   0:04.74 python                                 
>>     232 root      15  -5     0    0    0 S  7.8  0.0   0:09.71 spi_s3c24xx_gpi                        
>>    1546 root      15  -5     0    0    0 S  6.6  0.0   0:10.76 LIS302dl interr   
>>
>> compared to the current driver
>>
>>   Cpu(s): 18.0%us,  7.3%sy,  0.0%ni, 74.4%id,  0.0%wa,  0.0%hi,  0.3%si,  0.0%st
>>    1647 root      20   0  5168 2952 1748 S 17.4  2.4   0:01.10 python                   
>>     
>
> However, I think the spi_async() method should do quite a bit better
> than this. Instead of basically doing 5 queue_work()s per interrupt
> (the interrupt itself, plus reading of status,x,y,z), it would do just
> one.
>
> I made some mistake in the implementation so it doesn't work yet, so I
> don't have numbers for the improvement. I still think there is a race
> condition if we get the interrupt again before the spi_async completion
> handler has finished executing (so that the workqueue handler is already
> running)

For a SPI implementation, I'm pretty sure you will want to go back to an
edge triggered interrupt as you will not be clearing out the cause
during the ISR. I don't know if you started with the previous code or
the more recent code that changes it to level trigger. You will also
want to make sure your work function loops reading all the data until
the status says no more data or you will end up with no further
interrupts being generated when the system is too busy to keep up.

I'd like to also remind again that the 2 accelerometers are mutually
exclusive so they will have to block each other with a mutex or spin
lock. This brings up a question I have about the original changes: why
convert the mutex to a spin lock? What is the benefit? Is it because
there is an acquire in an ISR?

Cheers,
Sean