Skip to content

arm64: dts: qcom: shikra: Add OP-TEE node and disable unused CTI - #1260

Open
Bibek Kumar Patro (bibekpatro) wants to merge 4 commits into
qualcomm-linux:qcom-6.18.yfrom
bibekpatro:shikra-optee-coresight
Open

Bibek Kumar Patro (bibekpatro) wants to merge 4 commits into
qualcomm-linux:qcom-6.18.yfrom
bibekpatro:shikra-optee-coresight

Conversation

@bibekpatro

Copy link
Copy Markdown

Summary

  • Add optee node under firmware {} to enable OP-TEE as the TEE backend on Shikra; required for remoteproc subsystems (e.g. lpaicp) that use OP-TEE for firmware authentication and controlled startup
  • Disable cti@982b000 (cti_riscv): the RISCV core is not present on Shikra so this CTI node should not be probed

Test plan

  • Boot Shikra board, verify optee device appears in /sys/bus/platform/devices/
  • Confirm no coresight probe errors for cti@982b000
  • Trigger lpaicp load manually and confirm OP-TEE PAS authentication succeeds

🤖 Generated with Claude Code

Add the OP-TEE firmware node to enable the Trusted Execution
Environment on Shikra. This is required for remoteproc subsystems
that use OP-TEE as the secure backend for firmware authentication
and boot (e.g. lpaicp which has auto_boot = false and relies on
the TEE PAS service for controlled startup).

Change-Id: I139ff6b6e658ff41d0b792bf2029300017019d84
Co-Authored-By: Claude <noreply@anthropic.com>
The RISCV CTI node at 0x982b000 is not functional on Shikra as the
RISCV core is not present. Mark it disabled to prevent coresight
from attempting to probe it.

Change-Id: Ia7904334bff9de0e55ad4d08f6af5446edb8d9ef
Co-Authored-By: Claude <noreply@anthropic.com>
@qswat-orbit-external

Copy link
Copy Markdown

Merge Check Failed: No CR Numbers Found

Error: No Change Request numbers were found.

Please add Change Request numbers to your pull request description in the format CRs-Fixed: 12345 or link GitHub issues that are associated with Change Requests.

1 similar comment
@qswat-orbit-external

Copy link
Copy Markdown

Merge Check Failed: No CR Numbers Found

Error: No Change Request numbers were found.

Please add Change Request numbers to your pull request description in the format CRs-Fixed: 12345 or link GitHub issues that are associated with Change Requests.

…ware has none

The firmware resource table is passed to the PAS backend even when the
firmware carries none. This only works on the first boot, while the
cached pointer and its size are both zero.

Stopping the remote processor, or failing to start it, frees the cached
table and clears the pointer but leaves the size set. The next start
then pairs a NULL table with a non-zero size.

The SCM backend substitutes an empty table and hides the problem. The
TEE backend copies from the NULL pointer:

  remoteproc remoteproc2: powering up cdsp
  pc : __pi_memcpy_generic+0x110/0x22c
  lr : qcom_pas_tee_get_rsc_table+0xf4/0x25c
  Call trace:
   __pi_memcpy_generic+0x110/0x22c (P)
   qcom_pas_get_rsc_table+0x38/0x60
   qcom_pas_parse_firmware+0xa0/0x100
   rproc_boot+0x2d4/0x380
   state_store+0x40/0x100

Change-Id: I48b0cc8ff9a78e24425c9e554944e2631880e4ad
Fixes: a4584bf ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call")
Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
@qswat-orbit-external

Copy link
Copy Markdown

Merge Check Failed: No CR Numbers Found

Error: No Change Request numbers were found.

Please add Change Request numbers to your pull request description in the format CRs-Fixed: 12345 or link GitHub issues that are associated with Change Requests.

Set auto_boot = false for cdsp and lpaicp on Shikra. The mpss entry
already had auto_boot = false. With OP-TEE as the PAS backend,
subsystems must be started explicitly to allow the TEE authentication
flow to complete before handing off control.

Change-Id: I443b0bf92c1a465ad0be924de836a51cafa23744
Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
@qswat-orbit-external

Copy link
Copy Markdown

Merge Check Failed: No CR Numbers Found

Error: No Change Request numbers were found.

Please add Change Request numbers to your pull request description in the format CRs-Fixed: 12345 or link GitHub issues that are associated with Change Requests.

@qlijarvis

Copy link
Copy Markdown

PR #1260 — validate-patch

PR: #1260

Verdict Issues Detailed Report
❌ 2 Full report

Final Summary

  1. Lore link present: Not provided in agent output
  2. Lore link matches PR commits: Not provided in agent output
  3. Upstream patch status: Not provided in agent output
  4. PR present in qcom-next/topics: Partial - 2/4 commit(s) only have partial integration evidence
Verdict: ❌ — click to expand

🔍 Patch Validation Report

PR: #1260
Commits: 4 commits (2 vendor-specific DTS, 1 FROMLIST remoteproc, 1 vendor-specific remoteproc)
Verdict: ❌ FAIL


Commit 1/4: arm64: dts: qcom: shikra: Add OP-TEE firmware node

Upstream: N/A (vendor-specific)
Verdict: ⚠️ WARNING

Commit Message

Check Status Note
Subject matches upstream N/A No upstream source
Body preserves rationale ✅ Clear description of purpose
Fixes tag present/correct N/A Not a fix
Authorship preserved ✅ Bibek Kumar Patro
Backport note N/A Not a backport
Co-Authored-By usage ⚠️ Co-Authored-By: Claude <noreply@anthropic.com> - unusual for kernel commits

Diff

File Status Notes
arch/arm64/boot/dts/qcom/shikra.dtsi ✅ Adds OP-TEE firmware node

Issues

  • Co-Authored-By tag: The Co-Authored-By: Claude <noreply@anthropic.com> tag is unusual for kernel commits. If Claude is an AI assistant, this attribution is inappropriate. If this is a genuine co-author, use a real email address.
  • Change-Id: Gerrit-style Change-Id tags are not used in upstream kernel commits and should be removed before submission.

Final Summary

  1. Lore link present: No — vendor-specific Shikra DTS change; no lore link expected
  2. Lore link matches PR commits: N/A — no lore link to compare against
  3. Upstream patch status: N/A — vendor-specific platform enablement
  4. PR present in qcom-next/topics: Partial — integration_presence_report.md shows "partial - subject or partial tree evidence found, but full change was not verified"

Commit 2/4: arm64: dts: qcom: shikra: Disable cti_riscv node

Upstream: N/A (vendor-specific)
Verdict: ⚠️ WARNING

Commit Message

Check Status Note
Subject matches upstream N/A No upstream source
Body preserves rationale ✅ Clear explanation of why node is disabled
Fixes tag present/correct N/A Not a fix
Authorship preserved ✅ Bibek Kumar Patro
Backport note N/A Not a backport
Co-Authored-By usage ⚠️ Co-Authored-By: Claude <noreply@anthropic.com> - unusual for kernel commits

Diff

File Status Notes
arch/arm64/boot/dts/qcom/shikra.dtsi ✅ Adds status = "disabled" to cti_riscv node

Issues

  • Co-Authored-By tag: Same issue as commit 1/4 - Co-Authored-By: Claude <noreply@anthropic.com> is inappropriate.
  • Change-Id: Gerrit-style Change-Id tag should be removed.

Final Summary

  1. Lore link present: No — vendor-specific Shikra DTS change; no lore link expected
  2. Lore link matches PR commits: N/A — no lore link to compare against
  3. Upstream patch status: N/A — vendor-specific platform fix
  4. PR present in qcom-next/topics: Present — integration_presence_report.md shows "present - all checked added lines are present"

Commit 3/4: FROMLIST: remoteproc: qcom: pas: pass no resource table when the firmware has none

Upstream: MISSING - No lore.kernel.org link found
Verdict: ❌ FAIL

Commit Message

Check Status Note
Subject matches upstream ❌ Cannot verify - no lore link provided
Body preserves rationale ✅ Detailed problem description with crash trace
Fixes tag present/correct ✅ Fixes: a4584bff63c8 present
Authorship preserved ⚠️ Jorge Ramirez-Ortiz is original author, Bibek Kumar Patro is submitter
Backport note N/A Not a backport
Co-Authored-By usage ✅ Not used

Diff

File Status Notes
drivers/remoteproc/qcom_q6v5_pas.c ❌ Cannot verify against upstream - no lore link

Issues

CRITICAL: Missing lore.kernel.org link

This commit uses the FROMLIST: prefix, which requires a Link: https://lore.kernel.org/r/<message-id> tag in the commit message. Per the validate-patch skill:

FROMLIST: — Posted to mailing list, not yet merged — Yes — lore.kernel.org link required

The commit message must include a line like:

Link: https://lore.kernel.org/r/<message-id>/

Without this link, it is impossible to:

  1. Verify that the patch content matches what was posted to the mailing list
  2. Check the upstream review status (ACKed/NACKed/Pending)
  3. Ensure the correct revision is being used (v1, v2, v3, etc.)
  4. Validate that authorship and sign-offs are preserved correctly

Authorship note for FROMLIST commits:

  • The original author (Jorge Ramirez-Ortiz) must appear in Signed-off-by: — ✅ Present
  • The submitter (Bibek Kumar Patro) appears in From: and adds their own Signed-off-by: — ✅ Correct
  • This authorship pattern is correct for FROMLIST: commits

Change-Id tag:

  • Gerrit-style Change-Id should be removed before upstream submission

Final Summary

  1. Lore link present: ❌ No — FROMLIST: prefix requires a lore.kernel.org link, but none found
  2. Lore link matches PR commits: ❌ Cannot verify — no lore link to compare against
  3. Upstream patch status: ❌ Cannot determine — no lore link to check review status
  4. PR present in qcom-next/topics: Partial — integration_presence_report.md shows "partial - subject or partial tree evidence found, but full change was not verified"

Commit 4/4: remoteproc: qcom: shikra: Disable auto-boot for all PIL subsystems

Upstream: N/A (vendor-specific)
Verdict: ✅ PASS (with minor warnings)

Commit Message

Check Status Note
Subject matches upstream N/A No upstream source
Body preserves rationale ✅ Clear explanation of why auto_boot is disabled
Fixes tag present/correct N/A Not a fix
Authorship preserved ✅ Bibek Kumar Patro
Backport note N/A Not a backport
Co-Authored-By usage ✅ Not used

Diff

File Status Notes
drivers/remoteproc/qcom_q6v5_pas.c ✅ Changes auto_boot from true to false for cdsp and lpaicp

Issues

  • Change-Id: Gerrit-style Change-Id tag should be removed if submitting upstream.
  • Vendor-specific: This change is specific to Shikra platform behavior with OP-TEE, unlikely to be upstreamed as-is.

Final Summary

  1. Lore link present: No — vendor-specific Shikra platform configuration; no lore link expected
  2. Lore link matches PR commits: N/A — no lore link to compare against
  3. Upstream patch status: N/A — vendor-specific platform configuration
  4. PR present in qcom-next/topics: Present — integration_presence_report.md shows "present - all checked added lines are present"

Overall PR Verdict: ❌ FAIL

Critical Issues

  1. Commit 3/4 FAIL: The FROMLIST: prefix requires a Link: https://lore.kernel.org/r/<message-id> tag, but none is present. This commit cannot be validated without the lore link.

Warnings

  1. Commits 1/4 and 2/4: Inappropriate Co-Authored-By: Claude <noreply@anthropic.com> tags. If Claude is an AI assistant, this attribution should be removed. Kernel commits should only credit human contributors with valid email addresses.

  2. All commits: Gerrit-style Change-Id tags should be removed before upstream submission (though they may be acceptable in vendor trees).

Recommendation

DO NOT MERGE until the following corrections are made:

  1. Commit 3/4 (CRITICAL): Add the lore.kernel.org link to the commit message:

    Link: https://lore.kernel.org/r/<message-id>/
    

    Then re-run validation to verify the patch content matches the lore posting and check upstream review status.

  2. Commits 1/4 and 2/4: Remove the Co-Authored-By: Claude <noreply@anthropic.com> lines.

  3. All commits (optional): Remove Change-Id tags if these commits will be submitted upstream.

After corrections, re-submit the PR for validation.


Integration Status Summary

Per integration_presence_report.md:

  • Overall status: PARTIAL
  • Present commits: 2/4 (commits 2/4 and 4/4)
  • Partial commits: 2/4 (commits 1/4 and 3/4)
  • Missing commits: 0/4

The partial status for commits 1/4 and 3/4 indicates that while some evidence was found in qcom-next, the full changes could not be verified. This aligns with the validation findings that these commits have issues requiring correction.

Deterministic Integration Presence

Integration Presence Report

This report is generated by Jarvis before validate-patch runs.
It is the authoritative source for whether PR changes are already present
in qcom-next or in the kernel topic branches.

Kernel repo: /local/mnt/workspace/sgaud/Qgenie/image_pipeline/kernel
qcom-next ref: ba86f67908354f41f269ca1d64b22e4624766dcf
topics remote: topics -> https://github.com/qualcomm-linux/kernel-topics
topics fetch: fetched

Commit Subject qcom-next topics Final
1/4 [PATCH 1/4] arm64: dts: qcom: shikra: Add OP-TEE firmware node partial - subject or partial tree evidence found, but full change was not verified missing - no subject, patch-id, or full tree-content match found partial
2/4 [PATCH 2/4] arm64: dts: qcom: shikra: Disable cti_riscv node present - all checked added lines are present skipped - not checked because qcom-next already contains the change present
3/4 [PATCH 3/4] FROMLIST: remoteproc: qcom: pas: pass no resource table partial - subject or partial tree evidence found, but full change was not verified missing - no subject, patch-id, or full tree-content match found partial
4/4 [PATCH 4/4] remoteproc: qcom: shikra: Disable auto-boot for all PIL present - all checked added lines are present skipped - not checked because qcom-next already contains the change present

Final Status

overall_status: PARTIAL
present_commits: 2/4
partial_commits: 2/4
missing_commits: 0/4
topics_checked_for_commits: 2/4
final_summary: PR present in qcom-next/topics: Partial - 2/4 commit(s) only have partial integration evidence

@qlijarvis

Copy link
Copy Markdown

PR #1260 — checker-log-analyzer

PR: #1260
Checker run: https://github.com/qualcomm-linux/kernel-config/actions/runs/37843179933

Checker Result Summary
Checker Result Summary
checkpatch ❌ 4 commits with errors: Change-Id, missing Signed-off-by, Co-Authored-By format
dt-binding-check ⏭️ No DT binding changes
dtb-check ✅ All DTB validations passed
sparse-check ✅ No sparse warnings
check-uapi-headers ✅ No UAPI changes
check-patch-compliance ❌ 3 commits missing required prefix, 1 commit missing Link
tag-check ❌ 3 commits missing required subject prefix (mandatory for qcom-6.18.y)

Detailed report: Full report

Checker analysis — click to expand

🤖 CI Checker Analysis (checker-log-analyzer)

PR: #1260 - Shikra remoteproc and firmware updates
Source: https://github.com/qualcomm-linux/kernel-config/actions/runs/37843179933
Target branch: qcom-6.18.y

Checker Result Summary
checkpatch ❌ 4 commits with errors: Change-Id, missing Signed-off-by, Co-Authored-By format
dt-binding-check ⏭️ No DT binding changes
dtb-check ✅ All DTB validations passed
sparse-check ✅ No sparse warnings
check-uapi-headers ✅ No UAPI changes
check-patch-compliance ❌ 3 commits missing required prefix, 1 commit missing Link
tag-check ❌ 3 commits missing required subject prefix (mandatory for qcom-6.18.y)

❌ checkpatch

Root cause: All 4 commits contain Gerrit Change-Id tags that must be removed before upstream submission. Commits 1 and 2 also have missing Signed-off-by and incorrect Co-Authored-By format.

Failure details:

Commit 1: 55ae639 ("arm64: dts: qcom: shikra: Add OP-TEE firmware node")

ERROR: Remove Gerrit Change-Id's before submitting upstream
#12: Change-Id: I139ff6b6e658ff41d0b792bf2029300017019d84

WARNING: Non-standard signature: Co-Authored-By:
#13: Co-Authored-By: Claude <noreply@anthropic.com>

WARNING: 'Co-authored-by:' is the preferred signature form

ERROR: Missing Signed-off-by: line(s)

total: 2 errors, 2 warnings

Commit 2: b8847ef ("arm64: dts: qcom: shikra: Disable cti_riscv node")

ERROR: Remove Gerrit Change-Id's before submitting upstream
#10: Change-Id: Ia7904334bff9de0e55ad4d08f6af5446edb8d9ef

WARNING: Non-standard signature: Co-Authored-By:
#11: Co-Authored-By: Claude <noreply@anthropic.com>

WARNING: 'Co-authored-by:' is the preferred signature form

ERROR: Missing Signed-off-by: line(s)

total: 2 errors, 2 warnings

Commit 3: c4dad8b ("FROMLIST: remoteproc: qcom: pas: pass no resource table when the firmware has none")

ERROR: Remove Gerrit Change-Id's before submitting upstream
#28: Change-Id: I48b0cc8ff9a78e24425c9e554944e2631880e4ad

WARNING: Unknown commit id 'a4584bff63c8', maybe rebased or not pulled?
#29: Fixes: a4584bff63c8 ("remoteproc: pas: Extend parse_fw callback to fetch resources via SMC call")

total: 1 errors, 1 warnings

Commit 4: 5b5c29b ("remoteproc: qcom: shikra: Disable auto-boot for all PIL subsystems")

ERROR: Remove Gerrit Change-Id's before submitting upstream
#12: Change-Id: I443b0bf92c1a465ad0be924de836a51cafa23744

total: 1 errors

Fix:

For all 4 commits, remove the Change-Id: line:

git rebase -i f19f3cdab691f3d6d8bcccf03e46669ba4e714ea
# Mark each commit as 'edit'
# For each commit:
git commit --amend  # Remove Change-Id line from commit message
git rebase --continue

For commits 1 and 2, additionally:

  1. Add Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com> to the commit message
  2. Change Co-Authored-By: to Co-developed-by: (lowercase 'd') and add a matching Signed-off-by: for the co-author, or remove it entirely if Claude is not a genuine co-author

Reproduce locally:

./scripts/checkpatch.pl --strict --ignore FILE_PATH_CHANGES --git f19f3cdab691f3d6d8bcccf03e46669ba4e714ea..5b5c29bff168e437fe53d38c99790215d9c10ea6

❌ check-patch-compliance

Root cause: Three commits lack required subject prefix tags (FROMLIST:, FROMGIT:, UPSTREAM:, BACKPORT:), and one FROMLIST: commit is missing the mandatory Link: trailer.

Failure details:

Checking commit: arm64: dts: qcom: shikra: Add OP-TEE firmware node
Commit summary does not start with a required prefix

Checking commit: arm64: dts: qcom: shikra: Disable cti_riscv node
Commit summary does not start with a required prefix

Checking commit: FROMLIST: remoteproc: qcom: pas: pass no resource table when the firmware has none
No 'Link' found in commit message

Checking commit: remoteproc: qcom: shikra: Disable auto-boot for all PIL subsystems
Commit summary does not start with a required prefix

Fix:

Commits 1, 2, 4 — Add appropriate prefix:

  • If these patches are posted to lore.kernel.org → use FROMLIST: and add Link: trailer
  • If vendor-only with no upstream equivalent → use QCLINUX: (note: checker will still fail, known limitation)
  • If work-in-progress → use PENDING: (note: checker will still fail, known limitation)

Commit 3 — Add Link: trailer pointing to the lore.kernel.org message:

git rebase -i <base>
# Mark commit c4dad8b34f33 as 'edit'
git commit --amend
# Add to commit body:
# Link: https://lore.kernel.org/r/<message-id>
git rebase --continue

Reproduce locally:

# For each commit, verify with b4:
b4 am --single-message -C -l -3 <lore-link> -o /tmp/out

❌ tag-check — Mandatory Subject Prefix

Root cause: Target branch is qcom-6.18.y (not qcom-next or qcom-next-staging), so every commit must start with a valid prefix tag. Three commits are missing prefixes.

Failure details:

Commit 1: 55ae639

Subject: arm64: dts: qcom: shikra: Add OP-TEE firmware node
❌ Missing required prefix

Commit 2: b8847ef

Subject: arm64: dts: qcom: shikra: Disable cti_riscv node
❌ Missing required prefix

Commit 4: 5b5c29b

Subject: remoteproc: qcom: shikra: Disable auto-boot for all PIL subsystems
❌ Missing required prefix

Commit 3: c4dad8b

Subject: FROMLIST: remoteproc: qcom: pas: pass no resource table when the firmware has none
✅ Has valid prefix (FROMLIST:)

Fix:

Choose the correct prefix based on patch origin:

Prefix When to use
FROMLIST: Posted to lore.kernel.org, not yet merged
FROMGIT: In a maintainer git tree
UPSTREAM: Merged into Linus's mainline tree
BACKPORT: Upstream patch backported with modifications
QCLINUX: Vendor-only, no upstream equivalent
PENDING: Work-in-progress, not posted upstream
WORKAROUND: Temporary fix not suitable for upstream
git rebase -i f19f3cdab691f3d6d8bcccf03e46669ba4e714ea
# Mark commits 1, 2, 4 as 'edit'
# For each:
git commit --amend -m "FROMLIST: <original subject>"  # or QCLINUX:/PENDING: as appropriate
git rebase --continue

Reproduce locally:

git log f19f3cdab691..5b5c29bff168 --format="%H %s" | while read sha subject; do
  if ! echo "$subject" | grep -qE '^(FROMLIST|FROMGIT|UPSTREAM|BACKPORT|QCLINUX|PENDING|WORKAROUND):'; then
    echo "MISSING prefix: $sha $subject"
  fi
done

Verdict

5 blockers must be fixed before merge:

  1. All 4 commits: Remove Gerrit Change-Id: lines
  2. Commits 1 & 2: Add Signed-off-by: and fix Co-Authored-By: format
  3. Commits 1, 2, 4: Add required subject prefix (FROMLIST:/QCLINUX:/PENDING:)
  4. Commit 3: Add Link: trailer pointing to lore.kernel.org
  5. All commits: Ensure compliance with kernel coding style

Note: If commits 1, 2, 4 are vendor-only (QCLINUX:) or work-in-progress (PENDING:), the check-patch-compliance checker will continue to fail — this is a known checker limitation for non-upstream-linkable prefixes. However, the tag-check requirement is still satisfied by adding the prefix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants