-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SDA can be driven high (violates i2c specification) #89
Comments
Assigning @mbanth to bring to your attention - Joe proposes a simple fix for the 1b port implementation, where each line is a separate port. Will need regression testing but looks watertight on a quick inspection. The multibit port implementation is slightly trickier, since we can't make single pins on a port HiZ while others are outputting, but Joe suggests using the internal pullup mode of the port to at least mititate the issue - will probably need a note in documentation/in the release changelog that this behaviour has changed so that customers aren't surprised when the chip starts pulling up, as well as an option to toggle the old behaviour. |
At a meeting on 15 January 2024, @andrewdewhurst, @mbanth, @xross, @timmace-xmos, and @tomblackie-xmos agreed to defer work on this item until the work occurs to incorporate fwk_io's I2C functionality with lib_i2c (Jira AP-241). |
Added proposed fix from xmos#89
Just stumbled across this issue after noticing this irregularity on my scope. Picture one clearly shows SDA being driven high, as documented in the first post. The top 2 traces are digital, the bottom 2 are analogue. Updating the code results in a nice smooth rising edge from the action of the pull up resistor: Would be nice to see this small change merged so that we don't have to maintain a different version of this library ourselves. |
Confirmed. We've forked |
lib_i2c/lib_i2c/src/i2c_master.xc
Line 181 in 648351b
Both I2C bus lines should only ever be driven in open drain mode i.e. either drive low or drive high-z (input). This is the case for SCL but not the case for SDA when driving data on to the bus - it is driven high if the data is high. This is contrary to the specification and could lead to multiple hardware issues. (driving current into pullup supply voltage, multi voltage compatibility issues etc.)
Instead of:
p_sda <: data & 0x1;
Correct function here would be:
if (data & 1)
p_sda :> void;
else
p_sda <: 0;
I have made this change and done quick testing with success.
A similar change would be needed to the other i2c sources e.g. i2c_slave.xc and i2c_master_async.xc etc.
The text was updated successfully, but these errors were encountered: