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

[Core] Fixes I2C slave mode #1309

Merged
merged 1 commit into from Jan 23, 2018

Conversation

@avtolstoy
Copy link
Member

commented Apr 23, 2017

Problem

Core crashes when I2C is configured in slave mode.

Solution

  • Fixes incorrectly named interrupt handlers: HAL_I2C1_ER_Handler, HAL_I2C1_EV_Handler
  • Copied relevant I2C slave code from Photon/Electron implementation

Steps to Test

wiring/i2c_master_slave

Example App

N/A

References

Supersedes #1305


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)
@geekbozu

This comment has been minimized.

Copy link

commented Apr 24, 2017

So I pulled and tested this, I no longer can get valid I2C communication with these changes. Where as with my PR #1305 (which I updated as I did not perform unit testing), I am able to properly send and receive data. I will take a look into this with my logic analyzer tomorrow just wanted to chime in before bed.

@geekbozu

This comment has been minimized.

Copy link

commented Apr 25, 2017

So I think this block is not needed, and removing it fixes the problem.

https://github.com/spark/firmware/blob/57fd7cbc3a16ec6cd2e4829eb62a72800fd1a7cd/hal/src/core/i2c_hal.c#L660-L666

The comment claims that with clock stretching enabled it will pull SCL low if DR is empty. However this is only partially true.

The stm will only stretch the clock if it believes it has more data to send. It decides if there is more data to send based on the Master's ACK/NACK of the last transfered byte. If it recieves an ACK it will stretch the clock until the next byte is ready to be shifted out. How-ever if it recieves a nack it will not hold the clock as according to the I2C specification that is how the master signals to the slave that the current transfer is over.

Other

I believe this also applies to the photon/electron as well. According to the reference manual for those devices the same logic as above is applied.

Source:

http://www.st.com/content/ccc/resource/technical/document/reference_manual/59/b9/ba/7f/11/af/43/d5/CD00171190.pdf/files/CD00171190.pdf/jcr:content/translations/en.CD00171190.pdf

@technobly technobly added this to the 0.8.0 milestone May 4, 2017

@avtolstoy avtolstoy force-pushed the feature/core/i2c-slave-fixes branch from 57fd7cb to 73eafb4 Jan 18, 2018

@avtolstoy avtolstoy requested a review from sergeuz Jan 18, 2018

@m-mcgowan m-mcgowan self-requested a review Jan 23, 2018

@m-mcgowan m-mcgowan modified the milestones: 0.8.0, 0.8.0-rc.2 Jan 23, 2018

@m-mcgowan m-mcgowan merged commit 5cee525 into develop Jan 23, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@m-mcgowan m-mcgowan deleted the feature/core/i2c-slave-fixes branch Jan 23, 2018

@m-mcgowan m-mcgowan added the bug label Feb 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.