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

Second round of cleanups. #54

Merged
merged 8 commits into from
Dec 28, 2017
Merged

Second round of cleanups. #54

merged 8 commits into from
Dec 28, 2017

Conversation

roadrunner2
Copy link
Contributor

This is a second set of code cleanups - the largest part is the ones flagged by checkpatch (so hopefully that will avoid style complaints when we attempt to upstream this 😉 ).

After this I have some small fixes and enhancements lined up, plus finally the refinement of the packet/message data structures (including support multi-packet messages, and with that for more than 6 fingers).

applespi.c Outdated
/*
* MacBook (Pro) SPI keyboard and touchpad driver
*
* Copyright (c) 2015-2017 Federico Lorenzi
Copy link
Contributor

Choose a reason for hiding this comment

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

@roadrunner2: Given the work you put into that driver, you should add yourself there as author as well. You truly deserve it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, but it's fine. Federico did the original work, without which I wouldn't have had anything to contribute to.

Copy link
Owner

Choose a reason for hiding this comment

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

I definitely agree with @Dunedan here, you've done all the heavy lifting into getting the reverse engineered proof of concept I had into shape!

@l1k
Copy link
Contributor

l1k commented Dec 18, 2017

So this is almost too minor to mention, but just to avoid objections once it gets posted to the mailing list, in 7e51eac, for multi-line comments, kernel convention is to leave the first line empty and for single-line comments to not use caps, e.g.:

	/*
         * The data is stored in pairs of items, first a string containing
	 * the name of the item, followed by an 8-byte buffer containing the
	 * value in little-endian.
	 */

	/* check released keys */

The GPL license text at the top can be replaced with:

	SPDX-License-Identifier: GPL-2.0

Everything else LGTM. Thanks!

@cb22
Copy link
Owner

cb22 commented Dec 18, 2017

Looks good to me, thanks @roadrunner2!

Put all the module notes together in their respective paragraphs.

Added note about ensuring the modules are put in the initrd and that
doing so appears to solve the irqpoll issue on these machines.

Simplified instructions for setting up the modules in the initrd.
Replaced raw __set_bit()'s with input_set_capability() for consistency.
With this the explicit setting of evbit is not necessary.

Also, removed duplicate BTN_LEFT setting.
These are all style issues found by checkpatch.pl with --strict. No
functional changes.
As pointed out by Lukas Wunner.
@roadrunner2
Copy link
Contributor Author

@l1k Thanks for the feedback - don't worry about nit-picking, that's fine. Fixed the comments now too, and the license indicator.

@cb22 Pushed a new version of the branch: the changes are just the comment fixes (a new commit), as well as adjusting the copyright header.

@cb22 cb22 merged commit c106357 into cb22:master Dec 28, 2017
@roadrunner2 roadrunner2 deleted the cleanups2 branch December 29, 2017 05:47
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.

4 participants