Re: [PATCH 1/3] spi: spi-xilinx: Remove ISR race condition

From: Shubhrajyoti Datta
Date: Mon Oct 12 2015 - 08:21:12 EST


On Thu, Oct 8, 2015 at 8:40 PM, Jean-Francois Dagenais
<jeff.dagenais@xxxxxxxxx> wrote:
>
>> On Jun 4, 2013, at 1:32 PM, Mark Brown <broonie@xxxxxxxxxx> wrote:
>>
>> Applied but this is a bit sad, having to defer the refill to process
>> context means that we're adding extra latency which takes us further
>> away from being able to saturate the bus. There ought to be a way to
>> avoid the issue though I can't think of a non-racy one - I guess level
>> triggered interrupts aren't an option?
>
> Hi all,
>
> We've been using an improved version of this driver in production for years here
> in the 3.4 tree. I had to intervene in order to improve performance. I managed
> to double it (not bad).
Thats cool.



>
> Like the other threads I've seen about this, my strategy involved limiting the
> reads to the registers, which, through a pci-e link were kind of long. Here's
> the yet un-submitted commit header I carry in my clone:

Why not send the patches.

>
> =============
> spi: xilinx - minimize iomem reads
>
> If this IP core is accessed through bridges like PCI-e, reads are rather
> costly. Doing many reads or read-modify-writes is thus long and strenuous
> on the CPU (active waiting).
>
> The transfer workflow of this driver allows some assumptions to be made and
> exploited to minimize reads as much as possible.
>
> These two assumptions are made:
> - since we are in control of the CR register, cache it so we don't have to
> read it all the time to modify it.

Makes sense.

> - FIFO (either depth of 1 or 16) are always maxed out, unless the remaining
> bytes are less than the FIFO depth. For this reason, we can predict the
> state of the FIFOs without checking with the status register to check if
> they are empty or full.

Can you explain more details.
>
> Reading 32Mb flash at 32MHz on a core with 8bits/word and FIFO enabled (16),
> has dropped from ~60s to ~30s. Still high, but better. Most of the delay still
> comes from emptying the RX FIFO one "word" at a time.
> ===============
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/