Hi Arnd,
thanks for your review!
Am 29.06.2022 um 18:06 schrieb Arnd Bergmann:
On Wed, Jun 29, 2022 at 3:16 AM Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
Use dma_map_single() for a2091 driver (leave bounce buffer
logic unchanged).
Use dma_set_mask_and_coherent() to avoid explicit cache
flushes.
Compile-tested only.
CC: linux-scsi@xxxxxxxxxxxxxxx
Link: https://lore.kernel.org/r/6d1d88ee-1cf6-c735-1e6d-bafd2096e322@xxxxxxxxx
Signed-off-by: Michael Schmitz <schmitzmic@xxxxxxxxx>
Reviewed-by: Arnd Bergmann <arnd@xxxxxxxx>
+
+ addr = dma_map_single(hdata->dev, scsi_pointer->ptr,
+ len, DMA_DIR(dir_in));
+ if (dma_mapping_error(hdata->dev, addr)) {
+ dev_warn(hdata->dev, "cannot map SCSI data block %p\n",
+ scsi_pointer->ptr);
+ return 1;
+ }
+ scsi_pointer->dma_handle = addr;
/* don't allow DMA if the physical address is bad */
if (addr & A2091_XFER_MASK) {
+ /* drop useless mapping */
+ dma_unmap_single(hdata->dev, scsi_pointer->dma_handle,
+ scsi_pointer->this_residual,
+ DMA_DIR(dir_in));
I think you could save the extra map/unmap here if you wanted, but that
would risk introducing bugs since it requires a larger rework.
Not sure how to do that ...
+ scsi_pointer->dma_handle = (dma_addr_t) NULL;
+
wh->dma_bounce_len = (scsi_pointer->this_residual + 511) & ~0x1ff;
wh->dma_bounce_buffer = kmalloc(wh->dma_bounce_len,
GFP_KERNEL);
Not your bug, but if there is memory above the A2091_XFER_MASK limit,
this would need to use GFP_DMA instead of GFP_KERNEL.
Same argument goes for gvp11 - though last time I checked (and certainly
at the time these drivers were written), there really was no difference
between using GFP_DMA and GFP_KERNEL on m68k, hence the need to check
the allocated buffer is indeed suitable for DMA, and perhaps revert to
chip RAM (slow, useless for most other purposes but definitely below 16
MB) if that fails (as the gvp11 driver does).
Most users will opt for loading the kernel to a RAM chunk at a higher
physical address, making the lower chunk unusable (except as chip RAM),
meaning allocating a bounce buffer within the first 16 MB will most
likely fail anyway AIUI (but Geert would know that for sure).
Cheers,
Michael
Arnd
