Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35725 )
Change subject: device/software_i2c: Add function to recover bus ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c File src/device/software_i2c.c:
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c@3... PS1, Line 360: If we can set SDA
when using software i2c, we can always set sda. […]
Done
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c@3... PS1, Line 362: Check the 'incomplete_write_byte' fault injector : * for details.
this probably doesn't apply to coreboot, right?
Done
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c@3... PS1, Line 368: wait(bus);
You only need these if there's a risk that our side would leave the bus in a wedged state, which sho […]
The software controller connected to this pulls both lines low (the Aspeed AST). I can add the same code in the caller, but having it here seems to best way to make sure that the bus is in a valid and known state when the functions returns.
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c@3... PS1, Line 371: as we need to give 9 falling-rising edges
ah, now that makes much more sense. […]
Done
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c@3... PS1, Line 371: By this time SCL is high
Right, I meant wait with a timeout (like the existing functions using wait_for_scl() already do). […]
what's here to do?
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c@3... PS1, Line 373: while (i++ < RECOVERY_CLK_CNT * 2) {
sending random start conditions might also make some devices unhappy; what the kernel does here seem […]
Done