Skip to content

STM32: Remove i2c_read() and i2c_write() redirects to HAL_I2C_IsDeviceReady() #4717

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

Merged
merged 1 commit into from
Jul 24, 2017

Conversation

bscott-zebra
Copy link
Contributor

Description

Some I2C devices require specific zero length read/write sequences which
the HAL_I2C_IsDeviceReady() redirect interferes with. After Removing
these redirects, it was confirmed that zero length reads and writes
would both still work correctly for detecting presence/absence of an
I2C device on a bus.

Status

READY

Migrations

NO

Todos

Run I2C CI tests on all STM32 targets to confirm no regressions

Steps to test or reproduce

Build an STM32 based target image with an instantiated I2C bus and use an oscilloscope or I2C bus analyzer to confirm that both an I2C read without data and an I2C write without data can be generated, and the expected results returned, using code such as:

#include <mbed.h>
...
I2C testI2c(PA_10, PA_9);
printf("Write to 0x54 = %d\r\n", testI2c.write(0x54, 0, 0)); // Non-existent I2C device (expect 1 result)
printf("Write to 0xA0 = %d\r\n", testI2c.write(0xA0, 0, 0)); // EEPROM (expect 0 result)
printf("Read from 0x54 = %d\r\n", testI2c.read(0x54, 0, 0)); // Non-existent I2C device (expect 1 result)
printf("Read from 0xA0 = %d\r\n", testI2c.read(0xA0, 0, 0)); // EEPROM (expect 0 result)

…eReady()

Some I2C devices require specific zero length read/write sequences which
the HAL_I2C_IsDeviceReady() redirect interferes with.  After Removing
these redirects, it was confirmed that zero length reads and writes
would both still work correctly for detecting presence/absence of an
I2C device on a bus.
@bscott-zebra
Copy link
Contributor Author

@LMESTM
Here's the pull request first discussed on this commit.

@LMESTM
Copy link
Contributor

LMESTM commented Jul 6, 2017

@monkiineko thx - I'll and start tests, hopefully tomorrow

@adbridge
Copy link
Contributor

adbridge commented Jul 7, 2017

Travis failure is :
$ make -C events/equeue test clean

make: Entering directory `/home/travis/build/ARMmbed/mbed-os/events/equeue'
gcc -c -MMD -O2 -I. -I.. -std=c99 -Wall -D_XOPEN_SOURCE=600 tests/tests.c -o tests/tests.o
gcc -c -MMD -O2 -I. -I.. -std=c99 -Wall -D_XOPEN_SOURCE=600 equeue.c -o equeue.o
gcc -c -MMD -O2 -I. -I.. -std=c99 -Wall -D_XOPEN_SOURCE=600 equeue_posix.c -o equeue_posix.o
gcc -O2 -I. -I.. -std=c99 -Wall -D_XOPEN_SOURCE=600 tests/tests.o equeue.o equeue_posix.o -pthread -o tests/tests
tests/tests
beginning tests...
background_test: failed at line 519

which is :
    equeue_dispatch(&q, 0);
    test_assert(ms == 10);

@adbridge
Copy link
Contributor

adbridge commented Jul 7, 2017

Closing and reopening to re-trigger travis to see if this was just a weird timing error on the machine.

@LMESTM
Copy link
Contributor

LMESTM commented Jul 7, 2017

@monkiineko I'm busy with other things, but I asked @jeromecoutant to do the testing - he'll report the results
Then I'd like to check with an analyzer that everything happens as previously

@jeromecoutant
Copy link
Collaborator

jeromecoutant commented Jul 7, 2017

Yes, I have ran tests this morning for each STM32 family:

1 2 3 4 5
Target Test Suite Test Case Toolchain Result
NUCLEO_F767ZI tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_F767ZI tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  LM75B Temperature Read ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_F767ZI tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_F767ZI tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_F207ZG tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_L476RG tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_F103RB tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_F446RE tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_F091RC tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  LM75B Temperature Read ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  LM75B Temperature Read ARM OK
NUCLEO_F303ZE tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  LM75B Temperature Read ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_F446RE tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_L152RE tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_L073RZ tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  LM75B Temperature Read ARM OK
NUCLEO_L476RG tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  LM75B Temperature Read ARM OK
NUCLEO_F207ZG tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_L152RE tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  LM75B Temperature Read ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_F303ZE tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  EEProm 2nd WR 10  Bytes ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_F207ZG tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_F446RE tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  EEProm WR Single Byte ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  EEProm 2nd WR 2 Bytes ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  EEProm WR 2 Bytes ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_F103RB tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_L073RZ tests-assumptions-i2c I2C - is SCL connected? ARM OK
NUCLEO_F091RC tests-api-i2c I2C -  EEProm WR 100 Bytes ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  Instantiation of I2C Object ARM OK
NUCLEO_L152RE tests-api-i2c I2C -  EEProm 2nd WR 100 Bytes ARM OK
NUCLEO_F103RB tests-api-i2c I2C -  EEProm 2nd WR Single Byte ARM OK
NUCLEO_F091RC tests-assumptions-i2c I2C - is SDA connected? ARM OK
NUCLEO_L073RZ tests-api-i2c I2C -  EEProm WR 10  Bytes ARM OK
NUCLEO_L476RG tests-api-i2c I2C -  LM75B Temperature Read ARM OK
NUCLEO_F303ZE tests-api-i2c I2C -  LM75B Temperature Read ARM OK

@bscott-zebra
Copy link
Contributor Author

@LMESTM @jeromecoutant
Thanks Laurent and Jerome. Results look good.

@LMESTM
Copy link
Contributor

LMESTM commented Jul 10, 2017

@monkiineko I've also used a logic analyzer and have verified the behavior is as expected, on NUCLEO_F446RE (I2C_IP_VERSION_V1) and NUCLEO_L476RG (I2C_IP_VERSION_V2). So I'll approve the PR.
Thanks !

@0xc0170
Copy link
Contributor

0xc0170 commented Jul 10, 2017

/morph test

@mbed-bot
Copy link

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 755

Build Prep failed!

@studavekar
Copy link
Contributor

Git checkout timed out

09:15:36 FATAL: Could not checkout efc5c47e55b8a30c2acb366808975411e4bae7fd
09:15:36 hudson.plugins.git.GitException: Command "git checkout -f efc5c47e55b8a30c2acb366808975411e4bae7fd" returned status code -1:
09:15:36 stdout: 
09:15:36 stderr: 
09:15:36 	at org.jenkinsci.plugins.gitclient.CliGitAPIImpl.launchCommandIn(CliGitAPIImpl.java:1799)

/morph test

@mbed-bot
Copy link

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 778

Build failed!

@0xc0170
Copy link
Contributor

0xc0170 commented Jul 13, 2017

/morph test

@mbed-bot
Copy link

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 801

Test failed!

@theotherjimmy
Copy link
Contributor

Failure not related

/morph test

@mbed-bot
Copy link

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 830

All builds and test passed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants