Upgrade to kernel 5.6 #185
Conversation
…-config * Change `default.nix` to match kernel changeset, recalculate sha256 key * Change LEDs patch file since `sun50i-a64-pinephone.dts` is now replaced by `sun50i-a64-pinephone.dtsi` Signed-off-by: S Dao <si.dao@outlook.com>
…-config * Change `default.nix` to match kernel changeset, recalculate sha256 key * Change LEDs patch file since `sun50i-a64-pinephone.dts` is now replaced by `sun50i-a64-pinephone.dtsi` Signed-off-by: S Dao <si.dao@outlook.com>
* Upgrade to kernel 5.6.x * Add `rev` to hardware to specify hardware revision to build with Signed-off-by: S Dao <si.dao@outlook.com>
|
Hi! 👋 Thanks for the contribution, and welcome! First of all, see #146, another attempt at upgrading the kernel. On which @masipcat said (June 11th):
So this is why it hasn't been upgraded yet. Now, I'll review your PR in isolation as if there wasn't another PR for the update, as I'm sure some of the comments may be helpful. |
|
❌ The documentation build fails, since this is apparently using a new option without defining it. Now, do we need this option? I don't think we need it. The way the u-boot works, and the way the device tree files are named, we can have a build that works with all Pinephone revisions. If we can't it's a big failure from upstream. (Side-note, the status of U-Boot probably needs to be checked too.)
|
| @@ -13,10 +13,11 @@ | |||
| screen = { | |||
| width = 720; height = 1440; | |||
| }; | |||
| rev = if "" == builtins.getEnv "MOBILE_NIXOS_DEVICE_HW_REV" then "1.0" else builtins.getEnv "MOBILE_NIXOS_DEVICE_HW_REV"; | |||
There was a problem hiding this comment.
While there is an environment variable that can affect the build for the device name, it should probably be deprecated as this causes build reproducibility issues. This should be done entirely through configuration if wanted.
| Subject: [PATCH] dts: pinephone: Setup default on and panic LEDs | ||
|
|
||
| * The green LED defaults to on. | ||
| * The red LED is the panic indicator. | ||
|
|
||
| The green LED defaults to on for an expected transition through red, | ||
| yellow and green during the different boot stages. | ||
|
|
||
| Signed-off-by: S Dao <si.dao@outlook.com> |
There was a problem hiding this comment.
(Personal peeve) I would have liked to see Co-authored-by since you changed the author of the patch. Though I do see the patch has relatively non-trivial changes too, so this is why changing the author isn't an issue in itself.
| @@ -21,6 +22,6 @@ | |||
| installTargets = [ "install" "dtbs" ]; | |||
| postInstall = postInstall + '' | |||
| mkdir -p "$out/dtbs/allwinner" | |||
| cp -v "$buildRoot/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtb" "$out/dtbs/allwinner/" | |||
| cp -v "$buildRoot/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone-${hardware.rev}.dtb" "$out/dtbs/allwinner/sun50i-a64-pinephone.dtb" | |||
There was a problem hiding this comment.
This is where we should instead copy the versioned FDTs.
More things to look at:
- https://gitlab.com/pine64-org/u-boot/-/commit/1f770b7060840137fd94af6fbf4788fcacc1a5fd
- https://gitlab.com/pine64-org/u-boot/-/commit/b401e51cc1078ede81cc6f53d120f9c77a301952
- https://gitlab.com/pine64-org/u-boot/-/commit/16d23194ec7eb3663bca1032eff1dc2b134cd4f8
These commits are proof that copying all Pinephone FDTs is the solution, and we should look into updating U-Boot soon :).
|
Another nit, sorry for piling on 😊. The PR title should reference the device name, since this is a pretty heterogeneous ecosystem :). |
|
I expect to solve issue related to pkg-config missing when do |
|
#187 should solve the issue with |
|
OKay, let's me try again |
Upgrade to kernel 5.6
default.nixto match kernel changeset, recalculate sha256 keysun50i-a64-pinephone.dtsis now replaced bysun50i-a64-pinephone.dtsirevto hardware to specify hardware revision to build with (input byMOBILE_NIXOS_DEVICE_HW_REV)e.g
MOBILE_NIXOS_DEVICE_HW_REV="1.2" nix-build --argstr device pine64-pinephone-braveheart -A build.disk-imageThere is not much difference in term of kernel config between v5.5 and v5.6 therefore keeping kernel config as of now.