Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29104 )
Change subject: sdm845: Port I2C driver ......................................................................
Patch Set 35:
(5 comments)
Still has some issues that look like merge mistakes. Clean those up and rebase to the front?
https://review.coreboot.org/#/c/29104/35/src/mainboard/google/cheza/mainboar... File src/mainboard/google/cheza/mainboard.c:
https://review.coreboot.org/#/c/29104/35/src/mainboard/google/cheza/mainboar... PS35, Line 46: i2c_init(QUPV3_1_SE4, I2C_SPEED_FAST); nit: Please add a comment on the side explaining what this is (e.g. /* speaker amp */)
https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/i2c.c File src/soc/qualcomm/sdm845/i2c.c:
https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/i2c.c@158 PS35, Line 158: /* If stretch bit is set to 1 master will send repeated start Incorrect comment style, please use
/* * This * one */
or
/* This one */
(or just shorten it, all this needs to say is something like /* stretch means end with repeated start, not stop */)
https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/include/soc... File src/soc/qualcomm/sdm845/include/soc/qupv3_fw_config.h:
https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/include/soc... PS35, Line 20: #include <soc/qcom_qup_se.h> This should not be in this patch, something needs to be cleaned up earlier in the series.
https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/qupv3_fw_co... File src/soc/qualcomm/sdm845/qupv3_fw_config.c:
https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/qupv3_fw_co... PS35, Line 60: ???
https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/spi_qup.c File src/soc/qualcomm/sdm845/spi_qup.c:
https://review.coreboot.org/#/c/29104/35/src/soc/qualcomm/sdm845/spi_qup.c@a... PS35, Line 49: ???