Page MenuHomeFreeBSD

gpioiic: never drive lines active high
ClosedPublic

Authored by avg on Jul 16 2020, 8:26 AM.

Details

Summary

I2C communication is done by a combination of driving a line low or letting
it float, so that it is either pulled up or driven low by another party.

rS355276 besides the stated goal of the change -- using the new GPIO API --
also change the logic, so that active state is signaled by actively driving
a line.

That worked with iicbb prior to rS362042, but stopped working after that
commit on at least some hardware. My guess that the breakage was related to
getting an ACK bit. A device expected to be able to drive SDA actively low,
but controller was actively driving it high for some time.

Anyway, this change seems to fix the problem.

Tested using gpioiic on Orange Pi PC Plus with HTU21 sensor.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

avg requested review of this revision.Jul 16 2020, 8:26 AM

Also, I think that it did not make much sense to preset a pin to low before re-configuring it as output.
If the pin's initial state would be low then the preset was a NOP.
If the pin's initial state would be high, then that should be not an issue as its previous state would be kept just a for bit longer before we changed it to low.
But not sure.

This revision is now accepted and ready to land.Jul 16 2020, 10:02 AM
In D25684#568391, @avg wrote:

Also, I think that it did not make much sense to preset a pin to low before re-configuring it as output.
If the pin's initial state would be low then the preset was a NOP.
If the pin's initial state would be high, then that should be not an issue as its previous state would be kept just a for bit longer before we changed it to low.
But not sure.

The idea is avoid unwanted changes in the pin state. And also, this is a safety measure as the pin can be fiddled from userland.
One could set the output to high and when the pin is set to output you would get an unwanted state set until you set the pin back to low.
Sometimes this is harmless, sometimes it is not.
I prefer to always set the pin value before setting the pin to output mode, but as noted in the comment, this is not always supported.
With I2C, the only issue is having the data pin set to high eventually, the data would not be affected as it is synced by the clock pin.
Thanks to find this bug.

This revision was automatically updated to reflect the committed changes.