Duncan Laurie 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 7:
(3 comments)
https://review.coreboot.org/c/coreboot/+/36029/7/src/drivers/i2c/rt1011/chip... File src/drivers/i2c/rt1011/chip.h:
https://review.coreboot.org/c/coreboot/+/36029/7/src/drivers/i2c/rt1011/chip... PS7, Line 25: /* ACPI _HID (required) */ Now that this isn't using the generic driver you might consider removing the hid setting and have the driver set it directly, since the kernel will only probe if the string is "10EC1011" the board doesn't actually have the freedom to set the HID. (and doesn't need to look up the magic value)
https://review.coreboot.org/c/coreboot/+/36029/7/src/drivers/i2c/rt1011/rt10... File src/drivers/i2c/rt1011/rt1011.c:
https://review.coreboot.org/c/coreboot/+/36029/7/src/drivers/i2c/rt1011/rt10... PS7, Line 59: nit: empty line
https://review.coreboot.org/c/coreboot/+/36029/7/src/drivers/i2c/rt1011/rt10... PS7, Line 119: CHIP_NAME("Realtek RT1011 Codec").enable_dev CHIP_NAME(x) is just ".name = x," so when expanded this looks a bit odd without a newline.
It compiles and everything, but is easier to follow when split on 2 lines.