tuxedo-keyboard: fix compilation for kernel 6.10 and 6.11 #336633
Conversation
|
Please change the commit message format to fit the guidelines in CONTRIBUTING.md |
|
Also, it may be worth renaming the package to fit its presumably now extended scope? |
Keksgesicht
force-pushed
the
master
branch
2 times, most recently
from
66c5da9
to
19cd864
Compare
|
I also renamed the related config options to the new scope of the tuxedo driver. I hope this is ok. |
|
There are already commit in the new upstream repo to fix compile errors with kernel 6.11. Thus, I will most likely provide a follow up pull request the coming days to update this driver to version 4.6.3. |
Keksgesicht
changed the title
pkgs/top-level/linux-kernels.nix
Outdated
| @@ -488,7 +488,7 @@ in { | |||
|
|
|||
| rust-out-of-tree-module = if lib.versionAtLeast kernel.version "6.7" then callPackage ../os-specific/linux/rust-out-of-tree-module { } else null; | |||
|
|
|||
| tuxedo-keyboard = if lib.versionAtLeast kernel.version "4.14" then callPackage ../os-specific/linux/tuxedo-keyboard { } else null; | |||
| tuxedo-driver = if lib.versionAtLeast kernel.version "4.14" then callPackage ../os-specific/linux/tuxedo-driver { } else null; | |||
There was a problem hiding this comment.
As you renamed the package here, could you add an alias to block at line 608, in-case someone didn't use the module.
Something like this:
tuxedo-keyboard = self.tuxedo-drivers;
Keksgesicht
force-pushed
the
master
branch
2 times, most recently
from
f5a12de
to
a7b81bf
Compare
|
Now it also runs with kernel 6.11 on my Aura 15 Gen1. |
Keksgesicht
changed the title
Keksgesicht
force-pushed
the
master
branch
2 times, most recently
from
c3395c6
to
22dba23
Compare
|
@blanky0230 What do you think? As you are the maintainer could you review my changes please so it's clear to committers in what state this is? |
There was a problem hiding this comment.
Hate to nitpick catching up with reviews this late but: Since the manufacturer's repo is named "tuxedo-drivers" (plural), I think the corresponding linuxPackage and nixosModule in nixpkgs should share the same name in order to minimize the possibility for mix-ups and user's confusion in the future.
Other than that this PR is looking good to me 👍
|
doesn't build for me: I just updated my flake (on unstable) to the latest version and copied tuxedo-drivers/default.nix into boot.extraModulePackages = [
(config.boot.kernelPackages.callPackage ./tuxedo-new.nix { })
]; |
|
@WhiteBlackGoose. I tested you method myself and it compiles. Your error message still mentions In my {
inputs = {
nixpkgs-master = {
type = "github";
owner = "NixOS";
repo = "nixpkgs";
ref = "master";
rev = "c5989b0173fa669297d1f92cee201cf77aa6c3eb";
};
};
outputs = {
nixpkgs-master,
}@inputs: {
nixosConfigurations = {
"myMachineName" = nixpkgs-master.lib.nixosSystem {
};
};
};
} |
|
I added the config as @Keksgesicht mentioned in his comment. Build works fine, however somehow I still don't see any backlight on my keyboard. I also have this |
|
Why haven't you tried using Also, did you reboot after a |
|
I would love to help testing this. Is there a way to enable this without setting all of my main config to this branch? |
|
@chmanie disable the old Then try to something like that to your config: { config, ... }:
let
cfgLinuxKernel = config.boot.kernelPackages;
tuxedo-pkg-file = pkgs/os-specific/linux/tuxedo-drivers/default.nix;
tuxedo-drivers = cfgLinuxKernel.callPackage tuxedo-pkg-file {};
in
{
boot.kernelModules = [ "tuxedo_keyboard" ];
boot.extraModulePackages = [ tuxedo-drivers ];
}An alternative would be to clone your current nixpkgs commit and rebase the changes of this PR onto it. Then use this version of nixpkgs for your machine config. Just add the local path to your input. Another thought that I had is to only use the package from the commit of this PR, but this does not work as the kernel minor version already changed over the time: { inputs, system, ... }:
let
pkgs-master = import inputs.nixpkgs-master { # see a few messages above
inherit system;
};
cfgLinuxKernel = pkgs-master.linuxKernel.packages.linux_6_11;
tuxedo-drivers = cfgLinuxKernel.tuxedo-keyboard;
in
{
boot.kernelModules = [ "tuxedo_keyboard" ];
boot.extraModulePackages = [ tuxedo-drivers ];
} |
I also did try
Yes both use the switch parameter and then reboot |
|
@papanito does this show any file: realpath /run/*-system/kernel-modules/lib/modules/*/updates/src/tuxedo*or (this is only the case before the latest commit where the build commands changed) realpath /run/*-system/kernel-modules/lib/modules/*/tuxedo*Also look whether your kernel loaded the modules: lsmod | grep tuxedoReport what is not the case. Did it work with an LTS kernel and the older |
It never worked on my InfinityBook, but I assumed also maybe cause the package seemed very outdated.
|
|
@papanito do you have something like this? /sys/devices/platform/tuxedo_keyboard/leds/rgb:kbd_backlightTry changing echo "51" | sudo tee /sys/devices/platform/tuxedo_keyboard/leds/rgb:kbd_backlight/brightness
echo "0 255 0" | sudo tee /sys/devices/platform/tuxedo_keyboard/leds/rgb:kbd_backlight/multi_intensity |
|
My tuxedo has
hence I can see But when I do this The keyboard still shows no light :-( |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/installing-tuxedo-keyboard-drivers/40161/7 |
|
If does not create any error. Not even in the log (see |
|
Thanks @Keksgesicht for your time and effort. I probably have to talk to them yes.... |
|
@Keksgesicht thank you, I tested it the way you described. Here are some logs, not sure if this is expected or not (Tuxedo InfinityBook 14 Gen8): And testing tuxedo-rs |
|
Lol I feel a bit stupid, but actually it works once I enable the backlight using |
|
@chmanie looks fine so far. You could test if the For the failed temperature reading I am not sure whether this driver or a generic one for the Intel or AMD platform provides this sensor. Try running |
Co-Authored-By: April John <aprl@acab.dev> Co-Authored-By: Dominik Xaver Hörl <hoe.dom@gmx.de> thanks to the work of: - NixOS#293017 - NixOS#343483
|
So, I rebased and fixed a merge conflict because someone added some system76 things above of my changes in Is there something else I have to do? How do I get the attention of a committer to merge this? |
|
The |
Description of changes
It seems like the tuxedo-keyboard has moved to GitLab a little time ago. Thus, I changed the source git the new GitLab URL and updated the version number. This also fixes a bug when compiling with Kernel 6.10.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.