Skip to content
Open
Changes from 1 commit
Commits
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
34 changes: 34 additions & 0 deletions debos-recipes/qualcomm-linux-debian-flash.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
{{- $build_rb1 = "true" }}
{{- end -}}

{{- $target_boards := or .target_boards "all" }}

architecture: arm64

actions:
Expand Down Expand Up @@ -165,11 +167,35 @@ actions:
# path to unpacked qcom-ptool tarball
QCOM_PTOOL="$(ls -d "${ROOTDIR}/../qcom-ptool.tar.gz.d/qcom-ptool-"*)"

# 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)
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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?

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 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 if [ -e "${targets_file}" ]; then can go away - it'd be unconditionally setting skip_board to true if $board.name (in the iteration below) isn't present in build/targets.txt.

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

# unpack boot binaries
mkdir -v build/{{ $board.name }}_boot-binaries
unzip "${ROOTDIR}/../{{ $board.boot_binaries_download.filename }}" \
Expand Down Expand Up @@ -256,6 +282,7 @@ actions:
"${QCOM_PTOOL}/ptool.py" -x ptool-partitions.xml
)

if [ "${skip_board}" = false ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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!

Copy link
Contributor

Choose a reason for hiding this comment

The 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}"
Expand Down Expand Up @@ -287,24 +314,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
Expand All @@ -321,6 +354,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 }}

Expand Down