Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47235 )
Change subject: drivers/i2c/rx6110sa: Add basic ACPI support ......................................................................
Patch Set 5: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/47235/3/src/drivers/i2c/rx6110sa/rx... File src/drivers/i2c/rx6110sa/rx6110sa.c:
https://review.coreboot.org/c/coreboot/+/47235/3/src/drivers/i2c/rx6110sa/rx... PS3, Line 180: .speed = config->bus_speed ? : I2C_SPEED_FAST,
Maybe add some code somewhere to throw a warning if it's an unexpected value and fall back to slow?
Using an enum is a good idea, too
https://review.coreboot.org/c/coreboot/+/47235/5/src/drivers/i2c/rx6110sa/rx... File src/drivers/i2c/rx6110sa/rx6110sa.c:
https://review.coreboot.org/c/coreboot/+/47235/5/src/drivers/i2c/rx6110sa/rx... PS5, Line 176: !dev->enabled CB:47148 made this check unnecessary
https://review.coreboot.org/c/coreboot/+/47235/5/src/drivers/i2c/rx6110sa/rx... PS5, Line 215: BIOS_INFO nit: I'd use BIOS_DEBUG