Skip to content

Conversation

@towynlin
Copy link
Contributor

towynlin pushed a commit that referenced this pull request Jul 17, 2014
Use Mat McGowan's SPI bus arbitration code
@towynlin towynlin merged commit 160e2df into master Jul 17, 2014
@technobly
Copy link
Contributor

I never tested this branch... and Andy's and Satish's branches were based on this one... which I did test with 100% success. I was recommending Satish's core-common-lib bug-fix/ota-flash-reliability branch as it was the cleanest code and comments. Can you comment on the difference between that and mdma's branch?

@towynlin
Copy link
Contributor Author

Sure @technobly, thanks for asking!

Both branches solved the problem in all our internal testing. This code, to me, made more clear the actual problem and solution, but that's perhaps a matter of taste and background, since you thought the opposite.

Additionally, the software arbitration solves some problems that interrupt masking does not, as described by Mat here:
https://community.spark.io/t/bug-bounty-ota-flash-reliability-issue/5438/55?u=zachary

@technobly
Copy link
Contributor

Ah... I guess I missed the part where Andy's code and MDMA's code were more significantly different... After looking at MDMA's posts again and his code I'm agreeing with you that it seems like a more natural fix. I would like to test it though to make sure it works as well as Andy's interrupt masking, so I'll do that and report back after I collect enough data.

@technobly
Copy link
Contributor

150 passed the dual curl script with 0 failures. Since they all work the same for this test, it seems that it does then come down to the mutex with code vs. int masking argument. The code mutex does seem to be easier to maintain if the system should change in the future.

@m-mcgowan m-mcgowan deleted the m-mcgowan/spibus_arbiter branch September 4, 2014 19:07
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.

5 participants