[coreboot-gerrit] Change in coreboot[master]: soc/mediatek/mt8192: Add SPI flash controller DMA read function

[Hung-Te Lin (Code Review)]

Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44851 )

Change subject: soc/mediatek/mt8192: Add SPI flash controller DMA read function
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/44851/1/src/soc/mediatek/mt8192/fla... 
File src/soc/mediatek/mt8192/flash_controller.c:

https://review.coreboot.org/c/coreboot/+/44851/1/src/soc/mediatek/mt8192/fla... 
PS1, Line 142: 	size_t done = 0;
             : 	uintptr_t dma_buf = (uintptr_t)_dma_coherent;
             : 	size_t dma_buf_len = REGION_SIZE(dma_coherent);
             : 	u32 next;
             : 
             : 	if (!IS_ALIGNED((uintptr_t)addr, SFLASH_DMA_ALIGN)) {
             : 		next = MIN(ALIGN_UP((uintptr_t)addr, SFLASH_DMA_ALIGN) -
             : 			   (uintptr_t)addr, len);
             : 		if (pio_read(addr, buf, next))
             : 			return -1;
             : 		done += next;
             : 	}
             : 
             : 	while (len - done >= SFLASH_DMA_ALIGN) {
             : 		next = MIN(dma_buf_len, ALIGN_DOWN(len - done,
             : 			   SFLASH_DMA_ALIGN));
             : 		if (dma_read(addr + done, buf + done, next, dma_buf,
             : 			     dma_buf_len))
             : 			return -1;
             : 		done += next;
             : 	}
             : 	next = len - done;
             : 	if (next > 0 && pio_read(addr + done, buf + done, next))
             : 		return -1;
can we replace this by purely DMA calls, eg:

u32 start = addr - addr % SFLASH_DMA_ALIGN;
 u32 skip = addr - start;
 u32 total = ALIGN_UP(skip + len, SFLASH_DMA_ALIGN);
 u32 drop = total - skip - len;

size_t done = 0;
 u32 next = start;

/* DMA: [ skip | len | drop ] */

for (; done < total; ) {
    read_len = MIN(dma_buf_len, total - done);
    dma_read(start + done, read_len);
    done += read_len;

// decide the range to copy into buffer
    if (done == total)
       read_len -= drop;

memcpy(buf, dma_buf + skip, read_len - skip);
    skip = 0;
 }