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

macbook15-spi-driver: init at 0.1 #113214

Closed
wants to merge 1 commit into from
Closed

Conversation

hlolli
Copy link
Member

@hlolli hlolli commented Feb 15, 2021

Motivation for this change

This kernel-module is part of macbook pro 15+ hardware support NixOS/nixos-hardware#231

Things done

This module is fork of a fork https://github.com/roadrunner2/macbook12-spi-driver which includes a branch "mbp15" which as far as I know is the only way to get the touchbar working on macbooks 2018 and later (without which there aren't any F-keys available). The reason I forked it, is because I failed to be able to get the ambient light sensor nor keyboard backlight working, without using code from the contents from the "touchbar" branch (the main branch). So I merged together the changes on both branches to get all working. I'll make sure to maintain this module as changes come upstream.

  • 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/)
  • 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.

{ lib, stdenv, kernel, fetchFromGitHub, }:

stdenv.mkDerivation rec {
name = "macbook15-spi-driver";
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
name = "macbook15-spi-driver";
pname = "macbook15-spi-driver";

Comment on lines +27 to +29
license = lib.licenses.gpl2Only;
platforms = platforms.linux;
maintainers = [ lib.maintainers.hlolli ];
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
license = lib.licenses.gpl2Only;
platforms = platforms.linux;
maintainers = [ lib.maintainers.hlolli ];
license = licenses.gpl2Only;
platforms = platforms.linux;
maintainers = [ maintainers.hlolli ];


stdenv.mkDerivation rec {
name = "macbook15-spi-driver";
version = "0.1";
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect version.

};

buildPhase = ''
make -C ${kernel.dev}/lib/modules/${kernel.modDirVersion}/build \
Copy link
Member

Choose a reason for hiding this comment

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

You can do makeFlags = [ "KDIR=${kernel.dev}/lib/modules/${kernel.modDirVersion}/build" ]; here: https://github.com/hlolli/macbook15-spi-driver/blob/86a2f37ca0d2a4729a32b636dc48f39c522dd8ad/Makefile#L9

also see nixpkgs/pkgs/os-specific/linux/exfat/default.nix

@Mic92
Copy link
Member

Mic92 commented Mar 21, 2021

This has now a merge conflict.

@Mic92
Copy link
Member

Mic92 commented May 29, 2021

Are you still working on this?

@hlolli
Copy link
Member Author

hlolli commented May 29, 2021

This has left my radar for now. Sadly linux isn't allowed where I'm working now. This kernel module was as far as I remember crucial to get things to work at the time I tested it. Feel free to take this PR over the finishing line, if I know myself, then I'll not be using this computer for another 3 months.

@Mic92
Copy link
Member

Mic92 commented May 29, 2021

I cannot really test this. Kernel driver require regular maintenance as kernel upgrades will break them. Let's close this until we have a real user/maintainer.

@Mic92 Mic92 closed this May 29, 2021
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

3 participants