Change in coreboot[master]: soc/amd/common/block/lpc/spi_dma: Implement SPI DMA functionality

[Furquan Shaikh (Code Review)]

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Eric Peers, Felix Held.
Furquan Shaikh 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 5:

(8 comments)

File src/soc/amd/common/block/lpc/spi_dma.c:

https://review.coreboot.org/c/coreboot/+/56228/comment/f20b04ec_630562c5 
PS5, Line 134: if (spi_dma_has_error())
Print out an error in this case so that it is clear at what address the error occurred? Also, should this clear out the error before returning?

https://review.coreboot.org/c/coreboot/+/56228/comment/bbfe1072_a20609fc 
PS5, Line 144: else
nit: else is not required as the if block above returns.

https://review.coreboot.org/c/coreboot/+/56228/comment/a4105bf7_3fb5aac0 
PS5, Line 173: udelay(10);
Just curious - why this granularity?

https://review.coreboot.org/c/coreboot/+/56228/comment/e5f088c7_5bc15026 
PS5, Line 177: );
Would it be helpful to also dump `stopwatch_duration_usecs()` to see how long transactions are taking?

https://review.coreboot.org/c/coreboot/+/56228/comment/5513197f_165a184a 
PS5, Line 179: transaction.size - transaction.remaining
rdev API expects the function to return < 0 in case of any error. I am guessing if transaction.remaining is non-zero, then there was some kind of error. In that case should this return -1 instead of bytes read. Reason I say this is because I see some callers checking < 0 instead of != exp_size.

https://review.coreboot.org/c/coreboot/+/56228/comment/921df8ea_102a790e 
PS5, Line 185: assert(offset + size < CONFIG_ROM_SIZE);
             : 	assert(size);
Do we really need these checks? rdev API already takes care of this as part of `normalize_and_ok()`

https://review.coreboot.org/c/coreboot/+/56228/comment/386da023_c6de9c6d 
PS5, Line 188: size < LPC_ROM_DMA_MIN_ALIGNMENT
nit: Should this check be moved to can_use_dma() so that all attributes are checked in one place?

https://review.coreboot.org/c/coreboot/+/56228/comment/a0296823_a1e109d3 
PS5, Line 222: /* Internal only registers: RN/CZN+ */
If this is applicable only to certain platforms, I think it would be best to use a Kconfig that enables setting of this bit.