diff mbox

[4/4] spi: pl022: Fix race in giveback() leading to driver lock-up

Message ID 54F08D8D.5000705@nokia.com (mailing list archive)
State Accepted
Commit cd6fa8d2ca53cac3226fdcffcf763be390abae32
Headers show

Commit Message

Alexander Sverdlin Feb. 27, 2015, 3:30 p.m. UTC
spi: pl022: Fix race in giveback() leading to driver lock-up

Commit fd316941c ("spi/pl022: disable port when unused") introduced a race,
which leads to possible driver lock up (easily reproducible on SMP).

The problem happens in giveback() function where the completion of the transfer
is signalled to SPI subsystem and then the HW SPI controller is disabled. Another
transfer might be setup in between, which brings driver in locked-up state.

Exact event sequence on SMP:

core0                                   core1

                                        => pump_transfers()
                                        /* message->state == STATE_DONE */
                                          => giveback()
                                            => spi_finalize_current_message()

=> pl022_unprepare_transfer_hardware()
=> pl022_transfer_one_message
  => flush()
  => do_interrupt_dma_transfer()
    => set_up_next_transfer()
    /* Enable SSP, turn on interrupts */
    writew((readw(SSP_CR1(pl022->virtbase)) |
           SSP_CR1_MASK_SSE), SSP_CR1(pl022->virtbase));

...

=> pl022_interrupt_handler()
  => readwriter()

                                        /* disable the SPI/SSP operation */
                                        => writew((readw(SSP_CR1(pl022->virtbase)) &
                                                  (~SSP_CR1_MASK_SSE)), SSP_CR1(pl022->virtbase));

Lockup! SPI controller is disabled and the data will never be received. Whole
SPI subsystem is waiting for transfer ACK and blocked.

So, only signal transfer completion after disabling the controller.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
---
 drivers/spi/spi-pl022.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mark Brown March 6, 2015, 7:49 p.m. UTC | #1
On Fri, Feb 27, 2015 at 04:30:21PM +0100, Alexander Sverdlin wrote:
> spi: pl022: Fix race in giveback() leading to driver lock-up
> 
> Commit fd316941c ("spi/pl022: disable port when unused") introduced a race,
> which leads to possible driver lock up (easily reproducible on SMP).

Applied, but a few things:
 - Since this is a bug fix it should have been the first patch of the
   series rather than the last.  That ensures that fixes can be sent to
   Linus and the stable tree without any unneeded dependencies on other
   trees.
 - For all the patches in this series you've duplicated the subject line
   at the start of the message body, meaning the line gets duplicated in
   the commit.
diff mbox

Patch

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index ad78766..6aea517 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -508,12 +508,12 @@  static void giveback(struct pl022 *pl022)
 	pl022->cur_msg = NULL;
 	pl022->cur_transfer = NULL;
 	pl022->cur_chip = NULL;
-	spi_finalize_current_message(pl022->master);
 
 	/* disable the SPI/SSP operation */
 	writew((readw(SSP_CR1(pl022->virtbase)) &
 		(~SSP_CR1_MASK_SSE)), SSP_CR1(pl022->virtbase));
 
+	spi_finalize_current_message(pl022->master);
 }
 
 /**