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, Karthik Ramasubramanian, 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 4:
(3 comments)
File src/soc/amd/common/block/include/amdblocks/lpc.h:
https://review.coreboot.org/c/coreboot/+/56228/comment/40650366_25a58a5a PS3, Line 121: LPC_ROM_DMA_EC_HOST_CONTROL 0xb8
PPR 55570 has this register definition. […]
I mostly used the stoney BKDG, but the registers I used were apart of the picasso PPR. I had Felix verify the registers on Cezanne and they were present.
File src/soc/amd/common/block/lpc/spi_dma.c:
https://review.coreboot.org/c/coreboot/+/56228/comment/56be0795_d3b62520 PS3, Line 103: ctrl |= LPC_ROM_DMA_CTRL_ERROR; /* Clear error */ I was using the stoney BKDG since it provides more details:
DmaErrorStatus. Read; Write-1-to-clear. Reset: 0. 1=Previous transfer has error. 0=Previous transfer has completed successfully.
I've never actually encountered an error so I haven't validated the behavior.
I would prefer to set it since it won't cause any problems if it is read only, but it will clear the error if it isn't.
Thoughts?
https://review.coreboot.org/c/coreboot/+/56228/comment/4c8ed17a_1ab752aa PS3, Line 209: val |= BIT(6);
Is this the magic bit that needs to be set? A macro for that bit? What does this bit do to make the […]
Yeah this is the magic bit: b/179699789#comment11 has the explanation. I didn't add a macro since it's an internal only register. I also didn't link the bug here since this is public code.
-
Raul Rangel (Code Review)