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.

Sign up for GitHub

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

u-boot: ROCK64 RAM init improvements #191538

Merged
merged 1 commit into from
Nov 25, 2023
Merged

u-boot: ROCK64 RAM init improvements #191538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion pkgs/misc/arm-trusted-firmware/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ in {
platform = "rk3328";
extraMeta.platforms = ["aarch64-linux"];
filesToInstall = [ "build/${platform}/release/bl31/bl31.elf"];
platformCanUseHDCPBlob = true;
};

armTrustedFirmwareRK3399 = buildArmTrustedFirmware rec {
Expand Down
42 changes: 19 additions & 23 deletions pkgs/misc/uboot/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,32 +497,28 @@ in {
filesToInstall = ["u-boot.bin"];
};

ubootRock64 = let
rkbin = fetchFromGitHub {
owner = "ayufan-rock64";
repo = "rkbin";
rev = "f79a708978232a2b6b06c2e4173c5314559e0d3a";
sha256 = "0h7xm4ck3p3380c6bqm5ixrkxwcx6z5vysqdwvfa7gcqx5d6x5zz";
};
in buildUBoot {
ubootRock64 = buildUBoot {
defconfig = "rock64-rk3328_defconfig";
extraMeta = {
platforms = [ "aarch64-linux" ];
license = lib.licenses.unfreeRedistributableFirmware;
};
extraMeta.platforms = [ "aarch64-linux" ];
BL31="${armTrustedFirmwareRK3328}/bl31.elf";
filesToInstall = [ "u-boot.itb" "idbloader.img"];
# Derive MAC address from cpuid
# Submitted upstream: https://patchwork.ozlabs.org/patch/1203686/
extraConfig = ''
CONFIG_MISC_INIT_R=y
'';
# Close to being blob free, but the U-Boot TPL causes random memory
# corruption
postBuild = ''
./tools/mkimage -n rk3328 -T rksd -d ${rkbin}/rk33/rk3328_ddr_786MHz_v1.13.bin idbloader.img
cat spl/u-boot-spl.bin >> idbloader.img
filesToInstall = [ "u-boot.itb" "idbloader.img" "u-boot-rockchip.bin" ];
};

# A special build with much lower memory frequency (666 vs 1600 MT/s) which
# makes ROCK64 V2 boards stable. This is necessary because the DDR3 routing
# on that revision is marginal and not uncoditionally stable at the specified
# frequency. If your ROCK64 is unstable you can try this u-boot variant to
# see if it works better for you. The only disadvantage is lowered memory
# bandwidth.
ubootRock64v2 = buildUBoot {
Copy link
Member

Choose a reason for hiding this comment

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

minor: how about converting this to a function that accepts either { memFreq = "666"; } or { memFreq = "1600"; } as argument and calling it from all-packages with { memFreq = "666"; }?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, the memory frequency is not just a tunable (it is also half, i.e. 333MHz / 800MHz), there is a whole host of timing parameters that need to be configured to make it work. There are only two specification in the u-boot tree, the "regular" 1600MT/s and the 666MT/s. I've actually reverse-engineered most of the memory controller, but I don't have the time currently to write a driver which would allow us to do something like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think @wamserma meant just templating the memFreq value into the .dtsi name to select the correct one without duplicating the whole buildUBoot invocation.

Copy link
Member

Choose a reason for hiding this comment

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

If you want to further improve eval-time robustness, it could be checked with either an attrset of valid values, or couple of ifs. Otherwise it would fail during a needless attempt at building, which is still probably fine.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it was just about templating it into the name because it is more readable (and thus clearer) than the replacement expression.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a try at implementing this, but I struggled with where to put the function. There is a single callPackage in all-packages for these, which then get inherited up. So the only place I can see to put this is to put the function at the top together with buildUBoot and then call it twice. But that IMO makes the file relatively hard to read.

prePatch = ''
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
prePatch = ''
postPatch = ''

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would require a change to buildUBoot to avoid overwriting the postPatch defined there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, this can be done, but would require adding a new argument to buildUBoot.

substituteInPlace arch/arm/dts/rk3328-rock64-u-boot.dtsi \
--replace rk3328-sdram-lpddr3-1600.dtsi rk3328-sdram-lpddr3-666.dtsi
'';
defconfig = "rock64-rk3328_defconfig";
extraMeta.platforms = [ "aarch64-linux" ];
BL31="${armTrustedFirmwareRK3328}/bl31.elf";
filesToInstall = [ "u-boot.itb" "idbloader.img" "u-boot-rockchip.bin" ];
};

ubootRockPro64 = buildUBoot {
Expand Down
1 change: 1 addition & 0 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28876,6 +28876,7 @@ with pkgs;
ubootRaspberryPi4_64bit
ubootRaspberryPiZero
ubootRock64
ubootRock64v2
ubootRockPi4
ubootRockPro64
ubootROCPCRK3399
Expand Down