Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/27483 )
Change subject: sdm845: Add SPI QUP driver ......................................................................
Patch Set 51:
(4 comments)
Thanks, this looks good now... but it is interdependent with the I2C patch. This patch adds the underlying qcom_qup_se.c stuff, but the I2C patch adds qup_isr_handle() which this patch relies on, so they don't depend on each other. Patches always need to be atomic, so please pull all the shared stuff from the I2C patch into this one.
https://review.coreboot.org/#/c/27483/51/src/mainboard/google/cheza/bootbloc... File src/mainboard/google/cheza/bootblock.c:
https://review.coreboot.org/#/c/27483/51/src/mainboard/google/cheza/bootbloc... PS51, Line 19: #include <soc/qcom_qup_se.h> nit: please order alphabetically
https://review.coreboot.org/#/c/27483/51/src/soc/qualcomm/sdm845/qspi.c File src/soc/qualcomm/sdm845/qspi.c:
https://review.coreboot.org/#/c/27483/51/src/soc/qualcomm/sdm845/qspi.c@19 PS51, Line 19: arch No, this is supposed to be <device/mmio.h>
https://review.coreboot.org/#/c/27483/51/src/soc/qualcomm/sdm845/spi_qup.c File src/soc/qualcomm/sdm845/spi_qup.c:
https://review.coreboot.org/#/c/27483/51/src/soc/qualcomm/sdm845/spi_qup.c@1... PS51, Line 150: /n" I think this is supposed to be a \n?
https://review.coreboot.org/#/c/27483/51/src/soc/qualcomm/sdm845/spi_qup.c@1... PS51, Line 153: } You should actually return an error value here which you can then pass out of qup_spi_claim_bus().
-
Julius Werner (Code Review)