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 Crashes Kernel on Raspberry Pi Pico #21187

Open
tanneberger opened this issue Feb 4, 2025 · 6 comments · Fixed by #21188
Open

I2C Crashes Kernel on Raspberry Pi Pico #21187

tanneberger opened this issue Feb 4, 2025 · 6 comments · Fixed by #21188
Labels
Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@tanneberger
Copy link

Description

I want to use the veml7700 brightness sensor in combination with the Raspberry Pi PICO. Sadly, RIOT crashes after calling i2c_init (see the code below).

I tested the same code on the feather-nrf52840-sense there it works flawlessly.

int main(void) {
    i2c_t dev = I2C_DEV(0);
    i2c_init(dev); // crashes here

    uint16_t reg_value = 0;
    uint16_t device_addr = 0x10;

    i2c_acquire(dev);

    uint16_t conf_val1 = 6144;
    printf("writing value: %i", conf_val1);

    i2c_write_byte(dev, device_addr, 0, I2C_NOSTOP);
    i2c_write_bytes(dev, device_addr, &conf_val1, 2, 0);


    while (true) {
        i2c_read_regs(dev, device_addr, 0x04, &reg_value, 2, 0);
        printf("%i\n", reg_value);
    }

    i2c_release(dev);
    return 0;
}

Output I get by connecting a logic analyzer:

Image

Steps to reproduce the issue

In the Makefile, I add the line USEMODULE += periph_i2c in order to use i2c. Just compile this program for the BOARD=rpi-pico

Expected results

Don't crash.

Versions

I run this on RIOT version 2024.10 (b51f093a395bdd575cff94b7d26be415fa46771d)

@maribu
Copy link
Member

maribu commented Feb 4, 2025

Thx for the report. Could you try again without i2c_init()?

(Alternatively you could disable the auto initialisation of the I2C buses and then do by hand. I'm afk, but I think the module is named periph_init_i2c that does the auto initialisation. You can add DISABLE_MODULE += periph_init_i2c to the apps Makefile for that; assuming that I correctly nemorized the module name.)

@dylad
Copy link
Member

dylad commented Feb 4, 2025

Hi @tanneberger
As @maribu suggested, you can try without i2c_init() as this part is in general done automatically for you by RIOT.
Except if you use DISABLE_MODULE += periph_init_i2c
You can check this with make -C path/to/your/app info-build if you are uncertain. (Look at the USEMODULE / DISABLE_MODULES).

However, a second call to i2c_init() should not hurt that much so there is definitely something wrong underneath.

@dylad dylad added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Feb 4, 2025
@maribu
Copy link
Member

maribu commented Feb 4, 2025

However, a second call to i2c_init() should not hurt that much so there is definitely something wrong underneath.

That is not generally true. Most MCU peripherals are initialized in a way that the code is naturally idempotent. But some periph drivers rely on being called exactly once.

Our documentation in again substandard here, but in https://doc.riot-os.org/group__drivers__periph.html#autotoc_md1719 it is hinted.

@dylad
Copy link
Member

dylad commented Feb 4, 2025

Perhaps we could keep track of the state of the peripheral and early return if the initialization was already done or trigger an assert.
A Kernel panic is a high price to pay for a periph reinit.

@maribu
Copy link
Member

maribu commented Feb 4, 2025

@tanneberger: This issue to be auto-closed upon merge, rather than by intent, because I wrote maybe fixes #21187 in PR #21188.

Could you you report back if the issue was indeed the second call to i2c_init()?

And sorry for the doc being wrong here :/

@miri64
Copy link
Member

miri64 commented Feb 4, 2025

Could you you report back if the issue was indeed the second call to i2c_init()?

Let's re-open the issue until then.

@miri64 miri64 reopened this Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants