Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27483 )
Change subject: sdm845: Add SPI QUP driver ......................................................................
Patch Set 58:
(1 comment)
https://review.coreboot.org/c/coreboot/+/27483/42/src/soc/qualcomm/sdm845/sp... File src/soc/qualcomm/sdm845/spi_qup.c:
https://review.coreboot.org/c/coreboot/+/27483/42/src/soc/qualcomm/sdm845/sp... PS42, Line 229: write32(&qup[se_bus].regs->geni_m_irq_clear, m_irq);
Made changes in this driver to clear interrupt only after handling it, because recently we ran into […]
I see... sounds like your hardware works different than most I have seen. Normally, I would expect that the hardware takes the reading of the FIFO register itself (geni_rx_fifon) as a signal that there's space in the FIFO again now and it can start fetching more bytes. In that case you want to clear the interrupt before reading the FIFO register, because otherwise it may be possible that after you read the FIFO register but before you cleared the interrupt the hardware would already have fetched new data into the FIFO. It would not assert the interrupt again for that new data (because it is still set), then software advances to the point where it clears the interrupt, and then software doesn't know that there's still data stuck in the FIFO (because it won't assert another interrupt) and might get hung.
In this case, if the hardware really takes the clearing of the interrupt rather than the draining of the FIFO as the signal to start fetching more data, your previous code was correct (first drain FIFO, then clear interrupt). Sorry for leading you the wrong way. (Then again, this would be much easier for me if I had access to documentation describing these sorts of hardware behavior details...)