QCLINUX: arm64: dts: qcom: lemans and monaco: Rework RTSS Mailbox DT to use syscon and staging overlays - #1768
sargarram7 wants to merge 7 commits into
Conversation
|
Dmitry Baryshkov (@lumag), Ricardo Salveti (@ricardosalveti), Nicolas Dechesne (@ndechesne), Viswanath Kraleti (@vkraleti), Komal Bajaj (@Komal-Bajaj) would you please review the changes. |
sargarram7
changed the title
c953cb6 to
e751c72
Compare
|
Thank you Dmitry Baryshkov (@lumag) , I have addressed comments. |
PR #1768 — validate-patchPR: #1768
Final Summary
|
PR #1768 — checker-log-analyzerPR: #1768
Detailed report: Full report
|
e63bcc7 to
64ff024
Compare
09b5c80 to
e2f7c91
Compare
64ff024 to
8bffa16
Compare
|
hi Dmitry Baryshkov (@lumag), Ricardo Salveti (@ricardosalveti), |
| }; | ||
|
|
||
| apss_shared_ipc: syscon@17c0000c { | ||
| compatible = "qcom,sa8775p-tcsr", "syscon"; |
There was a problem hiding this comment.
What exactly is behind this address? TCSR is at the other address, so it can't be TCSR.
There was a problem hiding this comment.
0x17C0000C is SoC level top router APSS HM CSR register used for special event notifications such as boot milestones and runtime events.
APSS NHLOS, HLOS updates the milestone/magic value in TCSR and
triggers doorbell via 0x17C0000C, with the target potentially being OSM, SSC, RTSS, LPASS, Modem, xDSPs, AOP/RPMH or other subsystems.
Since this mechanism is not RTSS mailbox-specific and may be used by multiple APSS services, I would like to avoid introducing coupling through an RTSS mailbox DT reg property.
If syscon is not preferred, would it be acceptable to keep the register local to the driver and map it through platform resources (devm_ioremap_resource()) instead? This would keep the implementation self-contained while avoiding
mailbox specific DT binding for shared SoC resource.
There was a problem hiding this comment.
Are you going to upstream at least this patch? It is changing a common dtsi, so not really isolated.
There was a problem hiding this comment.
we have plan in 3 phases: phase 1, 2 will have downstream + overlay patches, after phase 2 commit work will be starting on upstreaming work.
There was a problem hiding this comment.
This will be a pure downstream change affecting a generic dtsi, not something I would recommend doing, but up to Dmitry Baryshkov (@lumag) and Shiraz Hashim (@shashim-quic).
Remember that downstream changes to the generic files can break systemready (dt check).
There was a problem hiding this comment.
Dmitry Baryshkov (@lumag) , Shiraz Hashim (@shashim-quic) could please review this open point.
There was a problem hiding this comment.
No, you can't describe this as a TCSR. Also, I don't think that this register space is not page aligned. If you don't want to describe the hardware properly, it might be better to follow PDC example. Other option would be to describe this register space correctly and make PDC use it.
There was a problem hiding this comment.
- PDC doesn't appear suitable because the SPI edge/level mapping is fixed to predefined indices zero for pdc base and one for attributes.
- Using the register as mailbox reg resource works today but implicitly associates ownership with the mailbox driver, while the register is shared resource and future any services may require access. let me know about this option as well.
- Dedicated syscon node with a phandle reference from the RTSS mailbox node provides cleaner ownership modeling and better handing by fixing page alignment issue and changing the compatible string to default.
e.g. apss_shared_ipc:syscon@17c00000 { compatible = "syscon"; reg = <0x0 0x17c00000 0x0 0x1000>; };
reference it using qcom,syscon-ipc = <&apss_shared_ipc 0xc>;
| }; | ||
|
|
||
| apss_shared_ipc: syscon@17c0000c { | ||
| compatible = "qcom,qcs8300-tcsr", "syscon"; |
There was a problem hiding this comment.
Same question. Is it really TCSR?
There was a problem hiding this comment.
answered with R7098 line comment
…ee overlay" This reverts commit 1923b1c. The original overlay accessed the APSS TCSR mutex/status registers via direct memory-mapped reg ranges and shipped RTSS as a standalone lemans-rtss-mb.dtso overlay. Upstream review feedback on this design requires: - Accessing the TCSR registers via a syscon phandle instead of a direct reg-mapped node. - Folding the RTSS mailbox node into the shared lemans-staging.dtso overlay instead of a separate, RTSS-only DTBO. The following commits reintroduce this support using that syscon-based, staging-overlay design. Signed-off-by: Ram Sargar <rsargar@qti.qualcomm.com>
…ee overlay" This reverts commit 1179022. Same as the lemans overlay, the original Monaco overlay accessed the APSS TCSR mutex/status registers via direct memory-mapped reg ranges and shipped RTSS as a standalone monaco-rtss-mb.dtso overlay. Upstream review feedback on this design requires: - Accessing the TCSR registers via a syscon phandle instead of a direct reg-mapped node. - Folding the RTSS mailbox node into the shared monaco-staging.dtso overlay instead of a separate, RTSS-only DTBO. The following commits reintroduce this support using that syscon-based, staging-overlay design. Signed-off-by: Ram Sargar <rsargar@qti.qualcomm.com>
8bffa16 to
beec106
Compare
|
hi Dmitry Baryshkov (@lumag), Ricardo Salveti (@ricardosalveti), |
|
Commits have no tag, should all start with QCLINUX. |
| interrupt-controller; | ||
| #interrupt-cells = <3>; | ||
| #mbox-cells = <2>; | ||
| }; |
There was a problem hiding this comment.
Do we have consumers for ipcc2-4 at this point?
There was a problem hiding this comment.
Yes, these channels are required for RTSS communication. While the current consumer is qcom-ipcc, the actual end user is rtss-mailbox.
This change addresses the existing RTSS channel limitation. The newly added channels have already been allocated from ipcc1 by Qualcomm and are intended to support:
- New RTSS clients for customer-specific use cases.
- Future Qualcomm internal use cases.
- Specialized robotics workloads that require additional RTSS communication paths from end user side
Without reserving these channels, RTSS scalability is constrained and future clients cannot be integrated without further IPCC channel rework. This update is intended to close that gap and provide sufficient channel capacity for upcoming RTSS consumers.
| @@ -0,0 +1,99 @@ | |||
| # SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause | |||
There was a problem hiding this comment.
Binding should should come before it is used by another patch.
There was a problem hiding this comment.
Change the order of the commits.
beec106 to
0dcb3f5
Compare
done. |
Add missing rtss mailbox dt schema file. Signed-off-by: Ram Sargar <rsargar@qti.qualcomm.com>
… Mailbox Add ipcc1-4 mailbox nodes, widen the tcsr syscon's reg range to cover both TCSR request/response registers, and add the apss_shared_ipc syscon node for the Lemans platform. These back the syscon/mbox phandle references used by the Lemans RTSS mailbox node (qcom,rtss-mailbox). Signed-off-by: Ram Sargar <rsargar@qti.qualcomm.com>
Add the rtss_mailbox overlay node for the Lemans platform, using ipcc1-ipcc4 and the tcsr/apss_shared_ipc syscon nodes. Place it under &soc, alongside the other simple-bus child nodes in this file. Signed-off-by: Ram Sargar <rsargar@qti.qualcomm.com>
… for RTSS Mailbox Add ipcc1-4 mailbox nodes, widen the tcsr syscon's reg range to cover both TCSR request/response registers, and add the apss_shared_ipc syscon node for the Monaco platform. These back the syscon/mbox phandle references used by the Monaco RTSS mailbox node (qcom,rtss-mailbox). Reserve the memory regions used by the SAIL/RTSS subsystem for its TZ mailbox, shared-memory mailbox, and OTA buffer on Monaco. These regions back the memory-region references used by the Monaco RTSS mailbox node. Signed-off-by: Ram Sargar <rsargar@qti.qualcomm.com>
Add the rtss_mailbox overlay node for the Monaco platform, mirroring the Lemans RTSS mailbox node. Place it under &soc, alongside the other simple-bus child nodes in this file. Signed-off-by: Ram Sargar <rsargar@qti.qualcomm.com>
0dcb3f5 to
215e02f
Compare
PR #1768 — validate-patchPR: #1768
Final Summary
|
PR #1768 — checker-log-analyzerPR: #1768
Detailed report: Full report
|
🔨 Build Failure Analysis — PR #1768PR: #1768 VerdictThis is NOT a compilation failure. The build failed during the automerge step with 145+ merge conflicts across the codebase. The PR itself does not introduce any compilation errors. The failure occurred when the CI system attempted to merge the PR branch ( Key Finding: Only 1 of the 145+ conflicting files (
Root Cause: The automerge process is attempting to integrate multiple topic branches simultaneously, and there are widespread conflicts between those branches. The PR's changes to the Makefile (removing two DTBO entries) likely conflict with other branches that also modify the Makefile in nearby lines. Recommendation: This PR cannot be evaluated for compilation correctness until the merge conflicts are resolved. The conflicts must be resolved either by:
📎 Detailed analysis: Full report |
🔨 Build Failure Analysis — PR #1768PR: #1768
VerdictAll failures are pre-existing integration conflicts. This PR introduces ZERO build or merge errors. The PR only modifies 9 files (Makefile, RTSS mailbox bindings, lemans/monaco DTS files), none of which conflict. The 145+ merge conflicts are between the baseline 📎 Detailed analysis: Full report |
🔨 Build Failure Analysis — PR #1768PR: #1768
VerdictThis is not a compilation failure. The build failed during the pre-compilation automerge step with 151 merge conflicts. The PR itself does not introduce compilation errors — the conflicts arise from integrating this PR's changes (RTSS mailbox refactoring for lemans/monaco) with other concurrent topic branches in the integration tree. The PR changes are limited to 9 files related to RTSS mailbox bindings and device trees, but conflicts occurred across 151 files throughout the kernel tree, indicating widespread parallel development activity in the integration branch. 📎 Detailed analysis: Full report |
🔨 Build Failure Analysis — PR #1768PR: #1768
VerdictAll 5+ merge conflicts are pre-existing integration issues, not introduced by this PR. The PR does not modify any of the conflicting files. 📎 Detailed analysis: Full report |
PR #1768 — validate-patchPR: #1768
Final Summary
|
PR #1768 — checker-log-analyzerPR: #1768
Detailed report: Full report
|
🔨 Build Failure Analysis — PR #1768PR: #1768
VerdictAll 7 merge conflicts are pre-existing integration issues between the PR's base branch and 📎 Detailed analysis: Full report |
🔨 Build Failure Analysis — PR #1768PR: #1768
VerdictThis is not a compilation error. The build failed during the automerge step due to a pre-existing merge conflict in a file unrelated to this PR's changes. 📎 Detailed analysis: Full report |
PR #1768 — validate-patchPR: #1768
Final Summary
|
PR #1768 — checker-log-analyzerPR: #1768
Detailed report: Full report
|
4 participants
Target milestone: QLI-2.1
Summary
Reworks the RTSS Mailbox device-tree support on Lemans and Monaco to address maintainers review feedback on the original design merged via #1327: syscon-based TCSR access, staging-overlay consolidation, and removal of the standalone RTSS-only DTBOs.
Impact
time for configs that include the staging overlay, per the existing FIT config).
Unchanged
Tracking issue
qualcomm-linux/meta-qcom#3085
This PR dependency
Related PRs
RTSS interface enablement is handled using a staging DTBO overlay to carry the rtss mailbox node, both bootup and DTBO patching with it applied have been verified on target.
Testing
Change Request
CRs-Fixed: 4668068
RTSS Feature Enablement Exception
https://jira-dc.qualcomm.com/jira/browse/QLIJIRA-123