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

Allow GPIO example built with Linux v6.10+ #287

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

jeremy90307
Copy link
Contributor

gpio_request_array() and gpio_free_array() were
removed in Kernel v6.10-rc1. Use gpio_request()
and gpio_free() instead to make the example run
successfully on v6.10+.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Use preprocessor directives for the compatibility among several Linux kernel versions.

@jeremy90307
Copy link
Contributor Author

I understand. I will fix it by using preprocessor directives.

@jserv
Copy link
Contributor

jserv commented Dec 4, 2024

I understand. I will fix it by using preprocessor directives.

Ensure that LKM can be built with Linux v5.15, v6.1, and v6.6+.

Comment on lines 18 to 20
/* gpio_request_array() and gpio_free_array()
* were removed in kernel v6.10-rc1 .
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention in git commit messages rather than program comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

You should denote the commit hash associated with the removal of gpio_request_array and gpio_free_array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed this comment and mentioned it in the commit message.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, always squash and refine the messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Read https://cbea.ms/git-commit/ carefully and enforce the rules. You should refine the git commit messages.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have removed this comment and mentioned it in the commit message.

You must refer to the exact git commit hash and summarize the rationale.

examples/bh_threaded.c Outdated Show resolved Hide resolved
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Fix conflicts and use git rebase -i to squash commits.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Read https://cbea.ms/git-commit/ carefully and enforce the rules.

@jeremy90307
Copy link
Contributor Author

Thank you, I will make the necessary corrections.

@jserv
Copy link
Contributor

jserv commented Dec 4, 2024

Thank you, I will make the necessary corrections.

Demonstrate your solution through concrete actions rather than making verbal promises.

examples/bh_threaded.c Outdated Show resolved Hide resolved
examples/bh_threaded.c Outdated Show resolved Hide resolved
@jeremy90307
Copy link
Contributor Author

Is there anything to improve in this commit? Thank you.

@jserv
Copy link
Contributor

jserv commented Dec 5, 2024

Is there anything to improve in this commit?

Carefully study the principles of https://cbea.ms/git-commit/ . When writing a commit message, use the imperative mood in the subject line, specifically employing precise action verbs. For instance, use "Allow" rather than passive constructions.
Ensure your commit message body is wrapped at 72 characters without cutting off words mid-line. More critically, focus on explaining the what and why of your changes, not the how. The message should provide substantive context about the rationale behind the code modification.

Take commit d2d54ca for example. We prefer the notation commit b8780c363d80 ("sched: remove sleep_on() and friends"). This format is preferred because it concisely combines the commit hash with a descriptive subject line. The message should go beyond mere description and dive into the underlying reasoning for the change.

When contributing to a technical project, communication should be direct and purposeful. The phrase "Thank you" becomes unnecessary when the primary goal is collaborative technical work. In a professional development context, your interactions should focus on the technical details, code quality, and project improvement.

@jserv jserv changed the title Fix GPIO example to run on Kernel v6.10+ Fix GPIO example to run on Linux v6.10+ Dec 5, 2024
@jeremy90307 jeremy90307 force-pushed the master branch 2 times, most recently from 1a1e5b7 to ee6a0da Compare December 6, 2024 15:53
@jeremy90307 jeremy90307 requested a review from jserv December 9, 2024 01:59
@jserv jserv changed the title Fix GPIO example to run on Linux v6.10+ Allow GPIO example built with Linux v6.10+ Dec 9, 2024
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Compose Git commit messages in the third person, and include a test summary specific to Raspberry Pi deployment or testing.

@jserv
Copy link
Contributor

jserv commented Dec 9, 2024

Refine the git commit message as following:

Commit dbcedec ("gpiolib: legacy: Remove unused gpio_request_array()
and gpio_free_array()") indicated that there are no more users of these
functions; therefore, they were removed.

And address the considerations of removal fro Linux kernel developers' perspectives.

@jeremy90307
Copy link
Contributor Author

jeremy90307 commented Dec 9, 2024

@jserv
Copy link
Contributor

jserv commented Dec 9, 2024

https://github.com/user-attachments/assets/bc43c25b-f02a-43c7-9bd1-a178f94b99f5 Successfully ran on Linux v6.12.1.

You should explain the testing procedure within git commit messages as well.

@jeremy90307
Copy link
Contributor Author

My testing steps only include compiling, connecting an LED and a button, and finally loading the module.

@jeremy90307 jeremy90307 force-pushed the master branch 2 times, most recently from 07f689e to d1658dc Compare December 10, 2024 09:25
Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Rebase the latest master branch to pass CI pipeline.

Copy link
Contributor

@jserv jserv left a comment

Choose a reason for hiding this comment

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

Replace Resolves:#285 with shorter Close #285.

Since the commit dbcedec ("gpiolib: legacy: Remove unused
gpio_request_array() and gpio_free_array()"), these functions had no
users in kernel and were subsequently removed to simplify the library.

These functions have been removed from GPIO examples for Linux
v6.10+ to ensure compatibility across all kernel versions.

Testing detail:

- Tested on Raspberry Pi 5B with Raspberry Pi OS (Debian 12, Linux
  version 6.12.1-v8-16k+)

- Verified the GPIO examples compile and load successfully

- Verified GPIO17 interrupt turns on the LED (GPIO4)

- Verified GPIO18 interrupt turns off the LED (GPIO4)

Close sysprog21#285
@jeremy90307 jeremy90307 requested a review from jserv December 11, 2024 09:20
@jserv jserv merged commit e442916 into sysprog21:master Dec 11, 2024
1 check passed
@jserv
Copy link
Contributor

jserv commented Dec 11, 2024

Thank @jeremy90307 for contributing!

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