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

Make the driver ready for linux upstream #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

c0d3z3r0
Copy link
Contributor

@c0d3z3r0 c0d3z3r0 commented Feb 5, 2018

more will follow:

  • more simplifications of the driver
  • replace deprecated stuff
  • incorporate changes requested/suggested on linux mailing list
  • ... (let's see)

ret = -ENOMEM;
}

/* value not assigned => wmi call with return value (e.g. change settings) */
Copy link
Owner

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

Copy link
Contributor Author

@c0d3z3r0 c0d3z3r0 Feb 6, 2018

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

ret = -EIO;

/* value assigned -> wmi call returning output */
else if(value) {
Copy link
Owner

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, working on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (ACPI_FAILURE(status))
return -EIO;

return thinkpad_wmi_extract_output_string(&output, value);
obj = output.pointer;
Copy link
Owner

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see comment before

}

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);
Copy link
Owner

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@c0d3z3r0 c0d3z3r0 changed the title Simplify wmi calls Make the driver ready for linux mainline Feb 6, 2018
@c0d3z3r0 c0d3z3r0 changed the title Make the driver ready for linux mainline Make the driver ready for linux upstream Feb 6, 2018
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.

2 participants