Felix Held 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 1:
(3 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 371: as we need to give 9 falling-rising edges
Most simple I2C slave don't interpret a stop condition as a stop condition in the middle of a transf […]
ah, now that makes much more sense. should be added to the comment
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c@3... PS1, Line 371: By this time SCL is high
It's not guaranteed that SCL is high here if the device is clock-stretching. […]
when the bus hardware is really stuck, the code would hang then
https://review.coreboot.org/c/coreboot/+/35725/1/src/device/software_i2c.c@3... PS1, Line 373: while (i++ < RECOVERY_CLK_CNT * 2) {
I think an idiomatic way to write this on top of the existing primitives would be […]
the thing that gets sent between the stop conditions isn't a start condition, since scl is low when sda gets pulled low; for a start condition scl has to be low when sda gets pulled low. the two signals are basically 90 degrees out of phase and so you only get either a series of start conditions or a series of stop conditions (if i don't remember i2c completely wrong; iirc start/restart/stop conditions are encoded by toggling sda when scl is low)