-
Notifications
You must be signed in to change notification settings - Fork 6
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
Make the driver ready for linux upstream #6
base: master
Are you sure you want to change the base?
Conversation
drivers/platform/x86/thinkpad-wmi.c
Outdated
ret = -ENOMEM; | ||
} | ||
|
||
/* value not assigned => wmi call with return value (e.g. change settings) */ |
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.
IMHO this is cleaner in a separate function
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.
That point is open to dispute; Personally I like this better because it removes redundant code... let's see what it looks like at the end when everything else is finished
drivers/platform/x86/thinkpad-wmi.c
Outdated
ret = -EIO; | ||
|
||
/* value assigned -> wmi call returning output */ | ||
else if(value) { |
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.
Add braces if you have empty lines (see scripts/checkpatch.pl to automatically lint the code)
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.
yep, working on 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.
done
drivers/platform/x86/thinkpad-wmi.c
Outdated
if (ACPI_FAILURE(status)) | ||
return -EIO; | ||
|
||
return thinkpad_wmi_extract_output_string(&output, value); | ||
obj = output.pointer; |
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.
IMHO it was better in smaller functions
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.
see comment before
drivers/platform/x86/thinkpad-wmi.c
Outdated
} | ||
|
||
static int thinkpad_wmi_set_bios_settings(const char *settings) | ||
{ | ||
return thinkpad_wmi_simple_call(LENOVO_SET_BIOS_SETTINGS_GUID, | ||
settings); | ||
return thinkpad_wmi_call(LENOVO_SET_BIOS_SETTINGS_GUID, settings, NULL); |
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 like thinkpad_wmi_simple_call() better than having to specify NULL all the time
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.
done
more will follow: