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

I2C not working in 'Release' configuration #304

Closed
holzachr opened this issue Jan 26, 2017 · 11 comments
Closed

I2C not working in 'Release' configuration #304

holzachr opened this issue Jan 26, 2017 · 11 comments

Comments

@holzachr
Copy link

holzachr commented Jan 26, 2017

When building in Debug configuration, I2C communication is working.
When building in Release configuration, communication gets stuck before it really starts.

This is what the bus looks like at the first communication after reset (Debug config):
i2c debug working - zoomed out

This is what the bus looks like at the first communication try after reset (Release config):
i2c release failing - zoomed out

Close look at where data is supposed to start:
i2c release failing - zoomed in 1

In error case, i2c_master_cmd_begin() does not return (with time set to portMAX_DELAY) or returns ESP_ERR_TIMEOUT for any smaller value for ticks_.

This is my initialialization.
First I reconfigure GPIO32 + 33 for general use:
i2c_config_t stI2cConfig;
REG_CLR_BIT(RTC_IO_XTAL_32K_PAD_REG, RTC_IO_X32P_MUX_SEL);
REG_CLR_BIT(RTC_IO_XTAL_32K_PAD_REG, RTC_IO_X32N_MUX_SEL);
stI2cConfig.mode = I2C_MODE_MASTER;
stI2cConfig.sda_io_num = GPIO_NUM_33;
stI2cConfig.sda_pullup_en = GPIO_PULLUP_ENABLE;
stI2cConfig.scl_io_num = GPIO_NUM_32;
stI2cConfig.scl_pullup_en = GPIO_PULLUP_ENABLE;
stI2cConfig.master.clk_speed = 400000;
i2c_param_config(I2C_NUM_0, &stI2cConfig);
i2c_set_data_mode(I2C_NUM_0, I2C_DATA_MODE_MSB_FIRST, I2C_DATA_MODE_MSB_FIRST);
i2c_driver_install(I2C_NUM_0, stI2cConfig.mode, 0, 0, 0);

This is how I access the bus:
// I2C access: write register, then read value
// Read register 1: W[0x68] [0x01] R[0x68] [0x00 = Value]
i2c_cmd_handle_t i2cHandle = i2c_cmd_link_create();
i2c_master_start(i2cHandle);
i2c_master_write_byte(i2cHandle, (I2C_ADDRESS << 1), true);
i2c_master_write_byte(i2cHandle, (uint8_t)enRegister, true);
i2c_master_write_byte(i2cHandle, (I2C_ADDRESS << 1) | 0x01, true);
i2c_master_read_byte(i2cHandle, pu8Value, 1);
esp_err_t result = i2c_master_cmd_begin(I2C_NUM_0, i2cHandle, portMAX_DELAY);
i2c_cmd_link_delete(i2cHandle);
return (result == ESP_OK);

I tried to stick with the I2C example as much as possible.

Any hints or reproduction are very welcome - thanks.

@holzachr
Copy link
Author

holzachr commented Feb 4, 2017

I took time to try the esp-idf/examples/peripherals/i2c example, which behaves the same.

I don't have an actual BH1750 sensor, so I set up the example as suggested in the README.md file (by connecting the ESP32's master & slave ports GPIO18<-->GPIO25 and GPIO19<-->GPIO26), and started a run in "debug" configuration, which looks good.
Then I set menuconfig to "release", make clean && make -j8 flash, and the master I2C peripheral is struggeling.
I tried this procedure with two separate ESP-WROOM-32 modules each on a Watterott ESP-WROOM-32-BREAKOUT board.

i2c_example_debug.txt
i2c_example_release.txt

@mludvig
Copy link

mludvig commented Feb 21, 2017

I just spent a good part of the day trying to make an I2C LCD display work and only now found this bug report. Recompiling in DEBUG mode indeed helped and my display now works as expected.

Using esp-idf git b8e2edc and gcc-5.2.0 [xtensa-esp32-elf-gcc (crosstool-NG crosstool-ng-1.22.0-59-g8d95cad) 5.2.0] compiling on ARM [Raspbian GNU/Linux 8 (jessie)]

@costaud
Copy link
Collaborator

costaud commented Feb 28, 2017

I will take this issue : )

@bobik78
Copy link

bobik78 commented Mar 10, 2017

Did you configure the Debug/Release option through menuconfig?

@holzachr
Copy link
Author

Right, configure through menuconfig.

@bobik78
Copy link

bobik78 commented Mar 14, 2017

I am trying to work with I2C EEPROM (FM24C64) and I can confirm that RELEASE configuration doesn't work at all and I2C bus is stuck on the beginning of any activity.
But, also DEBUG configuration not works so good and after some I2C activity I start getting ERR_TIMEOUT errors from i2c_master_cmd_begin command .
Do you have any idea how to solve this for now?

@emtantra
Copy link

@bobik78 im also with same issue (i2c matster EEPROM Selective (Random) Read problem #422)

@bobik78
Copy link

bobik78 commented Mar 16, 2017 via email

@emtantra
Copy link

we should thank to @nkolban for that.

@costaud
Copy link
Collaborator

costaud commented Mar 19, 2017

I think this one can fix the issue. Try to merge into idk ASAP.

driver/i2c.c

static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
{
       ......
        if (cmd->op_code == I2C_CMD_WRITE) {
            uint32_t wr_filled = 0;
            //TODO: to reduce interrupt number
            if (cmd->data) {
                while (p_i2c->tx_fifo_remain > 0 && cmd->byte_num > 0) {
                    WRITE_PERI_REG(I2C_DATA_APB_REG(i2c_num), *cmd->data++);
                    p_i2c->tx_fifo_remain--;
                    cmd->byte_num--;
                    wr_filled++;
                }
            } else {
                WRITE_PERI_REG(I2C_DATA_APB_REG(i2c_num), cmd->byte_cmd);
                p_i2c->tx_fifo_remain--;
                cmd->byte_num--;
                wr_filled ++;
            }
            //Workaround for register field operation.
            I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num = wr_filled;
            .......

igrr pushed a commit that referenced this issue Mar 23, 2017
This issue is reported from #304.
We found that when we operate the hw command registers in I2C struct, sometimes the behaviour would be different in DEBUG/RELEASE optimisation level:

The code looks like this:
I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num -= 0;

In DEBUG configuration:
I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num -= 0;
400f3ab0:    3388          l32i.n    a8, a3, 12
400f3ab2:    14c882            addi    a8, a8, 20
400f3ab5:    a08840            addx4    a8, a8, a4
400f3ab8:    0020c0            memw
400f3abb:    2898          l32i.n    a9, a8, 8
400f3abd:    0020c0            memw
400f3ac0:    28b8          l32i.n    a11, a8, 8
400f3ac2:    74a090            extui    a10, a9, 0, 8
400f3ac5:    00af92            movi    a9, 0xffffff00
400f3ac8:    109b90            and    a9, a11, a9
400f3acb:    2099a0            or    a9, a9, a10
400f3ace:    0020c0            memw
400f3ad1:    2899          s32i.n    a9, a8, 8

In RELEASE configuration:
I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num -= 0;
400f2ba2:    580572            l8ui    a7, a5, 88
400f2ba5:    747070            extui    a7, a7, 0, 8
400f2ba8:    0020c0            memw
400f2bab:    584572            s8i    a7, a5, 88

Looks like the compiler will make it a 8bit operation after optimisation.
But the register value changes from 0x901 to 0x101.
After this 8-bit optimisation, the 11th bit changed from 1 to zero, which caused this error.

We are still trying to find out why that happens, because there might be some risk when operating the register struct.
This is a workaround to avoid "-=" operation on I2C register struct fields.
igrr added a commit that referenced this issue Mar 23, 2017
…l' into 'master'


bugfix: i2c driver not working in 'RELEASE' configuration

This issue is reported from #304.

We found that when we operate the hw command registers in I2C struct, sometimes the behaviour would be different in DEBUG/RELEASE optimisation level:

The code looks like this:
```
I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num -= 0;
```

In DEBUG configuration:
```
I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num -= 0;
400f3ab0:    3388          l32i.n    a8, a3, 12
400f3ab2:    14c882            addi    a8, a8, 20
400f3ab5:    a08840            addx4    a8, a8, a4
400f3ab8:    0020c0            memw
400f3abb:    2898          l32i.n    a9, a8, 8
400f3abd:    0020c0            memw
400f3ac0:    28b8          l32i.n    a11, a8, 8
400f3ac2:    74a090            extui    a10, a9, 0, 8
400f3ac5:    00af92            movi    a9, 0xffffff00
400f3ac8:    109b90            and    a9, a11, a9
400f3acb:    2099a0            or    a9, a9, a10
400f3ace:    0020c0            memw
400f3ad1:    2899          s32i.n    a9, a8, 8
```

In RELEASE configuration:

```
I2C[i2c_num]->command[p_i2c->cmd_idx].byte_num -= 0;
400f2ba2:    580572            l8ui    a7, a5, 88
400f2ba5:    747070            extui    a7, a7, 0, 8
400f2ba8:    0020c0            memw
400f2bab:    584572            s8i    a7, a5, 88
```

Looks like the compiler will make it a 8bit operation after optimisation.

But the register value changes from 0x901 to 0x101.

After this 8-bit optimisation, the 11th bit changed from 1 to zero, which caused this error.

We are still trying to find out why that happens, because there might be some risk when operating the register struct.

This is a workaround to avoid "-=" operation on I2C register struct fields.

See merge request !592
@holzachr
Copy link
Author

holzachr commented Apr 3, 2017

Works fine for me - thanks everyone!

@holzachr holzachr closed this as completed Apr 3, 2017
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

5 participants