Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31083 )
Change subject: cheza: configure gpios and gcc clocks for sound ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/#/c/31083/1/src/soc/qualcomm/sdm845/bootblock.c File src/soc/qualcomm/sdm845/bootblock.c:
https://review.coreboot.org/#/c/31083/1/src/soc/qualcomm/sdm845/bootblock.c@... PS1, Line 29: sdm845_sound_init(); Any specific reason this must be done in the bootblock? If not, please do it in ramstage (from mainboard.c, since it's board-specific whether we actually want it).
https://review.coreboot.org/#/c/31083/1/src/soc/qualcomm/sdm845/sound.c File src/soc/qualcomm/sdm845/sound.c:
https://review.coreboot.org/#/c/31083/1/src/soc/qualcomm/sdm845/sound.c@30 PS1, Line 30: GPIO_PULL_DOWN, GPIO_2MA, GPIO_OUTPUT); Which I2S bus to use is board-specific, so this function has to go into mainboard.c.
https://review.coreboot.org/#/c/31083/1/src/soc/qualcomm/sdm845/sound.c@46 PS1, Line 46: write32((void *)GCC_LPASS_Q6_AXI_CBCR, 0x80000001); This function should be implemented in clock.c and use the GCC register overlay from there.