stm32-rs / stm32f0xx-hal

A Rust `embedded-hal` implementation for all MCUs in the STM32 F0 family
BSD Zero Clause License
134 stars 59 forks source link

First write on I2C bus never NACK's #132

Open barafael opened 3 years ago

barafael commented 3 years ago

Problem:

When performing the first write transaction of the i2c scanner example, there is never a NACK, even if no device is attached on the bus.

Hardware required:

1x STM32F072 Nucleo

2x appropriate (for example 4.7KOhm) pullup resistor (alternatively, use any known OK I2C breakout board with a known I2C address which already includes pullups)

1x https://github.com/stm32-rs/stm32f0xx-hal/blob/master/examples/i2c_find_address.rs (for easier pin access, you may want to change the SDA pin to be PB9 instead of PB7)

Steps to reproduce:

  1. Wire up the pull up resistors from SCL/SDA to 3v3.

  2. Run the i2c scanner example: https://github.com/stm32-rs/stm32f0xx-hal/blob/master/examples/i2c_find_address.rs

    • Expected: _devices == 0
    • Actual: _devices == 1, ACK at address 0
    • If you see a lot of ACKs, your pull-ups could be wrong
  3. Change start address in line 32: https://github.com/stm32-rs/stm32f0xx-hal/blob/fba9834b59fa7567ffd604afed2bcd8d07c4e904/examples/i2c_find_address.rs#L32 to e.g. 6 and re-run the scan.

    • Expected: _devices == 0
    • Actual: _devices == 1, ACK at address e.g. 6
  4. Before the scan loop, add this line: let _ = i2c.write(0x4, &[]); and re-run the scan.

    • Expected: _devices == 0
    • Actual: _devices == 0 (yay!)
  5. (Optional) Remove the line added in step 4. Attach an I2C sensor with known address to the bus and re-run the scan.

    • Expected: _devices == 1
    • Actual: _devices == 2
barafael commented 3 years ago

I can confirm that this problem does not occur when using the bitbang-hal.

Note: bitbang hal has currently a bug where all empty transactions are ACK'd. This is fixed here: https://github.com/sajattack/bitbang-hal/pull/22 Of course, with the bug in place, the scan results in an ACK from all 127 addresses on the bus :D

therealprof commented 3 years ago

Not to spoil your fun but a bitbang implementation doesn't really have any relevance here. Any reason why you're not using the internal pull-ups?

barafael commented 3 years ago

Good point. I am using external ones on my PCB, so I thought the internal ones are not required.

therealprof commented 3 years ago

I've just pulled up a F072 Nucleo and some I2C hardware... Let's see whether I can find something. Which version did you test? Master?

barafael commented 3 years ago

As per my Cargo.toml: 0.17.1

therealprof commented 3 years ago

I can confirm the behaviour. On an empty write a NACK to address 0 is sometimes not detected/reported by the hardware. Checking with the RM I don't see any way to improve that. What's preventing you from actually sending some data?

barafael commented 3 years ago

Can you check whether you can reproduce with other addresses, too? 0 is general call address so might be special. For me, it was always the very first write that had this behaviour, no matter the address, and after that it worked perfectly always.

Sending no data is actually necessary for smbus quick command. And if I do send some data in the scan, some device on the bus might actually interpret it. Sending address with no data is also sometimes required for waking up devices from sleep mode (I think). And, every i2c scanner works like this :) What did you find in the RM?

therealprof commented 3 years ago

Can you check whether you can reproduce with other addresses, too? 0 is general call address so might be special.

I haven't tried. I have tried a better version of my I2C scanner though and that one works by sending a byte of data which did work great.

Sending no data is actually necessary for smbus quick command.

SMBus is a different mode and a bit special in a lot of regards (like defining a timeout).

And if I do send some data in the scan, some device on the bus might actually interpret it. Sending address with no data is also sometimes required for waking up devices from sleep mode (I think). And, every i2c scanner works like this :)

No idea. Sounds like undefined behaviour to me.

What did you find in the RM?

Nothing really. It doesn't say anything special about NBYTES == 0. It does read a bit as if that case was not really considered though. I also checked the errata sheet and there're half a ton (scary) I2C related errata but nothing sounding like this particular behaviour.

barafael commented 3 years ago

sending a byte of data which did work great.

Referring to this: https://github.com/barafael/smbus-graph/blob/main/smbus-graph.pdf I am not convinced it's ok to scan by sending a byte.

i2cdetect man page does not recommend scanning either with just write-byte or with just quick-write: https://linux.die.net/man/8/i2cdetect

Regardless of this issue - do you confirm this behaviour does only occur if writing 0 bytes?

therealprof commented 3 years ago

i2cdetect man page does not recommend scanning either with just write-byte or with just quick-write: https://linux.die.net/man/8/i2cdetect

It does suggest not to use SMBus methods to scan the bus. I don't see anything saying it will not attempt to write to the bus.

Regardless of this issue - do you confirm this behaviour does only occur if writing 0 bytes?

I can confirm I've only seen it when writing 0 bytes and writing more than 0 bytes fixes it. I also haven't seen anything in the manual that would allow me to change anything about that; I can only check for a NACK after sending a START condition which I can only do after setting the address and configuring NBYTES.

If there's indeed an (hardware) implementation error causing only the first 0-bytes write to auto-NACK then the only "fix" I can think of would be to do just that during initialisation. But then again, if we did that we'd be writing random values to the bus which according to you is dangerous so it's definitely not something I'd consider.

barafael commented 3 years ago

Writing one byte to the bus is an smbus method btw :)

I'll check some C implementations (mbed and ST HAL) and possibly check with my ST contact.

Thank you for taking the time and effort!

therealprof commented 3 years ago

Why would that be an SMBus method? 🤔 And what is the point of using I2C if you're not going to transmit anything at all?

Also I have not see scanning documented anywhere in the specification, IIRC someone came up with this clever (ab-)use of I2C and everyone else copied the idea because it can be handy at times. I think the i2cdetect documentation puts it very well and clearly that this is a hack:

This program can confuse your I2C bus, cause data loss and worse!
jaxter184 commented 3 years ago

If this is a hack, should the i2c_find_address example be removed/replaced?

therealprof commented 3 years ago

What isn't a hack? 😅 Maybe we can improve the documentation to set the expectations right?

kevin406972552 commented 2 years ago

Is there a solution to the problem now?