Cheng-Yi Chiang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36029 )
Change subject: drivers/i2c/rt1011: Add a driver for RT1011 ......................................................................
Patch Set 6:
(5 comments)
Hi Paul, Thanks for reviewing the patch. I have run clang-format on this patch. Also added more explanation to the commit message. Please let me know if you have other concern. Thanks!
https://review.coreboot.org/c/coreboot/+/36029/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36029/5//COMMIT_MSG@8 PS5, Line 8:
Oh, and please add the reasoning, why this driver is needed in coreboot anyway, and is not enough in […]
Done
https://review.coreboot.org/c/coreboot/+/36029/5//COMMIT_MSG@11 PS5, Line 11: setup
Verb is spelled with a space: set up.
Done
https://review.coreboot.org/c/coreboot/+/36029/5//COMMIT_MSG@9 PS5, Line 9: Set calibration data in device property for RT1011 when config : CHROMEOS_DSM_CALIB is turned on. : Kernel rt1011 codec driver will use these device properties to setup : codec accordingly.
Please use the full text width (75 characters) and separate paragraphs by a blank line.
Done
https://review.coreboot.org/c/coreboot/+/36029/5//COMMIT_MSG@13 PS5, Line 13:
What datasheet (name, revision) did you use? Is it public?
Done
https://review.coreboot.org/c/coreboot/+/36029/5/src/drivers/i2c/rt1011/rt10... File src/drivers/i2c/rt1011/rt1011.c:
https://review.coreboot.org/c/coreboot/+/36029/5/src/drivers/i2c/rt1011/rt10... PS5, Line 69: printk(BIOS_ERR, "cannot get dsm_calib from VPD\n");
Error messages should be more elaborate.
Done