Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37882 )
Change subject: src/drivers/i2c: add i2c driver for Realtek 1308 ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37882/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37882/2//COMMIT_MSG@9 PS2, Line 9: Adding Add
https://review.coreboot.org/c/coreboot/+/37882/2//COMMIT_MSG@10 PS2, Line 10: 1. Please mention the datasheet name and revision you used for implementing this. 2. Did you use some existing code as template?
https://review.coreboot.org/c/coreboot/+/37882/2/src/drivers/i2c/rt1308/chip... File src/drivers/i2c/rt1308/chip.h:
https://review.coreboot.org/c/coreboot/+/37882/2/src/drivers/i2c/rt1308/chip... PS2, Line 26: uint32_t uid; Does the comment match? Please change the variable, as `uid` normally means *unique identifier*. Also, use bool if possible.
https://review.coreboot.org/c/coreboot/+/37882/2/src/drivers/i2c/rt1308/rt13... File src/drivers/i2c/rt1308/rt1308.c:
https://review.coreboot.org/c/coreboot/+/37882/2/src/drivers/i2c/rt1308/rt13... PS2, Line 104: Codec Is *amplifier* and *codec* the same?
-
Paul Menzel (Code Review)