[PATCH] (repost) lis302dl-queue-work-in-interrupt-handler.patch

[Sean McNeil]

Simon Kagstrom wrote:
> (I should have stated RFC somewhere here again)
>
> On Mon, 22 Sep 2008 17:33:32 +0100
> Andy Green <andy at openmoko.com> wrote:
>
>   
>> I don't think the workqueue way is ideal either, that is what was in
>> there originally as the workaround for not being able to use SPI in
>> interrupt context, but it doubles the number of context switches per
>> interrupt, and we already have a lot of them.
>>     
>
> Yes, this is true. I just thought it was a more elegant way of
> implementing the driver. But you are right. Looking at it in top shows
> that it consumes quite a bit of resources:
>
>   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                   
>
> So, I guess this is a no-go for that reason if nothing else. Well, well.
>
> (Switching to the other reply)
>
> On Mon, 22 Sep 2008 09:35:12 -0700
> Sean McNeil <sean at mcneil.com> wrote:
>
>   
>> 2 things:
>>
>> 1) If you change the interrupt to a level instead of edge trigger,
>> then you can get rid of the code to jump start and the while loop. It
>> also prevents lockup when the system is extremely busy. I don't think
>> your implementation would prevent that.
>>     
>
> Why is the interrupt edge-triggered by the way? I just assumed this as
> given.
>
>   
>> 2) I was under the impression that the reg_read didn't prevent
>> simultaneous access of the two accelerometers. This would cause
>> problems since the chip selects need to be mutually exclusive when
>> accessing them.
>>     
>
> There is a mutex taken in reg_read/reg_write, but this is private per
> instance. Perhaps it should be driver-global then?
>
>
> So about the hanging problem: Did I understand the situation correct
> that it is caused by the device having reported values into the
> out-registers while in the interrupt handler (which is subsequently
> missed as this is an edge interrupt).

I think the fundamental problem is that the interrupt pin is a level
where it indicates that data is available to read from the
accelerometer. So, I actually think your approach would work just fine.
You can either:

a) use a level interrupt and do things in the irq handler.
b) use an edge trigger interrupt and do things in a work_queue where it
assures that the line will go high again so that a new interrupt can happen.

Which means you have to loop until no data in the work_queue as you have
done.

Cheers,
Sean