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

linuxPackages.hid-nintendo: init at 3.1 #108348

Merged
merged 1 commit into from May 20, 2021
Merged

Conversation

rencire
Copy link
Contributor

@rencire rencire commented Jan 3, 2021

Motivation for this change

Add dkms-hid-nintendo "out-of-tree" kernel module.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • [] Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
    • No binary files available; tested with a Nintendo Switch Pro Controller.
    • Added the module to configuration.nix.
      • i.e. boot.extraModulePackages = with config.boot.kernelPackages; [ hid-nintendo ];
    • Output from modinfo:
      > modinfo hid_nintendo
      filename:       /run/current-system/kernel-modules/lib/modules/5.9.16/extra/hid-nintendo.ko.xz
      description:    Driver for Nintendo Switch Controllers
      author:         Daniel J. Ogorchock <djogorchock@gmail.com>
      license:        GPL
      alias:          hid:b0005g*v0000057Ep00002007
      alias:          hid:b0005g*v0000057Ep00002006
      alias:          hid:b0003g*v0000057Ep0000200E
      alias:          hid:b0005g*v0000057Ep00002009
      alias:          hid:b0003g*v0000057Ep00002009
      depends:        hid,ff-memless,led-class
      retpoline:      Y
      name:           hid_nintendo
      vermagic:       5.9.16 SMP mod_unload
      
    • Tested with Nintendo Switch Pro Controller
    • Controller is detected via either USB or Bluetooth.
    • Tested with games that support SDL by remapping the controller buttons, works fine.
    • Also tested buttons w/ Steam Input; mostly works fine when it is connected. However, bluetooth sometimes disconnects, but outside scope of this PR. (might be related to Add detection of upcoming hid-nintendo joy-con/pro controller kernel driver ValveSoftware/steam-for-linux#6651)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@SuperSandro2000
Copy link
Member

3 packages failed to build:
linuxPackages-libre.hid-nintendo linuxPackages_4_4.hid-nintendo linuxPackages_4_9.hid-nintendo

Can you please blacklist those kernels to produce less build failures?

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 108348 run on x86_64-linux 1

2 packages marked as broken and skipped:
  • linuxPackages_hardkernel_4_14.hid-nintendo
  • linuxPackages_hardkernel_latest.hid-nintendo
3 packages failed to build:
  • linuxPackages-libre.hid-nintendo
  • linuxPackages_4_4.hid-nintendo
  • linuxPackages_4_9.hid-nintendo
15 packages built:
  • linuxPackages.hid-nintendo (linuxPackages_5_4.hid-nintendo)
  • linuxPackages_4_14.hid-nintendo
  • linuxPackages_4_19.hid-nintendo
  • linuxPackages_5_10.hid-nintendo (linuxPackages_latest.hid-nintendo)
  • linuxPackages_5_9.hid-nintendo
  • linuxPackages_hardened.hid-nintendo
  • linuxPackages_latest-libre.hid-nintendo
  • linuxPackages_latest_hardened.hid-nintendo
  • linuxPackages_latest_xen_dom0.hid-nintendo
  • linuxPackages_latest_xen_dom0_hardened.hid-nintendo
  • linuxPackages_lqx.hid-nintendo
  • linuxPackages_testing_bcachefs.hid-nintendo
  • linuxPackages_xen_dom0.hid-nintendo
  • linuxPackages_xen_dom0_hardened.hid-nintendo
  • linuxPackages_zen.hid-nintendo

@rencire
Copy link
Contributor Author

rencire commented Jan 7, 2021

3 packages failed to build:
linuxPackages-libre.hid-nintendo linuxPackages_4_4.hid-nintendo linuxPackages_4_9.hid-nintendo

Can you please blacklist those kernels to produce less build failures?

thanks for feedback. I'm not sure how to do this in the code. In some of the examples, I see we have an if then else statement guarding against certain versions. Is this the approach to take?

Trying this, and it didn't seem to work?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-ignore-specific-kernel-versions-when-building-kernel-modules/10919/1

Copy link
Member

@KamilaBorowska KamilaBorowska left a comment

Choose a reason for hiding this comment

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

Seems to work for me with JoyCons.

SN30 pro+ in Switch mode did not work, but that's not an official controller, and it seems to be a known issue, so that's fine.

license = licenses.gpl2;
maintainers = [ maintainers.rencire ];
platforms = platforms.linux;
broken = builtins.elem kernel.version ["libre" "4.4" "4.9"];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
broken = builtins.elem kernel.version ["libre" "4.4" "4.9"];
broken = stdenv.lib.versionOlder kernel.version "4.14";

Instead of restricting specific versions, you may want to use versionOlder. linux-libre likely failed due to linux-libre being broken rather than due to an issue with the package, so it shouldn't be explicitly blocked.

@@ -0,0 +1,42 @@
# References:
Copy link
Member

Choose a reason for hiding this comment

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

May as well remove the references, they are generic Linux kernel module packing information rather than anything hid-nintendo specific.

license = licenses.gpl2;
maintainers = [ maintainers.rencire ];
platforms = platforms.linux;
broken = stdenv.lib.versionOlder kernel.version "4.14";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
broken = stdenv.lib.versionOlder kernel.version "4.14";
broken = lib.versionOlder kernel.version "4.14";

See #108938.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would work too:

Suggested change
broken = stdenv.lib.versionOlder kernel.version "4.14";
broken = versionOlder kernel.version "4.14";

installFlags = [ "INSTALL_MOD_PATH=${placeholder "out"}" ];
installTargets = [ "modules_install" ];

meta = with stdenv.lib; {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
meta = with stdenv.lib; {
meta = with lib; {

license = licenses.gpl2;
maintainers = [ maintainers.rencire ];
platforms = platforms.linux;
broken = stdenv.lib.versionOlder kernel.version "4.14";
Copy link
Contributor

Choose a reason for hiding this comment

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

This would work too:

Suggested change
broken = stdenv.lib.versionOlder kernel.version "4.14";
broken = versionOlder kernel.version "4.14";

@jakobrs
Copy link
Contributor

jakobrs commented May 9, 2021

This lgtm, but you should update to 3.1 and squash the commits.

@cole-h
Copy link
Member

cole-h commented May 10, 2021

@ofborg eval

(GitHub fix your API)

@jakobrs
Copy link
Contributor

jakobrs commented May 10, 2021

I can confirm that this evaluates and builds (on both linuxPackages and linuxPackages_latest), and that the module seems to work (I was able to insmod it). I haven't tried pairing a controller but I don't see why it shouldn't work.

@rencire rencire changed the title linuxPackages.hid-nintendo: init at 3.0 linuxPackages.hid-nintendo: init at 3.1 May 10, 2021
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/380

@Lassulus
Copy link
Member

Result of nixpkgs-review pr 108348 1

6 packages marked as broken and skipped:
  • linuxPackages-libre.hid-nintendo
  • linuxPackages_4_4.hid-nintendo
  • linuxPackages_4_9.hid-nintendo
  • linuxPackages_hardkernel_4_14.hid-nintendo
  • linuxPackages_hardkernel_latest.hid-nintendo
  • linuxPackages_latest-libre.hid-nintendo
13 packages built:
  • linuxPackages.hid-nintendo (linuxPackages_5_10.hid-nintendo)
  • linuxPackages_4_14.hid-nintendo
  • linuxPackages_4_19.hid-nintendo
  • linuxPackages_5_11.hid-nintendo (linuxPackages_latest.hid-nintendo)
  • linuxPackages_5_12.hid-nintendo
  • linuxPackages_5_4.hid-nintendo
  • linuxPackages_hardened.hid-nintendo
  • linuxPackages_latest_hardened.hid-nintendo
  • linuxPackages_lqx.hid-nintendo
  • linuxPackages_testing_bcachefs.hid-nintendo
  • linuxPackages_xanmod.hid-nintendo
  • linuxPackages_zen.hid-nintendo
  • tests.trivial

@Lassulus Lassulus merged commit 213cde9 into NixOS:master May 20, 2021
@rencire rencire deleted the add-hid-nintendo branch January 3, 2023 04:06
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.

None yet

7 participants