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

I2C: put inline helpers in IRAM for slave ISRs #6898

Merged
merged 7 commits into from Dec 15, 2019

Conversation

@Tech-TX
Copy link
Contributor

Tech-TX commented Dec 11, 2019

heavily modified to insure that the 'inline helpers' are truly inline (they are, verified in the dump)

@Tech-TX

This comment has been minimized.

Copy link
Contributor Author

Tech-TX commented on cf1d9fd Dec 11, 2019

missed these when I removed the other inline on reply(uint8_t ack)

@Tech-TX

This comment has been minimized.

Copy link
Contributor Author

Tech-TX commented Dec 12, 2019

Did you want me to close the other pull request #6894
as this one corrects the same issue in a different way?

The 'inline' attribute on some of the routines seemed to override the 'ICACHE_RAM_ATTR' attribute, and some of the slave ISR routines were ending up in Flash, or were split; part was in Flash, part in IRAM. This caused exceptions when I2C slave events happened during OTA download.

@devyte

This comment has been minimized.

Copy link
Collaborator

devyte commented Dec 12, 2019

@Tech-TX yes please, let's keep this one.

…alley up into the master section [issue 6875]
@devyte devyte added this to the 2.6.3 milestone Dec 12, 2019
 into I2c_slave_nonstatic
@Tech-TX

This comment has been minimized.

Copy link
Contributor Author

Tech-TX commented Dec 13, 2019

Latest commit added devyte's comment about not inlining reply(), removed the inline attribute from 2 public functions in the slave side, and moved twi_scl_valley() out of the middle of the slave code and into the master area, as it's a master-only function.

@Tech-TX Tech-TX mentioned this pull request Dec 13, 2019
5 of 6 tasks complete
@Tech-TX

This comment has been minimized.

Copy link
Contributor Author

Tech-TX commented Dec 13, 2019

As only one person has seen this error so far, would you mind waiting until Sunday evening or later to merge it? I'd like to run every test against it that I can think of, since the changes touched nearly every lowest-level function. I've eyeballed all of the changes 3 ways, but I'm the eternal pessimist.

@devyte
devyte approved these changes Dec 15, 2019
@devyte devyte merged commit 5612738 into esp8266:master Dec 15, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Tech-TX Tech-TX deleted the Tech-TX:I2c_slave_nonstatic branch Dec 15, 2019
@d-a-v d-a-v changed the title change to make inline helpers truly inline [issue 6875] I2C: put inline helpers in IRAM for slave ISRs Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.