Hey there Heiko, On 6/12/23 06:35, Heiko Schocher wrote:
I have not the deep knowledge of this specific i2c driver, but may also an option is to setint (*deblock)(struct udevice *bus); in static const struct dm_i2c_ops mvtwsi_i2c_ops = { for this driver and do there the stuff needed to deblock the bus, as you have done in twsi_i2c_reinit() in your patch?
I'm not sure that this is a good fit. The comment explaining deblock says this is for situations where "Resetting the I2C master does not help. The only way is to force the bus through a series of transitions to make sure that all slaves are done with the transaction."
Which reads to me like it's for situations where some slave is holding down SDA (making it impossible for us to send a start/stop) and several SCL pulses are required in order to shake it loose.
In this case, the *bus* is okay and the *master* is upset (ironically, I think, because the Realtek just did its own "deblock" equivalent), so a master reset is really all that's required here. We also know the specific state that it runs off to, so it's easy to detect this situation and resolve it.
Currently this deblock function is only used from i2c core in i2c command, but may it makes sense to add in dm_i2c_xfer function in drivers/i2c/i2c-uclass.c a call to i2c_deblock() in case bus is blocked?
I wouldn't object to such a change, but since deblock is an "active" operation that might have side effects, this probably needs a bigger discussion (beyond what I'm trying to do here) since other users might be unhappy about this happening without their explicit say-so.
Thanks for your feedback, Sam
