* [PATCH v5 01/12] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2020-03-18 0:15 ` Vladimir Oltean
2020-03-18 0:15 ` [PATCH v5 02/12] spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA Vladimir Oltean
` (7 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-18 0:15 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
peng.ma-3arQi8VN3Tc
From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
The SPI_MCR_PCSIS macro assumes that the controller has a number of chip
select signals equal to 6. That is not always the case, but actually is
described through the driver-specific "spi-num-chipselects" device tree
binding. LS1028A for example only has 4 chip selects.
Don't write to the upper bits of the PCSIS field, which are reserved in
the reference manual.
Fixes: 349ad66c0ab0 ("spi:Add Freescale DSPI driver for Vybrid VF610 platform")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v5:
None.
Changes in v4:
None.
Changes in v3:
None.
Changes in v2:
Remove duplicate phrase in commit message.
drivers/spi/spi-fsl-dspi.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 50e3382f0c50..6ca35881881b 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -22,7 +22,7 @@
#define SPI_MCR 0x00
#define SPI_MCR_MASTER BIT(31)
-#define SPI_MCR_PCSIS (0x3F << 16)
+#define SPI_MCR_PCSIS(x) ((x) << 16)
#define SPI_MCR_CLR_TXF BIT(11)
#define SPI_MCR_CLR_RXF BIT(10)
#define SPI_MCR_XSPI BIT(3)
@@ -1200,7 +1200,10 @@ static const struct regmap_config dspi_xspi_regmap_config[] = {
static void dspi_init(struct fsl_dspi *dspi)
{
- unsigned int mcr = SPI_MCR_PCSIS;
+ unsigned int mcr;
+
+ /* Set idle states for all chip select signals to high */
+ mcr = SPI_MCR_PCSIS(GENMASK(dspi->ctlr->num_chipselect - 1, 0));
if (dspi->devtype_data->trans_mode == DSPI_XSPI_MODE)
mcr |= SPI_MCR_XSPI;
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 02/12] spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-03-18 0:15 ` [PATCH v5 01/12] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR Vladimir Oltean
@ 2020-03-18 0:15 ` Vladimir Oltean
2020-03-18 0:15 ` [PATCH v5 03/12] spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode Vladimir Oltean
` (6 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-18 0:15 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
peng.ma-3arQi8VN3Tc
From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
In XSPI mode, the 32-bit PUSHR register can be written to separately:
the higher 16 bits are for commands and the lower 16 bits are for data.
This has nicely been hacked around, by defining a second regmap with a
width of 16 bits, and effectively splitting a 32-bit register into 2
16-bit ones, from the perspective of this regmap_pushr.
The problem is the assumption about the controller's endianness. If the
controller is little endian (such as anything post-LS1046A), then the
first 2 bytes, in the order imposed by memory layout, will actually hold
the TXDATA, and the last 2 bytes will hold the CMD.
So take the controller's endianness into account when performing split
writes to PUSHR. The obvious and simple solution would have been to call
regmap_get_val_endian(), but that is an internal regmap function and we
don't want to change regmap just for this. Therefore, we just re-read
the "big-endian" device tree property.
Fixes: 58ba07ec79e6 ("spi: spi-fsl-dspi: Add support for XSPI mode registers")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v5:
None.
Changes in v4:
None.
Changes in v3:
None.
Changes in v2:
Parse "big-endian" device tree bindings instead of taking the decision
based on compatible SoC.
drivers/spi/spi-fsl-dspi.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 6ca35881881b..be717776dd98 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -103,10 +103,6 @@
#define SPI_FRAME_BITS(bits) SPI_CTAR_FMSZ((bits) - 1)
#define SPI_FRAME_EBITS(bits) SPI_CTARE_FMSZE(((bits) - 1) >> 4)
-/* Register offsets for regmap_pushr */
-#define PUSHR_CMD 0x0
-#define PUSHR_TX 0x2
-
#define DMA_COMPLETION_TIMEOUT msecs_to_jiffies(3000)
struct chip_data {
@@ -240,6 +236,13 @@ struct fsl_dspi {
int words_in_flight;
+ /*
+ * Offsets for CMD and TXDATA within SPI_PUSHR when accessed
+ * individually (in XSPI mode)
+ */
+ int pushr_cmd;
+ int pushr_tx;
+
void (*host_to_dev)(struct fsl_dspi *dspi, u32 *txdata);
void (*dev_to_host)(struct fsl_dspi *dspi, u32 rxdata);
};
@@ -673,12 +676,12 @@ static void dspi_pushr_cmd_write(struct fsl_dspi *dspi, u16 cmd)
*/
if (dspi->len > dspi->oper_word_size)
cmd |= SPI_PUSHR_CMD_CONT;
- regmap_write(dspi->regmap_pushr, PUSHR_CMD, cmd);
+ regmap_write(dspi->regmap_pushr, dspi->pushr_cmd, cmd);
}
static void dspi_pushr_txdata_write(struct fsl_dspi *dspi, u16 txdata)
{
- regmap_write(dspi->regmap_pushr, PUSHR_TX, txdata);
+ regmap_write(dspi->regmap_pushr, dspi->pushr_tx, txdata);
}
static void dspi_xspi_write(struct fsl_dspi *dspi, int cnt, bool eoq)
@@ -1259,6 +1262,7 @@ static int dspi_probe(struct platform_device *pdev)
struct fsl_dspi *dspi;
struct resource *res;
void __iomem *base;
+ bool big_endian;
ctlr = spi_alloc_master(&pdev->dev, sizeof(struct fsl_dspi));
if (!ctlr)
@@ -1284,6 +1288,7 @@ static int dspi_probe(struct platform_device *pdev)
/* Only Coldfire uses platform data */
dspi->devtype_data = &devtype_data[MCF5441X];
+ big_endian = true;
} else {
ret = of_property_read_u32(np, "spi-num-chipselects", &cs_num);
@@ -1305,6 +1310,15 @@ static int dspi_probe(struct platform_device *pdev)
ret = -EFAULT;
goto out_ctlr_put;
}
+
+ big_endian = of_device_is_big_endian(np);
+ }
+ if (big_endian) {
+ dspi->pushr_cmd = 0;
+ dspi->pushr_tx = 2;
+ } else {
+ dspi->pushr_cmd = 2;
+ dspi->pushr_tx = 0;
}
if (dspi->devtype_data->trans_mode == DSPI_XSPI_MODE)
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 03/12] spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-03-18 0:15 ` [PATCH v5 01/12] spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR Vladimir Oltean
2020-03-18 0:15 ` [PATCH v5 02/12] spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA Vladimir Oltean
@ 2020-03-18 0:15 ` Vladimir Oltean
2020-03-18 0:15 ` [PATCH v5 04/12] spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode Vladimir Oltean
` (5 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-18 0:15 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
peng.ma-3arQi8VN3Tc
From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
In DMA mode, dspi_setup_accel does not get called, which results in the
dspi->oper_word_size variable (which is used by dspi_dma_xfer) to not be
initialized properly.
Because oper_word_size is zero, a few calculations end up being
incorrect, and the DMA transfer eventually times out instead of sending
anything on the wire.
Set up native transfers (or 8-on-16 acceleration) using dspi_setup_accel
for DMA mode too.
Also take the opportunity and simplify the DMA buffer handling a little
bit.
Fixes: 6c1c26ecd9a3 ("spi: spi-fsl-dspi: Accelerate transfers using larger word size if possible")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v5:
None.
Changes in v4:
Rebased on top of "spi: spi-fsl-dspi: fix DMA mapping".
Stopped uselessly writing to SPI_CTAR in dspi_transfer_one_message,
since we already do that in dspi_setup_accel which we now call.
Update message->actual_length before submitting the DMA transfer.
Changes in v3:
Pretty much re-did the patch. Before, dspi_setup_accel was called just
once at the beginning of dspi_dma_xfer. Now it is called in the while
loop. Everything else is just refactoring that follows along.
Changes in v2:
None.
drivers/spi/spi-fsl-dspi.c | 86 ++++++++++++++------------------------
1 file changed, 32 insertions(+), 54 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index be717776dd98..8f2b73cc6ed7 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -119,7 +119,6 @@ struct fsl_dspi_devtype_data {
enum dspi_trans_mode trans_mode;
u8 max_clock_factor;
int fifo_size;
- int dma_bufsize;
};
enum {
@@ -138,7 +137,6 @@ static const struct fsl_dspi_devtype_data devtype_data[] = {
[VF610] = {
.trans_mode = DSPI_DMA_MODE,
.max_clock_factor = 2,
- .dma_bufsize = 4096,
.fifo_size = 4,
},
[LS1021A] = {
@@ -167,19 +165,16 @@ static const struct fsl_dspi_devtype_data devtype_data[] = {
},
[LS2080A] = {
.trans_mode = DSPI_DMA_MODE,
- .dma_bufsize = 8,
.max_clock_factor = 8,
.fifo_size = 4,
},
[LS2085A] = {
.trans_mode = DSPI_DMA_MODE,
- .dma_bufsize = 8,
.max_clock_factor = 8,
.fifo_size = 4,
},
[LX2160A] = {
.trans_mode = DSPI_DMA_MODE,
- .dma_bufsize = 8,
.max_clock_factor = 8,
.fifo_size = 4,
},
@@ -191,9 +186,6 @@ static const struct fsl_dspi_devtype_data devtype_data[] = {
};
struct fsl_dspi_dma {
- /* Length of transfer in words of dspi->fifo_size */
- u32 curr_xfer_len;
-
u32 *tx_dma_buf;
struct dma_chan *chan_tx;
dma_addr_t tx_dma_phys;
@@ -352,7 +344,7 @@ static void dspi_rx_dma_callback(void *arg)
int i;
if (dspi->rx) {
- for (i = 0; i < dma->curr_xfer_len; i++)
+ for (i = 0; i < dspi->words_in_flight; i++)
dspi_push_rx(dspi, dspi->dma->rx_dma_buf[i]);
}
@@ -366,12 +358,12 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
int time_left;
int i;
- for (i = 0; i < dma->curr_xfer_len; i++)
+ for (i = 0; i < dspi->words_in_flight; i++)
dspi->dma->tx_dma_buf[i] = dspi_pop_tx_pushr(dspi);
dma->tx_desc = dmaengine_prep_slave_single(dma->chan_tx,
dma->tx_dma_phys,
- dma->curr_xfer_len *
+ dspi->words_in_flight *
DMA_SLAVE_BUSWIDTH_4_BYTES,
DMA_MEM_TO_DEV,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
@@ -389,7 +381,7 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
dma->rx_desc = dmaengine_prep_slave_single(dma->chan_rx,
dma->rx_dma_phys,
- dma->curr_xfer_len *
+ dspi->words_in_flight *
DMA_SLAVE_BUSWIDTH_4_BYTES,
DMA_DEV_TO_MEM,
DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
@@ -437,46 +429,42 @@ static int dspi_next_xfer_dma_submit(struct fsl_dspi *dspi)
return 0;
}
+static void dspi_setup_accel(struct fsl_dspi *dspi);
+
static int dspi_dma_xfer(struct fsl_dspi *dspi)
{
struct spi_message *message = dspi->cur_msg;
struct device *dev = &dspi->pdev->dev;
- struct fsl_dspi_dma *dma = dspi->dma;
- int curr_remaining_bytes;
- int bytes_per_buffer;
int ret = 0;
- curr_remaining_bytes = dspi->len;
- bytes_per_buffer = dspi->devtype_data->dma_bufsize /
- dspi->devtype_data->fifo_size;
- while (curr_remaining_bytes) {
- /* Check if current transfer fits the DMA buffer */
- dma->curr_xfer_len = curr_remaining_bytes /
- dspi->oper_word_size;
- if (dma->curr_xfer_len > bytes_per_buffer)
- dma->curr_xfer_len = bytes_per_buffer;
+ /*
+ * dspi->len gets decremented by dspi_pop_tx_pushr in
+ * dspi_next_xfer_dma_submit
+ */
+ while (dspi->len) {
+ /* Figure out operational bits-per-word for this chunk */
+ dspi_setup_accel(dspi);
+
+ dspi->words_in_flight = dspi->len / dspi->oper_word_size;
+ if (dspi->words_in_flight > dspi->devtype_data->fifo_size)
+ dspi->words_in_flight = dspi->devtype_data->fifo_size;
+
+ message->actual_length += dspi->words_in_flight *
+ dspi->oper_word_size;
ret = dspi_next_xfer_dma_submit(dspi);
if (ret) {
dev_err(dev, "DMA transfer failed\n");
- goto exit;
-
- } else {
- const int len = dma->curr_xfer_len *
- dspi->oper_word_size;
- curr_remaining_bytes -= len;
- message->actual_length += len;
- if (curr_remaining_bytes < 0)
- curr_remaining_bytes = 0;
+ break;
}
}
-exit:
return ret;
}
static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
{
+ int dma_bufsize = dspi->devtype_data->fifo_size * 2;
struct device *dev = &dspi->pdev->dev;
struct dma_slave_config cfg;
struct fsl_dspi_dma *dma;
@@ -501,16 +489,16 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
}
dma->tx_dma_buf = dma_alloc_coherent(dma->chan_tx->device->dev,
- dspi->devtype_data->dma_bufsize,
- &dma->tx_dma_phys, GFP_KERNEL);
+ dma_bufsize, &dma->tx_dma_phys,
+ GFP_KERNEL);
if (!dma->tx_dma_buf) {
ret = -ENOMEM;
goto err_tx_dma_buf;
}
dma->rx_dma_buf = dma_alloc_coherent(dma->chan_rx->device->dev,
- dspi->devtype_data->dma_bufsize,
- &dma->rx_dma_phys, GFP_KERNEL);
+ dma_bufsize, &dma->rx_dma_phys,
+ GFP_KERNEL);
if (!dma->rx_dma_buf) {
ret = -ENOMEM;
goto err_rx_dma_buf;
@@ -547,12 +535,10 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
err_slave_config:
dma_free_coherent(dma->chan_rx->device->dev,
- dspi->devtype_data->dma_bufsize,
- dma->rx_dma_buf, dma->rx_dma_phys);
+ dma_bufsize, dma->rx_dma_buf, dma->rx_dma_phys);
err_rx_dma_buf:
dma_free_coherent(dma->chan_tx->device->dev,
- dspi->devtype_data->dma_bufsize,
- dma->tx_dma_buf, dma->tx_dma_phys);
+ dma_bufsize, dma->tx_dma_buf, dma->tx_dma_phys);
err_tx_dma_buf:
dma_release_channel(dma->chan_tx);
err_tx_channel:
@@ -566,6 +552,7 @@ static int dspi_request_dma(struct fsl_dspi *dspi, phys_addr_t phy_addr)
static void dspi_release_dma(struct fsl_dspi *dspi)
{
+ int dma_bufsize = dspi->devtype_data->fifo_size * 2;
struct fsl_dspi_dma *dma = dspi->dma;
if (!dma)
@@ -573,15 +560,13 @@ static void dspi_release_dma(struct fsl_dspi *dspi)
if (dma->chan_tx) {
dma_unmap_single(dma->chan_tx->device->dev, dma->tx_dma_phys,
- dspi->devtype_data->dma_bufsize,
- DMA_TO_DEVICE);
+ dma_bufsize, DMA_TO_DEVICE);
dma_release_channel(dma->chan_tx);
}
if (dma->chan_rx) {
dma_unmap_single(dma->chan_rx->device->dev, dma->rx_dma_phys,
- dspi->devtype_data->dma_bufsize,
- DMA_FROM_DEVICE);
+ dma_bufsize, DMA_FROM_DEVICE);
dma_release_channel(dma->chan_rx);
}
}
@@ -833,7 +818,7 @@ static void dspi_setup_accel(struct fsl_dspi *dspi)
dspi->oper_word_size = DIV_ROUND_UP(dspi->oper_bits_per_word, 8);
/*
- * Update CTAR here (code is common for both EOQ and XSPI modes).
+ * Update CTAR here (code is common for EOQ, XSPI and DMA modes).
* We will update CTARE in the portion specific to XSPI, when we
* also know the preload value (DTCP).
*/
@@ -960,13 +945,6 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
regmap_update_bits(dspi->regmap, SPI_MCR,
SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF,
SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF);
- /*
- * Static CTAR setup for modes that don't dynamically adjust it
- * via dspi_setup_accel (aka for DMA)
- */
- regmap_write(dspi->regmap, SPI_CTAR(0),
- dspi->cur_chip->ctar_val |
- SPI_FRAME_BITS(transfer->bits_per_word));
spi_take_timestamp_pre(dspi->ctlr, dspi->cur_transfer,
dspi->progress, !dspi->irq);
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 04/12] spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
` (2 preceding siblings ...)
2020-03-18 0:15 ` [PATCH v5 03/12] spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode Vladimir Oltean
@ 2020-03-18 0:15 ` Vladimir Oltean
2020-03-18 0:15 ` [PATCH v5 05/12] spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight Vladimir Oltean
` (4 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-18 0:15 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
peng.ma-3arQi8VN3Tc
From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
If dspi->words_in_flight is populated with the hardware FIFO size,
then in dspi_fifo_read it will attempt to read more data at the end of a
buffer that is not a multiple of 16 bytes in length. It will probably
time out attempting to do so.
So limit the num_fifo_entries variable to the actual number of FIFO
entries that is going to be used.
Fixes: d59c90a2400f ("spi: spi-fsl-dspi: Convert TCFQ users to XSPI FIFO mode")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v5:
None.
Changes in v4:
Patch is new.
drivers/spi/spi-fsl-dspi.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 8f2b73cc6ed7..51224b772680 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -739,13 +739,16 @@ static void dspi_eoq_fifo_write(struct fsl_dspi *dspi)
int num_fifo_entries = dspi->devtype_data->fifo_size;
u16 xfer_cmd = dspi->tx_cmd;
+ if (num_fifo_entries * dspi->oper_word_size > dspi->len)
+ num_fifo_entries = dspi->len / dspi->oper_word_size;
+
dspi->words_in_flight = num_fifo_entries;
/* Fill TX FIFO with as many transfers as possible */
- while (dspi->len && num_fifo_entries--) {
+ while (num_fifo_entries--) {
dspi->tx_cmd = xfer_cmd;
/* Request EOQF for last transfer in FIFO */
- if (dspi->len == dspi->oper_word_size || num_fifo_entries == 0)
+ if (num_fifo_entries == 0)
dspi->tx_cmd |= SPI_PUSHR_CMD_EOQ;
/* Write combined TX FIFO and CMD FIFO entry */
dspi_pushr_write(dspi);
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 05/12] spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
` (3 preceding siblings ...)
2020-03-18 0:15 ` [PATCH v5 04/12] spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode Vladimir Oltean
@ 2020-03-18 0:15 ` Vladimir Oltean
[not found] ` <20200318001603.9650-6-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2020-03-18 0:15 ` [PATCH v5 07/12] spi: spi-fsl-dspi: Avoid NULL pointer in dspi_slave_abort for non-DMA mode Vladimir Oltean
` (3 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-18 0:15 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
peng.ma-3arQi8VN3Tc
From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
dspi->words_in_flight is a variable populated in the *_write functions
and used in the dspi_fifo_read function. It is also used in
dspi_fifo_write, immediately after transmission, to update the
message->actual_length variable used by higher layers such as spi-mem
for integrity checking.
But it may happen that the IRQ which calls dspi_fifo_read to be
triggered before the updating of message->actual_length takes place. In
that case, dspi_fifo_read will decrement dspi->words_in_flight to -1,
and that will cause an invalid modification of message->actual_length.
For that, we make the simplest fix possible: to not decrement the actual
shared variable in dspi->words_in_flight from dspi_fifo_read, but
actually a copy of it which is on stack.
But even if dspi_fifo_read from the next IRQ does not interfere with the
dspi_fifo_write of the current chunk, the *next* dspi_fifo_write still
can. So we must assume that everything after the last write to the TX
FIFO can be preempted by the "TX complete" IRQ, and the dspi_fifo_write
function must be safe against that. This means refactoring the 2
flavours of FIFO writes (for EOQ and XSPI) such that the calculation of
the number of words to be written is common and happens a priori. This
way, the code for updating the message->actual_length variable works
with a copy and not with the volatile dspi->words_in_flight.
After some interior debate, the dspi->progress variable used for
software timestamping was *not* backed up against preemption in a copy
on stack. Because if preemption does occur between
spi_take_timestamp_pre and spi_take_timestamp_post, there's really no
point in trying to save anything. The first-in-time
spi_take_timestamp_post call with a dspi->progress higher than the
requested xfer->ptp_sts_word_post will trigger xfer->timestamped = true
anyway and will close the deal.
To understand the above a bit better, consider a transfer with
xfer->ptp_sts_word_pre = xfer->ptp_sts_word_post = 3, and
xfer->bits_per_words = 8 (so byte 3 needs to be timestamped). The DSPI
controller timestamps in chunks of 4 bytes at a time, and preemption
occurs in the middle of timestamping the first chunk:
spi_take_timestamp_pre(0)
.
. (preemption)
.
. spi_take_timestamp_pre(4)
.
. spi_take_timestamp_post(7)
.
spi_take_timestamp_post(3)
So the reason I'm not bothering to back up dspi->progress for that
spi_take_timestamp_post(3) is that spi_take_timestamp_post(7) is going
to (a) be more honest, (b) provide better accuracy and (c) already
render the spi_take_timestamp_post(3) into a noop by setting
xfer->timestamped = true anyway.
Fixes: d59c90a2400f ("spi: spi-fsl-dspi: Convert TCFQ users to XSPI FIFO mode")
Reported-by: Michael Walle <michael-QKn5cuLxLXY@public.gmane.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v5:
Enhanced the patch such that dspi->words_in_flight is protected against
races with the next dspi_fifo_write too. This implied rather major
refactoring of dspi_xspi_fifo_write and dspi_eoq_fifo_write
unfortunately. Perhaps the good side of things is that the
dspi_xspi_write function has now disappeared (has merged with
dspi_xspi_fifo_write).
Changes in v4:
Patch is new.
drivers/spi/spi-fsl-dspi.c | 111 +++++++++++++++++--------------------
1 file changed, 52 insertions(+), 59 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 51224b772680..f7e1e7085e31 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -669,17 +669,26 @@ static void dspi_pushr_txdata_write(struct fsl_dspi *dspi, u16 txdata)
regmap_write(dspi->regmap_pushr, dspi->pushr_tx, txdata);
}
-static void dspi_xspi_write(struct fsl_dspi *dspi, int cnt, bool eoq)
+static void dspi_xspi_fifo_write(struct fsl_dspi *dspi, int num_words)
{
+ int num_bytes = num_words * dspi->oper_word_size;
u16 tx_cmd = dspi->tx_cmd;
- if (eoq)
+ /*
+ * If the PCS needs to de-assert (i.e. we're at the end of the buffer
+ * and cs_change does not want the PCS to stay on), then we need a new
+ * PUSHR command, since this one (for the body of the buffer)
+ * necessarily has the CONT bit set.
+ * So send one word less during this go, to force a split and a command
+ * with a single word next time, when CONT will be unset.
+ */
+ if (!(dspi->tx_cmd & SPI_PUSHR_CMD_CONT) && num_bytes == dspi->len)
tx_cmd |= SPI_PUSHR_CMD_EOQ;
/* Update CTARE */
regmap_write(dspi->regmap, SPI_CTARE(0),
SPI_FRAME_EBITS(dspi->oper_bits_per_word) |
- SPI_CTARE_DTCP(cnt));
+ SPI_CTARE_DTCP(num_words));
/*
* Write the CMD FIFO entry first, and then the two
@@ -688,7 +697,7 @@ static void dspi_xspi_write(struct fsl_dspi *dspi, int cnt, bool eoq)
dspi_pushr_cmd_write(dspi, tx_cmd);
/* Fill TX FIFO with as many transfers as possible */
- while (cnt--) {
+ while (num_words--) {
u32 data = dspi_pop_tx(dspi);
dspi_pushr_txdata_write(dspi, data & 0xFFFF);
@@ -697,58 +706,15 @@ static void dspi_xspi_write(struct fsl_dspi *dspi, int cnt, bool eoq)
}
}
-static void dspi_xspi_fifo_write(struct fsl_dspi *dspi)
-{
- int num_fifo_entries = dspi->devtype_data->fifo_size;
- int bytes_in_flight;
- bool eoq = false;
-
- /* In XSPI mode each 32-bit word occupies 2 TX FIFO entries */
- if (dspi->oper_word_size == 4)
- num_fifo_entries /= 2;
-
- /*
- * Integer division intentionally trims off odd (or non-multiple of 4)
- * numbers of bytes at the end of the buffer, which will be sent next
- * time using a smaller oper_word_size.
- */
- dspi->words_in_flight = dspi->len / dspi->oper_word_size;
-
- if (dspi->words_in_flight > num_fifo_entries)
- dspi->words_in_flight = num_fifo_entries;
-
- bytes_in_flight = dspi->words_in_flight * dspi->oper_word_size;
-
- /*
- * If the PCS needs to de-assert (i.e. we're at the end of the buffer
- * and cs_change does not want the PCS to stay on), then we need a new
- * PUSHR command, since this one (for the body of the buffer)
- * necessarily has the CONT bit set.
- * So send one word less during this go, to force a split and a command
- * with a single word next time, when CONT will be unset.
- */
- if (!(dspi->tx_cmd & SPI_PUSHR_CMD_CONT) &&
- bytes_in_flight == dspi->len)
- eoq = true;
-
- dspi_xspi_write(dspi, dspi->words_in_flight, eoq);
-}
-
-static void dspi_eoq_fifo_write(struct fsl_dspi *dspi)
+static void dspi_eoq_fifo_write(struct fsl_dspi *dspi, int num_words)
{
- int num_fifo_entries = dspi->devtype_data->fifo_size;
u16 xfer_cmd = dspi->tx_cmd;
- if (num_fifo_entries * dspi->oper_word_size > dspi->len)
- num_fifo_entries = dspi->len / dspi->oper_word_size;
-
- dspi->words_in_flight = num_fifo_entries;
-
/* Fill TX FIFO with as many transfers as possible */
- while (num_fifo_entries--) {
+ while (num_words--) {
dspi->tx_cmd = xfer_cmd;
/* Request EOQF for last transfer in FIFO */
- if (num_fifo_entries == 0)
+ if (num_words == 0)
dspi->tx_cmd |= SPI_PUSHR_CMD_EOQ;
/* Write combined TX FIFO and CMD FIFO entry */
dspi_pushr_write(dspi);
@@ -765,8 +731,10 @@ static u32 dspi_popr_read(struct fsl_dspi *dspi)
static void dspi_fifo_read(struct fsl_dspi *dspi)
{
+ int num_fifo_entries = dspi->words_in_flight;
+
/* Read one FIFO entry and push to rx buffer */
- while (dspi->words_in_flight--)
+ while (num_fifo_entries--)
dspi_push_rx(dspi, dspi_popr_read(dspi));
}
@@ -832,23 +800,48 @@ static void dspi_setup_accel(struct fsl_dspi *dspi)
static void dspi_fifo_write(struct fsl_dspi *dspi)
{
+ int num_fifo_entries = dspi->devtype_data->fifo_size;
struct spi_transfer *xfer = dspi->cur_transfer;
struct spi_message *msg = dspi->cur_msg;
- int bytes_sent;
+ int num_words, num_bytes;
dspi_setup_accel(dspi);
+ /* In XSPI mode each 32-bit word occupies 2 TX FIFO entries */
+ if (dspi->oper_word_size == 4)
+ num_fifo_entries /= 2;
+
+ /*
+ * Integer division intentionally trims off odd (or non-multiple of 4)
+ * numbers of bytes at the end of the buffer, which will be sent next
+ * time using a smaller oper_word_size.
+ */
+ num_words = dspi->len / dspi->oper_word_size;
+ if (num_words > num_fifo_entries)
+ num_words = num_fifo_entries;
+
+ /* Update total number of bytes that were transferred */
+ num_bytes = num_words * dspi->oper_word_size;
+ msg->actual_length += num_bytes;
+ dspi->progress += num_bytes / DIV_ROUND_UP(xfer->bits_per_word, 8);
+
+ /*
+ * Update shared variable for use in the next interrupt (both in
+ * dspi_fifo_read and in dspi_fifo_write).
+ */
+ dspi->words_in_flight = num_words;
+
spi_take_timestamp_pre(dspi->ctlr, xfer, dspi->progress, !dspi->irq);
if (dspi->devtype_data->trans_mode == DSPI_EOQ_MODE)
- dspi_eoq_fifo_write(dspi);
+ dspi_eoq_fifo_write(dspi, num_words);
else
- dspi_xspi_fifo_write(dspi);
-
- /* Update total number of bytes that were transferred */
- bytes_sent = dspi->words_in_flight * dspi->oper_word_size;
- msg->actual_length += bytes_sent;
- dspi->progress += bytes_sent / DIV_ROUND_UP(xfer->bits_per_word, 8);
+ dspi_xspi_fifo_write(dspi, num_words);
+ /*
+ * Everything after this point is in a potential race with the next
+ * interrupt, so we must never use dspi->words_in_flight again since it
+ * might already be modified by the next dspi_fifo_write.
+ */
spi_take_timestamp_post(dspi->ctlr, dspi->cur_transfer,
dspi->progress, !dspi->irq);
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 07/12] spi: spi-fsl-dspi: Avoid NULL pointer in dspi_slave_abort for non-DMA mode
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
` (4 preceding siblings ...)
2020-03-18 0:15 ` [PATCH v5 05/12] spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight Vladimir Oltean
@ 2020-03-18 0:15 ` Vladimir Oltean
2020-03-18 0:16 ` [PATCH v5 09/12] spi: spi-fsl-dspi: Move invariant configs out of dspi_transfer_one_message Vladimir Oltean
` (2 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-18 0:15 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
peng.ma-3arQi8VN3Tc
From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
The driver does not create the dspi->dma structure unless operating in
DSPI_DMA_MODE, so it makes sense to check for that.
Fixes: f4b323905d8b ("spi: Introduce dspi_slave_abort() function for NXP's dspi SPI driver")
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v5:
None.
Changes in v4:
Patch is new.
drivers/spi/spi-fsl-dspi.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index b65c21d048f9..81e22b6eadc7 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -1192,8 +1192,10 @@ static int dspi_slave_abort(struct spi_master *master)
* Terminate all pending DMA transactions for the SPI working
* in SLAVE mode.
*/
- dmaengine_terminate_sync(dspi->dma->chan_rx);
- dmaengine_terminate_sync(dspi->dma->chan_tx);
+ if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
+ dmaengine_terminate_sync(dspi->dma->chan_rx);
+ dmaengine_terminate_sync(dspi->dma->chan_tx);
+ }
/* Clear the internal DSPI RX and TX FIFO buffers */
regmap_update_bits(dspi->regmap, SPI_MCR,
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 09/12] spi: spi-fsl-dspi: Move invariant configs out of dspi_transfer_one_message
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
` (5 preceding siblings ...)
2020-03-18 0:15 ` [PATCH v5 07/12] spi: spi-fsl-dspi: Avoid NULL pointer in dspi_slave_abort for non-DMA mode Vladimir Oltean
@ 2020-03-18 0:16 ` Vladimir Oltean
2020-03-18 0:16 ` [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS Vladimir Oltean
2020-03-18 19:03 ` [PATCH v5 00/12] NXP DSPI bugfixes and support for LS1028A Michael Walle
8 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-18 0:16 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
peng.ma-3arQi8VN3Tc
From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
The operating mode (DMA, XSPI, EOQ) is not going to change across the
lifetime of the device. So it makes no sense to keep writing to SPI_RSER
on each message. Move this configuration to dspi_init instead.
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v5:
None.
Changes in v4:
Patch is new.
drivers/spi/spi-fsl-dspi.c | 55 ++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 26 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index fcc6f20b6631..5873752a091e 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -900,7 +900,6 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
{
struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr);
struct spi_device *spi = message->spi;
- enum dspi_trans_mode trans_mode;
struct spi_transfer *transfer;
int status = 0;
@@ -942,30 +941,11 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
spi_take_timestamp_pre(dspi->ctlr, dspi->cur_transfer,
dspi->progress, !dspi->irq);
- trans_mode = dspi->devtype_data->trans_mode;
- switch (trans_mode) {
- case DSPI_EOQ_MODE:
- regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE);
- dspi_fifo_write(dspi);
- break;
- case DSPI_XSPI_MODE:
- regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_CMDTCFE);
- dspi_fifo_write(dspi);
- break;
- case DSPI_DMA_MODE:
- regmap_write(dspi->regmap, SPI_RSER,
- SPI_RSER_TFFFE | SPI_RSER_TFFFD |
- SPI_RSER_RFDFE | SPI_RSER_RFDFD);
+ if (dspi->devtype_data->trans_mode == DSPI_DMA_MODE) {
status = dspi_dma_xfer(dspi);
- break;
- default:
- dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
- trans_mode);
- status = -EINVAL;
- goto out;
- }
+ } else {
+ dspi_fifo_write(dspi);
- if (trans_mode != DSPI_DMA_MODE) {
if (dspi->irq) {
wait_for_completion(&dspi->xfer_done);
reinit_completion(&dspi->xfer_done);
@@ -975,11 +955,12 @@ static int dspi_transfer_one_message(struct spi_controller *ctlr,
} while (status == -EINPROGRESS);
}
}
+ if (status)
+ break;
spi_transfer_delay_exec(transfer);
}
-out:
message->status = status;
spi_finalize_current_message(ctlr);
@@ -1170,7 +1151,7 @@ static const struct regmap_config dspi_xspi_regmap_config[] = {
},
};
-static void dspi_init(struct fsl_dspi *dspi)
+static int dspi_init(struct fsl_dspi *dspi)
{
unsigned int mcr;
@@ -1184,6 +1165,26 @@ static void dspi_init(struct fsl_dspi *dspi)
regmap_write(dspi->regmap, SPI_MCR, mcr);
regmap_write(dspi->regmap, SPI_SR, SPI_SR_CLEAR);
+
+ switch (dspi->devtype_data->trans_mode) {
+ case DSPI_EOQ_MODE:
+ regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_EOQFE);
+ break;
+ case DSPI_XSPI_MODE:
+ regmap_write(dspi->regmap, SPI_RSER, SPI_RSER_CMDTCFE);
+ break;
+ case DSPI_DMA_MODE:
+ regmap_write(dspi->regmap, SPI_RSER,
+ SPI_RSER_TFFFE | SPI_RSER_TFFFD |
+ SPI_RSER_RFDFE | SPI_RSER_RFDFD);
+ break;
+ default:
+ dev_err(&dspi->pdev->dev, "unsupported trans_mode %u\n",
+ dspi->devtype_data->trans_mode);
+ return -EINVAL;
+ }
+
+ return 0;
}
static int dspi_slave_abort(struct spi_master *master)
@@ -1339,7 +1340,9 @@ static int dspi_probe(struct platform_device *pdev)
if (ret)
goto out_ctlr_put;
- dspi_init(dspi);
+ ret = dspi_init(dspi);
+ if (ret)
+ goto out_clk_put;
dspi->irq = platform_get_irq(pdev, 0);
if (dspi->irq <= 0) {
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
` (6 preceding siblings ...)
2020-03-18 0:16 ` [PATCH v5 09/12] spi: spi-fsl-dspi: Move invariant configs out of dspi_transfer_one_message Vladimir Oltean
@ 2020-03-18 0:16 ` Vladimir Oltean
2020-04-20 14:38 ` Shawn Guo
2020-04-20 18:06 ` Geert Uytterhoeven
2020-03-18 19:03 ` [PATCH v5 00/12] NXP DSPI bugfixes and support for LS1028A Michael Walle
8 siblings, 2 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-03-18 0:16 UTC (permalink / raw)
To: broonie-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, michael-QKn5cuLxLXY,
peng.ma-3arQi8VN3Tc
From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
For debugging, it is useful to have access to the DSPI controller
signals. On the reference design board, these are exported to either the
mikroBUS1 or mikroBUS2 connector (according to the CPLD register
BRDCFG3[SPI3]).
Signed-off-by: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
---
Changes in v5:
None.
Changes in v4:
None.
Changes in v3:
None.
Changes in v2:
Change compatible string for spidev node.
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index 6d05b76c2c7a..0d27b5667b8c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -83,6 +83,20 @@
};
};
+&dspi2 {
+ bus-num = <2>;
+ status = "okay";
+
+ /* mikroBUS1 */
+ spidev@0 {
+ compatible = "rohm,dh2228fv";
+ spi-max-frequency = <20000000>;
+ fsl,spi-cs-sck-delay = <100>;
+ fsl,spi-sck-cs-delay = <100>;
+ reg = <0>;
+ };
+};
+
&esdhc {
sd-uhs-sdr104;
sd-uhs-sdr50;
--
2.17.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS
2020-03-18 0:16 ` [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS Vladimir Oltean
@ 2020-04-20 14:38 ` Shawn Guo
2020-04-20 15:10 ` Vladimir Oltean
2020-04-20 18:06 ` Geert Uytterhoeven
1 sibling, 1 reply; 22+ messages in thread
From: Shawn Guo @ 2020-04-20 14:38 UTC (permalink / raw)
To: Vladimir Oltean
Cc: broonie, linux-spi, linux-kernel, robh+dt, mark.rutland,
devicetree, eha, angelo, andrew.smirnov, gustavo, weic, mhosny,
michael, peng.ma
On Wed, Mar 18, 2020 at 02:16:03AM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> For debugging, it is useful to have access to the DSPI controller
> signals. On the reference design board, these are exported to either the
> mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> BRDCFG3[SPI3]).
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
> Changes in v5:
> None.
>
> Changes in v4:
> None.
>
> Changes in v3:
> None.
>
> Changes in v2:
> Change compatible string for spidev node.
>
> arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> index 6d05b76c2c7a..0d27b5667b8c 100644
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> @@ -83,6 +83,20 @@
> };
> };
>
> +&dspi2 {
> + bus-num = <2>;
> + status = "okay";
> +
> + /* mikroBUS1 */
> + spidev@0 {
> + compatible = "rohm,dh2228fv";
Is the compatible documented?
Shawn
> + spi-max-frequency = <20000000>;
> + fsl,spi-cs-sck-delay = <100>;
> + fsl,spi-sck-cs-delay = <100>;
> + reg = <0>;
> + };
> +};
> +
> &esdhc {
> sd-uhs-sdr104;
> sd-uhs-sdr50;
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS
2020-04-20 14:38 ` Shawn Guo
@ 2020-04-20 15:10 ` Vladimir Oltean
2020-04-21 3:00 ` Shawn Guo
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2020-04-20 15:10 UTC (permalink / raw)
To: Shawn Guo
Cc: Mark Brown, linux-spi, lkml, Rob Herring, Mark Rutland,
devicetree, Esben Haabendal, angelo, andrew.smirnov,
Gustavo A. R. Silva, Wei Chen, Mohamed Hosny, Michael Walle,
peng.ma
On Mon, 20 Apr 2020 at 17:38, Shawn Guo <shawnguo@kernel.org> wrote:
>
> On Wed, Mar 18, 2020 at 02:16:03AM +0200, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > For debugging, it is useful to have access to the DSPI controller
> > signals. On the reference design board, these are exported to either the
> > mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> > BRDCFG3[SPI3]).
> >
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> > Changes in v5:
> > None.
> >
> > Changes in v4:
> > None.
> >
> > Changes in v3:
> > None.
> >
> > Changes in v2:
> > Change compatible string for spidev node.
> >
> > arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > index 6d05b76c2c7a..0d27b5667b8c 100644
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > @@ -83,6 +83,20 @@
> > };
> > };
> >
> > +&dspi2 {
> > + bus-num = <2>;
> > + status = "okay";
> > +
> > + /* mikroBUS1 */
> > + spidev@0 {
> > + compatible = "rohm,dh2228fv";
>
> Is the compatible documented?
>
> Shawn
>
> > + spi-max-frequency = <20000000>;
> > + fsl,spi-cs-sck-delay = <100>;
> > + fsl,spi-sck-cs-delay = <100>;
> > + reg = <0>;
> > + };
> > +};
> > +
> > &esdhc {
> > sd-uhs-sdr104;
> > sd-uhs-sdr50;
> > --
> > 2.17.1
> >
I don't really know what's the status with spidev compatibles. I do
see other device trees are using this one, I thought it should be
fine.
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS
2020-04-20 15:10 ` Vladimir Oltean
@ 2020-04-21 3:00 ` Shawn Guo
0 siblings, 0 replies; 22+ messages in thread
From: Shawn Guo @ 2020-04-21 3:00 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Mark Brown, linux-spi, lkml, Rob Herring, Mark Rutland,
devicetree, Esben Haabendal, angelo, andrew.smirnov,
Gustavo A. R. Silva, Wei Chen, Mohamed Hosny, Michael Walle,
peng.ma
On Mon, Apr 20, 2020 at 06:10:35PM +0300, Vladimir Oltean wrote:
> On Mon, 20 Apr 2020 at 17:38, Shawn Guo <shawnguo@kernel.org> wrote:
> >
> > On Wed, Mar 18, 2020 at 02:16:03AM +0200, Vladimir Oltean wrote:
> > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > >
> > > For debugging, it is useful to have access to the DSPI controller
> > > signals. On the reference design board, these are exported to either the
> > > mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> > > BRDCFG3[SPI3]).
> > >
> > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > ---
> > > Changes in v5:
> > > None.
> > >
> > > Changes in v4:
> > > None.
> > >
> > > Changes in v3:
> > > None.
> > >
> > > Changes in v2:
> > > Change compatible string for spidev node.
> > >
> > > arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 ++++++++++++++
> > > 1 file changed, 14 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > > index 6d05b76c2c7a..0d27b5667b8c 100644
> > > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > > @@ -83,6 +83,20 @@
> > > };
> > > };
> > >
> > > +&dspi2 {
> > > + bus-num = <2>;
> > > + status = "okay";
> > > +
> > > + /* mikroBUS1 */
> > > + spidev@0 {
> > > + compatible = "rohm,dh2228fv";
> >
> > Is the compatible documented?
> >
> > Shawn
> >
> > > + spi-max-frequency = <20000000>;
> > > + fsl,spi-cs-sck-delay = <100>;
> > > + fsl,spi-sck-cs-delay = <100>;
> > > + reg = <0>;
> > > + };
> > > +};
> > > +
> > > &esdhc {
> > > sd-uhs-sdr104;
> > > sd-uhs-sdr50;
> > > --
> > > 2.17.1
> > >
>
> I don't really know what's the status with spidev compatibles. I do
> see other device trees are using this one, I thought it should be
> fine.
My understanding is that every compatible needs to be documented.
Shawn
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS
2020-03-18 0:16 ` [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS Vladimir Oltean
2020-04-20 14:38 ` Shawn Guo
@ 2020-04-20 18:06 ` Geert Uytterhoeven
2020-04-20 18:10 ` Vladimir Oltean
1 sibling, 1 reply; 22+ messages in thread
From: Geert Uytterhoeven @ 2020-04-20 18:06 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Mark Brown, linux-spi, Linux Kernel Mailing List, Shawn Guo,
Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, eha,
Angelo Dureghello, Andrey Smirnov, Gustavo A. R. Silva, weic,
mhosny, michael, peng.ma
Hi Vladimir,
On Wed, Mar 18, 2020 at 1:17 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> For debugging, it is useful to have access to the DSPI controller
> signals. On the reference design board, these are exported to either the
> mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> BRDCFG3[SPI3]).
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Thanks for your patch!
> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> @@ -83,6 +83,20 @@
> };
> };
>
> +&dspi2 {
> + bus-num = <2>;
> + status = "okay";
> +
> + /* mikroBUS1 */
> + spidev@0 {
Please use generic node names, e.g. "dac@0".
> + compatible = "rohm,dh2228fv";
> + spi-max-frequency = <20000000>;
> + fsl,spi-cs-sck-delay = <100>;
> + fsl,spi-sck-cs-delay = <100>;
> + reg = <0>;
> + };
> +};
> +
> &esdhc {
> sd-uhs-sdr104;
> sd-uhs-sdr50;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS
2020-04-20 18:06 ` Geert Uytterhoeven
@ 2020-04-20 18:10 ` Vladimir Oltean
0 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2020-04-20 18:10 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Mark Brown, linux-spi, Linux Kernel Mailing List, Shawn Guo,
Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Esben Haabendal, Angelo Dureghello, Andrey Smirnov,
Gustavo A. R. Silva, Wei Chen, Mohamed Hosny, Michael Walle,
peng.ma
Hi Geert,
On Mon, 20 Apr 2020 at 21:07, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Vladimir,
>
> On Wed, Mar 18, 2020 at 1:17 AM Vladimir Oltean <olteanv@gmail.com> wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > For debugging, it is useful to have access to the DSPI controller
> > signals. On the reference design board, these are exported to either the
> > mikroBUS1 or mikroBUS2 connector (according to the CPLD register
> > BRDCFG3[SPI3]).
> >
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> Thanks for your patch!
>
> > --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
> > @@ -83,6 +83,20 @@
> > };
> > };
> >
> > +&dspi2 {
> > + bus-num = <2>;
> > + status = "okay";
> > +
> > + /* mikroBUS1 */
> > + spidev@0 {
>
> Please use generic node names, e.g. "dac@0".
>
It's not a DAC. It's really an unpopulated pin header. I would have
really liked to have access to that as a char device with the default
board DTS, via spidev. That being said, there are warnings to not use
the "spidev" compatible in device trees. So if what I want is not
possible, I'd rather drop the patch altogether.
> > + compatible = "rohm,dh2228fv";
> > + spi-max-frequency = <20000000>;
> > + fsl,spi-cs-sck-delay = <100>;
> > + fsl,spi-sck-cs-delay = <100>;
> > + reg = <0>;
> > + };
> > +};
> > +
> > &esdhc {
> > sd-uhs-sdr104;
> > sd-uhs-sdr50;
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Thanks,
-Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 00/12] NXP DSPI bugfixes and support for LS1028A
[not found] ` <20200318001603.9650-1-olteanv-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
` (7 preceding siblings ...)
2020-03-18 0:16 ` [PATCH v5 12/12] arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS Vladimir Oltean
@ 2020-03-18 19:03 ` Michael Walle
[not found] ` <d37b6e0f8a35ae61bbfe147cd5809ec2-QKn5cuLxLXY@public.gmane.org>
8 siblings, 1 reply; 22+ messages in thread
From: Michael Walle @ 2020-03-18 19:03 UTC (permalink / raw)
To: Vladimir Oltean
Cc: broonie-DgEjT+Ai2ygdnm+yROfE0A, linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
eha-/iRVSOupHO4, angelo-BIYBQhTR83Y,
andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w,
gustavo-L1vi/lXTdts+Va1GwOuvDg, weic-DDmLM1+adcrQT0dZR+AlfA,
mhosny-DDmLM1+adcrQT0dZR+AlfA, peng.ma-3arQi8VN3Tc
Am 2020-03-18 01:15, schrieb Vladimir Oltean:
> From: Vladimir Oltean <vladimir.oltean-3arQi8VN3Tc@public.gmane.org>
>
> This series addresses a few issues that were missed during the previous
> series "[PATCH 00/12] TCFQ to XSPI migration for NXP DSPI driver", on
> SoCs other than LS1021A and LS1043A. DMA mode has been completely
> broken
> by that series, and XSPI mode never worked on little-endian
> controllers.
>
> Then it introduces support for the LS1028A chip, whose compatible has
> recently been documented here:
>
> https://lore.kernel.org/linux-devicetree/20200218171418.18297-1-michael-QKn5cuLxLXY@public.gmane.org/
>
> The device tree for the LS1028A SoC is extended with DMA channels
> definition, such that even though the default operating mode is XSPI,
> one can simply change DSPI_XSPI_MODE to DSPI_DMA_MODE in the
> devtype_data structure of the driver and use that instead.
>
> Compared to earlier v4, the only change is in patch 05/12 to fix a race
> condition signaled by Michael Walle here:
> https://lkml.org/lkml/2020/3/17/740
>
> I don't expect the "fixes" patches to reach very far down the stable
> pipe, since there has been pretty heavy refactoring in this driver.
>
> For testing, benchmarking and debugging, the mikroBUS connector on the
> LS1028A-RDB is made available via spidev.
Patches 1-11:
Tested-by: Michael Walle <michael-QKn5cuLxLXY@public.gmane.org>
Thanks Vladimir for the great work.
-michael
>
> Vladimir Oltean (12):
> spi: spi-fsl-dspi: Don't access reserved fields in SPI_MCR
> spi: spi-fsl-dspi: Fix little endian access to PUSHR CMD and TXDATA
> spi: spi-fsl-dspi: Fix bits-per-word acceleration in DMA mode
> spi: spi-fsl-dspi: Avoid reading more data than written in EOQ mode
> spi: spi-fsl-dspi: Protect against races on dspi->words_in_flight
> spi: spi-fsl-dspi: Replace interruptible wait queue with a simple
> completion
> spi: spi-fsl-dspi: Avoid NULL pointer in dspi_slave_abort for non-DMA
> mode
> spi: spi-fsl-dspi: Fix interrupt-less DMA mode taking an XSPI code
> path
> spi: spi-fsl-dspi: Move invariant configs out of
> dspi_transfer_one_message
> spi: spi-fsl-dspi: Add support for LS1028A
> arm64: dts: ls1028a: Specify the DMA channels for the DSPI
> controllers
> arm64: dts: ls1028a-rdb: Add a spidev node for the mikroBUS
>
> .../boot/dts/freescale/fsl-ls1028a-rdb.dts | 14 +
> .../arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 6 +
> drivers/spi/spi-fsl-dspi.c | 324 +++++++++---------
> 3 files changed, 182 insertions(+), 162 deletions(-)
^ permalink raw reply [flat|nested] 22+ messages in thread