Skip to contentSkip to conversation

spi: pl022: Fix race in giveback() leading to driver lock-up

PATCH Applied Alexander Sverdlin 2 participants 2 messages Feb 27, 2015 – Mar 6, 2015 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

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.

---
 drivers/spi/spi-pl022.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/spi/spi-pl022.c +1 -1
index ad78766..6aea517 100644
@@ -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);
 }
 
 /**

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

#2 lore raw
In reply to a message not in the archive
Quoted 4 lines · Alexander Sverdlin

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.