From e120cc0dcf2880a4c5c0a6cb27b655600a1cfa1d Mon Sep 17 00:00:00 2001 From: Daniel Santos Date: Sun, 5 Jan 2014 17:39:26 -0600 Subject: [PATCH 1/6] spidev: fix hang when transfer_one_message fails This corrects a problem in spi_pump_messages() that leads to an spi message hanging forever when a call to transfer_one_message() fails. This failure occurs in my MCP2210 driver when the cs_change bit is set on the last transfer in a message, an operation which the hardware does not support. Rationale Since the transfer_one_message() returns an int, we must presume that it may fail. If transfer_one_message() should never fail, it should return void. Thus, calls to transfer_one_message() should properly manage a failure. Fixes: ffbbdd21329f3 (spi: create a message queueing infrastructure) Signed-off-by: Daniel Santos Signed-off-by: Mark Brown Cc: stable@vger.kernel.org --- drivers/spi/spi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 18cc625d887f..0e215237383b 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -735,7 +735,9 @@ static void spi_pump_messages(struct kthread_work *work) ret = master->transfer_one_message(master, master->cur_msg); if (ret) { dev_err(&master->dev, - "failed to transfer one message from queue\n"); + "failed to transfer one message from queue: %d\n", ret); + master->cur_msg->status = ret; + spi_finalize_current_message(master); return; } } From b6fb8d3a1f156c50a35f88b9b55f404034493938 Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Thu, 9 Jan 2014 15:23:55 +0200 Subject: [PATCH 2/6] spi: Check conflicting CS based on spi->chip_select instead of device name Commit e13ac47bec20 (spi: Use stable dev_name for ACPI enumerated SPI slaves) changed the SPI device naming to be based on ACPI device name instead of carrying bus number and chip select for devices enumerated from ACPI namespace. In case of a buggy BIOS that lists multiple SPI devices sharing the same chip select (even though they should use different) the current code fails to detect that and allows the devices to be added to the bus. Fix this by walking through the bus and comparing spi->chip_select instead of device name. This should work regardless what the device name will be in future. Signed-off-by: Mika Westerberg Acked-by: Jarkko Nikula Signed-off-by: Mark Brown --- drivers/spi/spi.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 0e215237383b..51e00c6436aa 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -370,6 +370,17 @@ static void spi_dev_set_name(struct spi_device *spi) spi->chip_select); } +static int spi_dev_check(struct device *dev, void *data) +{ + struct spi_device *spi = to_spi_device(dev); + struct spi_device *new_spi = data; + + if (spi->master == new_spi->master && + spi->chip_select == new_spi->chip_select) + return -EBUSY; + return 0; +} + /** * spi_add_device - Add spi_device allocated with spi_alloc_device * @spi: spi_device to register @@ -384,7 +395,6 @@ int spi_add_device(struct spi_device *spi) static DEFINE_MUTEX(spi_add_lock); struct spi_master *master = spi->master; struct device *dev = master->dev.parent; - struct device *d; int status; /* Chipselects are numbered 0..max; validate. */ @@ -404,12 +414,10 @@ int spi_add_device(struct spi_device *spi) */ mutex_lock(&spi_add_lock); - d = bus_find_device_by_name(&spi_bus_type, NULL, dev_name(&spi->dev)); - if (d != NULL) { + status = bus_for_each_dev(&spi_bus_type, NULL, spi, spi_dev_check); + if (status) { dev_err(dev, "chipselect %d already in use\n", spi->chip_select); - put_device(d); - status = -EBUSY; goto done; } From 9e8f4882cc49205a688b616c1b6c17bade10587f Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 21 Jan 2014 16:10:05 +0100 Subject: [PATCH 3/6] spi: Spelling s/finised/finished/ Signed-off-by: Geert Uytterhoeven Signed-off-by: Mark Brown --- drivers/spi/spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 51e00c6436aa..56b41099b40c 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -640,7 +640,7 @@ out: * * Called by SPI drivers using the core transfer_one_message() * implementation to notify it that the current interrupt driven - * transfer has finised and the next one may be scheduled. + * transfer has finished and the next one may be scheduled. */ void spi_finalize_current_transfer(struct spi_master *master) { From 0516712c6548e252adf7aebf70d67842cd734483 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 21 Jan 2014 16:10:06 +0100 Subject: [PATCH 4/6] spi: Clarify transfer_one() w.r.t. spi_finalize_current_transfer() Signed-off-by: Geert Uytterhoeven Signed-off-by: Mark Brown --- include/linux/spi/spi.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 8c62ba74dd91..e766e0e2fe58 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -282,10 +282,12 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv) * @prepare_message: set up the controller to transfer a single message, * for example doing DMA mapping. Called from threaded * context. - * @transfer_one: transfer a single spi_transfer. When the - * driver is finished with this transfer it must call - * spi_finalize_current_transfer() so the subsystem can issue - * the next transfer + * @transfer_one: transfer a single spi_transfer. + * - return 0 if the transfer is finished, + * - return 1 if the transfer is still in progress. When + * the driver is finished with this transfer it must + * call spi_finalize_current_transfer() so the subsystem + * can issue the next transfer * @unprepare_message: undo any work done by prepare_message(). * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS * number. Any individual value may be -ENOENT for CS lines that From bd6857a0c630207484a03ddc470fab34b23f80bb Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 21 Jan 2014 16:10:07 +0100 Subject: [PATCH 5/6] spi: Correct set_cs() documentation The documentation for spi_master.set_cs() says: assert or deassert chip select, true to assert i.e. its "enable" parameter uses assertion-level logic. This does not match the implementation of spi_set_cs(), which calls spi_master.set_cs() with the wanted logic level of the chip select line, which depends on the polarity of the chip select signal. Correct the documentation to match the implementation. Signed-off-by: Geert Uytterhoeven Signed-off-by: Mark Brown --- include/linux/spi/spi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index e766e0e2fe58..9f5242df9311 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -277,7 +277,7 @@ static inline void spi_unregister_driver(struct spi_driver *sdrv) * @unprepare_transfer_hardware: there are currently no more messages on the * queue so the subsystem notifies the driver that it may relax the * hardware by issuing this call - * @set_cs: assert or deassert chip select, true to assert. May be called + * @set_cs: set the logic level of the chip select line. May be called * from interrupt context. * @prepare_message: set up the controller to transfer a single message, * for example doing DMA mapping. Called from threaded From 13a4279880229240af38486611c94587492b24d3 Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Sat, 18 Jan 2014 22:05:22 +0800 Subject: [PATCH 6/6] spi: core: Fix transfer failure when master->transfer_one returns positive value master->transfer_one returns positive value is not a error. So set ret to 0 when master->transfer_one returns positive value. Otherwise, I hit "spi_master spi0: failed to transfer one message from queue" error when my transfer_one callback returns 1. Signed-off-by: Axel Lin Signed-off-by: Mark Brown --- drivers/spi/spi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 56b41099b40c..8ed1aee13c6f 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -599,8 +599,10 @@ static int spi_transfer_one_message(struct spi_master *master, goto out; } - if (ret > 0) + if (ret > 0) { + ret = 0; wait_for_completion(&master->xfer_completion); + } trace_spi_transfer_stop(msg, xfer);