drivers: qcom: rpmh/cmd_db: fix completion tracking and remove unused scaffolding - #57
Open
Selvam (zelvam95) wants to merge 4 commits into
Open
Conversation
Sumit Garg (b49020)
force-pushed
the
qcom-next
branch
2 times, most recently
from
September 7, 2026 07:43
1a117cb to
dab3efd
Compare
Member
|
Please rebase this PR to tip of qcom-next. |
Selvam (zelvam95)
force-pushed
the
feature/qcom-cmd_db-rpmh-refactor
branch
from
September 8, 2026 09:40
beb1e8c to
64512ac
Compare
Selvam (zelvam95)
changed the title
Contributor
Author
Rebase done. |
Contributor
Author
|
Corresponding upstream PR is tracked here: OP-TEE/optee_os#7958 |
Selvam (zelvam95)
force-pushed
the
feature/qcom-cmd_db-rpmh-refactor
branch
from
September 10, 2026 08:54
64512ac to
93b66f4
Compare
The AMC-finished interrupt status is never cleared, so once it is set it stays latched "complete" for the rest of the boot: every later completion check reads that stale bit instead of a fresh per-command signal, regardless of whether the new command was ever actually processed. Clear it before triggering a command and after consuming its completion, so every wait observes a fresh signal. Also program the per-command completion-wait mask before triggering: the TCS has a register for it that nothing was writing to, so the hardware had no way to know which enabled commands actually need an AOP response before the TCS counts as done. rpmh_send_command() already waits for completion before returning, so rpmh_barrier_single()'s second wait on the same request was already redundant, and would now time out instead of reading a harmless stale bit. Drop it along with its call sites. Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Tested by: Naresh Nunna <nnunna@qti.qualcomm.com>
rpmh_create_handle() has always rejected every drv_id but RSC_DRV_SECURE. Everything around it was built for a broader case that never occurs: a runtime multi-DRV config table with a single entry, a per-DRV base-address array and register step, and an async multi-slot TCS state machine built to walk that table. None of it has a real caller. Replace all of it with a single fixed RSC base and a compile-time TCS layout. The command-dispatch loop is otherwise unchanged: it still builds a full command array with enable/wait masks, since the TCS already supports dispatching a batch and there is no reason to narrow that just because the one real caller only ever submits one command. Separately, poll the actual TCS_STATUS_CONTROLLER_IDLE hardware bit before dispatch, replacing a software idle flag that nothing in this path ever clears back to busy. Also drop the per-TCS AMC/non-AMC setup that init used to do across all 4 TCSs. The send path already asserts AMC mode on the one TCS it ever dispatches through, right before triggering, so priming it at init was redundant, and the other three TCSs are never touched again by anything. hal_rpmh_convert_to_amc()/convert_to_tcs() go with it, since nothing else called them. Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Tested by: Naresh Nunna <nnunna@qti.qualcomm.com>
cmd_db_get_priority() had exactly one caller, removed from the RPMH driver in the previous commit, leaving it and the address-lookup path it depends on unreachable. Remove both and simplify search_entry() to the resource-ID lookup its only remaining caller uses. entry_header keeps its own priority[] field, since it mirrors the on-disk layout the AOP command DB writes. While touching search_entry(), also skip any slot whose reported slv_id falls outside the valid range before scanning its entries, so a slot with a corrupted slv_id but a stray non-zero cnt can't be walked. It previously relied solely on an unpopulated slot's cnt being 0 to make the inner loop a no-op. Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Tested by: Naresh Nunna <nnunna@qti.qualcomm.com>
Selvam (zelvam95)
force-pushed
the
feature/qcom-cmd_db-rpmh-refactor
branch
from
September 10, 2026 12:28
be9929d to
a66d510
Compare
rpmh_hal.h hardcoded every RSC/TCS register offset and MSGID/ADDR field value in one file shared by every platform that could ever enable this driver. Move the register offsets, control/status bits, and MSGID/ADDR field shifts and masks into a new rpmh_target.h under each platform directory. Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Tested by: Naresh Nunna <nnunna@qti.qualcomm.com>
Selvam (zelvam95)
force-pushed
the
feature/qcom-cmd_db-rpmh-refactor
branch
from
September 10, 2026 13:17
a66d510 to
6f26bd1
Compare
IDineshChoudhary
approved these changes
Sep 14, 2026
IDineshChoudhary
left a comment
IDineshChoudhary
left a comment
There was a problem hiding this comment.
Couple of review comments (but minor ones), looks good otherwise - approving.
IDineshChoudhary
suggested changes
Sep 14, 2026
IDineshChoudhary
left a comment
IDineshChoudhary
left a comment
There was a problem hiding this comment.
resetting approval based on equivalent upstream change review at OP-TEE/optee_os#7958
Contributor
Author
Thanks Dinesh. I will address the comments provided here as well and push to the upstream PR since active review is happening there. Once the change is merged there, will pull it to qcom-next. |
| udelay(1); | ||
| } | ||
|
|
||
| /* Clear the AMC-finished interrupt now that it's been consumed. */ |
Contributor
There was a problem hiding this comment.
Do we still need this when we are clearing AMC every time before sending tcs (Line 151-53)?
| uint32_t data, uint32_t *req_id); | ||
|
|
||
| /* Wait for command completion */ | ||
| void rpmh_barrier_single(struct rpmh_client *handle, uint32_t req_id); |
Contributor
There was a problem hiding this comment.
This function may be useful in case we have any Async voting scenario, are we assuming that we will not have any such scenario?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Title
drivers: qcom: rpmh/cmd_db: fix completion tracking and remove unused scaffolding
Dependencies / stacking
This PR is stacked on: PR #47 (now merged)
Description
The RPMH client has a real completion-tracking bug: the AMC-finished
interrupt status is never cleared around dispatch, and the
per-command completion-wait register is never programmed, so after
the first command of a boot every later completion check can read
stale state instead of a fresh per-command signal. This series fixes
that.
While auditing the surrounding code, it also removes a substantial
amount of driver machinery that has no real caller — a runtime
multi-DRV config table, a per-DRV base-address array, an async
multi-slot TCS state machine, and CMD_DB's paired priority/address
lookup path.
rpmh_create_handle()has always accepted only thesecure DRV, and only active-set commands are ever issued through it;
none of the removed code was reachable. In its place, RPMH's register
layout is split into a per-platform header, mirroring QFPROM's
existing structure, and CMD_DB's slave-ID lookup gains a range check
it was missing.
None of this changes behavior for the one real caller (QFPROM's fuse
rail sequencing) beyond fixing the completion-tracking bug itself —
everything else is dead-code removal or a defensive hardening that
was already correct in practice.
Testing
Built
qcom-kodiakandqcom-lemans, both with and withoutCFG_QFPROM_PROGRAMMING=y CFG_QCOM_QFPROM_FUSEPROV=y, plus a one-offqcom-lemansbuild forcingCFG_QCOM_RPMH_CLIENT=y CFG_QCOM_CMD_DB=yto confirm the new per-platform header resolves correctly there too.
All combinations build clean;
scripts/checkpatch.shis clean onevery commit in the series.