Commit 0b6bfad4 authored by Serge Semin's avatar Serge Semin Committed by Mark Brown

spi: spi-dw: Remove extraneous locking

There is no point in having the commit 19b61392 ("spi: spi-dw: Add
lock protect dw_spi rx/tx to prevent concurrent calls") applied. The
commit author made an assumption that the problem with the rx data
mismatch was due to the lack of the data protection. While most likely it
was caused by the lack of the memory barrier. So having the
commit bfda0445 ("spi: dw: use "smp_mb()" to avoid sending spi data
error") applied would be enough to fix the problem.

Indeed the spin unlock operation makes sure each memory operation issued
before the release will be completed before it's completed. In other words
it works as an implicit one way memory barrier. So having both smp_mb()
and the spin_unlock_irqrestore() here is just redundant. One of them would
be enough. It's better to leave the smp_mb() since the Tx/Rx buffers
consistency is provided by the data transfer algorithm implementation:
first we initialize the buffers pointers, then make sure the assignments
are visible by the other CPUs by calling the smp_mb(), only after that
enable the interrupt, which handler uses the buffers.
Signed-off-by: default avatarSerge Semin <Sergey.Semin@baikalelectronics.ru>
Link: https://lore.kernel.org/r/20200920112914.26501-5-Sergey.Semin@baikalelectronics.ruSigned-off-by: default avatarMark Brown <broonie@kernel.org>
parent ffb7ca54
...@@ -141,11 +141,9 @@ static inline u32 rx_max(struct dw_spi *dws) ...@@ -141,11 +141,9 @@ static inline u32 rx_max(struct dw_spi *dws)
static void dw_writer(struct dw_spi *dws) static void dw_writer(struct dw_spi *dws)
{ {
u32 max; u32 max = tx_max(dws);
u16 txw = 0; u16 txw = 0;
spin_lock(&dws->buf_lock);
max = tx_max(dws);
while (max--) { while (max--) {
/* Set the tx word if the transfer's original "tx" is not null */ /* Set the tx word if the transfer's original "tx" is not null */
if (dws->tx_end - dws->len) { if (dws->tx_end - dws->len) {
...@@ -157,16 +155,13 @@ static void dw_writer(struct dw_spi *dws) ...@@ -157,16 +155,13 @@ static void dw_writer(struct dw_spi *dws)
dw_write_io_reg(dws, DW_SPI_DR, txw); dw_write_io_reg(dws, DW_SPI_DR, txw);
dws->tx += dws->n_bytes; dws->tx += dws->n_bytes;
} }
spin_unlock(&dws->buf_lock);
} }
static void dw_reader(struct dw_spi *dws) static void dw_reader(struct dw_spi *dws)
{ {
u32 max; u32 max = rx_max(dws);
u16 rxw; u16 rxw;
spin_lock(&dws->buf_lock);
max = rx_max(dws);
while (max--) { while (max--) {
rxw = dw_read_io_reg(dws, DW_SPI_DR); rxw = dw_read_io_reg(dws, DW_SPI_DR);
/* Care rx only if the transfer's original "rx" is not null */ /* Care rx only if the transfer's original "rx" is not null */
...@@ -178,7 +173,6 @@ static void dw_reader(struct dw_spi *dws) ...@@ -178,7 +173,6 @@ static void dw_reader(struct dw_spi *dws)
} }
dws->rx += dws->n_bytes; dws->rx += dws->n_bytes;
} }
spin_unlock(&dws->buf_lock);
} }
static void int_error_stop(struct dw_spi *dws, const char *msg) static void int_error_stop(struct dw_spi *dws, const char *msg)
...@@ -294,21 +288,18 @@ static int dw_spi_transfer_one(struct spi_controller *master, ...@@ -294,21 +288,18 @@ static int dw_spi_transfer_one(struct spi_controller *master,
{ {
struct dw_spi *dws = spi_controller_get_devdata(master); struct dw_spi *dws = spi_controller_get_devdata(master);
struct chip_data *chip = spi_get_ctldata(spi); struct chip_data *chip = spi_get_ctldata(spi);
unsigned long flags;
u8 imask = 0; u8 imask = 0;
u16 txlevel = 0; u16 txlevel = 0;
u32 cr0; u32 cr0;
int ret; int ret;
dws->dma_mapped = 0; dws->dma_mapped = 0;
spin_lock_irqsave(&dws->buf_lock, flags);
dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE); dws->n_bytes = DIV_ROUND_UP(transfer->bits_per_word, BITS_PER_BYTE);
dws->tx = (void *)transfer->tx_buf; dws->tx = (void *)transfer->tx_buf;
dws->tx_end = dws->tx + transfer->len; dws->tx_end = dws->tx + transfer->len;
dws->rx = transfer->rx_buf; dws->rx = transfer->rx_buf;
dws->rx_end = dws->rx + transfer->len; dws->rx_end = dws->rx + transfer->len;
dws->len = transfer->len; dws->len = transfer->len;
spin_unlock_irqrestore(&dws->buf_lock, flags);
/* Ensure dw->rx and dw->rx_end are visible */ /* Ensure dw->rx and dw->rx_end are visible */
smp_mb(); smp_mb();
...@@ -466,7 +457,6 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) ...@@ -466,7 +457,6 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
dws->master = master; dws->master = master;
dws->dma_addr = (dma_addr_t)(dws->paddr + DW_SPI_DR); dws->dma_addr = (dma_addr_t)(dws->paddr + DW_SPI_DR);
spin_lock_init(&dws->buf_lock);
spi_controller_set_devdata(master, dws); spi_controller_set_devdata(master, dws);
......
...@@ -143,7 +143,6 @@ struct dw_spi { ...@@ -143,7 +143,6 @@ struct dw_spi {
size_t len; size_t len;
void *tx; void *tx;
void *tx_end; void *tx_end;
spinlock_t buf_lock;
void *rx; void *rx;
void *rx_end; void *rx_end;
int dma_mapped; int dma_mapped;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment