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

Jump to bottom

ubootRock64, ubootRockPro64: use upstream U-Boot #74580

Merged
merged 1 commit into from Dec 6, 2019
Merged

ubootRock64, ubootRockPro64: use upstream U-Boot #74580

merged 1 commit into from Dec 6, 2019

Conversation

lopsided98
Copy link
Contributor

@lopsided98 lopsided98 commented Nov 29, 2019 •
edited

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

@ofborg ofborg bot requested review from dezgeg and samueldr November 29, 2019 05:23
flokli
flokli reviewed Nov 29, 2019
View reviewed changes
pkgs/misc/uboot/default.nix Outdated Show resolved Hide resolved
@bennofs
Copy link
Contributor

bennofs commented Nov 29, 2019 •
edited

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
Copy link
Contributor Author

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

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

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

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

@thefloweringash
Copy link
Member

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.

@lopsided98 lopsided98 mentioned this pull request Dec 1, 2019
Merged
@lopsided98
ubootRock64, ubootRockPro64: use upstream U-Boot
4d6921a 
The Rock64 still needs a binary TPL to avoid memory initialization issues.
@lopsided98
Copy link
Contributor Author

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

@flokli flokli merged commit c3e22dd into NixOS:master Dec 6, 2019
@lopsided98 lopsided98 deleted the uboot-rock64-upstream branch April 27, 2020 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@flokli flokli flokli approved these changes

@dezgeg dezgeg Awaiting requested review from dezgeg

@samueldr samueldr Awaiting requested review from samueldr

Assignees
No one assigned
Labels
8.has: clean-up 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lopsided98 @bennofs @thefloweringash @Mic92 @flokli