Commit Graph

93 Commits

Author SHA1 Message Date
Lukas Wunner e1483ac030
spi: bcm2835: Fix use-after-free on unbind
bcm2835_spi_remove() accesses the driver's private data after calling
spi_unregister_controller() even though that function releases the last
reference on the spi_controller and thereby frees the private data.

Fix by switching over to the new devm_spi_alloc_master() helper which
keeps the private data accessible until the driver has unbound.

Fixes: f8043872e7 ("spi: add driver for BCM2835")
Reported-by: Sascha Hauer <s.hauer@pengutronix.de>
Reported-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v3.10+: 123456789abc: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v3.10+
Cc: Vladimir Oltean <olteanv@gmail.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/ad66e0a0ad96feb848814842ecf5b6a4539ef35c.1605121038.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-11-12 15:05:35 +00:00
Martin Hundebøll bc7f2cd755
spi: bcm2835: remove use of uninitialized gpio flags variable
Removing the duplicate gpio chip select level handling in
bcm2835_spi_setup() left the lflags variable uninitialized. Avoid trhe
use of such variable by passing default flags to
gpiochip_request_own_desc().

Fixes: 5e31ba0c05 ("spi: bcm2835: fix gpio cs level inversion")
Signed-off-by: Martin Hundebøll <martin@geanix.com>
Link: https://lore.kernel.org/r/20201105090615.620315-1-martin@geanix.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-11-06 11:23:26 +00:00
Martin Hundebøll 5e31ba0c05
spi: bcm2835: fix gpio cs level inversion
The work on improving gpio chip-select in spi core, and the following
fixes, has caused the bcm2835 spi driver to use wrong levels. Fix this
by simply removing level handling in the bcm2835 driver, and let the
core do its work.

Fixes: 3e5ec1db8b ("spi: Fix SPI_CS_HIGH setting when using native and GPIO CS")
Cc: <stable@vger.kernel.org>
Signed-off-by: Martin Hundebøll <martin@geanix.com>
Link: https://lore.kernel.org/r/20201014090230.2706810-1-martin@geanix.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-10-29 12:41:30 +00:00
Jason Yan cbd632ea8e
spi: bcm2835: Make polling_limit_us static
This eliminates the following sparse warning:

drivers/spi/spi-bcm2835.c:78:14: warning: symbol 'polling_limit_us' was
not declared. Should it be static?

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Jason Yan <yanaijie@huawei.com>
Link: https://lore.kernel.org/r/20200912072211.602735-1-yanaijie@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-09-14 15:50:15 +01:00
Krzysztof Kozlowski 65acd82c4e
spi: bcm2835: Simplify with dev_err_probe()
Common pattern of handling deferred probe can be simplified with
dev_err_probe().  Less code and the error value gets printed.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/20200901152713.18629-4-krzk@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-09-08 18:19:25 +01:00
Martin Sperl 9df2003df7
spi: bcm2835: support effective_speed_hz
Setting spi_transfer->effective_speed_hz in transfer_one so that
it can get used in cs_change_delay configured with delay as a muliple
of SPI clock cycles.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Link: https://lore.kernel.org/r/20200709074120.110069-2-mkl@pengutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-07-17 00:30:44 +01:00
Robin Murphy 26751de25d
spi: bcm2835: Micro-optimise FIFO loops
The blind and counted loops are always called with nonzero count, so
convert them to do-while loops that lead to slightly more efficient
code generation. With GCC 8.3 this shaves off 1-2 instructions per
iteration in each case.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Link: https://lore.kernel.org/r/9242863077acf9a64e4b3720e479855b88d19e82.1592261248.git.robin.murphy@arm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-07-01 22:19:31 +01:00
Robin Murphy afe7e36360
spi: bcm2835: Micro-optimise IRQ handler
The IRQ handler only needs the struct spi_controller for the sake of
the completion at the end of a transfer. Passing the struct bcm2835_spi
directly as the IRQ data allows that level of indirection to be pushed
into the completion path for the reverse lookup, and avoided entirely
in all other cases.

This saves one explicit load in the critical path, plus (for a GCC 8.3
build) two registers worth of stack frame overhead.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Link: https://lore.kernel.org/r/6b401cb521539caffab21f05b4c8cba6c9d27c6e.1592261248.git.robin.murphy@arm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-07-01 22:19:30 +01:00
Robin Murphy ac4648b5d8
spi: bcm3835: Tidy up bcm2835_spi_reset_hw()
It doesn't need a struct spi_controller, and every callsite has
already retrieved the appropriate struct bcm2835_spi, so just pass
that directly.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Link: https://lore.kernel.org/r/eca458ae1a0d3934d0627f90e25d294fefd4b13d.1592261248.git.robin.murphy@arm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-07-01 22:19:29 +01:00
Mark Brown d62069c22e
spi: bcm2835: Remove shared interrupt support
This reverts commit ecfbd3cf3b since Lukas Wunner noticed that we
start operating on the hardware before we check to see if this is a
spurious interrupt.

Reported-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-05-29 18:48:46 +01:00
Martin Sperl ecfbd3cf3b
spi: bcm2835: Enable shared interrupt support
bcm2711, Rasberry Pi 4's SoC, shares one interrupt for multiple
instances of the bcm2835 SPI controller. So this enables shared
interrupt support for them.

The early bail out in the interrupt routine avoids messing with buffers
of transfers being done by other means. Otherwise, the driver can handle
receiving interrupts asserted by other controllers during an IRQ based
transfer.

Signed-off-by: Martin Sperl <kernel@martin.sperl.org>
Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/20200528185805.28991-1-nsaenzjulienne@suse.de
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-05-29 17:49:54 +01:00
Florian Fainelli 118eb0e52e
spi: bcm2835: Implement shutdown callback
Make sure we clear the FIFOs, stop the block, disable the clock and
release the DMA channel.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/20200528190605.24850-1-f.fainelli@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-05-29 17:49:53 +01:00
Lukas Wunner 05897c710e
spi: bcm2835: Tear down DMA before turning off SPI controller
On unbind of the BCM2835 SPI driver, the SPI controller is disabled
first and the DMA channels are terminated and torn down afterwards.

This seems backwards:  In the theoretical case that DMA is active,
it might try to fill the SPI FIFOs even after the controller has
been disabled.

Reverse the order, thereby mirroring what's done on ->probe().

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Link: https://lore.kernel.org/r/ac79f1e3d6fd9a1f5e0cb4008c43b98ea70be3c2.1589557526.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-05-20 18:16:41 +01:00
Mark Brown c38a4905ca
Merge branch 'for-5.7' of https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi into spi-5.8 2020-05-20 17:55:07 +01:00
Lukas Wunner 9dd277ff92
spi: bcm2835: Fix controller unregister order
The BCM2835 SPI driver uses devm_spi_register_controller() on bind.
As a consequence, on unbind, __device_release_driver() first invokes
bcm2835_spi_remove() before unregistering the SPI controller via
devres_release_all().

This order is incorrect:  bcm2835_spi_remove() tears down the DMA
channels and turns off the SPI controller, including its interrupts
and clock.  The SPI controller is thus no longer usable.

When the SPI controller is subsequently unregistered, it unbinds all
its slave devices.  If their drivers need to access the SPI bus,
e.g. to quiesce their interrupts, unbinding will fail.

As a rule, devm_spi_register_controller() must not be used if the
->remove() hook performs teardown steps which shall be performed
after unbinding of slaves.

Fix by using the non-devm variant spi_register_controller().  Note that
the struct spi_controller as well as the driver-private data are not
freed until after bcm2835_spi_remove() has finished, so accessing them
is safe.

Fixes: 247263dba2 ("spi: bcm2835: use devm_spi_register_master()")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v3.13+
Link: https://lore.kernel.org/r/2397dd70cdbe95e0bc4da2b9fca0f31cb94e5aed.1589557526.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-05-20 15:16:11 +01:00
Wei Yongjun dd4441ab1f
spi: bcm2835: Fix error return code in bcm2835_dma_init()
Fix to return negative error code -ENOMEM from the dma mapping error
handling case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Link: https://lore.kernel.org/r/20200506125607.90952-1-weiyongjun1@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-05-06 15:55:14 +01:00
Jacko Dirks e37687c98a
spi: bcm2835: Fixes bare use of unsigned
Signed-off-by: Jacko Dirks <jdirks.linuxdev@gmail.com>
Link: https://lore.kernel.org/r/20200503200033.GA3256@vasteMachine
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-05-05 13:50:22 +01:00
Lukas Wunner 603e92ff10
spi: bcm2835: Raise maximum number of slaves to 4
The "RevPi Connect Flat" PLC offered by KUNBUS has 4 slaves attached
to the BCM2835 SPI master.  Raise the maximum number of slaves in the
driver accordingly.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Link: https://lore.kernel.org/r/01453fd062de2d49bd74a847e13a0781cbf8143d.1578572268.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
2020-01-09 21:25:44 +00:00
Jim Quinlan f4dc4abdce
spi: bcm2835: no dev_err() on clk_get() -EPROBE_DEFER
Use dev_dbg() on -EPROBE_DEFER and dev_err() on all
other errors.

Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Link: https://lore.kernel.org/r/20191216230802.45715-2-jquinlan@broadcom.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2019-12-17 12:37:34 +00:00
Peter Ujfalusi 6133fed053
spi: bcm2835: Use dma_request_chan() instead dma_request_slave_channel()
dma_request_slave_channel() is a wrapper on top of dma_request_chan()
eating up the error code.

By using dma_request_chan() directly the driver can support deferred
probing against DMA.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Link: https://lore.kernel.org/r/20191212135550.4634-4-peter.ujfalusi@ti.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2019-12-16 11:56:46 +00:00
Peter Ujfalusi 666224b43b
spi: bcm2835: Release the DMA channel if probe fails after dma_init
The DMA channel was not released if either devm_request_irq() or
devm_spi_register_controller() failed.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Link: https://lore.kernel.org/r/20191212135550.4634-3-peter.ujfalusi@ti.com
Signed-off-by: Mark Brown <broonie@kernel.org>
2019-12-16 11:56:27 +00:00
Chris Packham c2f102f1e8
spi: bcm2835: fix typo in comment
GPIOS_OUT_LOW should be GPIOD_OUT_LOW.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Link: https://lore.kernel.org/r/20191105214134.25142-1-chris.packham@alliedtelesis.co.nz
Signed-off-by: Mark Brown <broonie@kernel.org>
2019-11-05 23:48:39 +00:00
Mark Brown b769c5ba8a
Merge branch 'spi-5.4' into spi-next 2019-09-15 10:32:06 +01:00
Lukas Wunner 2b8279aec1
spi: bcm2835: Speed up RX-only DMA transfers by zero-filling TX FIFO
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>
2019-09-11 15:57:46 +01:00
Lukas Wunner 8259bf667a
spi: bcm2835: Speed up TX-only DMA transfers by clearing RX FIFO
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>
2019-09-11 15:57:30 +01:00
Lukas Wunner 571e31fa60
spi: bcm2835: Cache CS register value for ->prepare_message()
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>
2019-09-11 15:56:30 +01:00
Lukas Wunner 1513ceee70
spi: bcm2835: Drop dma_pending flag
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>
2019-09-11 15:52:33 +01:00
Lukas Wunner 4c524191c0
spi: bcm2835: Work around DONE bit erratum
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>
2019-09-10 11:28:44 +01:00
YueHaibing 6ba794dfbb
spi: bcm2835: use devm_platform_ioremap_resource() to simplify code
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>
2019-09-04 17:16:28 +01:00
Linus Walleij 3bd158c56a
spi: bcm2835: Convert to use CS GPIO descriptors
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>
2019-08-28 14:11:01 +01:00
Mark Brown 795227660d
Merge branch 'spi-5.3' into spi-5.4 2019-08-23 12:00:22 +01:00
Stephen Boyd 6b8ac10e0d
spi: Remove dev_err() usage after platform_get_irq()
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>
2019-08-02 12:15:43 +01:00
Lukas Wunner 8d8bef5036
spi: bcm2835: Fix 3-wire mode if DMA is enabled
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>
2019-07-24 19:44:59 +01:00
Mark Brown 4343f61103 Linux 5.2-rc4
-----BEGIN PGP SIGNATURE-----
 
 iQFSBAABCAA8FiEEq68RxlopcLEwq+PEeb4+QwBBGIYFAlz8fAYeHHRvcnZhbGRz
 QGxpbnV4LWZvdW5kYXRpb24ub3JnAAoJEHm+PkMAQRiG1asH/3ySguxqtqL1MCBa
 4/SZ37PHeWKMerfX6ZyJdgEqK3B+PWlmuLiOMNK5h2bPLzeQQQAmHU/mfKmpXqgB
 dHwUbG9yNnyUtTfsfRqAnCA6vpuw9Yb1oIzTCVQrgJLSWD0j7scBBvmzYqguOkto
 ThwigLUq3AILr8EfR4rh+GM+5Dn9OTEFAxwil9fPHQo7QoczwZxpURhScT6Co9TB
 DqLA3fvXbBvLs/CZy/S5vKM9hKzC+p39ApFTURvFPrelUVnythAM0dPDJg3pIn5u
 g+/+gDxDFa+7ANxvxO2ng1sJPDqJMeY/xmjJYlYyLpA33B7zLNk2vDHhAP06VTtr
 XCMhQ9s=
 =cb80
 -----END PGP SIGNATURE-----
gpgsig -----BEGIN PGP SIGNATURE-----
 
 iQFHBAABCgAxFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAlz+mPkTHGJyb29uaWVA
 a2VybmVsLm9yZwAKCRAk1otyXVSH0BXUB/95xjqF6hzkrX9vO1ewx58dWoq9g5HL
 ovD61aVBgyaZT2LnUt+iVLag6tdKg6pOVAoJnpEeCbDcMJ3cDawoIXaXnWdO7Npc
 shw75MqHQCyghc5oOuSBc91d/4DKVpy0dSkhTBUmz5Vu7cBnN2imFLwrgViJT2C7
 UsRyAX6AaUV+V52lp/EAklb1SMcgYl3Yn6vA9tHKwsCpSWsCMZz/uxI/TkslnDEx
 VIH6ZUxnZEcyXaGnpKea0TOWMyG/q7gRTPS2NCrapqt1fNdx7D5X7Q4WZWKKVrdn
 6ADQ9YPLSvORTXqDU3CWqHQeJbn/0KUEjOx4xMH8JR4ePc5Xat06dkEN
 =NNyc
 -----END PGP SIGNATURE-----

Merge tag 'v5.2-rc4' into spi-5.3

Linux 5.2-rc4
2019-06-10 18:52:53 +01:00
Thomas Gleixner c942fddf87 treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 157
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>
2019-05-30 11:26:37 -07:00
Lukas Wunner af505208e9
spi: bcm2835: Drop assignment of dma_slave_config direction
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>
2019-05-22 13:22:32 +01:00
Lukas Wunner 5f336ea53b
spi: bcm2835: Replace spi_master by spi_controller
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>
2019-05-22 13:22:18 +01:00
Nicolas Saenz Julienne 3393f7d924
spi: bcm2835: only split transfers that exceed DLEN if DMA available
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>
2019-05-13 13:14:38 +01:00
Hoan Nguyen An bfada2080b
spi: bcm2835: Remove spi_alloc_master() error printing
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>
2019-05-13 13:14:07 +01:00
Martin Sperl 154f7da56f
spi: bcm2835: add driver stats to debugfs
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>
2019-05-08 17:59:47 +09:00
Martin Sperl ff245d90eb
spi: bcm2835: make the polling duration limits configurable
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>
2019-05-08 17:59:21 +09:00
Martin Sperl c41d62b06c
spi: bcm2835: added comment about different bus behaviour of DMA mode
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>
2019-05-08 17:58:54 +09:00
Martin Sperl 7f1922eb3a
spi: bcm2835: Avoid 64-bit arithmetic in xfer len calc
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>
2019-05-08 17:58:28 +09:00
Martin Sperl 9ac3f90db4
spi: bcm2835: bcm2835_spi_transfer_one_poll remove unnecessary argument
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>
2019-05-08 17:57:57 +09:00
Meghana Madhyastha 8b7bd10eb0
spi/spi-bcm2835: Split transfers that exceed DLEN
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>
2019-05-02 10:37:55 +09:00
Lukas Wunner 2527704d84
spi: bcm2835: Synchronize with callback on DMA termination
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>
2018-12-04 16:51:55 +00:00
Lukas Wunner 2e0733bc5a
spi: bcm2835: Speed up FIFO access if fill level is known
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>
2018-12-04 16:51:41 +00:00
Lukas Wunner b31a9299bc
spi: bcm2835: Polish transfer of DMA prologue
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>
2018-12-04 16:51:12 +00:00
Mark Brown 5651248697
Merge branch 'for-4.20' of https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi into spi-4.21 2018-11-29 16:23:39 +00:00
Lukas Wunner 29bdedfd9c
spi: bcm2835: Unbreak the build of esoteric configs
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>
2018-11-29 16:02:01 +00:00