commit 8f96c434dfbc85ffa755d6634c8c1cb2233fcf24 upstream.
spi_geni_remove() accesses the driver's private data after calling
spi_unregister_master() even though that function releases the last
reference on the spi_master and thereby frees the private data.
Moreover, since commit 1a9e489e61 ("spi: spi-geni-qcom: Use OPP API to
set clk/perf state"), spi_geni_probe() leaks the spi_master allocation
if the calls to dev_pm_opp_set_clkname() or dev_pm_opp_of_add_table()
fail.
Fix by switching over to the new devm_spi_alloc_master() helper which
keeps the private data accessible until the driver has unbound and also
avoids the spi_master leak on probe.
Fixes: 561de45f72 ("spi: spi-geni-qcom: Add SPI driver support for GENI based QUP")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v4.20+: 5e844cc37a: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v4.20+
Cc: Rajendra Nayak <rnayak@codeaurora.org>
Cc: Girish Mahadevan <girishm@codeaurora.org>
Link: https://lore.kernel.org/r/dfa1d8c41b8acdfad87ec8654cd124e6e3cb3f31.1607286887.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 7174dc655ef0578877b0b4598e69619d2be28b4d upstream.
If the call to devm_spi_register_master() fails on probe of the GPIO SPI
driver, the spi_master struct is erroneously not freed:
After allocating the spi_master, its reference count is 1. The driver
unconditionally decrements the reference count on unbind using a devm
action. Before calling devm_spi_register_master(), the driver
unconditionally increments the reference count because on success,
that function will decrement the reference count on unbind. However on
failure, devm_spi_register_master() does *not* decrement the reference
count, so the spi_master is leaked.
The issue was introduced by commits 8b797490b4 ("spi: gpio: Make sure
spi_master_put() is called in every error path") and 79567c1a32 ("spi:
gpio: Use devm_spi_register_master()"), which sought to plug leaks
introduced by 9b00bc7b90 ("spi: spi-gpio: Rewrite to use GPIO
descriptors") but missed this remaining leak.
The situation was later aggravated by commit d3b0ffa1d7 ("spi: gpio:
prevent memory leak in spi_gpio_probe"), which introduced a
use-after-free because it releases a reference on the spi_master if
devm_add_action_or_reset() fails even though the function already
does that.
Fix by switching over to the new devm_spi_alloc_master() helper.
Fixes: 9b00bc7b90 ("spi: spi-gpio: Rewrite to use GPIO descriptors")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Cc: <stable@vger.kernel.org> # v4.17+: 5e844cc37a: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.1-: 8b797490b4: spi: gpio: Make sure spi_master_put() is called in every error path
Cc: <stable@vger.kernel.org> # v5.1-: 45beec3519: spi: bitbang: Introduce spi_bitbang_init()
Cc: <stable@vger.kernel.org> # v5.1-: 79567c1a32: spi: gpio: Use devm_spi_register_master()
Cc: <stable@vger.kernel.org> # v5.4-: d3b0ffa1d7: spi: gpio: prevent memory leak in spi_gpio_probe
Cc: <stable@vger.kernel.org> # v4.17+
Cc: Navid Emamdoost <navid.emamdoost@gmail.com>
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
Link: https://lore.kernel.org/r/86eaed27431c3d709e3748eb76ceecbfc790dd37.1607286887.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 236924ee531d6251c8d10e9177b7742a60534ed5 upstream.
If the call to devm_spi_register_controller() fails on probe of the
Qualcomm Atheros AR934x/QCA95xx SPI driver, the spi_controller struct is
erroneously not freed. Fix by switching over to the new
devm_spi_alloc_master() helper.
Moreover, the controller's clock is enabled on probe but not disabled if
any of the subsequent probe steps fail.
Finally, there's an ordering issue in ar934x_spi_remove() wherein the
clock is disabled even though the controller is not yet unregistered.
It is unregistered after ar934x_spi_remove() by the devres framework.
As long as it is not unregistered, SPI transfers may still be ongoing
and disabling the clock may break them. It is not possible to use
devm_spi_register_controller() in this case, so move to the non-devm
variant.
All of these bugs have existed since the driver was first introduced,
so it seems fair to fix them together in a single commit.
Fixes: 047980c582 ("spi: add driver for ar934x spi controller")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v5.7+: 5e844cc37a: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.7+
Cc: Chuanhong Guo <gch981213@gmail.com>
Link: https://lore.kernel.org/r/1d58367d74d55741e0c2730a51a2b65012c8ab33.1607286887.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 0f4ad8d59f33b24dd86739f3be23e6af1a86f5a9 upstream.
If the call to devm_spi_register_controller() fails on probe of the
MediaTek SPI NOR driver, the spi_controller struct is erroneously not
freed.
Since commit a1daaa991e ("spi: spi-mtk-nor: use dma_alloc_coherent()
for bounce buffer"), the same happens if the call to
dmam_alloc_coherent() fails.
Since commit 3bfd9103c7 ("spi: spi-mtk-nor: Add power management
support"), the same happens if the call to mtk_nor_enable_clk() fails.
Fix by switching over to the new devm_spi_alloc_master() helper.
Fixes: 881d1ee9fe ("spi: add support for mediatek spi-nor controller")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Ikjoon Jang <ikjn@chromium.org>
Cc: <stable@vger.kernel.org> # v5.7+: 5e844cc37a: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.7+
Cc: Chuanhong Guo <gch981213@gmail.com>
Link: https://lore.kernel.org/r/d5b9f0289465394e73dedb8ec51e180a8f1dffc9.1607286887.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[ Upstream commit 2c2b3ad2c4c801bab1eec7264ea6991b1e4e8f2c ]
If cs-gpios property is used in devicetree then ctlr->num_chipselect value
may be changed by spi_get_gpio_descs().
So use ctlr->max_native_cs instead of ctlr->num_chipselect to set SPI_MCR
Fixes: 4fcc7c2292 (spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR)
Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
Link: https://lore.kernel.org/r/20201201085916.63543-1-fido_max@inbox.ru
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 440408dbadfe47a615afd0a0a4a402e629be658a ]
Consider an spi driver with a .probe but without a .remove callback (e.g.
rtc-ds1347). The function spi_drv_probe() is called to bind a device and
so dev_pm_domain_attach() is called. As there is no remove callback
spi_drv_remove() isn't called at unbind time however and so calling
dev_pm_domain_detach() is missed and the pm domain keeps active.
To fix this always use both spi_drv_probe() and spi_drv_remove() and
make them handle the respective callback not being set. This has the
side effect that for a (hypothetical) driver that has neither .probe nor
remove the clk and pm domain setup is done.
Fixes: 33cf00e570 ("spi: attach/detach SPI device to the ACPI power domain")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20201119161604.2633521-1-u.kleine-koenig@pengutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 763eab7074f6e71babd85d796156f05a675f9510 ]
pm_runtime_get_sync will increment pm usage counter even it
failed. Forgetting to pm_runtime_put_noidle will result in
reference leak in two callers(tegra_slink_setup and
tegra_slink_resume), so we should fix it.
Fixes: dc4dc36056 ("spi: tegra: add spi driver for SLINK controller")
Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
Link: https://lore.kernel.org/r/20201103141345.6188-1-zhangqilong3@huawei.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
When SPI DW memory ops support was introduced, there was a check for
excluding controllers which supplied their own CS function. Even so,
the mem_ops pointer is *always* presented to the SPI core.
This causes the SPI core sanity check in spi_controller_check_ops() to
refuse registration, since a mem_ops pointer is being supplied without
an exec_op member function.
The end result is failure of the SPI DW driver on sparx5 and similar
platforms.
The fix in the core SPI DW driver is to avoid presenting the mem_ops
pointer if the exec_op function is not set.
Fixes: 6423207e57 (spi: dw: Add memory operations support)
Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
Acked-by: Serge Semin <fancer.lancer@gmail.com>
Link: https://lore.kernel.org/r/20201120213414.339701-1-lars.povlsen@microchip.com
Signed-off-by: Mark Brown <broonie@kernel.org>
If set active without increase the usage count of pm, the dont use
autosuspend function will call the suspend callback to close the two
clocks of spi because the usage count is reduced to -1.
This will cause the warning dump below when the defer-probe occurs.
[ 129.379701] ecspi2_root_clk already disabled
[ 129.384005] WARNING: CPU: 1 PID: 33 at drivers/clk/clk.c:952 clk_core_disable+0xa4/0xb0
So add the get noresume function before set active.
Fixes: 43b6bf406c spi: imx: fix runtime pm support for !CONFIG_PM
Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
Link: https://lore.kernel.org/r/20201124085247.18025-1-xiaoning.wang@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Given the case that bootloader(such as UEFI)'s FSPI driver might not
handle all interrupts before loading kernel, those legacy interrupts
would assert immidiately once kernel's FSPI driver enable them. Further,
if it was FSPI_INTR_IPCMDDONE, the irq handler nxp_fspi_irq_handler()
would call complete(&f->c) to notify others. However, f->c might not be
initialized yet at that time, then cause kernel panic.
Of cause, we should fix this issue within bootloader. But it would be
better to have this pacth to make dirver more robust (by clearing all
interrupt status bits before enabling interrupts).
Suggested-by: Han Xu <han.xu@nxp.com>
Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
Link: https://lore.kernel.org/r/20201123025715.14635-1-ran.wang_1@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
I've discovered that due to the recent commit 49d7d695ca ("spi: dw:
Explicitly de-assert CS on SPI transfer completion") a concurrent usage of
the spidev devices with different chip-selects causes the "SPI transfer
timed out" error. The root cause of the problem has turned to be in a race
condition of the SPI-transfer execution procedure and the spi_setup()
method being called at the same time. In particular in calling the
spi_set_cs(false) while there is an SPI-transfer being executed. In my
case due to the commit cited above all CSs get to be switched off by
calling the spi_setup() for /dev/spidev0.1 while there is an concurrent
SPI-transfer execution performed on /dev/spidev0.0. Of course a situation
of the spi_setup() being called while there is an SPI-transfer being
executed for two different SPI peripheral devices of the same controller
may happen not only for the spidev driver, but for instance for MMC SPI +
some another device, or spi_setup() being called from an SPI-peripheral
probe method while some other device has already been probed and is being
used by a corresponding driver...
Of course I could have provided a fix affecting the DW APB SSI driver
only, for instance, by creating a mutual exclusive access to the set_cs
callback and setting/clearing only the bit responsible for the
corresponding chip-select. But after a short research I've discovered that
the problem most likely affects a lot of the other drivers:
- drivers/spi/spi-sun4i.c - RMW the chip-select register;
- drivers/spi/spi-rockchip.c - RMW the chip-select register;
- drivers/spi/spi-qup.c - RMW a generic force-CS flag in a CSR.
- drivers/spi/spi-sifive.c - set a generic CS-mode flag in a CSR.
- drivers/spi/spi-bcm63xx-hsspi.c - uses an internal mutex to serialize
the bus config changes, but still isn't protected from the race
condition described above;
- drivers/spi/spi-geni-qcom.c - RMW a chip-select internal flag and set the
CS state in HW;
- drivers/spi/spi-orion.c - RMW a chip-select register;
- drivers/spi/spi-cadence.c - RMW a chip-select register;
- drivers/spi/spi-armada-3700.c - RMW a chip-select register;
- drivers/spi/spi-lantiq-ssc.c - overwrites the chip-select register;
- drivers/spi/spi-sun6i.c - RMW a chip-select register;
- drivers/spi/spi-synquacer.c - RMW a chip-select register;
- drivers/spi/spi-altera.c - directly sets the chip-select state;
- drivers/spi/spi-omap2-mcspi.c - RMW an internally cached CS state and
writes it to HW;
- drivers/spi/spi-mt65xx.c - RMW some CSR;
- drivers/spi/spi-jcore.c - directly sets the chip-selects state;
- drivers/spi/spi-mt7621.c - RMW a chip-select register;
I could have missed some drivers, but a scale of the problem is obvious.
As you can see most of the drivers perform an unprotected
Read-modify-write chip-select register modification in the set_cs callback.
Seeing the spi_setup() function is calling the spi_set_cs() and it can be
executed concurrently with SPI-transfers exec procedure, which also calls
spi_set_cs() in the SPI core spi_transfer_one_message() method, the race
condition of the register modification turns to be obvious.
To sum up the problem denoted above affects each driver for a controller
having more than one chip-select lane and which:
1) performs the RMW to some CS-related register with no serialization;
2) directly disables any CS on spi_set_cs(dev, false).
* the later is the case of the DW APB SSI driver.
The controllers which equipped with a single CS theoretically can also
experience the problem, but in practice will not since normally the
spi_setup() isn't called concurrently with the SPI-transfers executed on
the same SPI peripheral device.
In order to generically fix the denoted bug I'd suggest to serialize an
access to the controller IO by taking the IO mutex in the spi_setup()
callback. The mutex is held while there is an SPI communication going on
on the SPI-bus of the corresponding SPI-controller. So calling the
spi_setup() method and disabling/updating the CS state within it would be
safe while there is no any SPI-transfers being executed. Also note I
suppose it would be safer to protect the spi_controller->setup() callback
invocation too, seeing some of the SPI-controller drivers update a HW
state in there.
Fixes: 49d7d695ca ("spi: dw: Explicitly de-assert CS on SPI transfer completion")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Link: https://lore.kernel.org/r/20201117094517.5654-1-Sergey.Semin@baikalelectronics.ru
Signed-off-by: Mark Brown <broonie@kernel.org>
It turns out the IRQs most like can be unmasked before the controller is
enabled with no problematic consequences. The manual doesn't explicitly
state that, but the examples perform the controller initialization
procedure in that order. So the commit da8f58909e ("spi: dw: Unmask IRQs
after enabling the chip") hasn't been that required as I thought. But
anyway setting the IRQs up after the chip enabling still worth adding
since it has simplified the code a bit. The problem is that it has
introduced a potential bug. The transfer handler pointer is now
initialized after the IRQs are enabled. That may and eventually will cause
an invalid or uninitialized callback invocation. Fix that just by
performing the callback initialization before the IRQ unmask procedure.
Fixes: da8f58909e ("spi: dw: Unmask IRQs after enabling the chip")
Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Link: https://lore.kernel.org/r/20201117094054.4696-1-Sergey.Semin@baikalelectronics.ru
Signed-off-by: Mark Brown <broonie@kernel.org>
Clang warns:
drivers/spi/spi-bcm2835aux.c:532:50: warning: variable 'err' is
uninitialized when used here [-Wuninitialized]
dev_err(&pdev->dev, "could not get clk: %d\n", err);
^~~
./include/linux/dev_printk.h:112:32: note: expanded from macro 'dev_err'
_dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
drivers/spi/spi-bcm2835aux.c:495:9: note: initialize the variable 'err'
to silence this warning
int err;
^
= 0
1 warning generated.
Restore the assignment so that the error value can be used in the
dev_err statement and there is no uninitialized memory being leaked.
Fixes: e13ee6cc47 ("spi: bcm2835aux: Fix use-after-free on unbind")
Link: https://github.com/ClangBuiltLinux/linux/issues/1199
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Link: https://lore.kernel.org/r/20201113180701.455541-1-natechancellor@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Here's my proposal to fix the use-after-free bugs reported by
Sascha Hauer and Florian Fainelli:
I scrutinized all SPI drivers in the v5.10 tree:
* There are 9 drivers with a use-after-free in the ->remove() hook
caused by accessing driver private data after spi_unregister_controller().
* There are 8 drivers which leak the spi_controller in the ->probe()
error path because of a missing spi_controller_put().
I'm introducing devm_spi_alloc_master/slave() which automatically
calls spi_controller_put() on ->remove(). This fixes both classes
of bugs while at the same time reducing code amount and complexity
in the ->probe() hook.
I propose that spi_controller_unregister() should no longer release
a reference on the spi_controller. Instead, drivers need to either
do it themselves or use one of the devm functions introduced herein.
The vast majority of drivers can be converted to the devm functions.
See the commit message of patch [1/4] for the rationale and details.
Enclosed are patches for 3 Broadcom drivers.
Patches for the other drivers are on this branch:
https://github.com/l1k/linux/commits/spi_fixes
@Florian Fainelli: Could you verify that there are no KASAN splats or
leaks with these patches? Unfortunately I do not have any SPI-capable
hardware at my disposal right now, so can only compile-test. You may
want to augment spi_controller_release() with a printk() to log when
the spi_controller is freed.
@Mark Brown: Patches [2/4] to [4/4] reference the SHA-1 of patch [1/4]
in their stable tags. Because the hash is unknown to me until you apply
the patch, I've used "123456789abc" as a placeholder. You'll have to
replace the hash if/when applying. Alternatively, only apply patch [1/4]
and I'll repost the other patches with the hash fixed up.
Thanks!
Lukas Wunner (4):
spi: Introduce device-managed SPI controller allocation
spi: bcm2835: Fix use-after-free on unbind
spi: bcm2835aux: Fix use-after-free on unbind
spi: bcm-qspi: Fix use-after-free on unbind
drivers/spi/spi-bcm-qspi.c | 34 ++++++++-------------
drivers/spi/spi-bcm2835.c | 24 +++++----------
drivers/spi/spi-bcm2835aux.c | 21 +++++--------
drivers/spi/spi.c | 58 +++++++++++++++++++++++++++++++++++-
include/linux/spi/spi.h | 19 ++++++++++++
5 files changed, 103 insertions(+), 53 deletions(-)
--
2.28.0
Normally the last reference on an spi_controller is released by
spi_unregister_controller(). In the case of the i.MX lpspi driver,
the spi_controller is registered with devm_spi_register_controller(),
so spi_unregister_controller() is invoked automatically after the driver
has unbound.
However the driver already releases the last reference in
fsl_lpspi_remove() through a gratuitous call to spi_master_put(),
causing a use-after-free when spi_unregister_controller() is
subsequently invoked by the devres framework.
Fix by dropping the superfluous spi_master_put().
Fixes: 944c01a889 ("spi: lpspi: enable runtime pm for lpspi")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v5.2+
Cc: Han Xu <han.xu@nxp.com>
Link: https://lore.kernel.org/r/ab3c0b18bd820501a12c85e440006e09ec0e275f.1604874488.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
bcm_qspi_remove() calls spi_unregister_master() even though
bcm_qspi_probe() calls devm_spi_register_master(). The spi_master is
therefore unregistered and freed twice on unbind.
Moreover, since commit 0392727c26 ("spi: bcm-qspi: Handle clock probe
deferral"), bcm_qspi_probe() leaks the spi_master allocation if the call
to devm_clk_get_optional() fails.
Fix by switching over to the new devm_spi_alloc_master() helper which
keeps the private data accessible until the driver has unbound and also
avoids the spi_master leak on probe.
While at it, fix an ordering issue in bcm_qspi_remove() wherein
spi_unregister_master() is called after uninitializing the hardware,
disabling the clock and freeing an IRQ data structure. The correct
order is to call spi_unregister_master() *before* those teardown steps
because bus accesses may still be ongoing until that function returns.
Fixes: fa236a7ef2 ("spi: bcm-qspi: Add Broadcom MSPI driver")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v4.9+: 123456789abc: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v4.9+
Cc: Kamal Dasu <kdasu.kdev@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/5e31a9a59fd1c0d0b795b2fe219f25e5ee855f9d.1605121038.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
SPI driver probing currently comprises two steps, whereas removal
comprises only one step:
spi_alloc_master()
spi_register_controller()
spi_unregister_controller()
That's because spi_unregister_controller() calls device_unregister()
instead of device_del(), thereby releasing the reference on the
spi_controller which was obtained by spi_alloc_master().
An SPI driver's private data is contained in the same memory allocation
as the spi_controller struct. Thus, once spi_unregister_controller()
has been called, the private data is inaccessible. But some drivers
need to access it after spi_unregister_controller() to perform further
teardown steps.
Introduce devm_spi_alloc_master() and devm_spi_alloc_slave(), which
release a reference on the spi_controller struct only after the driver
has unbound, thereby keeping the memory allocation accessible. Change
spi_unregister_controller() to not release a reference if the
spi_controller was allocated by one of these new devm functions.
The present commit is small enough to be backportable to stable.
It allows fixing drivers which use the private data in their ->remove()
hook after it's been freed. It also allows fixing drivers which neglect
to release a reference on the spi_controller in the probe error path.
Long-term, most SPI drivers shall be moved over to the devm functions
introduced herein. The few that can't shall be changed in a treewide
commit to explicitly release the last reference on the controller.
That commit shall amend spi_unregister_controller() to no longer release
a reference, thereby completing the migration.
As a result, the behaviour will be less surprising and more consistent
with subsystems such as IIO, which also includes the private data in the
allocation of the generic iio_dev struct, but calls device_del() in
iio_device_unregister().
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Link: https://lore.kernel.org/r/272bae2ef08abd21388c98e23729886663d19192.1605121038.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
In the case that the SPI mux isn't set, the transfer_one_message
function returns without finalizing the message. This means that
the transfer never completes, resulting in hung tasks and an
eventual kernel panic. Fix it by finalizing the transfer in this
case.
Fixes: 9211a441e6 ("spi: fsi: Check mux status before transfers")
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Link: https://lore.kernel.org/r/20201110214736.25718-1-eajames@linux.ibm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Commit f3186dd876 ("spi: Optionally use GPIO descriptors for CS GPIOs")
introduced the optional use of GPIO descriptors for chip selects.
A side-effect of this change: when a SPI bus uses GPIO descriptors,
all its client devices have SPI_CS_HIGH set in spi->mode. This flag is
required for the SPI bus to operate correctly.
This unfortunately breaks many client drivers, which use the following
pattern to configure their underlying SPI bus:
static int client_device_probe(struct spi_device *spi)
{
...
spi->mode = SPI_MODE_0;
spi->bits_per_word = 8;
err = spi_setup(spi);
..
}
In short, many client drivers overwrite the SPI_CS_HIGH bit in
spi->mode, and break the underlying SPI bus driver.
This is especially true for Freescale/NXP imx ecspi, where large
numbers of spi client drivers now no longer work.
Proposed fix:
-------------
When using gpio descriptors, depend on gpiolib to handle CS polarity.
Existing quirks in gpiolib ensure that this is handled correctly.
Existing gpiolib behaviour will force the polarity of any chip-select
gpiod to active-high (if 'spi-active-high' devicetree prop present) or
active-low (if 'spi-active-high' absent). Irrespective of whether
the gpio is marked GPIO_ACTIVE_[HIGH|LOW] in the devicetree.
Loose ends:
-----------
If this fix is applied:
- is commit 138c9c32f0
("spi: spidev: Fix CS polarity if GPIO descriptors are used")
still necessary / correct ?
Fixes: f3186dd876 ("spi: Optionally use GPIO descriptors for CS GPIOs")
Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Link: https://lore.kernel.org/r/20201106150706.29089-1-TheSven73@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Since commit 530b5affc6 ("spi: fsl-dspi: fix use-after-free in
remove path"), this driver causes a "NULL pointer dereference"
in dspi_suspend/resume.
This is because since this commit, the drivers private data point to
"dspi" instead of "ctlr", the codes in suspend and resume func were
not modified correspondly.
Fixes: 530b5affc6 ("spi: fsl-dspi: fix use-after-free in remove path")
Signed-off-by: Zhao Qiang <qiang.zhao@nxp.com>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Link: https://lore.kernel.org/r/20201103020546.1822-1-qiang.zhao@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
