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 16:
(2 comments)
Patch Set 16: Code-Review+2
Please be aware that your logic for "config not found" will now lead to a die() instead of continue to boot while not setting up the device at all.
Yes, according to the document,
static inline DEVTREE_CONST void *config_of(const struct device *dev) { if (dev && dev->chip_info) return dev->chip_info;
devtree_die(); }
this is true
Thank you for review
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 231: dev->chip_info;
Would it make sense to use config_of(dev) here?
Yes you are right. It would be better if I use config_of () here. Thanks!
https://review.coreboot.org/c/coreboot/+/44476/13/src/ec/kontron/kempld/kemp... PS13, Line 238: if (!config)
This could be dropped in case of config_of(dev)-usage because this API introduces a devtree_die() in […]
Done