From 42bdaaece121b3bb50fd4d1203d6d0170279f9fa Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 12 Mar 2019 19:43:31 +0100 Subject: spi: rspi: Fix register initialization while runtime-suspended The Renesas RSPI/QSPI driver performs SPI controller register initialization in its spi_operations.setup() callback, without calling pm_runtime_get_sync() first, which may cause spurious failures. So far this went unnoticed, as this SPI controller is typically used with a single SPI NOR FLASH containing the boot loader: 1. If the device's module clock is still enabled (left enabled by the bootloader, and not yet disabled by the clk_disable_unused() late initcall), register initialization succeeds, 2. If the device's module clock is disabled, register writes don't seem to cause lock-ups or crashes. Data received in the first SPI message may be corrupted, though. Subsequent SPI messages seem to be OK. E.g. on r8a7791/koelsch, one bit is lost while receiving the 6th byte of the JEDEC ID for the s25fl512s FLASH, corrupting that byte and all later bytes. But until commit a2126b0a010905e5 ("mtd: spi-nor: refine Spansion S25FL512S ID"), the 6th byte was not considered for FLASH identification. Fix this by moving all initialization from the .setup() to the .prepare_message() callback. The latter is always called after the device has been runtime-resumed by the SPI core. This also makes the driver follow the rule that .setup() must not change global driver state or register values, as that might break a transfer in progress. Fixes: 490c97747d5dc77d ("spi: rspi: Add runtime PM support, using spi core auto_runtime_pm") Signed-off-by: Geert Uytterhoeven Signed-off-by: Mark Brown --- drivers/spi/spi-rspi.c | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) (limited to 'drivers/spi/spi-rspi.c') diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c index 556870dcdf79..b30fed824e66 100644 --- a/drivers/spi/spi-rspi.c +++ b/drivers/spi/spi-rspi.c @@ -868,28 +868,6 @@ static int qspi_transfer_one(struct spi_controller *ctlr, } } -static int rspi_setup(struct spi_device *spi) -{ - struct rspi_data *rspi = spi_controller_get_devdata(spi->controller); - - rspi->max_speed_hz = spi->max_speed_hz; - - rspi->spcmd = SPCMD_SSLKP; - if (spi->mode & SPI_CPOL) - rspi->spcmd |= SPCMD_CPOL; - if (spi->mode & SPI_CPHA) - rspi->spcmd |= SPCMD_CPHA; - - /* CMOS output mode and MOSI signal from previous transfer */ - rspi->sppcr = 0; - if (spi->mode & SPI_LOOP) - rspi->sppcr |= SPPCR_SPLP; - - set_config_register(rspi, 8); - - return 0; -} - static u16 qspi_transfer_mode(const struct spi_transfer *xfer) { if (xfer->tx_buf) @@ -959,8 +937,24 @@ static int rspi_prepare_message(struct spi_controller *ctlr, struct spi_message *msg) { struct rspi_data *rspi = spi_controller_get_devdata(ctlr); + struct spi_device *spi = msg->spi; int ret; + rspi->max_speed_hz = spi->max_speed_hz; + + rspi->spcmd = SPCMD_SSLKP; + if (spi->mode & SPI_CPOL) + rspi->spcmd |= SPCMD_CPOL; + if (spi->mode & SPI_CPHA) + rspi->spcmd |= SPCMD_CPHA; + + /* CMOS output mode and MOSI signal from previous transfer */ + rspi->sppcr = 0; + if (spi->mode & SPI_LOOP) + rspi->sppcr |= SPPCR_SPLP; + + set_config_register(rspi, 8); + if (msg->spi->mode & (SPI_TX_DUAL | SPI_TX_QUAD | SPI_RX_DUAL | SPI_RX_QUAD)) { /* Setup sequencer for messages with multiple transfer modes */ @@ -1267,7 +1261,6 @@ static int rspi_probe(struct platform_device *pdev) init_waitqueue_head(&rspi->wait); ctlr->bus_num = pdev->id; - ctlr->setup = rspi_setup; ctlr->auto_runtime_pm = true; ctlr->transfer_one = ops->transfer_one; ctlr->prepare_message = rspi_prepare_message; -- cgit v1.2.3 From 26843bb128590edd7eba1ad7ce22e4b9f1066ce3 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 12 Mar 2019 19:45:13 +0100 Subject: spi: rspi: Fix sequencer reset during initialization While the sequencer is reset after each SPI message since commit 880c6d114fd79a69 ("spi: rspi: Add support for Quad and Dual SPI Transfers on QSPI"), it was never reset for the first message, thus relying on reset state or bootloader settings. Fix this by initializing it explicitly during configuration. Fixes: 0b2182ddac4b8837 ("spi: add support for Renesas RSPI") Signed-off-by: Geert Uytterhoeven Signed-off-by: Mark Brown --- drivers/spi/spi-rspi.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'drivers/spi/spi-rspi.c') diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c index b30fed824e66..3be8fbe80b08 100644 --- a/drivers/spi/spi-rspi.c +++ b/drivers/spi/spi-rspi.c @@ -271,7 +271,8 @@ static int rspi_set_config_register(struct rspi_data *rspi, int access_size) /* Sets parity, interrupt mask */ rspi_write8(rspi, 0x00, RSPI_SPCR2); - /* Sets SPCMD */ + /* Resets sequencer */ + rspi_write8(rspi, 0, RSPI_SPSCR); rspi->spcmd |= SPCMD_SPB_8_TO_16(access_size); rspi_write16(rspi, rspi->spcmd, RSPI_SPCMD0); @@ -315,7 +316,8 @@ static int rspi_rz_set_config_register(struct rspi_data *rspi, int access_size) rspi_write8(rspi, 0x00, RSPI_SSLND); rspi_write8(rspi, 0x00, RSPI_SPND); - /* Sets SPCMD */ + /* Resets sequencer */ + rspi_write8(rspi, 0, RSPI_SPSCR); rspi->spcmd |= SPCMD_SPB_8_TO_16(access_size); rspi_write16(rspi, rspi->spcmd, RSPI_SPCMD0); @@ -366,7 +368,8 @@ static int qspi_set_config_register(struct rspi_data *rspi, int access_size) /* Sets buffer to allow normal operation */ rspi_write8(rspi, 0x00, QSPI_SPBFCR); - /* Sets SPCMD */ + /* Resets sequencer */ + rspi_write8(rspi, 0, RSPI_SPSCR); rspi_write16(rspi, rspi->spcmd, RSPI_SPCMD0); /* Sets RSPI mode */ -- cgit v1.2.3 From 7e95b16625a3659a75c0ba4d5b0802324d16be13 Mon Sep 17 00:00:00 2001 From: Hoan Nguyen An Date: Tue, 23 Apr 2019 18:19:21 +0900 Subject: spi: rspi: Fix handling of QSPI code when transmit and receive Process handling QSPI when transmit/receive at qspi_trigger_transfer_out_in() as follows: Setting the trigger, is the number of bytes in the FIFO buffer to determine when there is an interrupt. Then check if the value of triggering number is 32-bytes or 1-byte, there will be corresponding processing Handling (if (n == QSPI_BUFFER_SIZE) esle) this is unnecessary, leads to the same processing of data transmission or reception, The difference here are with ret = rspi_wait_for_tx_empty(rspi); ret = rspi_wait_for_rx_full(rspi); When the nummber trigger is 32 bytes, we only write into FIFO when the FIFO is completely empty (interrupt transmission), and only receive if FIFO is full of 32 bytes of data. In the case of a nummber trigger that is 1 byte, in principle we still need to process rspi_wait_for_tx_empty/full so that FIFO is empty only with the amount of data we need to write to or equal to the number of bytes we need to receive, There is currently no processing of this. And in the current case with this patch, at this time it only needs at least 1 byte received in FIFO that has interrupt received, or FIFO at least 1bytes free can be written into FIFO, This patch therefore does not affect this processing. So we need to eliminate unnecessary waste processing (if (n == QSPI_BUFFER_SIZE) esle), more precisely in waiting for FIFO status. The same with handling in qspi_transfer_out()/qspi_transfer_in(). Signed-off-by: Hoan Nguyen An Signed-off-by: Mark Brown --- drivers/spi/spi-rspi.c | 71 ++++++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 43 deletions(-) (limited to 'drivers/spi/spi-rspi.c') diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c index 3be8fbe80b08..15f5723d9f95 100644 --- a/drivers/spi/spi-rspi.c +++ b/drivers/spi/spi-rspi.c @@ -739,27 +739,22 @@ static int qspi_trigger_transfer_out_in(struct rspi_data *rspi, const u8 *tx, while (len > 0) { n = qspi_set_send_trigger(rspi, len); qspi_set_receive_trigger(rspi, len); - if (n == QSPI_BUFFER_SIZE) { - ret = rspi_wait_for_tx_empty(rspi); - if (ret < 0) { - dev_err(&rspi->ctlr->dev, "transmit timeout\n"); - return ret; - } - for (i = 0; i < n; i++) - rspi_write_data(rspi, *tx++); + ret = rspi_wait_for_tx_empty(rspi); + if (ret < 0) { + dev_err(&rspi->ctlr->dev, "transmit timeout\n"); + return ret; + } + for (i = 0; i < n; i++) + rspi_write_data(rspi, *tx++); - ret = rspi_wait_for_rx_full(rspi); - if (ret < 0) { - dev_err(&rspi->ctlr->dev, "receive timeout\n"); - return ret; - } - for (i = 0; i < n; i++) - *rx++ = rspi_read_data(rspi); - } else { - ret = rspi_pio_transfer(rspi, tx, rx, n); - if (ret < 0) - return ret; + ret = rspi_wait_for_rx_full(rspi); + if (ret < 0) { + dev_err(&rspi->ctlr->dev, "receive timeout\n"); + return ret; } + for (i = 0; i < n; i++) + *rx++ = rspi_read_data(rspi); + len -= n; } @@ -796,19 +791,14 @@ static int qspi_transfer_out(struct rspi_data *rspi, struct spi_transfer *xfer) while (n > 0) { len = qspi_set_send_trigger(rspi, n); - if (len == QSPI_BUFFER_SIZE) { - ret = rspi_wait_for_tx_empty(rspi); - if (ret < 0) { - dev_err(&rspi->ctlr->dev, "transmit timeout\n"); - return ret; - } - for (i = 0; i < len; i++) - rspi_write_data(rspi, *tx++); - } else { - ret = rspi_pio_transfer(rspi, tx, NULL, len); - if (ret < 0) - return ret; + ret = rspi_wait_for_tx_empty(rspi); + if (ret < 0) { + dev_err(&rspi->ctlr->dev, "transmit timeout\n"); + return ret; } + for (i = 0; i < len; i++) + rspi_write_data(rspi, *tx++); + n -= len; } @@ -833,19 +823,14 @@ static int qspi_transfer_in(struct rspi_data *rspi, struct spi_transfer *xfer) while (n > 0) { len = qspi_set_receive_trigger(rspi, n); - if (len == QSPI_BUFFER_SIZE) { - ret = rspi_wait_for_rx_full(rspi); - if (ret < 0) { - dev_err(&rspi->ctlr->dev, "receive timeout\n"); - return ret; - } - for (i = 0; i < len; i++) - *rx++ = rspi_read_data(rspi); - } else { - ret = rspi_pio_transfer(rspi, NULL, rx, len); - if (ret < 0) - return ret; + ret = rspi_wait_for_rx_full(rspi); + if (ret < 0) { + dev_err(&rspi->ctlr->dev, "receive timeout\n"); + return ret; } + for (i = 0; i < len; i++) + *rx++ = rspi_read_data(rspi); + n -= len; } -- cgit v1.2.3