Message ID | 1274267100l.1747l.1l@i-dmzi_al.realan.de |
---|---|
State | New, archived |
Headers | show |
On Wed, 19 May 2010 13:05:00 +0200 Anders Larsen <al@alarsen.net> wrote: > On 2010-04-22 00:24:10, Andrew Morton wrote: > > Finally.. Wouldn't it be better to just fix the atmel SPI driver so > > that it doesn't barf when handed vmalloc'ed memory? Who do we ridicule > > about that? <checks, adds cc> > > You mean something like this instead? That looks simple enough. How do we get it tested, changelogged and merged up? Haavard, can you please take a look? > diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c > index c4e0442..a9ad5e8 100644 > --- a/drivers/spi/atmel_spi.c > +++ b/drivers/spi/atmel_spi.c > @@ -352,16 +352,30 @@ atmel_spi_dma_map_xfer(struct atmel_spi *as, struct spi_transfer *xfer) > > xfer->tx_dma = xfer->rx_dma = INVALID_DMA_ADDRESS; > if (xfer->tx_buf) { > - xfer->tx_dma = dma_map_single(dev, > - (void *) xfer->tx_buf, xfer->len, > - DMA_TO_DEVICE); > + if (is_vmalloc_addr(xfer->tx_buf)) > + xfer->tx_dma = dma_map_page(dev, > + vmalloc_to_page(xfer->tx_buf), > + (unsigned long)xfer->tx_buf & (PAGE_SIZE-1), > + xfer->len, > + DMA_TO_DEVICE); > + else > + xfer->tx_dma = dma_map_single(dev, > + (void *) xfer->tx_buf, xfer->len, > + DMA_TO_DEVICE); > if (dma_mapping_error(dev, xfer->tx_dma)) > return -ENOMEM; > } > if (xfer->rx_buf) { > - xfer->rx_dma = dma_map_single(dev, > - xfer->rx_buf, xfer->len, > - DMA_FROM_DEVICE); > + if (is_vmalloc_addr(xfer->rx_buf)) > + xfer->rx_dma = dma_map_page(dev, > + vmalloc_to_page(xfer->rx_buf), > + (unsigned long)xfer->rx_buf & (PAGE_SIZE-1), > + xfer->len, > + DMA_FROM_DEVICE); > + else > + xfer->rx_dma = dma_map_single(dev, > + xfer->rx_buf, xfer->len, > + DMA_FROM_DEVICE); > if (dma_mapping_error(dev, xfer->rx_dma)) { > if (xfer->tx_buf) > dma_unmap_single(dev, >
Anders, I just tested one path, the "if (xfer->rx_buf)...", on 2.6.33 plus the at91 patch http://maxim.org.za/AT91RM9200/2.6/2.6.33-at91.patch.gz running on AT91SAM9260. The test case involved doing i/o via the /dev/mtdblock interface -- but this only exercises the rx_buf/vmalloc path -- MTD reads a block into the cache-buf to merge the write data. Not sure that we have any use cases for the tx_buf path using MTD. -Ian On Friday 21 May 2010, Andrew Morton wrote: > On Wed, 19 May 2010 13:05:00 +0200 > > Anders Larsen <al@alarsen.net> wrote: > > On 2010-04-22 00:24:10, Andrew Morton wrote: > > > Finally.. Wouldn't it be better to just fix the atmel SPI > > > driver so that it doesn't barf when handed vmalloc'ed > > > memory? Who do we ridicule about that? <checks, adds cc> > > > > You mean something like this instead? > > That looks simple enough. How do we get it tested, > changelogged and merged up? Haavard, can you please take a > look? > > > diff --git a/drivers/spi/atmel_spi.c > > b/drivers/spi/atmel_spi.c index c4e0442..a9ad5e8 100644 > > --- a/drivers/spi/atmel_spi.c > > +++ b/drivers/spi/atmel_spi.c > > @@ -352,16 +352,30 @@ atmel_spi_dma_map_xfer(struct > > atmel_spi *as, struct spi_transfer *xfer) > > > > xfer->tx_dma = xfer->rx_dma = INVALID_DMA_ADDRESS; > > if (xfer->tx_buf) { > > - xfer->tx_dma = dma_map_single(dev, > > - (void *) xfer->tx_buf, xfer->len, > > - DMA_TO_DEVICE); > > + if (is_vmalloc_addr(xfer->tx_buf)) > > + xfer->tx_dma = dma_map_page(dev, > > + vmalloc_to_page(xfer->tx_buf), > > + (unsigned long)xfer->tx_buf & (PAGE_SIZE-1), > > + xfer->len, > > + DMA_TO_DEVICE); > > + else > > + xfer->tx_dma = dma_map_single(dev, > > + (void *) xfer->tx_buf, xfer->len, > > + DMA_TO_DEVICE); > > if (dma_mapping_error(dev, xfer->tx_dma)) > > return -ENOMEM; > > } > > if (xfer->rx_buf) { > > - xfer->rx_dma = dma_map_single(dev, > > - xfer->rx_buf, xfer->len, > > - DMA_FROM_DEVICE); > > + if (is_vmalloc_addr(xfer->rx_buf)) > > + xfer->rx_dma = dma_map_page(dev, > > + vmalloc_to_page(xfer->rx_buf), > > + (unsigned long)xfer->rx_buf & (PAGE_SIZE-1), > > + xfer->len, > > + DMA_FROM_DEVICE); > > + else > > + xfer->rx_dma = dma_map_single(dev, > > + xfer->rx_buf, xfer->len, > > + DMA_FROM_DEVICE); > > if (dma_mapping_error(dev, xfer->rx_dma)) { > > if (xfer->tx_buf) > > dma_unmap_single(dev,
Andrew Morton <akpm@linux-foundation.org> wrote: > On Wed, 19 May 2010 13:05:00 +0200 > Anders Larsen <al@alarsen.net> wrote: > > > On 2010-04-22 00:24:10, Andrew Morton wrote: > > > Finally.. Wouldn't it be better to just fix the atmel SPI driver so > > > that it doesn't barf when handed vmalloc'ed memory? Who do we ridicule > > > about that? <checks, adds cc> > > > > You mean something like this instead? > > That looks simple enough. How do we get it tested, changelogged and > merged up? Haavard, can you please take a look? Sure. Sorry for the late response; I've been traveling for the last two weeks. Did anyone check what other drivers do to handle this case? Surely this isn't the only driver which supports DMA? > > diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c > > index c4e0442..a9ad5e8 100644 > > --- a/drivers/spi/atmel_spi.c > > +++ b/drivers/spi/atmel_spi.c > > @@ -352,16 +352,30 @@ atmel_spi_dma_map_xfer(struct atmel_spi *as, struct spi_transfer *xfer) > > > > xfer->tx_dma = xfer->rx_dma = INVALID_DMA_ADDRESS; > > if (xfer->tx_buf) { > > - xfer->tx_dma = dma_map_single(dev, > > - (void *) xfer->tx_buf, xfer->len, > > - DMA_TO_DEVICE); > > + if (is_vmalloc_addr(xfer->tx_buf)) > > + xfer->tx_dma = dma_map_page(dev, > > + vmalloc_to_page(xfer->tx_buf), > > + (unsigned long)xfer->tx_buf & (PAGE_SIZE-1), > > + xfer->len, > > + DMA_TO_DEVICE); Ok, this should be fine for small transfers, but what happens if the transfer crosses a page boundary? Are there any guarantees that this will never happen? What callers are passing vmalloc'ed memory in the first place? Ditto for the rx path. Haavard
diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c index c4e0442..a9ad5e8 100644 --- a/drivers/spi/atmel_spi.c +++ b/drivers/spi/atmel_spi.c @@ -352,16 +352,30 @@ atmel_spi_dma_map_xfer(struct atmel_spi *as, struct spi_transfer *xfer) xfer->tx_dma = xfer->rx_dma = INVALID_DMA_ADDRESS; if (xfer->tx_buf) { - xfer->tx_dma = dma_map_single(dev, - (void *) xfer->tx_buf, xfer->len, - DMA_TO_DEVICE); + if (is_vmalloc_addr(xfer->tx_buf)) + xfer->tx_dma = dma_map_page(dev, + vmalloc_to_page(xfer->tx_buf), + (unsigned long)xfer->tx_buf & (PAGE_SIZE-1), + xfer->len, + DMA_TO_DEVICE); + else + xfer->tx_dma = dma_map_single(dev, + (void *) xfer->tx_buf, xfer->len, + DMA_TO_DEVICE); if (dma_mapping_error(dev, xfer->tx_dma)) return -ENOMEM; } if (xfer->rx_buf) { - xfer->rx_dma = dma_map_single(dev, - xfer->rx_buf, xfer->len, - DMA_FROM_DEVICE); + if (is_vmalloc_addr(xfer->rx_buf)) + xfer->rx_dma = dma_map_page(dev, + vmalloc_to_page(xfer->rx_buf), + (unsigned long)xfer->rx_buf & (PAGE_SIZE-1), + xfer->len, + DMA_FROM_DEVICE); + else + xfer->rx_dma = dma_map_single(dev, + xfer->rx_buf, xfer->len, + DMA_FROM_DEVICE); if (dma_mapping_error(dev, xfer->rx_dma)) { if (xfer->tx_buf) dma_unmap_single(dev,