skills/qcom-kernel-platform-backport: add LTS platform backport skill - #26
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a catalog skill for backporting upstream Qualcomm platform enablement to the LTS kernel.
Changes:
- Documents candidate selection, backporting, validation, and PR workflows.
- Adds helper scripts for commit handling and validation.
- Registers the skill across catalog metadata and documentation.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
skills/qcom-kernel-platform-backport/SKILL.md |
Defines the workflow. |
scripts/find-candidates.sh |
Finds candidate commits. |
scripts/dtbs-compare.sh |
Compares DTB validation results. |
scripts/check-series.sh |
Checks commit-series compliance. |
scripts/backport-commit.sh |
Applies and formats backports. |
references/ci-checkers.md |
Documents kernel CI behavior. |
skills.json |
Adds catalog metadata. |
README.md |
Lists the new skill. |
ROADMAP.md |
Marks the skill available. |
.claude-plugin/marketplace.json |
Registers the plugin. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Qualcomm AI ReviewClick to expand Deep Code ReviewQualcomm AI Deep Code Review AssistantCode Review: qcom-kernel-platform-backport Skill AdditionKey Findings SummaryTotal Issues Found: 8 Severity Breakdown:
Category Distribution:
Quick Statistics: Detailed Analysis1. Incorrect Conflict Resolution WorkflowSeverity: High Problem DescriptionThe script provides logically incorrect instructions for handling cherry-pick conflicts. After a conflict occurs, users are instructed to run echo "Cherry-pick failed. Please resolve conflicts, then run:"
echo " git cherry-pick --continue"
echo "Then re-run with the same arguments"Impact Analysis
Recommended FixRemove the misleading re-run instruction and provide correct guidance for manual commit message amendment: echo "Cherry-pick failed. Please resolve conflicts, then run:"
echo " git cherry-pick --continue"
echo ""
echo "After the cherry-pick completes, manually amend the commit message to add:"
echo " (cherry picked from commit $commit)"
echo " (backported from $upstream_branch)"
echo ""
echo "Use: git commit --amend"Implementation Guidance:
2. Insecure Temporary File CreationSeverity: High Problem DescriptionThe script writes checkpatch output to a hardcoded path without secure temporary file creation: ./scripts/checkpatch.pl --no-tree --terse --file $file \
> /tmp/checkpatch-series.log 2>&1Impact Analysis
Recommended FixReplace hardcoded path with secure temporary file creation using # Create secure temporary file
checkpatch_log=$(mktemp) || {
echo "Failed to create temporary file"
exit 1
}
# Ensure cleanup on exit
trap 'rm -f "$checkpatch_log"' EXIT INT TERM
# Use the secure temporary file
./scripts/checkpatch.pl --no-tree --terse --file $file \
> "$checkpatch_log" 2>&1
if [ -s "$checkpatch_log" ]; then
echo " Checkpatch warnings/errors:"
cat "$checkpatch_log"
fiAdditional Context
3. Unescaped Email Address in Regex PatternSeverity: Medium Problem DescriptionThe user's email address is used directly in a grep regex pattern without escaping special characters: me=$(git config user.email)
if ! git log -1 --format=%B $commit | grep -q "^Signed-off-by:.*<${me}>"; then
echo " Missing Signed-off-by for $me"
fiEmail addresses commonly contain regex metacharacters like Impact Analysis
Recommended FixEscape regex metacharacters before using the email in pattern matching: me=$(git config user.email)
# Escape regex special characters in email
me_escaped=$(printf '%s\n' "$me" | sed 's/[.[\*^$()+?{|]/\\&/g')
if ! git log -1 --format=%B $commit | grep -q "^Signed-off-by:.*<${me_escaped}>"; then
echo " Missing Signed-off-by for $me"
fi4. Missing Git Configuration ValidationSeverity: Medium Problem DescriptionThe script retrieves the user's email but doesn't validate that it exists: me=$(git config user.email)If Impact Analysis
Recommended FixAdd explicit validation with a clear error message: me=$(git config user.email)
if [ -z "$me" ]; then
echo "Error: Git user.email is not configured"
echo "Please run: git config --global user.email 'your.email@example.com'"
exit 1
fi5. Unescaped Path Variable in Sed PatternSeverity: Medium Problem DescriptionThe outdir=$(mktemp -d)
# Later in the code:
sed -E "s#^$outdir/#OUT/#"Paths from Impact Analysis
Recommended FixEscape regex metacharacters in the path variable: outdir=$(mktemp -d)
# Escape regex special characters
outdir_escaped=$(printf '%s\n' "$outdir" | sed 's/[.[\*^$()+?{|]/\\&/g')
# Use escaped version in sed pattern
sed -E "s#^$outdir_escaped/#OUT/#"6. Documentation Parameter MismatchSeverity: Medium Problem DescriptionThe usage documentation shows an incorrect parameter name: # Documentation shows:
--glob 'qcom/*.dts'
# But the script actually implements:
--subdir qcomImpact Analysis
Recommended FixUpdate the usage documentation to reflect the actual implementation: # Usage: dtbs-compare.sh [options] <old-ref> <new-ref>
# Options:
# --subdir <dir> Compare only DTBs in the specified subdirectory
# Example: --subdir qcom7. Non-Portable Command UsageSeverity: Medium Problem DescriptionThe script uses the git log --format="%H %s" $range | tacThe Impact Analysis
Recommended FixReplace # Instead of: git log --format="%H %s" $range | tac
# Use:
git log --format="%H %s" $range | awk '{lines[NR]=$0} END {for(i=NR;i>0;i--) print lines[i]}'Additional ContextThis awk solution stores all lines in an array and prints them in reverse order, providing identical functionality while maintaining POSIX compliance across all Unix-like systems. 8. Hardcoded Remote Name AssumptionSeverity: Low Problem DescriptionThe script hardcodes the linux-next remote name when checking commit ancestry: if git merge-base --is-ancestor $commit refs/remotes/linux-next/master 2>/dev/null; then
status="in-next"
fiUsers who add the linux-next remote with a different name (e.g., Impact Analysis
Recommended FixUse the user-configurable # Extract the first upstream ref from the --upstream parameter
upstream_ref="${upstream_refs[0]}"
# Check if commit is in the upstream branch
if git merge-base --is-ancestor $commit "$upstream_ref" 2>/dev/null; then
status="in-next"
fiPositive AspectsThis skill demonstrates several commendable practices:
SummaryThe qcom-kernel-platform-backport skill provides valuable automation for kernel backporting workflows, but requires attention to several critical issues before production deployment. The two high-severity issues—incorrect conflict resolution instructions and insecure temporary file handling—should be addressed immediately as they will cause workflow failures and introduce security vulnerabilities. The five medium-severity issues impact reliability and portability across different environments and should be resolved to ensure robust operation. The low-severity issue is a quality-of-life improvement that enhances flexibility. Once these recommendations are implemented, this skill will provide a solid foundation for automating complex kernel backporting workflows with appropriate safety checks and validation mechanisms. |
f70e9de to
98d5faa
Compare
![qualcomm-ai-code-review-assistant[bot]](https://avatars.githubusercontent.com/in/4128059?s=60&v=4){kind=small}
98d5faa to
c0fdac6
Compare
Boards enabled upstream do not work on the qualcomm-linux LTS branch until
their DTS, the SoC dtsi deltas they depend on, their bindings and a handful
of driver fixes are backported. Doing that by hand is repetitive and has a
few traps that are easy to fall into and expensive to find later: a
cherry-pick that silently reassigns authorship, a binding shared with
another SoC that regresses its DTBs, a driver enabled in defconfig but
absent from the BSP module list, and CI checkers that fail structurally on
any adapted backport.
Add a skill that walks the whole flow — candidate triage across mainline,
linux-next and the qcom SoC tree, authorship-preserving cherry-picks with
the UPSTREAM/BACKPORT/FROMGIT prefixes, a cross-platform CHECK_DTBS
regression check, hardware validation, and the pull request — plus four
helper scripts and a reference on what each Kernel Checker enforces and
which of its failures a backport series cannot avoid.
The scripts were exercised against a real 30-commit series backporting the
Arduino UNO Q (qrb2210-arduino-imola) onto qcom-6.18.y, then run end to end
on a second, unfamiliar platform — the Arduino VENTUNO Q
(monaco-arduino-monza, QCS8300) — which produced a 36-commit series booted
on hardware in the lab. That second run drove the following:
- backport-commit.sh reports a missing Link: at pick time instead of
leaving it for check-series.sh, deriving one from Message-ID: when
present. lore.kernel.org is behind Anubis bot protection, so the
fallback documents recovering the message-id from patchwork, with the
two checks that tell a genuine hit from a same-subject repost.
- check-series.sh audits the prefix against the diff. A cherry-pick can
diverge from the posted patch without conflicting — rename detection
moves it to another file, or the 3-way merge drops an already-present
hunk — leaving a BACKPORT wearing an UPSTREAM label that only
check-patch-compliance would catch. This found exactly that in the
VENTUNO Q series.
- dtbs-compare.sh gained --match for a family-scoped run, since a full
vendor sweep is serial in dt-validate and takes about an hour per side.
It also no longer counts a newly added board's inherited SoC dtsi
warnings as regressions, which had made it fail the very case the skill
exists for.
- SKILL.md documents the downstream qcom.config and prune.config
fragments that override defconfig in meta-qcom builds — one made a
QCLINUX defconfig commit redundant, the other silently disabled a
driver the board needed — plus a scripted reorder guarded against
dropping commits, and that a deferred consumer names its supplier,
which is what a missing module in the curated BSP list looks like.
Assisted-by: Claude Code:claude-fable-5
Signed-off-by: Ricardo Salveti <ricardo.salveti@oss.qualcomm.com>
c0fdac6 to
5c8ecec
Compare
|
Skill used to generate qualcomm-linux/kernel#1013. |
c6cccec
into
qualcomm-linux:main
Boards enabled upstream do not work on the qualcomm-linux LTS branch until their DTS, the SoC dtsi deltas they depend on, their bindings and a handful of driver fixes are backported. Doing that by hand is repetitive and has a few traps that are easy to fall into and expensive to find later: a cherry-pick that silently reassigns authorship, a binding shared with another SoC that regresses its DTBs, a driver enabled in defconfig but absent from the BSP module list, and CI checkers that fail structurally on any adapted backport.
Add a skill that walks the whole flow — candidate triage against the target branch, authorship-preserving cherry-picks with the UPSTREAM/BACKPORT/FROMGIT prefixes, a cross-platform CHECK_DTBS regression check, hardware validation, and the pull request — plus four helper scripts and a reference on what each Kernel Checker enforces and which of its failures a backport series cannot avoid.
Assisted-by: Claude Code:claude-fable-5