search

eblot / pyftdi

FTDI device driver written in pure Python
Other
509 stars 212 forks source link

Fake tri-state: Move SDA=output after ACK to before I2C write to avoid momentary shorts after ACK during I2C reads #290

Closed henryay1 closed 1 year ago

henryay1 commented 2 years ago

After the master sends the request to read from a slave and after the ACK from the slave, the _send_check_ack function ends with SDA being a HI output. During this period to when the master starts clocking in the data it wants to read, some slaves pull the SDA line low. This would short the SDA output to GND, causing it to draw its max current (16mA for the FT4232). This fix makes it so that if the next action after the ACK is a read, SDA remains as an input during this period of time.

eblot commented 2 years ago

While I understand the issue, I think the proposed patch clutters the implementation, so I think the patch needs to be simplified - shorter arg name, default value when faking tristate is not required, ...

Another point is that some devices, such as FT232H have a tristate mode, so would this feature required if such mode is enabled?

Note that I do not have time at the moment to check whether this patch may bring regressions.

henryay1 commented 2 years ago

I have simplified the implementation. Please take a look.

This feature is not necessary for a device that has an open drain/collector output. There is no current being pulled from the FTDI chip because of that output configuration. Thus, no short can be created.

eblot commented 2 years ago

I have simplified the implementation. Please take a look.

Much more readable, thanks.

This feature is not necessary for a device that has an open drain/collector output. There is no current being pulled from the FTDI chip because of that output configuration. Thus, no short can be created.

So should not be this feature only enable if real tristate mode not available?

henryay1 commented 2 years ago

Sorry, I'm a little confused by the double negative statement.

But, this is only necessary if real tristate mode is not available as the previous code is actively driving SDA HI right after the ACK. Some slaves pull SDA low during this time which would create a short.

With an open-collector/drain output, if you’re setting SDA HI, you’re turning off the internal transistor to allow the pull-up resistor to set the SDA line HI, thus no short is created.

Let me know if you have any more questions.

eblot commented 2 years ago

Let me know if you have any more questions.

Sorry for the misunderstanding, let me rephrase: do we need this feature when using a FT232H? In other words, should we not enable this feature only for -some- devices (the ones that are not Hi-Z capable), not all of them?

henryay1 commented 2 years ago

I took a look at the code a little more and found that simply shifting where cmd.extend(self._clk_lo_data_hi) is called would fix this issue. This greatly simplifies the implementation. See the latest commit.

henryay1 commented 2 years ago

Note for anyone reading this in the future:

In _send_check_ack: There is still a very brief amount of time where the slave may pull SDA low when FTDI's SDA is still outputting a HI right after a byte has been written and right before when we switch FTDI's SDA to an input thus creating a short.

There is no way around this as the slave device may react faster than the FTDI device and commands. This is a limitation of FTDI devices without an open-collect/drain output.

c0d3z3r0 commented 2 years ago

some slaves pull the SDA line low

can you name any specific slaves?

c0d3z3r0 commented 2 years ago

related: #314

henryay1 commented 2 years ago

can you name any specific slaves?

I've seen it on the following parts: ICP-10111, ICP-20100, TMD3719.

eblot commented 1 year ago

Thanks and sorry for the very long delay to consider this PR.

I do not have time to test it with a real device though.