Attention is currently required from: Daniel Kurtz, Kevin Chang, Paul Menzel. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56100 )
Change subject: grunt/treeya: add Realtek ALC5682 codec support ......................................................................
Patch Set 4:
(2 comments)
File src/mainboard/google/kahlee/variants/treeya/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/56100/comment/64e07a42_4c217256 PS4, Line 126: 1a
here you are explicitly configuring the Audio codec i2c address 0x1a (which is the same as da7219 ab […]
Unfortunately, that's not a simple request. To use a #define here, the definition needs to be in a header file that's included by the chip.h file of the section that it's in. In the da719 section above, we could add a da719.h file containing that macro into a drivers/i2c/da719 directory and include it directly from there into the chip.h file.
In this case, because generic is being used, there's really no good file that it can be easily included into unless we create a header to just add random generic information to. Not my preference.
I'd probably recommend we just leave this location as 1a.
File src/soc/amd/stoneyridge/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/56100/comment/b196e74f_1cb8e558 PS4, Line 17: #define I2C_DEVICE_PATH 0x1a
No, this board-specific audio component specific I2C address does not belong here in general SoC con […]
I think this should be something like:
#define RT58_I2C_ADDRESS 0x1a
I'd probably just put this into the audio.c file.
-
Martin Roth (Code Review)