Attention is currently required from: Bao Zheng, Jason Glenesk, Furquan Shaikh, Marshall Dawson, Karthik Ramasubramanian, Karthikeyan Ramasubramanian, Felix Held. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51404 )
Change subject: soc/amd/common: Introduce I2C driver common to all AMD SoCs ......................................................................
Patch Set 1:
(3 comments)
File src/soc/amd/common/block/i2c/i2c.c:
https://review.coreboot.org/c/coreboot/+/51404/comment/d1104b9b_737e4fb5 PS1, Line 60: /* : * Toggle SCL back and forth 9 times under 100KHz. A single read is : * needed after the writes to force the posted write to complete. : */
This code was put in place because an i2c device was writing data to SDA, but we stopped clocking mi […]
The issue is that the system can reset or change power state in the middle of a transaction. This was leaving devices in a bad state, and they were not responsive to standard I2C transactions anymore. If there's a way to reset that device with a GPIO, that's great, but many devices don't have that option.
We could do a cold reset to power the entire system off, then back on, but this code was sufficient to reset the stuck devices.
If someone wants to create a bug to re-investigate this and see if it's even still needed on the ryzen processors, I'd be all for that. It did what we needed it to do for stoney, and got around the stuck devices. That's all I can vouch for.
https://review.coreboot.org/c/coreboot/+/51404/comment/9f561aff_0d9ded9c PS1, Line 72: udelay(4); /* 4usec gets 85KHz for 1 pin, 70KHz for 4 pins */ Note for the future. This value was specific to stoney and should be tested for other platforms.
https://review.coreboot.org/c/coreboot/+/51404/comment/aac0cef5_d47d3925 PS1, Line 80: udelay(4); same as above.
-
Martin Roth (Code Review)