Attention is currently required from: Subrata Banik, Matt DeVillier, Tim Wawrzynczak, Karthik Ramasubramanian. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63209 )
Change subject: device/i2c_bus: Add detect function pointer in i2c_bus_ops ......................................................................
Patch Set 3:
(1 comment)
File src/include/device/i2c_bus.h:
https://review.coreboot.org/c/coreboot/+/63209/comment/d57cc860_34b64376 PS3, Line 14: bool (*detect)(struct device *dev, unsigned int addr);
I think what Julius might have been getting at is should we just use the i2c_simple interface instea […]
I meant we shouldn't have a separate detect() function pointer here. That still requires every driver to implement it even if they all just implement it by forwarding to their transfer() implementation. I'm saying we should tie those together at the generic level so that each individual driver doesn't have to worry about it.
I'm not saying necessarily that you need to use the i2c_simple interface instead, although that's one option (and it the detect is generally useful it's probably a good idea to at least offer it for the i2c_simple interface as well). But you can still have a detect specifically for the bus_operations interface but just writing a helper function like this:
bool i2c_dev_detect(struct device *dev, unsigned int addr) { struct i2c_msg seg = { .flags = 0, .slave = addr, .buf = NULL, .len = 0 }; return dev->ops->ops_i2c_bus->transfer(dev, &seg, 1) == 0; }