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

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

Merged
merged 1 commit into from Jul 24, 2017

Conversation

Projects
None yet
8 participants
@monkiineko
Contributor

monkiineko commented Jul 6, 2017

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)

STM32: Remove i2c_read() and i2c_write() redirects to HAL_I2C_IsDevic…
…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.
@monkiineko

This comment has been minimized.

Contributor

monkiineko commented Jul 6, 2017

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

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jul 6, 2017

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

@adbridge

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

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
@monkiineko

This comment has been minimized.

Contributor

monkiineko commented Jul 7, 2017

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

@LMESTM

This comment has been minimized.

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 !

@LMESTM

LMESTM approved these changes Jul 10, 2017

@0xc0170 0xc0170 removed the needs: review label Jul 10, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 10, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 10, 2017

Result: FAILURE

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

/morph test

Output

mbed Build Number: 755

Build Prep failed!

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Jul 10, 2017

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

This comment has been minimized.

mbed-bot commented Jul 11, 2017

Result: FAILURE

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

/morph test

Output

mbed Build Number: 778

Build failed!

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 13, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 13, 2017

Result: FAILURE

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

/morph test

Output

mbed Build Number: 801

Test failed!

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 17, 2017

Failure not related

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 18, 2017

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 830

All builds and test passed!

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Jul 24, 2017

@theotherjimmy theotherjimmy merged commit 57a4c95 into ARMmbed:master Jul 24, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment