[RFC] libertas: remove TX queues from GSPI driver

[Andrey Yurovsky]

On Mon, Jun 22, 2009 at 8:29 AM, Sebastian Andrzej
Siewior<sebastian at breakpoint.cc> wrote:
> * Andrey Yurovsky | 2009-06-22 08:10:32 [-0700]:
>
>>On Mon, Jun 22, 2009 at 1:04 AM, Sebastian Andrzej
>>Siewior<sebatian at breakpoint.cc> wrote:
>>> * Andrey Yurovsky | 2009-06-19 15:39:28 [-0700]:
>>>
>>>>Posting this as an RFC in case others want to help test it. ?This
>>>>work-in-progress patch removes the redundant command and data TX queues
>>>>from the Libertas GSPI driver, it also therefore removes a memcpy from
>>>>if_spi_host_to_card and the locking mechanisms for the queues in
>>>>question. ?The goal is to improve performance and reduce overhead by
>>>>simplifying the TX path.
>>>>
>>>>The driver works after this patch, however IEEE PS is somewhat broken
>>>>(especially temporarily waking the device to issue commands). ?This is
>>>>caused, in part, by the IRQ handler preempting the libertas main thread
>>>>(for example, when the main thread calls if_spi_host_to_card and an IRQ
>>>>comes in) so we need a synchronization solution that allows interrupt
>>>>servicing to be done when it's safe to do so, that is we need to lock
>>>>the bus properly.
>>>
>>> Why is it now okay now to block in if_spi_host_to_card()? Was this never
>>> an issue?
>>
>>We don't really block, if_spi_host_to_card is called by the main
>>thread (synchronized by the dnld_sent flag, which gets cleared by the
>>IRQ handler) and it writes out the command or TX packet directly and
>>then returns.
> you get blocked in spu_write(). I though earlier that
> if_spi_host_to_card() is called by the core driver and we are atomic due
> to network core or the libertas driver itself.

That's right, but we can block that thread.

>>The issue is that you can't sleep in a real IRQ
>>handler, like the GPIO IRQ that we have in this driver.
> That is not 100% true as of current mainline, read below.
>
>>
>>> Would it help if you replace request_irq() with request_threaded_irq()?
>>> With that change it should similar to other interfaces since you can
>>> read from the chip within the interrupt handler.
>>
>>It's an actual IRQ handler that takes care of a GPIO interrupt.  This
>>is different from, say, SDIO, where you use a bus interrupt that the
>>subsystem provides you.  We can never sleep in that handler, hence the
>>additional thread.  Almost any implementation of the underlying SPI
>>controller will sleep at some point, so we can't check, for example,
>>the IRQ status register right in the handler.
>
> If your interrupt controller is correctly implemented then you can sleep
> in the interrupt handler if you use request_threaded_irq(). That one got
> merged in v2.6.30 and you have to provide two callback functions:
> - one short check function which finds out if your device really issued
>  an interrupt. This only matters on shared lines
> - the real interrupt handler which executed "later" as a thread.

Thanks Sebastian, I didn't know about this new API and I'll read up
about it, that sounds like what we want to use.

  -Andrey