| Message ID | 1397457001-5266-6-git-send-email-21cnbao@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bf83fd6402a856eeb9a22c364c50ccf9bbdf9b17 |
| Headers | show |
On Mon, Apr 14, 2014 at 02:30:01PM +0800, Barry Song wrote: > From: Qipan Li <Qipan.Li@csr.com> > > sometimes t->tx can be equal with t->rx. for example, spidev will make > tx and rx point to spidev->buffer at the same time. currently, for this > case, we map the buffer BIDIRECTION to fix the cache consistency. I've applied this but such usage is out of spec - do we have any drivers doing this in mainline?
2014-04-15 4:06 GMT+08:00 Mark Brown <broonie@kernel.org>: > On Mon, Apr 14, 2014 at 02:30:01PM +0800, Barry Song wrote: >> From: Qipan Li <Qipan.Li@csr.com> >> >> sometimes t->tx can be equal with t->rx. for example, spidev will make >> tx and rx point to spidev->buffer at the same time. currently, for this >> case, we map the buffer BIDIRECTION to fix the cache consistency. > > I've applied this but such usage is out of spec - do we have any drivers > doing this in mainline? i felt strange too at the first look from internal gerrit. qipan told me there is one: drivers/spi/spidev.c static int spidev_message(struct spidev_data *spidev, struct spi_ioc_transfer *u_xfers, unsigned n_xfers) { buf = spidev->buffer; .. k_tmp->rx_buf = buf; ... k_tmp->tx_buf = buf; ... spi_message_add_tail(k_tmp, &msg); spidev_sync(spidev, &msg); } -barry -- 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
On Tue, Apr 15, 2014 at 09:43:24AM +0800, Barry Song wrote: > i felt strange too at the first look from internal gerrit. qipan told > me there is one: > drivers/spi/spidev.c OK, that needs fixing. That code predates anyone doing DMA.
2014-04-15 17:26 GMT+08:00 Mark Brown <broonie@kernel.org>: > On Tue, Apr 15, 2014 at 09:43:24AM +0800, Barry Song wrote: > >> i felt strange too at the first look from internal gerrit. qipan told >> me there is one: >> drivers/spi/spidev.c > > OK, that needs fixing. That code predates anyone doing DMA. if it is out of specification, it seems fixing spidev.c is not enough as spi transfer actually needs an precondition of tx!=rx now. -barry -- 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
On Tue, Apr 15, 2014 at 05:57:39PM +0800, Barry Song wrote: > 2014-04-15 17:26 GMT+08:00 Mark Brown <broonie@kernel.org>: > > OK, that needs fixing. That code predates anyone doing DMA. > if it is out of specification, it seems fixing spidev.c is not enough > as spi transfer actually needs an precondition of tx!=rx now. Well, it's enough to avoid bugs. Adding the validation to make sure nobody introduces the problem again is nice too but isn't as urgent.
diff --git a/drivers/spi/spi-sirf.c b/drivers/spi/spi-sirf.c index 3c12f39..0c039d4 100644 --- a/drivers/spi/spi-sirf.c +++ b/drivers/spi/spi-sirf.c @@ -383,7 +383,8 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t) struct dma_async_tx_descriptor *rx_desc, *tx_desc; sspi->dst_start = dma_map_single(&spi->dev, - sspi->rx, t->len, DMA_FROM_DEVICE); + sspi->rx, t->len, (t->tx_buf != t->rx_buf) ? + DMA_FROM_DEVICE : DMA_BIDIRECTIONAL); rx_desc = dmaengine_prep_slave_single(sspi->rx_chan, sspi->dst_start, t->len, DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT | DMA_CTRL_ACK); @@ -391,7 +392,9 @@ static int spi_sirfsoc_transfer(struct spi_device *spi, struct spi_transfer *t) rx_desc->callback_param = &sspi->rx_done; sspi->src_start = dma_map_single(&spi->dev, - (void *)sspi->tx, t->len, DMA_TO_DEVICE); + (void *)sspi->tx, t->len, + (t->tx_buf != t->rx_buf) ? + DMA_TO_DEVICE : DMA_BIDIRECTIONAL); tx_desc = dmaengine_prep_slave_single(sspi->tx_chan, sspi->src_start, t->len, DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);