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

Add basic debug logging #139

Merged
merged 4 commits into from Aug 29, 2018
Merged

Conversation

ktemkin
Copy link
Contributor

@ktemkin ktemkin commented Aug 18, 2018

This allows a linux-like debug logging mode:

  • If a debugger is connected, debug printk's are emitted via semihosting.
  • printk logs to a ringbuffer, which can be queried using greatfet dmesg.

@ktemkin ktemkin requested a review from dominicgs August 18, 2018 02:29
// Possible log levels.
enum debug_log_level {
LOGLEVEL_EMERGENCY,
LOGLEVEL_ALERT,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be accompanying pr_alert() and pr_critical()functions?

enum debug_log_level {
LOGLEVEL_EMERGENCY,
LOGLEVEL_ALERT,
LOGLEVEL_CRITITCAL,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be LOGLEVEL_CRITICAL? (spelling)

@dominicgs
Copy link
Contributor

My review is two minor nitpicks. The rest of it looks great and I'm happy to merge it if you have counter arguments to my nitpicks.

@ktemkin
Copy link
Contributor Author

ktemkin commented Aug 29, 2018

Review comments looked good; addressed them. Thanks; here's an updated PR. :)

@dominicgs dominicgs merged commit 084b1f0 into greatscottgadgets:master Aug 29, 2018
@dominicgs
Copy link
Contributor

Apparently you can't merge and then approve, but rest assured, I reviewed and approved of the last commit too.

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.

None yet

2 participants