stm32-rs / stm32f1xx-hal

A Rust embedded-hal HAL impl for the STM32F1 family based on japarics stm32f103xx-hal
Apache License 2.0
575 stars 179 forks source link

Usb clocks brake i2c (with lto=true) #311

Open fishrockz opened 3 years ago

fishrockz commented 3 years ago

When I set my clocks like

let clocks = rcc.cfgr.freeze(&mut flash.acr);

Then my i2c code works and i can talk to a sensor over i2c, im using the adafuit BME280 board once i get a bit further im happy to add a example to your repo if you like, im not very far along atm tho but hope to get there.

But i for many reasons it would be lovely to use the USB serial to print out the data much like the arduino examples do.

But when i do

let clocks = rcc
    .cfgr
    .use_hse(8.mhz())
    .sysclk(48.mhz())
    .pclk1(24.mhz())
    .freeze(&mut flash.acr);

As per your usb_serial example

Then the i2c read and write comands just block for ever.

let scl = gpiob.pb6.into_alternate_open_drain(&mut gpiob.crl);
let sda = gpiob.pb7.into_alternate_open_drain(&mut gpiob.crl);

let mut i2c = BlockingI2c::i2c1(
    dp.I2C1,
    (scl, sda),
    &mut afio.mapr,
    Mode::Standard {
        frequency: 100_000.hz(),
    },
    clocks,
    &mut rcc.apb1,
    1000,
    10,
    1000,
    1000,
);

led.set_low().unwrap(); // LED on
let mut read_buf: [u8; 1] = [0; 1];
let write_buf: [u8; 2] = [208, 1];
let read_res = i2c.write(0x77, &write_buf) // this blocks for ever with the usb compatible clocks

I can upload my full code if the above is not enough to spot the issue but i was hoping not to upload it anywhere until it was slightly more finished and tidy..

Many thanks Will

TheZoq2 commented 3 years ago

That's odd. I think I've successfully used USB with I2C before and had no issues.

What board are you using, a blue pill?

fishrockz commented 3 years ago

Yep, bluepill

I have also tried

    let mut i2c = BlockingI2c::i2c1(
        dp.I2C1,
        (scl, sda),
        &mut afio.mapr,

        Mode::Fast {
            frequency: 400_000.hz(),
            duty_cycle: DutyCycle::Ratio2to1,
        },
        clocks,
        &mut rcc.apb1,
        1000,
        10,
        1000,
        1000,
    );

With no luck

It seems to sit there waiting for data so my gues is that the clock is coming out at a speed the sensor cant cope with..

I will hook up the scope tonight and see if i can see if the clock is at least coming out at the right speed and may even be able to get it to give me even more clues on the data line..

It seems to sit there waiting for data so my gues is that the clock is coming out at a speed the sensor cant cope with..

fishrockz commented 3 years ago

IMG_20210226_091912

IMG_20210226_092149

Sorry for the less good pics. Even tho I have set the i2c to the same settings the starting pulse seems to be different when I change the clock.

fishrockz commented 3 years ago

I have spent some time investigating this and its seems that as the clock is increased the value set in i2c_trise i2c_ccr registers should tell the i2c unit how to use the clocks to create the correct timed start pulse, but that this is not working for my chip.

The blue pill that i have dose not have a stm32f103 but a clone, (its chip identifier is 0x2ba01477 when it should be 0x1ba01477) that said it dose seem to work well most of the time. I am looking to try to get a dev board with a genuine stm chip.

However, for me, no matter what value i try to set i2c_trise i2c_ccr for faster clocks the start pulse seem to be too quick as shown in the images in the previous comment and the only thing that seems to alter the start pulse length is sysclk. I assume that my non-genuine stm is causing this. hopefully this issue even after it is closed can be found by others having similar issues. I will try to get a genuine chip to see if this is the issue or if genuine chips also suffer this issue.

LongHairedHacker commented 3 years ago

I've just spent a couple of hours chasing after a similar if not the same problem.

My I2C setup is quite similar to yours, we probably copied from the same example:

    let i2c = i2c::BlockingI2c::i2c1(
        dp.I2C1,
        (scl, sda),
        &mut afio.mapr,
        i2c::Mode::Fast {
            frequency: 400_000.hz(),
            duty_cycle: i2c::DutyCycle::Ratio2to1,
        },
        clocks,
        &mut rcc.apb1,
        1000,
        10,
        1000,
        1000,
    );

If I try something like this the I2C works.

    let clocks = rcc
        .cfgr
        .use_hse(8.mhz())
        .sysclk(16.mhz())
        .pclk1(4.mhz())
        .freeze(&mut flash.acr);

DS1Z_QuickPrint39

If I use the "full" speed

    let clocks = rcc
        .cfgr
        .use_hse(8.mhz())
        .sysclk(72.mhz())
        .pclk1(36.mhz())
        .freeze(&mut flash.acr);

IC2 just seems to do nothing. DS1Z_QuickPrint37

At first I suspected that the short dip in the signal is a malformed start condition, but it is also present in the working trace if I zoom out: DS1Z_QuickPrint38

So most likely the dip is just the mirco resetting and the I2C peripheral does not generate a start condition at all.

It's likely that my bluepill board has fake chip on it. I have a couple more board to try, bought from different vendors over the last two years. In addition to that I should have some genuine chips somewhere that I can transplant onto on of them.

LongHairedHacker commented 3 years ago

Grabbed my hot air station and exchanged the chip on the bluepill for a supposedly genuine one. (At least my distributor sold it to me as genuine, and so far I had no reason to doubt that statement.) It shows the same behaviour. Works fine with a low sysckl and fails with a higher one.

I also tried all my other bluepills: same problem.

So either all my chips are fake (not entirely unlikely) or there is something else I missed.

LongHairedHacker commented 3 years ago

Okay If managed to narrow my problem down to something completely different.

While trying to come up with a minimal example that reproduces the problem, I noticed that the problem only occurs if the binary is build in release mode. So cargo run works fine and cargo run --release will get stuck.

If pushed my minimal reproducer to github: https://github.com/LongHairedHacker/i2c-test

It would be great if somebody else could verify my observations.

I'm afraid that this is some kind of compiler optimisation issue. As a next step I'll try update everything to the latest and greatest version.

fishrockz commented 3 years ago

So looking at your code, @LongHairedHacker https://github.com/LongHairedHacker/i2c-test/blob/main/src/main.rs#L67 and the stm32101/2/3/5/7xx reference manual prox page 724/1096 [1] it looks like you need to use fast mode to have 400khz bus speed, i don't know what the expected behaviour is if you try to operate out side of the defined operational parameters.

[1] https://www.element14.com/community/servlet/JiveServlet/downloadBody/49978-102-1-259998/STMicroelectronics.Reference_Manual.pdf

Also I have finally received some blue pills that have the correct chip identifier (0x1ba01477) so at least have a chance of being real. so i will try to reproduce my previous experiments and see if i can have any more luck.

LongHairedHacker commented 3 years ago

Good catch. That must have gotten mixed up while coping stuff around. However even if I change it to fast mode the problem is still pretty much the same.

I've also uncovered the problem in the mean time. My cargo.toml:57 is lto = 'fat'. Removing or commenting it out results in a working I2C, regardless of --release.

fishrockz commented 3 years ago

OOOO, i had lto=true but removing it allowed my i2c to work at higher clocks too, at least for my "genuine" chips but probably for all of mine.

Thank you for the tip!

fishrockz commented 3 years ago

As far as i can tell the issue is that both sck32f103xx based blue pills as well as stm32f103xx based blue pills can run i2c at low clocks with lto=true but both fail to use i2c if lto=true and the clock is faster.

chmanie commented 3 years ago

I added #338. Do you think it could be related? In my case setting the system clock to anything, really, panics already.

fishrockz commented 3 years ago

I added #338. Do you think it could be related? In my case setting the system clock to anything, really, panics already.

Um, I would recommend using this lib without lto=true

But I don't think this is related, as others have noted I would set the HSE before changing any other clock settings.

TheZoq2 commented 3 years ago

This sounds super interesting and certainly needs more looking into, especially as it differs between opt modes. Out of curiosity, what rust version is this? Could it potentially be related to the unsoundness that was mitigated in 1.52.2?

But I don't think this is related, as others have noted I would set the HSE before changing any other clock settings.

This should not make a difference. Clock settings are always applied in the same order and always in the freeze method. Or are you talking about making changes inside freeze?

fishrockz commented 3 years ago

This sounds super interesting and certainly needs more looking into, especially as it differs between opt modes. Out of curiosity, what rust version is this? Could it potentially be related to the unsoundness that was mitigated in 1.52.2?

[will@localhost ~]$ rustc --version
rustc 1.48.0 (7eac88abb 2020-11-16)

Looks like it could be, I will try updating and trying again

But I don't think this is related, as others have noted I would set the HSE before changing any other clock settings.

This should not make a difference. Clock settings are always applied in the same order and always in the freeze method. Or are you talking about making changes inside freeze?

The example @chmanie had only set the clock without the HSE in one freeze, I should have been more clear. Thanks for pointing it out :)

fishrockz commented 3 years ago

This sounds super interesting and certainly needs more looking into, especially as it differs between opt modes. Out of curiosity, what rust version is this? Could it potentially be related to the unsoundness that was mitigated in 1.52.2?

[will@localhost ~]$ rustc --version
rustc 1.48.0 (7eac88abb 2020-11-16)

Looks like it could be, I will try updating and trying again

[will@localhost bluepill]$ rustc --version rustc 1.54.0-nightly (ff2c947c0 2021-05-25)

So running in my test repo turns the -nightly on, not sure if nightly is actually needed

Any way I can still reproduce the i2c issue when setting lto=true with the new version.