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

zenergy: init at 0-unstable-2024-05-19 #297598

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

Conversation

wizardlink
Copy link

Description of changes

Adds a new kernel module, zenergy, this is necessary for accessing CPU power draw data without root permission, specifically MangoHud requires it to be able to show the user their CPU power draw.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@wizardlink
Copy link
Author

wizardlink commented Mar 21, 2024

I apologize if I missed something in advance, first time contributing to the repository! I am a little confused though with the testing and release note markdown, for the testing I couldn't find an example of a kernel module being tested, if it's even possible with the suite available; whereas the markdown file, it implies that not all additions will be covered, so would this PR be applicable still? Not quite sure.

pkgs/os-specific/linux/zenergy/default.nix Outdated Show resolved Hide resolved
pkgs/os-specific/linux/zenergy/default.nix Outdated Show resolved Hide resolved
Comment on lines +30 to +31
installPhase = ''
make modules_install KDIR=${kernelDirectory}
'';
Copy link
Member

Choose a reason for hiding this comment

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

It should be enough to set makeTarget and makeFlags like already done

Copy link
Author

Choose a reason for hiding this comment

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

I searched in the repository for examples and tried to replicate, but in the installPhase the KDIR environment wasn't set on top of not running modules_install with makeTargets = [ "modules_install" ].

Am I missing something?

Copy link
Author

Choose a reason for hiding this comment

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

@SuperSandro2000 can you also give me an update on this?

pkgs/os-specific/linux/zenergy/default.nix Outdated Show resolved Hide resolved
@SuperSandro2000
Copy link
Member

I am a little confused though with the testing and release note markdown, for the testing I couldn't find an example of a kernel module being tested, if it's even possible with the suite available; whereas the markdown file, it implies that not all additions will be covered, so would this PR be applicable still? Not quite sure.

If you are doing the change on top of nixos-unstable, you can just rebuild your system with it and try to use the kernel module like normally done.
We don't need a release note entry here.

@wizardlink
Copy link
Author

wizardlink commented Jun 13, 2024

I've been using that kernel module locally since the PR has been open, so I can attest that it's in working condition!

I'll be adjusting for the review ASAP. o7

@wizardlink wizardlink changed the title zenergy: init at a3e1244 zenergy: init at d65592b Jun 15, 2024
@SuperSandro2000 SuperSandro2000 changed the title zenergy: init at d65592b zenergy: init at 0-unstable-2024-05-19 Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants