Error using non-output pin for SCL in slave mode (IDFGH-269)

[antonio-fiol]

Environment

• Development Kit: ESP32-DevKitC
• Core (if using chip or module): ESP-WROOM32
• IDF version b1fa1ba
• Development Env: Make
• Operating System: Linux on raspberry pi
• Power Supply: USB

Problem Description

Commit 1ad68de (fixing #393) produces an error on the console when using a non-output pin in slave mode for SCL.

E (270) gpio: gpio_set_level(189): GPIO output gpio_num error

It seems that the message may not be critical, but it's really confusing.

Expected Behavior

Able to use pin 34 for SCL in slave mode.

Actual Behavior

The error stated in the description appears on the log.

Steps to repropduce

Modify the i2c sample to use pin 34 on the slave for SCL.

Suggested fix

Only call gpio_set_level if configuring MASTER mode, but not in SLAVE.

[loboris]

AFAIK, i2c slave must be able to drive SCL active low (clock stretching).

[stickbreaker]

@loboris under what circumstances does slave mode drive SCL low? I am working on a SlaveMode driver for the Arduino branch, I need this functionality to match the Arduino Slave protocol.

Chuck.

[antonio-fiol]

I agree that using a pin without output capability will stop me from possibly doing clock stretching from the ESP32 slave.
However, the rest of the code seems to accept the slave mode with SCL on a non-output pin as a valid combination.
In my case, I need a lot of output pins, nearly no inputs, and do not need to stretch the clock at all... so this combination would be really helpful.

[negativekelvin]

@stickbreaker

At any point in the data transfer process, an addressed slave can hold the SCL line low after the master releases it. The master is required to refrain from additional clock pulses or data transfer until such time as the slave releases the SCL line.

https://learn.sparkfun.com/tutorials/i2c

I guess if i2c driver and state machine never uses clk stretching in slave mode it would be ok to use an input only gpio.

[stickbreaker]

@negativekelvin How do I make the ESP32 clock Stretch in Slave Mode? I understand the i2c protocol, I have found no situations under which I can cause the i2c peripheral to clock stretch.

The Arduino i2c protocol, which was based on the capabilities of the AtMega168 automatically started clock stretching as soon as it ack'd the Slave Id, this paused the i2c Master while application code generated the data to be returned. The AtMega168 held SCL low until the driver code released it. During my investigation of the ESP32, there is no delay between the Slave Address Ack and data being set out. If the TX Fifo is empty the ESP32 sends 0xff.

If there is any known procedure to cause the i2c peripheral to instigate a SCL stretching operation, I WOULD LOVE to know it!

Chuck.

[loboris]

The clock will be stretched while processing the i2c interrupt or if the interrupts are disabled.
Look at my PR, in this modified driver more processing is done in interrupt routine and clock stretching may be required at higher speeds.
I think it newer happens in the current driver.

[stickbreaker]

@loboris I'll read through your changes. Just to verify I am understanding you, you have see the i2c peripheral stretch SCL? I played with it a couple of months ago and was never able to get it to stretch.

I'll have to resurrect my test code. During what interrupt are you seeing SCL stretching? I only remember seeing Rx and TX fifo interrupts and Slave_trans_complete during Slave Operations. If the peripheral was in slave Mode, TX_Fifo_empty would trigger until the fifo level meet the thrld. This resulted in stale data. I am hoping to fill the fifo when Slave_id is Ack'd.

I'll look through your code and try to see where the stretching happens, I am using the Arduino base not IDF so it is a little different.

Chuck.

[xiongyumail]

Hi @antonio-fiol

A standard I2C Slaver requires the ability to output SCL, as well as configuring SCL as open-drain output in Master mode. If you don't need this capability, you can modify the I2C code. But it's impractical to make changes on the master branch.

[antonio-fiol]

@xiongyumail I'm just saying:

• this is a regression introduced in commit b1fa1ba
• it won't stop anyone from using SCL on an input pin, but it's very confusing
• it is trivial to fix.

A regression
Besides the fact that the version before that commit does not give you the error, in the same function, you can read that it checks that SDA is a valid OUTPUT GPIO:

    I2C_CHECK(((GPIO_IS_VALID_OUTPUT_GPIO(sda_io_num))), I2C_SDA_IO_ERR_STR, ESP_ERR_INVALID_ARG);

But then it checks that SCL is either a valid OUTPUT GPIO or just a valid GPIO if we're requesting SLAVE mode.

    I2C_CHECK((GPIO_IS_VALID_OUTPUT_GPIO(scl_io_num)) ||
              (GPIO_IS_VALID_GPIO(scl_io_num) && mode == I2C_MODE_SLAVE),
              I2C_SCL_IO_ERR_STR,
              ESP_ERR_INVALID_ARG);

IMO, this proves that the original intention was to allow (restricted, maybe) SCL operation on an input-only pin.

Commit b1fa1ba breaks that scenario: the I2C_CHECK lines allow it, but then it outputs an error message.

Confusing
The message is at ERROR level, internally on gpio_set_level, and without any information to help you understand why. Oh, and actually it does not break anything that worked before.

Trivial fix
It is trivial to fix this issue, without reverting b1fa1ba and without causing the glitches described in the bug it fixes.
The fix is one line:

if( GPIO_IS_VALID_OUTPUT_GPIO(scl_io_num) ) gpio_set_level(scl_io_num, I2C_IO_INIT_LEVEL);

instead of

gpio_set_level(scl_io_num, I2C_IO_INIT_LEVEL);

The glitches can't appear simply because an input pin can not pull that line down.

[igrr]

This issue was recently fixed via #8312 (thanks @jkearins!)