Add fast-servo board support #24
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "fsagbuya/nix-servo:fast-servo"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Description
This patch aims to add firmware board support for Sinara Fast Servo.
Note:
ps7_initand device tree are generated manually via Vitis IDE.TODO:
v2022.2version.xparameters.hto make it work)fsbl.elf should not be committed. Just build it like the others.
@ -0,0 +10,4 @@zynq-cc108.dtb \zynq-cse-nand.dtb \zynq-cse-nor.dtb \diff --git a/arch/arm/dts/fast-servo.dts b/arch/arm/dts/fast-servo.dtsWhere is this file used? I only see the .dtb referenced.
The
.dtsfile generated is also needed to be copied to u-boot. But we only need the.dtbfor building the OS.https://stackoverflow.com/questions/21670967/how-to-compile-dts-linux-device-tree-source-files-to-dtb
Please remove the .dtb and generate it from the .dts.
.dtscan also be generated from these file recipes https://github.com/farbius/linux-vitis-zynq/tree/main/devicetree-src/zynq, see the build.sh script. Should I instead commit those files?Yes.
e90d0d6ea6tof31b46c70ef31b46c70eto7accfd2c427accfd2c42tod49c926197No. You just need to generate a few device tree file, the rest can be built without Vitis. The gateware isn't a concern yet.
And you don't need a 26k-line patch to build FSBL for a board.
@ -0,0 +14,4 @@new file mode 100644index 0000000000..918d44a32c--- /dev/null+++ b/arch/arm/dts/fast-servo.dtsIsn't that duplicated from the files above?
Any progress on getting this sorted? This is the biggest issue with this PR right now.
Will revert on committing only the
.dtsfile in the repo to be consistent with the linux kernel https://github.com/torvalds/linux/tree/master/arch/arm/boot/dts/xilinx. So the dtb builder will be consistent on both the fast-servo and not-os zynq. I'm currently fixing the not-os patch to extract the zc706 dts file.Updated the u-boot patch to prevent duplication.
@ -0,0 +688,4 @@CONFIG_ENV_OFFSET=0xE00000CONFIG_DM_GPIO=y-CONFIG_DEFAULT_DEVICE_TREE="zynq-zc706"+CONFIG_DEFAULT_DEVICE_TREE="fast-servo"How is that working with the board selection in nix?
Patch for fast-servo will only be applied if fast-servo board is selected.
d75530b07b/flake.nix (L239)@ -234,0 +219,4 @@}''mkdir -p $outif [ "${board}" = "fast-servo" ]; thenPurpose of this grossly inconsistent test?
Updated the name of the device-tree runCommand to
device-treefor clarity. This builds the device tree in two ways, one from the dts files committed to the repo and one fromnot-osbuild.There should be only one source of truth for the device tree, from which other formats are built using open tools and Nix derivations.
Ahh.. I get what you mean. Will revise the code to align with a more standard approach.
Understood.
Initially, I just copied the hw files from vitis to replicate the BSP template from this repo https://github.com/Xilinx/embeddedsw/tree/master/lib/sw_apps/zynq_fsbl/misc.
Will find a better approach for this.
A lot of Xilinx software is garbage and not an example to follow.
05f1a9ae33to825cc7c55dShould we not rely now on the Xilinx tool to build the fsbl? Need your suggestions.
Just copy the existing FSBL derivation that is in zynq-rs and change its settings to match the fast-servo hardware, fix #1 (i.e. use the Linux toolchain, there's no good reason it wouldn't work), and don't do anything else.
This is also what I did here, the challenging part is making this line work. https://git.m-labs.hk/M-Labs/zynq-rs/src/branch/master/flake.nix#L187. The tool currently supports only the 3 boards in the upstream (zc702, zc706 and zed), that's why the current approach is to make another directory for fast-servo with its hw files to have this supported. Thus, the 26k patch. :( We can also commit those files here and just patch the fsbl tool to look in our repo instead. This will make the patch shorter.
Can you just inject the required files with shell commands in postPatch or similar? That should be less than 30 lines of code, not 26000+.
I think this is possible. Will work on this solution. 👍
825cc7c55dto454096067f@ -166,2 +161,2 @@rev = "65c849ed46c88c67457e1fc742744f96db968ff1";sha256 = "1rvl06ha40dzd6s9aa4sylmksh4xb9dqaxq462lffv1fdk342pda";fsbl = { board ? "zc706" }: letfast-servo-hw = ./fast-servo/hw;Call the directory "fsbl-support" or similar, other software doesn't need this stuff.
@ -168,0 +165,4 @@src = pkgs.fetchFromGitHub {owner = "Xilinx";repo = "embeddedsw";# Previous version can't build fast-servo wdtps driverRemove this comment from here. Goes in the PR description.
@ -1,12 +1,9 @@diff --git a/lib/sw_apps/zynq_fsbl/src/Makefile b/lib/sw_apps/zynq_fsbl/src/MakefileDoes this patch still do anything? Or should you remove it entirely?
Tested without the patch and
fsblstill builds for this version. Will remove it.See the commit history. Xilinx fixed it upstream.
@ -168,0 +177,4 @@];postUnpack = ''mkdir -p $sourceRoot/lib/sw_apps/zynq_fsbl/misc/fast-servocp -r ${fast-servo-hw}/* $sourceRoot/lib/sw_apps/zynq_fsbl/misc/fast-servoIf you use -r you don't need *
@ -168,0 +190,4 @@'';installPhase = ''mkdir $outcp fsbl.elf $outWhat is the directory for? $out can be the elf file directly, no?
454096067fto51fdb70452@ -0,0 +10,4 @@dmapsemacpsgpiops+iicpsAnd we need this in FSBL because?
The additional drivers here are generated from the xsa file of fast-servo. Adding them makes the fast-servo fsbl build.
I'm pretty sure it builds without them. FSBL doesn't need I2C nor most of the other things. Why are they there?
I see. Will revise it for a minimal build, removing those that are not necessary.
Rather minimal patch. Just what is needed vs. the existing code in FSBL.
@ -161,0 +162,4 @@fsbl-support = pkgs.stdenv.mkDerivation {name = "fast-servo-fsbl-support";src = pkgs.fetchFromGitHub {owner = "elhep";No, we shouldn't depend on that repos.
@ -161,0 +170,4 @@postUnpack = "sourceRoot=$sourceRoot/builds/fast_servo_petalinux/project-spec/hw-description";installPhase = ''mkdir -p $outcp ps7_init.c $outHow different are they from the existing ones in FSBL?
Are the differences relevant?
Files with relevant differences are
ps7_init.candps7_init_gpl.cwith 3000+ diff, will just commit these to the repos.51fdb70452tod75530b07bUpdated. Still need to work out the device tree to have one source of truth (i.e.
.dtsfile).d75530b07btoef3d574020ef3d574020to17183a4b086fac78ce76to1ce024b8ea1ce024b8eato24d668ac46WIP: Add fast-servo board supportto Add fast-servo board support@ -292,0 +242,4 @@fidtc -I dts -O dtb -o ${board}.dtb ${board}.dtschmod +x ${board}.dtbWhy does this need to be executable?
I have added this before because
.dtbfile from linux kernel nix build is executable. So I did it as well in fast-servo.dtb for consistency. However from testing, it also boots fine without making it executable. Will just remove this.e980c9255ftob6c55eeb66@ -26,0 +31,4 @@'';"service/nix/run".source = pkgs.writeScript "nix" ''#!${pkgs.runtimeShell}- nix-store --load-db < /nix/store/nix-path-registrationThis pr-28.patch file should be in a separate PR I believe?
Will add this to a separate PR.
b6c55eeb66to08bcf27579ps7_init isn't the gateware.
@ -194,2 +209,3 @@patches = [] ++ pkgs.lib.optional (board == "fast-servo") ./fast-servo/u-boot.patch;preConfigure = ''export DEVICE_TREE=zynq-${board}export DEVICE_TREE=$([ "${board}" = "zc706" ] && echo "zynq-${board}" || echo "${board}")Why the difference?
Can't you just call the device tree zynq-fast-servo in u-boot?
Ah yes. I didn't think of that. Will change it to
zynq-fast-servoin u-boot.08bcf27579to49f9b3e095@ -347,3 +380,3 @@};packages.armv7l-linux = {zc706-u-boot = u-boot { board = "zc706"; };not-os = not-os-cfg.build.zynq_image;For which board is this? And isn't it redundant with the others?
Here's the current content of the package:
devicetree.dtbhere is not necessary since we are sourcing the device tree from u-boot. We can remove thedevicetree.dtbhere or remove thenot-osfrom the package list, sinceuImageanduRamdisk.image.gzis already inside sd-image package.In any case the board ambiguity needs to be resolved and cleaned up, perhaps deeper into the code.
49f9b3e095to2e0ba854e3Why the directory dts-support? There's only one file there? The patches don't have their own directory.
@ -344,0 +274,4 @@}''mkdir -p $outDTSDIR=$(mktemp -d /tmp/dts-XXXXXX)Not needed AFAICT? You can always create temporary build files in the current directory in a Nix derivation.
@ -354,2 +390,2 @@zc706-not-os = not-os-cfg.build.zynq_image;};packages.armv7l-linux =(build { board = "zc706"; }) //"build" is too general of a name.
Do you have a more suitable name in mind? I only get this from the
artiq-zynqflake.Maybe something like board-package-set
Will remove the
dts-supportdirectory and also add a directory for the patches.I didn't say the patches needed a dir, just pointed out the inconsistency.
2e0ba854e3to293f1eb54fCONFLICT (content): Merge conflict in pr-28.patch
293f1eb54ftoafa00efee3Conflict resolved.