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

Jump to bottom

[RFC] kernel: drop kmod-ledtrig-{default-on,heartbeat,netdev,timer} packages #3754

Closed
wants to merge 3 commits into from
Closed

[RFC] kernel: drop kmod-ledtrig-{default-on,heartbeat,netdev,timer} packages #3754

wants to merge 3 commits into from

Conversation

mans0n
Copy link
Member

@mans0n mans0n commented Jan 4, 2021 •
edited

The goal of this PR is to remove kmod dependency of the luci collection package.

When we run opkg install luci on self-built snapshot image, the installation failes
due to unmet kernel version dependency:

root@OpenWrt:/# opkg install luci
Installing luci (git-20.074.84698-ead5e81) to root...
Downloading https://downloads.openwrt.org/snapshots/packages/arm_xscale/luci/luci_git-20.074.84698-ead5e81_all.ipk
Collected errors:
 * pkg_hash_fetch_best_installation_candidate: Packages for kmod-ledtrig-default-on found, but incompatible with the architectures configured
 * pkg_hash_fetch_best_installation_candidate: Packages for kmod-ledtrig-heartbeat found, but incompatible with the architectures configured
 * pkg_hash_fetch_best_installation_candidate: Packages for kmod-ledtrig-netdev found, but incompatible with the architectures configured
 * pkg_hash_fetch_best_installation_candidate: Packages for kmod-ledtrig-timer found, but incompatible with the architectures configured
 * satisfy_dependencies_for: Cannot satisfy the following dependencies for luci:
 *      kernel (= 5.4.86-1-f9bcc5911da3ac3e6475f95070b6e091)
 * opkg_install_cmd: Cannot install package luci.

Of course, it can be easily resolved by adding luci or kmod-ledtrig-* packages at build time, but I admit it becomes a bit cumbersome sometimes.
Fortunately, the three of the four kernel modules are already built-in on most targets, and the last one, heartbeat, does not increase kernel size that much, so we can make all of them built-in and remove the kmod packages.

I also made a patch for the luci part. I'll open another PR for it, if this gets positive feedbacks.
EDIT: openwrt/luci#4719

@mans0n
kernel: enable CONFIG_LEDS_TRIGGER_HEARTBEAT
b508f5d 
The heartbeat trigger is used by luci-mod-system, which is installed
as a part of the standard luci package set. It seems the LED trigger will be
required quite often, so let's enable it by default.

This increses uncompressed kernel size by about 100 bytes on ath79/generic.

Signed-off-by: Sungbo Eo <mans0n@gorani.run>
@mans0n
treewide: do not disable LED triggers in target config
d893a99 
Those targets have already enabled some other LED triggers, so enabling
a few more won't be a big problem.

Signed-off-by: Sungbo Eo <mans0n@gorani.run>
@mans0n
kernel: drop empty kmod-ledtrig-* packages
7d70d69 
The following four led triggers are enabled in generic config.

* kmod-ledtrig-default-on
* kmod-ledtrig-heartbeat
* kmod-ledtrig-netdev
* kmod-ledtrig-timer

Drop the packages and remove them from DEVICE_PACKAGES.
There's no other package depending on them in this repo.

Signed-off-by: Sungbo Eo <mans0n@gorani.run>
@adschm adschm added kernel pull request/issue with Linux kernel related changes RFC pull request ready for comments treewide pull request/issue with change across more than single place labels Jan 4, 2021
@adschm
Copy link
Member

adschm commented Jan 4, 2021

This looks like a good idea to me. The targets that disabled triggers before are only a few (and mostly legacy), and the size requirement for heartbeat is obviously negligible.

The only remaining possible reason to disable the symbols on certain targets would be if the functionality is broken there. If we can exclude that to a fair level, I think we should merge this.

However, I'm no kernel expert, so better wait for an additional opinion before you go on with additional work :-)

@adschm
Copy link
Member

adschm commented Jan 4, 2021

And I'm not really sure whether the third patch should be a "kernel:" or a "treewide:". But that's really the least of our problems ...

@mans0n
Copy link
Member Author

mans0n commented Jan 5, 2021 •
edited

I had a run-test on oxnas with built-in netdev trigger and it worked well. And I don't think default-on or timer trigger would ever break on any platform.

@mans0n mans0n mentioned this pull request Jan 8, 2021
Merged
jow-
jow- approved these changes Jan 8, 2021
View reviewed changes
Copy link
Contributor

@jow- jow- left a comment

Choose a reason for hiding this comment

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

ACK from the LuCI side.

@adschm
Copy link
Member

adschm commented Jan 15, 2021

I decided to merge this, it's well-justified and I don't see any obvious issue with it.
Thanks!

@adschm adschm closed this Jan 15, 2021
@mans0n mans0n deleted the pr-kmod-ledtrig-heartbeat branch January 15, 2021 23:49
@mans0n
Copy link
Member Author

mans0n commented Jan 15, 2021

Thank you for review and merge, as always. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jow- jow- jow- approved these changes

Assignees
No one assigned
Labels
kernel pull request/issue with Linux kernel related changes RFC pull request ready for comments treewide pull request/issue with change across more than single place
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mans0n @adschm @jow-