T Michael Turney 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 5:
(7 comments)
https://review.coreboot.org/#/c/31083/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31083/1//COMMIT_MSG@2 PS1, Line 2: RajendraBabu
Add a space between the names?
Ack
https://review.coreboot.org/#/c/31083/1//COMMIT_MSG@7 PS1, Line 7: cheza: configure gpios and gcc clocks for sound
Use the SoC name as prefix?
Ack
https://review.coreboot.org/#/c/31083/1//COMMIT_MSG@9 PS1, Line 9: Configure mi2s, codec reset gpios and enable : GCC LPASS clocks required for audio support.
Please use the fully allowed text width.
Ack
https://review.coreboot.org/#/c/31083/1//COMMIT_MSG@14 PS1, Line 14: kumar
Capital K?
Ack
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 mainb […]
Ack
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.
Ack
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.
I need to better understand the clock interface to make this change. This is deferred for now and function moved to mainboard.c with rest of this code.
-
T Michael Turney (Code Review)