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

ubootRock64, ubootRockPro64: use upstream U-Boot #74580

Merged
merged 1 commit into from Dec 6, 2019

Conversation

@lopsided98
Copy link
Contributor

@lopsided98 lopsided98 commented Nov 29, 2019

Motivation for this change

This PR makes the ubootRock64 and ubootRockPro64 packages use upstream U-Boot. This allows us to get rid all of blobs for the RockPro64, but Rock64 still needs a binary TPL to avoid what appears to be random memory corruption.

The Rock64 usually boots, but lots of processes die with kernel oops and memtester reports lots of failures. Something is wrong with the SDRAM initialization in the upstream TPL.

The flashing procedure is somewhat different from the downstream U-Boot. Like before, idbloader.img must be placed at sector 64 of the eMMC/SD, but now u-boot.itb must also be placed at sector 16384. To make this less error prone, I created partitions at those offsets. I also had not left enough room in front of my root partition, so I had to reformat my eMMC. Once this PR is merged, I will update the wiki to include this information.

I also included a patch that moves ramdisk_addr_r to allow loading larger kernels. I used to just manually modify the environment to do this, but saveenv doesn't work yet with upstream U-Boot and this is a better fix anyway. I plan to submit this upstream as soon as it is reviewed here.

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 nix-review --run "nix-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.
Notify maintainers

cc @dezgeg @samueldr @thefloweringash @bennofs

pkgs/misc/uboot/default.nix Outdated Show resolved Hide resolved
@bennofs
Copy link
Contributor

@bennofs bennofs commented Nov 29, 2019

Build fails for me with:

builder for '/nix/store/1ygc1377fh5kxxmra7vd19r53v5l1f15-uboot-rock64-rk3328_defconfig-2019.10.drv' failed with exit code 2; last 10 log lines:
    COPY    u-boot.dtb
    MKIMAGE u-boot-dtb.img
    MKIMAGE tpl/u-boot-tpl.img
    CAT     idbloader.img
    CFGCHK  u-boot.cfg
  ./"arch/arm/mach-rockchip/make_fit_atf.py" \
  arch/arm/dts/rk3328-rock64.dtb > u-boot.its
  /nix/store/bk9y5mzvj50snn0kr02k9fw6pffmz9z6-bash-4.4-p23/bin/bash: ./arch/arm/mach-rockchip/make_fit_atf.py: /usr/bin/env: bad interpreter: No such file or directory

If I add a patchShebangs arch/arm/mach-rockchip, it works fine.

@lopsided98 lopsided98 force-pushed the lopsided98:uboot-rock64-upstream branch from f7967ea to 12f0c92 Nov 29, 2019
@lopsided98
Copy link
Contributor Author

@lopsided98 lopsided98 commented Nov 29, 2019

I only ever tested this on a non-NixOS machine, which seems to have a misconfigured sandbox.

The patch has not been submitted upstream yet. I wanted to get it reviewed here first. In particular, I was wondering I should apply the same fix to the RK3288. I know @samueldr and @lheckemann have/had RK3288 devices, but my guess is that they haven't needed this fix because armv7l kernels are much smaller.

@thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented Nov 30, 2019

Testing this on a Rock64, it boots, but I seem to get a random mac address each time. I'll try to find my serial console to dig deeper.

@lopsided98
Copy link
Contributor Author

@lopsided98 lopsided98 commented Nov 30, 2019

That's interesting, I noticed that my MAC address changed when I first switched to 2019.10, but it has stayed the same since then. I see the following message in U-Boot:

Warning: ethernet@ff540000 (eth0) using random MAC address - 02:9c:c7:32:89:bb

but that address does not get used in Linux.

@lopsided98
Copy link
Contributor Author

@lopsided98 lopsided98 commented Nov 30, 2019

Setting CONFIG_MISC_INIT_R=y enables the code that generates the MAC address from the cpuid. The algorithm takes the first 6 bytes of the SHA256 hash of the cpuid string and then sets a couple of bits to make sure its a valid MAC address.

Interestingly, once I enabled CONFIG_MISC_INIT_R, I got a different address from what I had with the rockchip U-Boot. It turns out that the rockchip U-Boot didn't read the cpuid correctly, and always set it to 00000000000000000000000000000000, resulting in the MAC address 86:e0:c0:ea:fa:a9

@lopsided98 lopsided98 force-pushed the lopsided98:uboot-rock64-upstream branch from 12f0c92 to 1680aee Nov 30, 2019
@thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented Dec 1, 2019

I've tested with the updated version and the mac address seems stable. The mac address I was using was the 86:e0:c0:ea:fa:a9, I had no idea that was an error. Thank you for getting to the bottom of that.

The Rock64 still needs a binary TPL to avoid memory initialization issues.
@lopsided98 lopsided98 force-pushed the lopsided98:uboot-rock64-upstream branch from 1680aee to 4d6921a Dec 5, 2019
@lopsided98
Copy link
Contributor Author

@lopsided98 lopsided98 commented Dec 5, 2019

I have submitted the patches for ramdisk_addr_r, CONFIG_MISC_INIT_R and extlinux.conf path length upstream.

@flokli
flokli approved these changes Dec 6, 2019
@flokli flokli merged commit c3e22dd into NixOS:master Dec 6, 2019
15 checks passed
15 checks passed
ubootRock64, ubootRockPro64 on x86_64-linux No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
ubootRock64, ubootRockPro64 on aarch64-linux Success
Details
@lopsided98 lopsided98 deleted the lopsided98:uboot-rock64-upstream branch Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.