>-----Original Message-----
>From: Alan Cox [mailto:[email protected]]
>Sent: Tuesday, November 16, 2010 3:11 PM
>To: Gorby, Russ
>Cc: [email protected]
>Subject: Re: [Meego-kernel] [PATCH] IFX runtime PM patch
>
>> [Gorby, Russ] the issue here is the relative timing of D3 suspend
>> between the SPI protocol and controller driver. You may be right that
>> it doesn't completely close the hole. It's here to mitigate the
>> problem. If I could count on top-down suspend and bottom up resume
>> this wouldn't be needed at all but I'm not aware of any guarantees
>> like that.
>
>If it takes out the modem when it gets it wrong then you need to sort
>the locking out so it can't suspend in the middle of waggling MRDY
>

[Gorby, Russ] 
The issue is not intra-driver locking.
It's a problem of coordination between 2 drivers.
The protocol driver is the one the one that does MRDY/SRDY handshake with the 
modem hardware via gpio lines, but it is the controller driver that drives the 
SPI bus, with the SPI core sitting between the 2 drivers.
If the former happens w/o the latter, that's the problem case.
This same situation arises w/ runtime suspend/resume, but since the controller 
autonomously resumes (and appears to do so fast enough), the modem stays alive. 
I was just trying to be thorough w/ the D3 suspend/resume case, it's not 
actually an error path I've instrumented and exercised. So if this seems like a 
wart I'll remove it.


>> [Gorby, Russ] It was explained in the preamble email (a little) but I
>> should have put it in this patch file as well. My apologies - I'm
>
>Better yet as a separate patch.
[Gorby, Russ] Fair enough - this was a performance related issue and not a 
functional one

>
>> still learning some of the process details. The issue here is that if
>> the controller driver has gone into PM-runtime-suspend at the time
>> I/O arrives for it from spi_async(), even though the controller
>> driver can autonomously resume, it takes some time and returns EBUSY
>> until it has resumed. Our protocol driver calls spi_async from a
>> tasklet, and we discovered during PM testing that if the tasklet is
>> immediately rescheduled we get a large number of iterations before
>> the resume completes. So, we added a delay timer in this case so the
>> CPU is used more appropriately.
>
>Ok that wants fixing in the SPI code, otherwise we have to fix every
>SPI device for the same thing which is bonkers.
>
>Ok that requires more thought on how to do it properly.
>
[Gorby, Russ] OK, let me know what you think. 

>> >> + clear_bit(IFX_SPI_STATE_IO_RETRY, &(ifx_dev->flags));
>> >> + del_timer(&ifx_dev->spi_retry_timer);
>> >
>> >And if the timer handler is running in parallel on another CPU isn't
>> >this going to cause confusion ?
>>
>> [Gorby, Russ] That shouldn't be possible - unless you see something I
>> do not. There can be only one SPI msg being processed at a time, and
>> one tasklet running. The tasklet is the only thing that calls
>> spi_async.  Spi_complete runs only if spi_async succeeded or returned
>> something other than EBUSY and the retry timer runs only if spi_async
>> returned EBUSY.
>
>Ok so in fact at the point you hit the del_timer the timer cannot in
>fact be running anyway but must have completed already ?

 [Gorby, Russ] correct

>
>> [Gorby, Russ] I gather you're referring to the intel_mid_ssp_...
>> driver. We have not determined which way to go on that. There is
>> missing functionality in each that would  need to be filled-out.
>
>Well on the basis that one driver has all Moorestown and Medfield
>integrated into one driver it's the one currently being subjected to
>upstream review.
>
>I don't know how we came to have two each of which doesn't do
>everything but it seem a rather silly situation and one that would be
>improved by having everybody working on the one driver.
>

[Gorby, Russ] I agree with you and would I like to resolve that situation asap, 
but I can't do any of that until the critical functional changes are accepted 
to meego which has pretty much stopped at this point as far as I can tell. I 
have multiple functionality and bugfix patches I would like to submit but until 
I know what exactly has been merged to meego I don't know how to craft the 
patches as they're all mostly against the same driver and are inherently serial 
for the most part.
_______________________________________________
MeeGo-kernel mailing list
[email protected]
http://lists.meego.com/listinfo/meego-kernel

Reply via email to