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

Fixing panics in release mode #72

Merged
merged 1 commit into from
Oct 2, 2020
Merged

Conversation

ryan-summers
Copy link
Member

This PR fixes #67 by increasing the delay after the software reset of the ADS7924 (power monitor external ADC) from 100uS to 500uS. It was found that 100uS was not always sufficient for the reset to occur.

Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

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

500 µs is really worrying long given the 2 µs that TI mentions.
https://e2e.ti.com/support/data-converters/f/73/t/755048
This is certainly ok for now (i.e. it doesn't hurt). But I suspect that something else is wrong.

@ryan-summers
Copy link
Member Author

Note that the link you're referring to is discussing the hardware reset pin, but we're dealing with the software reset here. It's very possible that the software reset is more complicated than the hardware reset circuit.

I'm posting on the TI forums to see if I can get any information about timing requirements - I'm not a fan of this being unspecified and it can result in hard-to-diagnose bugs in the field if it doesn't work right.

@jordens
Copy link
Member

jordens commented Oct 2, 2020

Sure. But for the device with the "similar reset circuitry" (as per the TI link), they cover software and hardware resets with the same specs in terms of dead time.

image

@jordens
Copy link
Member

jordens commented Oct 2, 2020

But I'm happy to leave it at 500 µs now and your trace clearly shows that it NAK'ed after 100 µs.

@ryan-summers
Copy link
Member Author

I'm going to merge this as-is, but there's an open TI request here: https://e2e.ti.com/support/data-converters/f/73/t/945550 - I'd really like to get some more information from them, as this could be a really annoying bug in the field.

@ryan-summers
Copy link
Member Author

Further investigations are tracked in #77

@ryan-summers ryan-summers merged commit 46f5e27 into develop Oct 2, 2020
@ryan-summers ryan-summers deleted the rs/issue-67/release-panic branch October 2, 2020 17:14
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.

Panics in release mode
2 participants