Change in coreboot[master]: soc/amd/common/block/lpc/spi_dma: Implement SPI DMA functionality
Attention is currently required from: Jason Glenesk, Marshall Dawson, Eric Peers, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56228 )
Change subject: soc/amd/common/block/lpc/spi_dma: Implement SPI DMA functionality ......................................................................
Patch Set 3:
(3 comments)
File src/soc/amd/common/block/lpc/spi_dma.c:
https://review.coreboot.org/c/coreboot/+/56228/comment/d73985a7_6ac0843e PS2, Line 69: if (!IS_ALIGNED((uintptr_t)target, LPC_ROM_DMA_MIN_ALIGNMENT))
given your comments about architectural constraints (RN, CZN+), do we need a check for that as well?
I wanted to avoid specifically checking for the SoC. Otherwise we will need to update the logic for every generation.
https://review.coreboot.org/c/coreboot/+/56228/comment/80142e46_3dc51ef6 PS2, Line 119: if (spi_dma_has_error())
does this need to move above the busy check?
I don't think so. If we encounter an error, the DMA controller is no longer busy.
https://review.coreboot.org/c/coreboot/+/56228/comment/05ccedf2_5eb771ec PS2, Line 158: udelay(10);
why 10? will different spi speeds yield a different number here?
It was just an arbitrary number. Doing very rough some math:
250 bytes / 10us @ 100 MHz (2-2-1) 166.65 bytes / 10us @ 66.6 MHz (2-2-1) 83.325 bytes / 10us @ 33.33 MHz (2-2-1)
The min block size is 64 bytes. From what I've seen we normally do 1k - 64k transactions, so I think 10us should be fine.
-
Raul Rangel (Code Review)