Issue2524 allow clocks below 100KHz by Tech-TX · Pull Request #6934 · esp8266/Arduino · GitHub
Skip to content

Issue2524 allow clocks below 100KHz#6934

Merged
devyte merged 11 commits into
esp8266:masterfrom
Tech-TX:issue2524
Dec 28, 2019
Merged

Issue2524 allow clocks below 100KHz#6934
devyte merged 11 commits into
esp8266:masterfrom
Tech-TX:issue2524

Conversation

@Tech-TX

@Tech-TX Tech-TX commented Dec 22, 2019

Copy link
Copy Markdown
Contributor

Corrects the fixed I2C bus clock selections and allows slower bus clocks down to ridiculously low bus speeds if you need that. Originally prompted by Earle's comment in #2524 about computing the bus clock timing.

Additionally, the dead Twi::stop was removed. The compiler was removing it in slave compiles anyways, as it wasn't called by any of the functions except for one oversight in the slave state machine. Slaves can't issue STOP commands. Although the Twi::stop was listed as an exposed function, it wasn't linked from Wire.h, so it was optimized out.

@Tech-TX

Tech-TX commented Dec 22, 2019

Copy link
Copy Markdown
Contributor Author

@Tech-TX Tech-TX left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devyte - [review integer ranges for overflow, minor constness ]

edit: divide by zero if they feed it 0Hz, uint32_t overflow below 233Hz
edit2: there's no overflow, my test program above was mixing ints and unsigned ints (corrected). The real code was correct. I knew I'd checked my math twice to insure I stayed within an unsigned int!

Might want to limit the lower end even further, as a single byte read or write is ~78ms at 250Hz.
(start, 7 bits address, read/write, ack, 8 bits data, ack, stop). Maybe somewhere around 500Hz to 1KHz would be a better lower limit. I don't expect any more than a few people trying to run it lower than 10KHz, normally. There were some radio PLLs that needed ~ 10KHz bus clock back in the '80s, but that's ancient history.

@Tech-TX Tech-TX changed the title Issue2524 Issue2524 allow clocks below 100KHz Dec 22, 2019
Comment thread cores/esp8266/core_esp8266_si2c.cpp Outdated
@devyte

devyte commented Dec 27, 2019

Copy link
Copy Markdown
Collaborator

@devyte devyte left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work here. Only minor changes before merge.

Comment thread cores/esp8266/core_esp8266_si2c.cpp Outdated
Comment thread cores/esp8266/core_esp8266_si2c.cpp
Comment thread cores/esp8266/core_esp8266_si2c.cpp
@devyte devyte merged commit 3197d2a into esp8266:master Dec 28, 2019
@devyte devyte added this to the 2.7.0 milestone Dec 28, 2019
@devyte devyte mentioned this pull request Dec 28, 2019
@Tech-TX Tech-TX deleted the issue2524 branch December 28, 2019 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants