Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44476 )
Change subject: ec/kontron/kempld: add option to configure I2C frequency ......................................................................
Patch Set 14:
(4 comments)
Thanks for the review!
https://review.coreboot.org/c/coreboot/+/44476/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44476/13//COMMIT_MSG@9 PS13, Line 9: _khz
Needs slight update.
Done
https://review.coreboot.org/c/coreboot/+/44476/13/src/ec/kontron/kempld/chip... File src/ec/kontron/kempld/chip.h:
https://review.coreboot.org/c/coreboot/+/44476/13/src/ec/kontron/kempld/chip... PS13, Line 19: I2C_FREQ_MAX = 2700,
Nit, technically these should have a KEMPLD_ prefix. All the `chip.h` are […]
Done
https://review.coreboot.org/c/coreboot/+/44476/13/src/ec/kontron/kempld/kemp... File src/ec/kontron/kempld/kempld_i2c.c:
https://review.coreboot.org/c/coreboot/+/44476/13/src/ec/kontron/kempld/kemp... PS13, Line 254: BIOS_WARNING
Warning is a bit too high, from `commonlib/loglevel.h`: […]
In that case, is BIOS_INFO okay?
https://review.coreboot.org/c/coreboot/+/44476/13/src/ec/kontron/kempld/kemp... PS13, Line 256: config->i2c_frequency);
Nit, while not mandated, it would be nice to have braces around the branches […]
Done