The BCM2835 SPI driver currently sets the SPI_CONTROLLER_MUST_TX flag.
When performing an RX-only transfer, this flag causes the SPI core to
allocate and DMA-map a dummy buffer which is copied to the TX FIFO.
The dummy buffer is necessary because the chip is not capable of
automatically clocking out null bytes.
Avoid the overhead induced by the dummy buffer by preallocating a
reusable DMA transaction which fills the TX FIFO by cyclically copying
from the zero page. The transaction requires very little CPU time to
submit and generates no interrupts while running. Specifics are
provided in kerneldoc comments.
[Nathan Chancellor contributed a DMA mapping fixup for an early version
of this commit, hence his Signed-off-by.]
Tested-by: Nuno Sá <nuno.sa@analog.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
Acked-by: Martin Sperl <kernel@martin.sperl.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Link: https://lore.kernel.org/r/f45920af18dbf06e34129bbc406f53dc9c5d1075.1568187525.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
The BCM2835 SPI driver currently sets the SPI_CONTROLLER_MUST_RX flag.
When performing a TX-only transfer, this flag causes the SPI core to
allocate and DMA-map a dummy buffer into which the RX FIFO contents are
copied. The dummy buffer is necessary because the chip is not capable
of disabling the receiver or automatically throwing away received data.
Not reading the RX FIFO isn't an option either since transmission is
halted once it's full.
Avoid the overhead induced by the dummy buffer by preallocating a
reusable DMA transaction which cyclically clears the RX FIFO. The
transaction requires very little CPU time to submit and generates no
interrupts while running. Specifics are provided in kerneldoc comments.
With a ks8851 Ethernet chip attached to the SPI controller, I am seeing
a 30 us reduction in ping time with this commit (1.819 ms vs. 1.849 ms,
average of 100,000 packets) as well as a 2% reduction in CPU time
(75:08 vs. 76:39 for transmission of 5 GByte over the SPI bus).
The commit uses the TX DMA interrupt to signal completion of a transfer.
This interrupt is raised once all bytes have been written to the
TX FIFO and it is then necessary to busy-wait for the TX FIFO to become
empty before the transfer can be finalized. As an alternative approach,
I have explored using the SPI controller's DONE interrupt to detect
completion. This interrupt is signaled when the TX FIFO becomes empty,
avoiding the need to busy-wait. However latency deteriorates compared
to the present commit and surprisingly, CPU time is slightly higher as
well:
It turns out that in 45% of the cases, no busy-waiting is needed at all
and in 76% of the cases, less than 10 busy-wait iterations are
sufficient for the TX FIFO to drain. This was measured on an RT kernel.
On a vanilla kernel, wakeup latency is worse and thus fewer iterations
are needed. The measurements were made with an SPI clock of 20 MHz,
they may differ slightly for slower or faster clock speeds.
Previously we always used the RX DMA interrupt to signal completion of a
transfer. Using the TX DMA interrupt now introduces a race condition:
TX DMA is always started before RX DMA so that bytes are already clocked
out while RX DMA is still being set up. But if a TX-only transfer is
very short, then the TX DMA interrupt may occur before RX DMA is set up.
If the interrupt happens to occur on the same CPU, setup of RX DMA may
even be delayed until after the interrupt was handled.
I've solved this by having the TX DMA callback clear the RX FIFO while
busy-waiting for the TX FIFO to drain, thus avoiding a dependency on
setup of RX DMA. Additionally, I am using a lock-free mechanism with
two flags, tx_dma_active and rx_dma_active plus memory barriers to
terminate RX DMA either by the TX DMA callback or immediately after
setting it up, whichever wins the race. I've explored an alternative
approach which temporarily disables the TX DMA callback until RX DMA
has been set up (using tasklet_disable(), local_bh_disable() or
local_irq_save()), but the performance was minimally worse.
[Nathan Chancellor contributed a DMA mapping fixup for an early version
of this commit, hence his Signed-off-by.]
Tested-by: Nuno Sá <nuno.sa@analog.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
Acked-by: Martin Sperl <kernel@martin.sperl.org>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Link: https://lore.kernel.org/r/874949385f28251e2dcaa9494e39a27b50e9f9e4.1568187525.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
The BCM2835 SPI driver needs to set up the clock polarity in its
->prepare_message() hook before spi_transfer_one_message() asserts chip
select to avoid a gratuitous clock signal edge (cf. commit acace73df2
("spi: bcm2835: set up spi-mode before asserting cs-gpio")).
Precalculate the CS register value (which selects the clock polarity)
once in ->setup() and use that cached value in ->prepare_message() and
->transfer_one(). This avoids one MMIO read per message and one per
transfer, yielding a small latency improvement. Additionally, a
forthcoming commit will use the precalculated value to derive the
register value for clearing the RX FIFO, which will eliminate the need
for an RX dummy buffer when performing TX-only DMA transfers.
Tested-by: Nuno Sá <nuno.sa@analog.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
Acked-by: Martin Sperl <kernel@martin.sperl.org>
Link: https://lore.kernel.org/r/d17c1d7fcdc97fffa961b8737cfd80eeb14f9416.1568187525.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
The BCM2835 SPI driver uses a flag to keep track of whether a DMA
transfer is in progress.
The flag is used to avoid terminating DMA channels multiple times if a
transfer finishes orderly while simultaneously the SPI core invokes the
->handle_err() callback because the transfer took too long. However
terminating DMA channels multiple times is perfectly fine, so the flag
is unnecessary for this particular purpose.
The flag is also used to avoid invoking bcm2835_spi_undo_prologue()
multiple times under this race condition. However multiple *concurrent*
invocations can no longer happen since commit 2527704d84 ("spi:
bcm2835: Synchronize with callback on DMA termination") because the
->handle_err() callback now uses the _sync() variant when terminating
DMA channels.
The only raison d'être of the flag is therefore that
bcm2835_spi_undo_prologue() cannot cope with multiple *sequential*
invocations. Achieve that by setting tx_prologue to 0 at the end of
the function. Subsequent invocations thus become no-ops.
With that, the dma_pending flag becomes unnecessary, so drop it.
Tested-by: Nuno Sá <nuno.sa@analog.com>
Tested-by: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
Acked-by: Martin Sperl <kernel@martin.sperl.org>
Link: https://lore.kernel.org/r/062b03b7f86af77a13ce0ec3b22e0bdbfcfba10d.1568187525.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit 3bd7f6589f ("spi: bcm2835: Overcome sglist entry length
limitation") amended the BCM2835 SPI driver with support for DMA
transfers whose buffers are not aligned to 4 bytes and require more than
one sglist entry.
When testing this feature with upcoming commits to speed up TX-only and
RX-only transfers, I noticed that SPI transmission sometimes breaks.
A function introduced by the commit, bcm2835_spi_transfer_prologue(),
performs one or two PIO transmissions as a prologue to the actual DMA
transmission. It turns out that the breakage goes away if the DONE bit
in the CS register is set when ending such a PIO transmission.
The DONE bit signifies emptiness of the TX FIFO. According to the spec,
the bit is of type RO, so writing it should never have any effect.
Perhaps the spec is wrong and the bit is actually of type RW1C.
E.g. the I2C controller on the BCM2835 does have an RW1C DONE bit which
needs to be cleared by the driver. Another, possibly more likely
explanation is that it's a hardware erratum since the issue does not
occur consistently.
Either way, amend bcm2835_spi_transfer_prologue() to always write the
DONE bit.
Usually a transmission is ended by bcm2835_spi_reset_hw(). If the
transmission was successful, the TX FIFO is empty and thus the DONE bit
is set when bcm2835_spi_reset_hw() reads the CS register. The bit is
then written back to the register, so we happen to do the right thing.
However if DONE is not set, e.g. because transmission is aborted with
a non-empty TX FIFO, the bit won't be written by bcm2835_spi_reset_hw()
and it seems possible that transmission might subsequently break. To be
on the safe side, likewise amend bcm2835_spi_reset_hw() to always write
the bit.
Tested-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
Acked-by: Martin Sperl <kernel@martin.sperl.org>
Link: https://lore.kernel.org/r/edb004dff4af6106f6bfcb89e1a96391e96eb857.1564825752.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Link: https://lore.kernel.org/r/20190904135918.25352-7-yuehaibing@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This converts the BCM2835 SPI master driver to use GPIO
descriptors for chip select handling.
The BCM2835 driver was relying on the core to drive the
CS high/low so very small changes were needed for this
part. If it managed to request the CS from the device tree
node, all is pretty straight forward.
However for native GPIOs this driver has a quite unorthodox
loopback to request some GPIOs from the SoC GPIO chip by
looking it up from the device tree using gpiochip_find()
and then offseting hard into its numberspace. This has
been augmented a bit by using gpiochip_request_own_desc()
but this code really needs to be verified. If "native CS"
is actually an SoC GPIO, why is it even done this way?
Should this GPIO not just be defined in the device tree
like any other CS GPIO? I'm confused.
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Chris Boot <bootc@bootc.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20190804003852.1312-1-linus.walleij@linaro.org
Signed-off-by: Mark Brown <broonie@kernel.org>
We don't need dev_err() messages when platform_get_irq() fails now that
platform_get_irq() prints an error message itself when something goes
wrong. Let's remove these prints with a simple semantic patch.
// <smpl>
@@
expression ret;
struct platform_device *E;
@@
ret =
(
platform_get_irq(E, ...)
|
platform_get_irq_byname(E, ...)
);
if ( \( ret < 0 \| ret <= 0 \) )
{
(
-if (ret != -EPROBE_DEFER)
-{ ...
-dev_err(...);
-... }
|
...
-dev_err(...);
)
...
}
// </smpl>
While we're here, remove braces on if statements that only have one
statement (manually).
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-spi@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20190730181557.90391-42-swboyd@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit 6935224da2 ("spi: bcm2835: enable support of 3-wire mode")
added 3-wire support to the BCM2835 SPI driver by setting the REN bit
(Read Enable) in the CS register when receiving data. The REN bit puts
the transmitter in high-impedance state. The driver recognizes that
data is to be received by checking whether the rx_buf of a transfer is
non-NULL.
Commit 3ecd37edaa ("spi: bcm2835: enable dma modes for transfers
meeting certain conditions") subsequently broke 3-wire support because
it set the SPI_MASTER_MUST_RX flag which causes spi_map_msg() to replace
rx_buf with a dummy buffer if it is NULL. As a result, rx_buf is
*always* non-NULL if DMA is enabled.
Reinstate 3-wire support by not only checking whether rx_buf is non-NULL,
but also checking that it is not the dummy buffer.
Fixes: 3ecd37edaa ("spi: bcm2835: enable dma modes for transfers meeting certain conditions")
Reported-by: Nuno Sá <nuno.sa@analog.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v4.2+
Cc: Martin Sperl <kernel@martin.sperl.org>
Acked-by: Stefan Wahren <wahrenst@gmx.net>
Link: https://lore.kernel.org/r/328318841455e505370ef8ecad97b646c033dc8a.1562148527.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Based on 3 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version [author] [kishon] [vijay] [abraham]
[i] [kishon]@[ti] [com] this program is distributed in the hope that
it will be useful but without any warranty without even the implied
warranty of merchantability or fitness for a particular purpose see
the gnu general public license for more details
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version [author] [graeme] [gregory]
[gg]@[slimlogic] [co] [uk] [author] [kishon] [vijay] [abraham] [i]
[kishon]@[ti] [com] [based] [on] [twl6030]_[usb] [c] [author] [hema]
[hk] [hemahk]@[ti] [com] this program is distributed in the hope
that it will be useful but without any warranty without even the
implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 1105 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Richard Fontana <rfontana@redhat.com>
Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070033.202006027@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The BCM2835 SPI driver still sets the "direction" member in struct
dma_slave_config even though it was deprecated five years ago with
commit d9ff958bb3 ("dmaengine: Mark the struct dma_slave_config
direction field deprecated") and is no longer evaluated by the BCM2835
DMA driver since commit 00648f4d0f ("dmaengine: bcm2835: remove
dma_slave_config direction usage").
Drop the superfluous assignment.
No functional change intended.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit 8caab75fd2 ('spi: Generalize SPI "master" to "controller"')
changed the "spi_master" nomenclature to "spi_controller", necessitating
a conversion of all drivers.
Perform this conversion for the BCM2835 SPI driver.
No functional change intended.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
There is no use for this when performing non DMA operations. So we
bypass the split.
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
Printing an error on memory allocation failure is unnecessary,
as the memory allocation core code already takes care of that.
Signed-off-by: Hoan Nguyen An <na-hoan@jinso.co.jp>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
To estimate efficiency add statistics on transfer types
(polling, interrupt and dma) used to debugfs.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Changelog:
V1 -> V2: applied feedback by Stefan Wahren
reorganized patchset
added extra rational, descriptions
fixed compile issue when CONFIG_DEBUG_FS is unset
Signed-off-by: Mark Brown <broonie@kernel.org>
Under some circumstances the default 30 us polling limit is not optimal
and may lead to long delays because we are waiting on an interrupt.
with this patch we have the possibility to influence this policy.
So make this limit (in us) configurable via a module parameters
(but also modifyable via /sys/modules/...)
This replicates similar code found in spi-bcm2835aux.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Changelog:
V1 -> V2: applied feedback by Stefan Wahren
reorganized patchset
added extra rational, descriptions
Signed-off-by: Mark Brown <broonie@kernel.org>
The DMA mode behaves slightly different than polling or interrupt driven
mode, so just document the fact
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Changelog:
V1 -> V2: applied feedback by Stefan Wahren
new in V2
Signed-off-by: Mark Brown <broonie@kernel.org>
Avoid 64 bit aritmetics when deciding if we need to use polling or not
This replicates: commit d704afffe6
("spi: bcm2835aux: Avoid 64-bit arithmetic in xfer len calc")
from spi-bcm2835aux
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Changelog:
V1 -> V2: applied feedback by Stefan Wahren
reorganized patchset
added extra rational, descriptions
Signed-off-by: Mark Brown <broonie@kernel.org>
Remove the unnecessary argument of xfer_time_us when calling
bcm2835_spi_transfer_one_poll.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Changelog:
V1 -> V2: applied feedback by Stefan Wahren
reorganized patchset
added extra rational, descriptions
Signed-off-by: Mark Brown <broonie@kernel.org>
Split spi transfers into chunks of <=65532 to enable the driver to
perform DMA transfer on big buffers. The DLEN register specifies the
number of bytes to transfer in DMA mode. It is 16-bit wide and thus the
maximum DMA transfer is 65535 bytes. Set the maximum to 65532 (4 byte
aligned) since the FIFO in DMA mode is accessed in 4 byte chunks.
->max_dma_len is the value the spi core uses when splitting the buffer
into scatter gather entries.
The BCM2835 DMA block has 2 types of channels/engines:
- Normal: Max length: 1G
- Lite: Max length: 65535
Even when using a Lite channel we will not exceed the max length, so
let's drop setting ->max_dma_len.
Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
Reviewed-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit b36f09c3c4 ("dmaengine: Add transfer termination
synchronization support") deprecated dmaengine_terminate_all() in favor
of dmaengine_terminate_sync() and dmaengine_terminate_async() to avoid
freeing resources used by the DMA callback before its execution has
concluded.
Commit de92436ac4 ("dmaengine: bcm2835-dma: Use vchan_terminate_vdesc()
instead of desc_free") amended the BCM2835 DMA driver with an
implementation of ->device_synchronize(), which is a prerequisite for
dmaengine_terminate_sync(). Thus, clients of the DMA driver (such as
the BCM2835 SPI driver) may now be converted to the new API.
It is generally desirable to use the _sync() variant except in atomic
context. There is only a single occurrence where the BCM2835 SPI driver
calls dmaengine_terminate_all() in atomic context and that is in
bcm2835_spi_dma_done() (the RX DMA channel's callback) to terminate the
TX DMA channel. The TX DMA channel doesn't have a callback (yet), hence
it is safe to use the _async() variant there.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
The RX and TX FIFO of the BCM2835 SPI master each accommodate 64 bytes
(16 32-bit dwords). The CS register provides hints on their fill level:
"Bit 19 RXR - RX FIFO needs Reading ([¾] full)
0 = RX FIFO is less than [¾] full (or not active TA = 0).
1 = RX FIFO is [¾] or more full. Cleared by reading sufficient
data from the RX FIFO or setting TA to 0."
"Bit 16 DONE - Transfer Done
0 = Transfer is in progress (or not active TA = 0).
1 = Transfer is complete. Cleared by writing more data to the
TX FIFO or setting TA to 0."
"If DONE is set [...], write up to 16 [dwords] to SPI_FIFO. [...]
If RXR is set read 12 [dwords] data from SPI_FIFO."
[Source: Pages 153, 154 and 158 of
https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
Note: The spec is missing the "¾" character, presumably due to
copy-pasting from a different charset. It also incorrectly
refers to 16 and 12 "bytes" instead of 32-bit dwords.]
In short, the RXR bit indicates that 48 bytes can be read and the DONE
bit indicates 64 bytes can be written. Leverage this knowledge to read
or write bytes blindly to the FIFO, without polling whether data can be
read or free space is available to write. Moreover, when a transfer is
starting, the TX FIFO is known to be empty, likewise allowing a blind
write of 64 bytes.
This cuts the number of bus accesses in half if the fill level is known.
Also, the (posted) write accesses can be pipelined on the AXI bus since
they are no longer interleaved with (non-posted) reads.
bcm2835_spi_transfer_one_poll() switches to interrupt mode when a time
limit is exceeded by calling bcm2835_spi_transfer_one_irq(). The TX
FIFO may contain data in this case, but is known to be empty when the
function is called from bcm2835_spi_transfer_one(). Hence only blindly
fill the TX FIFO in the latter case but not the former.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Tested-by: Eric Anholt <eric@anholt.net>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit 3bd7f6589f ("spi: bcm2835: Overcome sglist entry length
limitation") was unfortunately merged even though submission of a
refined version was imminent. Apply those refinements as an amendment:
* Drop no longer needed #include <asm/page.h>. The lines requiring
its inclusion were removed by the commit.
* Change type of tx_spillover flag from bool to unsigned int for
consistency with dma_pending flag and pursuant to Linus' dictum:
https://lkml.org/lkml/2017/11/21/384
* In bcm2835_rd_fifo_count() do not check for bs->rx_buf != NULL.
The function will never be called if that's the case.
* Amend kerneldoc of bcm2835_wait_tx_fifo_empty() to prevent its use in
situations where the function might spin forever. (In response to a
review comment by Stefan Wahren.)
* Sync only the cacheline containing the RX prologue back to memory,
not the full first sglist entry.
* Use sg_dma_address() and sg_dma_len() instead of referencing the
sglist entry members directly. Seems to be the more common syntax in
the tree, even for lvalues.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit e82b0b3828 ("spi: bcm2835: Fix race on DMA termination") broke
the build with COMPILE_TEST=y on arches whose cmpxchg() requires 32-bit
operands (xtensa, older arm ISAs).
Fix by changing the dma_pending flag's type from bool to unsigned int.
Fixes: e82b0b3828 ("spi: bcm2835: Fix race on DMA termination")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
When in DMA mode, the BCM2835 SPI controller requires that the FIFO is
accessed in 4 byte chunks. This rule is not fulfilled if a transfer
consists of multiple sglist entries, one per page, and the first entry
starts in the middle of a page with an offset not a multiple of 4.
The driver currently falls back to programmed I/O for such transfers,
incurring a significant performance penalty.
Overcome this hardware limitation by transferring the first few bytes of
a transfer without DMA such that the remainder of the first sglist entry
becomes a multiple of 4. Specifics are provided in kerneldoc comments.
An alternative approach would have been to split transfers in the
->prepare_message hook, but this may necessitate two transfers per page,
defeating the goal of clustering multiple pages together in a single
transfer for efficiency. E.g. if the first TX sglist entry's length is
23 and the first RX's is 40, the first transfer would send and receive
23 bytes, the second 40 - 23 = 17 bytes, the third 4096 - 17 = 4079
bytes, the fourth 4096 - 4079 = 17 bytes and so on. In other words,
O(n) transfers are necessary (n = number of sglist entries), whereas
the algorithm implemented herein only requires O(1) additional work.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Document the driver's data structure to lower the barrier to entry for
contributors.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit a30a555d74 ("spi: bcm2835: transform native-cs to gpio-cs on
first spi_setup") disabled the use of hardware-controlled native Chip
Select in favour of software-controlled GPIO Chip Select but left code
to support the former untouched. Remove it to simplify the driver and
ease the addition of new features and further optimizations.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
If a DMA transfer finishes orderly right when spi_transfer_one_message()
determines that it has timed out, the callbacks bcm2835_spi_dma_done()
and bcm2835_spi_handle_err() race to call dmaengine_terminate_all(),
potentially leading to double termination.
Prevent by atomically changing the dma_pending flag before calling
dmaengine_terminate_all().
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Fixes: 3ecd37edaa ("spi: bcm2835: enable dma modes for transfers meeting certain conditions")
Cc: stable@vger.kernel.org # v4.2+
Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
If submission of a DMA TX transfer succeeds but submission of the
corresponding RX transfer does not, the BCM2835 SPI driver terminates
the TX transfer but neglects to reset the dma_pending flag to false.
Thus, if the next transfer uses interrupt mode (because it is shorter
than BCM2835_SPI_DMA_MIN_LENGTH) and runs into a timeout,
dmaengine_terminate_all() will be called both for TX (once more) and
for RX (which was never started in the first place). Fix it.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Fixes: 3ecd37edaa ("spi: bcm2835: enable dma modes for transfers meeting certain conditions")
Cc: stable@vger.kernel.org # v4.2+
Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
The IRQ handler bcm2835_spi_interrupt() first reads as much as possible
from the RX FIFO, then writes as much as possible to the TX FIFO.
Afterwards it decides whether the transfer is finished by checking if
the TX FIFO is empty.
If very few bytes were written to the TX FIFO, they may already have
been transmitted by the time the FIFO's emptiness is checked. As a
result, the transfer will be declared finished and the chip will be
reset without reading the corresponding received bytes from the RX FIFO.
The odds of this happening increase with a high clock frequency (such
that the TX FIFO drains quickly) and either passing "threadirqs" on the
command line or enabling CONFIG_PREEMPT_RT_BASE (such that the IRQ
handler may be preempted between filling the TX FIFO and checking its
emptiness).
Fix by instead checking whether rx_len has reached zero, which means
that the transfer has been received in full. This is also more
efficient as it avoids one bus read access per interrupt. Note that
bcm2835_spi_transfer_one_poll() likewise uses rx_len to determine
whether the transfer has finished.
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Fixes: e34ff011c7 ("spi: bcm2835: move to the transfer_one driver model")
Cc: stable@vger.kernel.org # v4.1+
Cc: Mathias Duckeck <m.duckeck@kunbus.de>
Cc: Frank Pavlic <f.pavlic@kunbus.de>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Noralf Trønnes <noralf@tronnes.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
The license text is specifying GPL v2 or later but the MODULE_LICENSE
is set to GPL v2 which means GNU Public License v2 only. So choose the
license text as the correct one.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Acked-by: Florian Kauer <florian.kauer@koalo.de>
Acked-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
This should be fixed by commit 4c02cba18c
("pinctrl: bcm2835: Fix initial value for direction_output")
Signed-off-by: Axel Lin <axel.lin@ingics.com>
Acked-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Change the initialization order of the HW so that the interrupt
is only requested after the HW is initialized
Also the use of irq_of_parse_and_map is replaced by platform_get_irq.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
There is a bug in the alignment checking of transfers,
that results in DMA not being used for un-aligned
transfers that do not cross page-boundries, which is valid.
This is due to a missconception of the meaning PAGE_MASK
when implementing that check originally - (PAGE_SIZE - 1)
should have been used instead.
Also fixes a copy/paste error.
Reported-by: <robert@axium.co.nz>
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
This resulted in the use of polling mode when other approaches
(dma or interrupts) would have been more appropriate.
Happened for transfers longer than 477 bytes.
Reported-by: Noralf Tronnes <noralf@tronnes.org>
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
When using reverse polarity for clock (spi-cpol) on a device
the clock line gets altered after chip-select has been asserted
resulting in an additional clock beat, which confuses hardware.
This did not show when using native-CS, as the same register
is used to control cs as well as polarity, so the changes came
into effect at the same time. Unfortunately this is not true
with gpio-cs.
To avoid this situation this patch moves the setup of polarity
(spi-cpol and spi-cpha) outside of the chip-select into
prepare_message, which is run prior to asserting chip-select.
Also fixes resetting 3-wire mode after use of rx-mode, so that
a 3-Wire sequence TX, RX, TX works as well (right now it runs
TX, RX, RX instead)
Reported-by: Noralf Tronnes <noralf@tronnes.org>
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
fixes several warnings/error emmitted by the kbuild system:
* warn: cast from pointer to integer of different size
using size_t instead of u32
* error: 'SZ_4K' undeclared
moved to PAGE_SIZE and PAGE_MASK instead
Review showed also a typo in the same code where tx_buff
was checked twice instead of checking both rx and tx_buff.
Reported by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Conditions per spi_transfer are:
* transfer.len >= 96 bytes (to avoid mapping overhead costs)
* transfer.len < 65536 bytes (limitaion by spi-hw block - could get extended)
* an individual scatter/gather transfer length must be a multiple of 4
for anything but the last transfer - spi-hw block limit.
(some shortcut has been taken in can_dma to avoid unnecessary mapping of
pages which, for which there is a chance that there is a split with a
transfer length not a multiple of 4)
If it becomes a necessity these restrictions can get removed by additional
code.
Note that this patch requires a patch to dma-bcm2835.c by Noralf to
enable scatter-gather mode inside the dmaengine, which has not been
merged yet.
That is why no patch to arch/arm/boot/dts/bcm2835.dtsi is included - the
code works as before without dma when tx/rx are not set, but it writes
a message warning about dma not used:
spi-bcm2835 20204000.spi: no tx-dma configuration found - not using dma mode
To enable dma-mode add the following lines to the device-tree:
dmas = <&dma 6>, <&dma 7>;
dma-names = "tx", "rx";
Tested-by: Noralf Trønnes <noralf@tronnes.org> (private communication)
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
The polling mode of the driver is designed for transfers that run
less than 30us - it will only execute under those circumstances.
So it should run comfortably without getting interrupted by the
scheduler.
But there are situations where the raspberry pi is so overloaded
that it can take up to 80 jiffies until the polling thread gets
rescheduled - this has been observed especially under heavy
IO situations.
In such a situation we now fall back to the interrupt handler and
log the situation at debug level.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
The way that the timeout code is written in the polling function
the timeout does also trigger when interrupted or rescheduled while
in the polling loop.
This patch changes the timeout from effectively 20ms (=2 jiffies) to
1 second and removes the time that the transfer really takes out of
the computation, as - per design - this is <30us and the jiffie resolution
is 10ms so that does not make any difference what so ever.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
In cases of short transfer times the CPU is spending lots of time
in the interrupt handler and scheduler to reschedule the worker thread.
Measurements show that we have times where it takes 29.32us to between
the last clock change and the time that the worker-thread is running again
returning from wait_for_completion_timeout().
During this time the interrupt-handler is running calling complete()
and then also the scheduler is rescheduling the worker thread.
This time can vary depending on how much of the code is still in
CPU-caches, when there is a burst of spi transfers the subsequent delays
are in the order of 25us, so the value of 30us seems reasonable.
With polling the whole transfer of 4 bytes at 10MHz finishes after 6.16us
(CS down to up) with the real transfer (clock running) taking 3.56us.
So the efficiency has much improved and is also freeing CPU cycles,
reducing interrupts and context switches.
Because of the above 30us seems to be a reasonable limit for polling.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
Transforms the bcm-2835 native SPI-chip select to their gpio-cs equivalent.
This allows for some support of some optimizations that are not
possible due to HW-gliches on the CS line - especially filling
the FIFO before enabling SPI interrupts (by writing to CS register)
while the transfer is already in progress (See commit: e3a2be3030)
This patch also works arround some issues in bcm2835-pinctrl which does not
set the value when setting the GPIO as output - it just sets up output and
(typically) leaves the GPIO as low. When a fix for this is merged then this
gpio_set_value can get removed from bcm2835_spi_setup.
Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
