Skip to content
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

[TW#14953] Feature request: I2C clear bus function in driver. #922

Closed
PerMalmberg opened this issue Aug 20, 2017 · 4 comments
Closed

[TW#14953] Feature request: I2C clear bus function in driver. #922

PerMalmberg opened this issue Aug 20, 2017 · 4 comments
Assignees

Comments

@PerMalmberg
Copy link
Contributor

Quote from the I2C-bus specification and user manual, page 20, §3.1.16

If the data line (SDA) is stuck LOW, the master should send nine clock pulses. The device
that held the bus LOW should release it sometime within those nine clocks.

Would it be possible to have this implemented in the driver? It'd be a much more elegant solution than having to de-initialize, setup GPIO, generate clock pulses, the reinitialize the I2C driver.

I imagine having a signature as follows, so that it is possible to clear each bus separately:

bool i2c_clear_bus(i2c_port_t port_num, i2c_cmd_handle_t cmd_handle)
@CarlosGS
Copy link

Related to espressif/arduino-esp32#349

@FayeY FayeY changed the title Feature request: I2C clear bus function in driver. [TW#14953] Feature request: I2C clear bus function in driver. Aug 25, 2017
@costaud
Copy link
Collaborator

costaud commented Oct 18, 2017

If, only the data wire is held by the slave, the master can still send some data to the bus(although the sda won't change), so that there will be clock on scl wire to release the slave? Is it correct?

@costaud
Copy link
Collaborator

costaud commented Oct 19, 2017

OK, according to my test result, if the SDA is stuck, master can not send clock out, but the code can be simple like this to switch signal, generate clock on SCL, then set back the signals.

    gpio_set_direction(19, GPIO_MODE_OUTPUT); //scl
    gpio_set_direction(18, GPIO_MODE_OUTPUT); //sda
    for (int i = 0;i < 9; i++) {
        gpio_set_level(19, 1);
        gpio_set_level(19, 0);
    }
    i2c_set_pin(i2c_num, 18, 19, 1, 1, I2C_MODE_MASTER);

@costaud costaud self-assigned this Oct 19, 2017
igrr pushed a commit that referenced this issue Oct 23, 2017
Reported from different sources from github or bbs:

#680

#922

We tested reading several sensor or other I2C slave devices, if the power and SDA/SCL wires are in proper condition, everything works find with reading the slave.
If we remove the power supply for the slave during I2C is reading, or directly connect SDA or SCL to ground, this would  cause the I2C FSM get stuck in wrong state, all we can do is the reset the I2C hardware in this case.
After this commit, no matter whether the power supply of I2C slave is removed or SDA / SCL are shorted to ground, the driver can recover from wrong state.

We are not sure whether this the save issue with the reported one yet, but to make the driver more robust.

Further information:

1. For I2C master mode, we have tested different situations, e.g., to short the SDA/SCL directly to GND/VCC, to short the SDA to SCL, to un-plug the slave device, to power off the slave device. Under all of those situations, this version of driver can recover and keep working.
2. Some slave device will die by accident and keep the SDA in low level, in this case, master should send several clock to make the slave release the bus.
3. Slave mode of ESP32 might also get in wrong state that held the SDA low, in this case, master device could send a stop signal to make esp32 slave release the bus.

Modifications:

1. Disable I2C_MASTER_TRAN_COMP interrupt to void extra interrupt.
2. Disable un-used timeout interrupt for slave.
3. Add bus reset if error detected for master mode.
4. Add bus clear if SDA level is low when error detected.
5. Modify the argument type of i2c_set_pin.
6. add API to set timeout value
7. add parameter check for timing APIs
@PerMalmberg
Copy link
Contributor Author

Oh, I see we now have a i2c_master_clear_bus(). Awesome, @igrr :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants