[RFC] libertas: remove TX queues from GSPI driver

Sebastian Andrzej Siewior sebastian at breakpoint.cc
Mon Jun 22 11:29:51 EDT 2009


* 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.

>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.

>  -Andrey

Sebastian



More information about the libertas-dev mailing list