-
Notifications
You must be signed in to change notification settings - Fork 44
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
test-socwatch.sh: no need to load socwatch module manually #1157
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't delete all the corresponding code. We are barely getting started in this area which means things will change in the future and we may need this again soon.
In fact it should be very easy to make the module loading and unload optional with just this:
- sudo "$SOCWATCH_PATH"/drivers/insmod-socwatch -q
+ sudo "$SOCWATCH_PATH"/drivers/insmod-socwatch -q || true
- check_socwatch_module_loaded || die "socwatch module not loaded"
+ check_socwatch_module_loaded || dlogi "socwatch module not loaded"
cf6c2f5
to
5ec7437
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't find the commit message very accurate but... good enough.
We do need to load the driver manually; this PR just makes that optional.
} | ||
|
||
socwatch_test_once() | ||
{ | ||
local i="$1" | ||
dlogi "===== Loop($i/$loop_count) =====" | ||
dlogi "SoCWatch version: ${SOCWATCH_VERSION}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you could have just run the command here but no big deal.
We only use socwatch tool to read the power data for power measurement in this case, we can directly use the latest released version, so there is no need to pre-compile the driver. Signed-off-by: Keqiao Zhang <[email protected]>
5ec7437
to
8013d14
Compare
@fredoh9 can we merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
We only use socwatch tool to read the power data for power measurement in this case, we can directly use the latest released version, so there is no need to pre-compile the driver.