mobile-nixos icon indicating copy to clipboard operation
mobile-nixos copied to clipboard

pinephone: emmc mod support

Open colemickens opened this issue 2 years ago • 15 comments

Hello,

I finally got the soldering iron out and did the eMMC mod:

  • https://wiki.postmarketos.org/wiki/PINE64_PinePhone_(pine64-pinephone)#Increasing_eMMC_speed_.28Vccq_mod.29
  • https://gitlab.com/postmarketOS/pmaports/-/merge_requests/2665/diffs#f0127b3c9122c57291f0b4c703758c1628469ebc

It would be nice to have a "variant" for this, or a way for the user to enable an option that does the right u-boot things.

I'll try to circle back to this but my life is a huge mess and I've got a ton to do, :crossed_fingers: .

colemickens avatar Jul 31 '22 02:07 colemickens

Here's some notes.

~~I'm assuming the device still works at previous speeds without changing anything in the software, right?~~

If one performs the eMMC Vccq mod the phone will boot fine but without speed increase.

~~Yes.~~ That was from their MR description, but uh, reading the wiki page:

In theory a PinePhone with Vccq mod could boot an unmodified image (without DT change) from eMMC, albeit in DDR52 mode, but this isn't working as the eMMC is mounted read-only during boot

That's confusing now.


Then, looking at the changes, I see it's mostly a device tree overlay change.

No new device variant is needed. What is needed is to apply a device tree overlay on top of the existing dtb file, and ensure that the system boots with it.

"Ensuring the system boots with it" might be tricky once UEFI is in use. In that upcoming future, Tow-Boot may need to be involved, but not necessarily.

samueldr avatar Jul 31 '22 02:07 samueldr

I wonder what happens when the mod isn't applied and the file is used:

  • https://gitlab.com/postmarketOS/pmaports/-/blob/570d55b47efc28e0ea78fe39a54b6b79f33a2112/device/main/device-pine64-pinephone/pinephone-vccq-mod.dts#L6-7

It's not specified. Would it just configure the device wrongly in a benign manner, or would it somehow fry the device?

samueldr avatar Jul 31 '22 02:07 samueldr

From what I've read, it might not boot, but I've not seen any indications (or stark warnings) that it can cause damage. I specifically decided "I can do this mod without the software in place since it boots from emmc and worst case scenario it doesn't boot and I have to setup a new u-boot sdcard" (which obviously I now have to do since I didn't do this software change first).

"Ensuring the system boots with it" might be tricky once UEFI is in use.

"once UEFI is in use" oh boy, I've been away too long - that sounds like fairly-set future plans.

What is needed is to apply a device tree overlay on top of the existing dtb file, and ensure that the system boots with it.

Do you feel like this can/should be done via a uboot script the same way postmarketos has done it? Is there some other effort to figure out what to do WRT to dto + uefi?

colemickens avatar Jul 31 '22 04:07 colemickens

Do you feel like this can/should be done via a uboot script the same way postmarketos has done it? Is there some other effort to figure out what to do WRT to dto + uefi?

This should be done using the existing tooling in NixOS to apply device trees. Though we can provide a device-specific "quirk" option for this, making it easier for end-users.

I don't know if the tooling will work out of the box for this, if it does not that is the work to do to solve the issue.


Though, it would be better to have the platform firmware intrinsically know about the mod, and do DT fixups correctly accordingly, but this would require some way of detecting this, which AFAIK isn't possible.

Another option, something that would require more work, would be to add knowledge of the vcc mod inside Tow-Boot (how?) and have it as an option that can be toggled by the end-user and saved (using e.g. env save). I guess the previously mentioned DT fixups could look at that environment variable and apply the required changes in that case.

In a future where Tow-Boot gets a user interface for such things, it would basically work on the same principles at first.

samueldr avatar Jul 31 '22 04:07 samueldr

:/ https://lists.denx.de/pipermail/u-boot/2019-May/368388.html

colemickens avatar Jul 31 '22 04:07 colemickens

also: https://lists.denx.de/pipermail/u-boot/2021-August/458800.html

colemickens avatar Jul 31 '22 04:07 colemickens

:/ https://lists.denx.de/pipermail/u-boot/2019-May/368388.html

What I'm talking about is the existing and already working overlaying infra from NixOS, which pre-bakes overlays.

The "for later" goal would be in Tow-Boot to add more built-in knowledge about FDTs and overlaying, which would be its own thing, not building on top of the existing U-Boot tooling. I have my own opinions that AFAIK goes against how U-Boot would do any of it.


also: https://lists.denx.de/pipermail/u-boot/2021-August/458800.html

You'll have to phrase what you mean here. I don't know what I should understand from that thread.

samueldr avatar Jul 31 '22 17:07 samueldr

What I'm talking about is the existing and already working overlaying infra from NixOS, which pre-bakes overlays.

Gotcha. I can poke around at this, but my first instinct is that the "kernel package" that mobile-nixos is composing doesn't conform to nixos expectations fully:

{
   config = {
      # ...

      hardware.deviceTree.overlays = [
        {
          name = "pinephone-emmc-vccq-mod";
          dtsFile = ./dts-pinephone-emmc-vccq-mod.dts;
        }
      ];
  };
}

results in this error:

==>> nix eval --json .#toplevels.pinephone --apply x: { out=x.outPath; drv=x.drvPath; sys=x.system; }
error: attribute 'kernelPatches' missing

       at /nix/store/blw96pa8bmikk942mmgsavavi5d6f0z9-source/nixos/modules/hardware/device-tree.nix:63:40:

           62|     inherit (cfg.kernelPackage) src nativeBuildInputs depsBuildBuild;
           63|     patches = map (patch: patch.patch) cfg.kernelPackage.kernelPatches;
             |                                        ^
           64|     buildPhase = ''
(use '--show-trace' to show detailed location information)


You'll have to phrase what you mean here. I don't know what I should understand from that thread.

Sorry, that was a long thread, but part of the work in there includes supporting the DTBOVERLAY command in extlinux.conf in u-boot's pxe flow. But it doesn't seem like that's really relevant right now anyway.

colemickens avatar Jul 31 '22 20:07 colemickens

Gotcha. I can poke around at this, but my first instinct is that the "kernel package" that mobile-nixos is composing doesn't conform to nixos expectations fully:

Oh, that might be, but work was made in #488 to basically fix that. Mobile NixOS can now work coherently with "totally normal" NixOS-flavoured kernels, and it will be the way forward for mainline kernels. This would also help solve issues with features and modules not being enabled in mainline kernels.

samueldr avatar Jul 31 '22 21:07 samueldr

I've read through #488 several times and tried to read the diff and I'm still very, very confused. I am struggling to have any conceptual idea of what I'm trying to achieve or what various options I need to enable/disable to get there.

I'm feeling kinda overwhelmed with the options here and trying to get my head around the bigger picture of how this all fits together.

colemickens avatar Aug 07 '22 02:08 colemickens

error: The option `mobile.boot.stage-1.kernel.package' is defined multiple times.

       Definition values:
       - In `/nix/store/ka2krxc8nngpzzwp26xafn0yawv6w0rf-source/modules/initrd-kernel.nix': <derivation /nix/store/qn6p27p2nj4q8xhkil29kvy4nncr61rb-linux-5.15.59.drv>
       - In `/nix/store/4f4ly4w1gvjv001gcnzwd0acbdgfwh4z-source/hosts/pinephone/configuration.nix': <derivation /nix/store/g1rcsq88xvvva1lymd3a80lvcm6swsqm-linux-5.17.5.drv>
       - In `/nix/store/4f4ly4w1gvjv001gcnzwd0acbdgfwh4z-source/hosts/pinephone/configuration.nix': <derivation /nix/store/4dd99i9q4x02m3mv32bb86lmb7vjkaiq-linux-5.19.drv>
(use '--show-trace' to show detailed location information)

somehow three different kernel derivations in the mix... :(

with my attempt: https://gist.github.com/colemickens/47aa852ddda9f0592b9052146821aec8

colemickens avatar Aug 07 '22 02:08 colemickens

mobile.boot.stage-1.kernel.useNixOSKernel = true is what you would use to use a "NixOS-flavoured" kernel build. Then once this is done, you would use the NixOS kernel options as usual on NixOS. When doing so, do not set boot.stage-1.kernel.package yourself, it'll be handled for you. You can then use boot.kernelPackages to set the kernel package.

Though with that said, it's possible it won't work with the system type used by the Pinephone, I have not checked that.

samueldr avatar Aug 07 '22 03:08 samueldr

Yeah, I'm not sure. I tried that too, and basically the same issue:

https://gist.github.com/colemickens/c30b6053b50c4f3305433f08f0b4c98e

error: The option `mobile.boot.stage-1.kernel.package' is defined multiple times.

       Definition values:
       - In `/nix/store/ka2krxc8nngpzzwp26xafn0yawv6w0rf-source/modules/initrd-kernel.nix': <derivation /nix/store/4dd99i9q4x02m3mv32bb86lmb7vjkaiq-linux-5.19.drv>
       - In `/nix/store/q0i0lw000xah9pgzsmrx9vqdyn874n7n-source/hosts/pinephone/configuration.nix': <derivation /nix/store/g1rcsq88xvvva1lymd3a80lvcm6swsqm-linux-5.17.5.drv>
(use '--show-trace' to show detailed location information)

I'm not sure I even understand the error message. linuxPackages_latest is what I'm setting in my configuration.nix which should actually be 5.19... so I'm not sure why it says 5.17.5?

colemickens avatar Aug 07 '22 04:08 colemickens

Summarizing some Matrix convo, it seems there are two paths:

  1. use the nixos kernel infra, disable the option inside the mobile-nixos pinephone device definition that sets the kernel, define a nixos kernel for the pinephone
  2. add support in mobile-nixos for building dtbos and/or applying them during boot similar to how is done in nixos

presumably #2 is preferred, but I don't really understand the design choices behind have a different kernel builder, nor do I know anything about the pieces at play in building the dtbo and applying it without diving in more than I can probably justify right now.

colemickens avatar Aug 17 '22 03:08 colemickens

These are not two independent paths. You will need to implement option (2), whether the kernel is built with the NixOS kernel builder semantics or not.

I don't really understand the design choices behind have a different kernel builder

Plain and simply, vendor android kernel builds are severely broken, and this mostly papers around the issue.

There are some other tasks that could be added to the main kernel building infra too, like the kernel logo patches.

nor do I know anything about the pieces at play in building the dtbo and applying it

.dtb from the kernel build goes in, .dtbo is applied on top, new .dtb goes out. And rather than using "${boot.kernelPackage}/dtbs/..." we use "${config.hardware.deviceTree.package}/dtbs/"

Though really, (2) is likely already being intrinsically done via composition... As long as we're already using the "system" output to load the dtb files, it should already be using the correct dtbs folder from the device tree overlay infra.

  • https://github.com/NixOS/nixpkgs/blob/3d47bbaa26e7a771059d828eecf3bd8bf28a8b0f/nixos/modules/hardware/device-tree.nix#L201-L203
  • https://github.com/NixOS/nixpkgs/blob/3d47bbaa26e7a771059d828eecf3bd8bf28a8b0f/nixos/modules/system/activation/top-level.nix#L39-L41

samueldr avatar Aug 17 '22 18:08 samueldr