Skip to content

drivers: qcom: rpmh/cmd_db: fix completion tracking and remove unused scaffolding - #7958

Draft
zelvam95 wants to merge 4 commits into
OP-TEE:masterfrom
zelvam95:feature/rpmh-cmd_db-fixes
Draft

drivers: qcom: rpmh/cmd_db: fix completion tracking and remove unused scaffolding#7958
zelvam95 wants to merge 4 commits into
OP-TEE:masterfrom
zelvam95:feature/rpmh-cmd_db-fixes

Conversation

@zelvam95

@zelvam95 zelvam95 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Title

drivers: qcom: rpmh/cmd_db: fix completion tracking and remove unused scaffolding

Dependencies / stacking

This PR is stacked on: PR #7954. This needs to be reviewed and merged after the base PR is 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 the
secure 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-kodiak and qcom-lemans, both with and without
CFG_QFPROM_PROGRAMMING=y CFG_QCOM_QFPROM_FUSEPROV=y, plus a one-off
qcom-lemans build forcing CFG_QCOM_RPMH_CLIENT=y CFG_QCOM_CMD_DB=y
to confirm the new per-platform header resolves correctly there too.
All combinations build clean; scripts/checkpatch.sh is clean on
every commit in the series.

@zelvam95
zelvam95 force-pushed the feature/rpmh-cmd_db-fixes branch 2 times, most recently from 98b0294 to a9bbccf Compare August 27, 2026 13:35
@zelvam95 zelvam95 changed the title drivers: qcom: rpmh: fix stale completion tracking and trim unused code # drivers: qcom: rpmh: fix completion tracking and reduce to single-DRV Aug 27, 2026
@zelvam95 zelvam95 changed the title # drivers: qcom: rpmh: fix completion tracking and reduce to single-DRV drivers: qcom: rpmh: fix completion tracking and reduce to single-DRV Aug 27, 2026
@github-actions

Copy link
Copy Markdown

FYI @ldts @kishorebatta-ossqcom @b49020

@zelvam95
zelvam95 force-pushed the feature/rpmh-cmd_db-fixes branch 2 times, most recently from 3342d20 to dd8bfef Compare August 27, 2026 15:30
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>
@zelvam95
zelvam95 force-pushed the feature/rpmh-cmd_db-fixes branch from dd8bfef to bef491a Compare August 27, 2026 19:27
@zelvam95
zelvam95 marked this pull request as draft August 28, 2026 05:00
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>
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>
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. QFPROM already splits its own register layout
per platform (kodiak/qfprom_target.h vs lemans/qfprom_target.h) for
exactly this reason; RPMH had no equivalent split.

Move the register offsets, control/status bits, and MSGID/ADDR field
shifts and masks into a new rpmh_target.h under each platform
directory, resolved the same way QFPROM's register layout already
is. Lemans's copy is a duplicate of Kodiak's values, since RPMH is
not yet enabled there and nothing has shown the register layout
differs — the file exists so that assumption is easy to correct in
one place if it turns out not to hold.

Physical base addresses (RPMH_BASE_ADDR and friends) are unrelated to
this and stay where they already are.

Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com>
@zelvam95
zelvam95 force-pushed the feature/rpmh-cmd_db-fixes branch from bef491a to 54cdee1 Compare August 28, 2026 05:57
@zelvam95 zelvam95 changed the title drivers: qcom: rpmh: fix completion tracking and reduce to single-DRV drivers: qcom: rpmh/cmd_db: fix completion tracking and remove unused scaffolding Aug 28, 2026
@zelvam95
zelvam95 marked this pull request as ready for review August 28, 2026 06:46
@zelvam95
zelvam95 marked this pull request as draft August 30, 2026 10:23
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.

1 participant