Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42095 )
Change subject: sc7180: Remove QcLib specific changes from CB UART ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42095/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42095/1//COMMIT_MSG@20 PS1, Line 20:
This change is w.r.t the bug https://partnerissuetracker.corp.google. […]
Ack
https://review.coreboot.org/c/coreboot/+/42095/1/src/soc/qualcomm/sc7180/qup... File src/soc/qualcomm/sc7180/qupv3_config.c:
https://review.coreboot.org/c/coreboot/+/42095/1/src/soc/qualcomm/sc7180/qup... PS1, Line 55: if (protocol != SE_PROTOCOL_UART) {
The source clock is assumed as 19.2MHz, and for this no need to call clock_configure_qup(). […]
I thought all the other QUPs (SPI and I2C) are using DFSR so their frequency settings are implicitly initialized by clock_configure_dfsr(). Is that not how it works? I thought because 19.2MHz is the first entry in the DFSR array that would somehow implicitly make it use that as the base clock if you don't set anything else? (To be fair, I never quite understood the DFSR stuff as well as I probably should, I would appreciate more explanations on that.)
So if the other QUPs get their frequency set by that, then the UART QUP should also have its frequency set explicitly somehow.