-
Notifications
You must be signed in to change notification settings - Fork 20
Optional DTB #186
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
base: main
Are you sure you want to change the base?
Optional DTB #186
Changes from all commits
b783e2f
9234c19
80be92c
913db88
6883f28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,8 @@ | |
| {{- $build_rb1 = "true" }} | ||
| {{- end -}} | ||
|
|
||
| {{- $target_boards := or .target_boards "all" }} | ||
|
|
||
| architecture: arm64 | ||
|
|
||
| actions: | ||
|
|
@@ -165,11 +167,50 @@ actions: | |
| # path to unpacked qcom-ptool tarball | ||
| QCOM_PTOOL="$(ls -d "${ROOTDIR}/../qcom-ptool.tar.gz.d/qcom-ptool-"*)" | ||
|
|
||
| # build a list of supported dtbs from the dtbs tarball | ||
| dtbs_file="build/dtbs.txt" | ||
| : >"${dtbs_file}" | ||
| if [ -f "${ARTIFACTDIR}/dtbs.tar.gz" ]; then | ||
| tar -tzf "${ARTIFACTDIR}/dtbs.tar.gz" --wildcards '*.dtb' \ | ||
| >>"$dtbs_file" | ||
| fi | ||
|
|
||
| # optional list of board names to build | ||
| targets_file="build/targets.txt" | ||
| rm -f "${targets_file}" | ||
| case "{{ $target_boards }}" in | ||
| all) | ||
| # no override; build all possible boards (default) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You already have $boards defined, right? Could you make the later code simpler by unconditionally writing build/targets.txt, so the "all" magic is restricted to here, rather than needing to be understood elsewhere? I think that would avoid surprising semantics of when build/targets.txt doesn't exist.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me try to repeat what I understand you're suggesting: have the go template logic at the top to generate a targets.txt table based on the data from $boards, then have a pure shell script reading through that table afterwards. Is that correct?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that matches, yes, if by "at the top" you mean in the code area highlighted here? I think you could write $board.name for $board in $boards to build/targets.txt in the area of code highlighted here in the case that $target_boards is set to "all". Then below the This would take away the special handling when build/targets.txt isn't present, because it'd always be present, making the logic simpler and less surprising from my POV. |
||
| ;; | ||
| *) | ||
| echo "{{ $target_boards }}" | | ||
| tr ',' '\n' | | ||
| sed '/^$/d' | | ||
| sort -u >"${targets_file}" | ||
| ;; | ||
| esac | ||
|
|
||
| {{- range $board := $boards }} | ||
| ### board: {{ $board.name }} | ||
| ### platform: {{ $board.platform }} | ||
| ### silicon family: {{ $board.silicon_family }} | ||
|
|
||
| # set skip_board if target list present and board isn't listed | ||
| skip_board=false | ||
| if [ -e "${targets_file}" ]; then | ||
| if ! grep -Fxq "{{ $board.name }}" "${targets_file}"; then | ||
| skip_board=true | ||
| echo "Skipping board {{ $board.name }}: not in target list" | ||
| fi | ||
| fi | ||
| {{- if $board.dtb }} | ||
| # set skip_board if board has a dtb and dtb isn't present | ||
| if ! grep -Fxq "{{ $board.dtb }}" "${dtbs_file}"; then | ||
| skip_board=true | ||
| echo "Skipping board {{ $board.name }}: dtb not available" | ||
| fi | ||
| {{- end }} | ||
|
|
||
| # unpack boot binaries | ||
| mkdir -v build/{{ $board.name }}_boot-binaries | ||
| unzip "${ROOTDIR}/../{{ $board.boot_binaries_download.filename }}" \ | ||
|
|
@@ -256,6 +297,7 @@ actions: | |
| "${QCOM_PTOOL}/ptool.py" -x ptool-partitions.xml | ||
| ) | ||
|
|
||
| if [ "${skip_board}" = false ]; then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do the following lines need indenting, or is GitHub hiding that from me? Same question for the other if statements below.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does, but then I have to indent the whole file, becoming hard to read and review :) happy to do that though!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My opinion: especially because there are many matching if/fi close together, it's easier to read with the expected indentation and if someone is debugging in this area then your intent will be clearer. Factoring some of these into functions might help with readability too, but that's another can of worms so maybe not worth it. This is subjective of course so I'll leave it to you. |
||
| # create board-specific flash directory | ||
| flash_dir="${ARTIFACTDIR}/flash_{{ $board.name }}" | ||
| rm -rf "${flash_dir}" | ||
|
|
@@ -287,24 +329,30 @@ actions: | |
| -or -name '*.mbn' \ | ||
| \) \ | ||
| -exec cp --preserve=mode,timestamps -v '{}' "${flash_dir}" \; | ||
| fi | ||
| {{- if $board.u_boot_file }} | ||
| if [ "${skip_board}" = false ]; then | ||
| # copy U-Boot binary to boot.img; | ||
| # qcom-ptool/platforms/*/partitions.conf uses filename=boot.img | ||
| # boot_a and boot_b partitions | ||
| cp --preserve=mode,timestamps -v "${ARTIFACTDIR}/{{ $board.u_boot_file }}" \ | ||
| "${flash_dir}/boot.img" | ||
| fi | ||
| {{- end }} | ||
|
|
||
| {{- if $board.cdt_download }} | ||
| if [ "${skip_board}" = false ]; then | ||
| # unpack board CDT | ||
| unzip "${ROOTDIR}/../{{ $board.cdt_download.filename }}" \ | ||
| -d build/{{ $board.name }}_cdt | ||
| # copy just the CDT data; no partition or flashing files | ||
| cp --preserve=mode,timestamps -v build/{{ $board.name }}_cdt/{{ $board.cdt_filename }} \ | ||
| "${flash_dir}" | ||
| fi | ||
| {{- end }} | ||
|
|
||
| {{- if $board.dtb }} | ||
| if [ "${skip_board}" = false ]; then | ||
| # generate a dtb.bin FAT partition with just a single dtb for the current | ||
| # board; long-term this should really be a set of dtbs and overlays as to | ||
| # share dtb.bin across boards | ||
|
|
@@ -321,6 +369,7 @@ actions: | |
| tar -C build -xvf "${ARTIFACTDIR}/dtbs.tar.gz" "{{ $board.dtb }}" | ||
| # copy into the FAT as combined-dtb.dtb | ||
| mcopy -vmp -i "${dtb_bin}" "build/{{ $board.dtb }}" ::/combined-dtb.dtb | ||
| fi | ||
| {{- end }} | ||
| {{- end }} | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.