Skip to content

Nord Clock driver Support - #40

Draft
nnunna94 wants to merge 27 commits into
qualcomm-linux:qcom-nextfrom
nnunna94:qup-clk-nord
Draft

Nord Clock driver Support#40
nnunna94 wants to merge 27 commits into
qualcomm-linux:qcom-nextfrom
nnunna94:qup-clk-nord

Conversation

@nnunna94

Copy link
Copy Markdown
Contributor

No description provided.

Organizes PAS clock support under platform/$(PLATFORM_FLAVOR)/ so
future platforms can provide their own PAS clock implementation.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Move all Kodiak-specific logic and the PTA command handlers into
platform/kodiak/, and model each subsystem with a descriptor/ops
abstraction: every platform exposes a table via
qcom_pas_platform_subsys() that the generic pas_core.c drives.
Pure structural refactor.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Add qcom_clock_lucidevo_pll_enable(), a self-contained helper that
configures, locks and enables the main output of a Lucid-EVO PLL given its
register block base and a struct qcom_lucidevo_pll_config.

No caller yet; this provides the building block for per-processor PLL
bring-up.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Bring up the Compute DSP (CDSP0/1) via the PAS peripheral
authentication path on the Lemans platform.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Each PAS subsystem maps its controller window at runtime via
core_mmu_add_mapping(); these late mappings come from
CFG_RESERVED_VASPACE_SIZE and are never released. The six DSP windows
total ~146.5 MB but the previous 60 MB default fits only one, so
reserve 256 MB to cover them with headroom.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Add LPASS / ADSP (QDSP6 v68/v69) PAS bring-up for the Lemans platform
(IQ-9075-EVK), following the existing Lemans CDSP0/1 PAS + clock-driver
pattern and the Kodiak LPASS PTA layout.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
After boot the static memory map is frozen at count + 5 entries, so
core_mmu_add_mapping() failed once those spare slots were exhausted.

Grow the map through the same realloc hook as every other add path,
re-resolving RES_VASPACE afterwards.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Add the IRIS video-codec PAS driver for lemans, mirroring the kodiak
venus driver (which already targets IRIS hardware). The lemans IRIS
register layout is identical: WRAPPER_TZ at IRIS+0xc0000 with the same
XTSS_SW_RESET / FW / CPA / NONPIX offsets.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Add PAS bring-up for the two general-purpose Hexagon DSPs on Lemans
(SA8775P): GP-DSP0 (TURINGGDSP, image id 39) and GP-DSP1 (TURINGGDSP1,
image id 40). This follows the same architecture as the existing
CDSP0/1, LPASS and IRIS subsystems.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
The subsystem manages the Iris video core, so name the file and its
symbols accordingly to match the hardware it drives.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Move the PAS_ID_* definitions out of the per-platform target_config.h
files into the PTA's pas_data.h so they live in one place.

These IDs are really part of the PTA contract with the client rather
than a platform definition; centralizing them in the PTA is a first
step towards that abstraction.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Add io_read32_off()/io_write32_off() for reading/writing a 32-bit MMIO
register at a base address plus byte offset, and
io_read32_off_field()/io_write32_off_field() for getting/setting a
masked, shifted field within such a register.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
Add a driver for the Qualcomm Hardware Key Manager (HWKM), a hardware
IP block present on Qualcomm SoCs that manages cryptographic key slots
in a tamper-resistant key table. Keys stored in HWKM slots are never
exposed in plaintext to software above the security level they were
provisioned at; the hardware enforces per-slot access-control and
usage policies.

The driver exposes the following functionality to OP-TEE:

  - Hardware Unique Key (HUK): implements tee_otp_get_hw_unique_key()
    by performing a three-level key derivation using the SYSTEM_KDF
    command. A stable SKDK L3 mixing key is first derived from
    TZ_SKDK_L2 into the dedicated mixing key slot; the UKDK L3 KDK
    and the final L4 HUK are then derived with the mixing key folded
    in via BSVE.MKS_EN. Two Kconfig options control this behaviour:
    CFG_HWKM_HUK_MIX_SKDK (default y) enables the SKDK mixing step,
    and CFG_HWKM_HUK_FUSE_REGION_DIGEST (default 0x0) selects fuse
    regions whose SHA256 digest is bound into the KDF input.

  - Full command set: NIST_KEYGEN, SYSTEM_KDF, KEY_WRAP_EXPORT,
    KEY_UNWRAP_IMPORT, KEY_SLOT_CLEAR, KEY_SLOT_RDWR, and SET_TPKEY
    are all implemented and exposed through a transaction queue API.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
Acked-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Add a MAINTAINERS entry for the new Qualcomm Hardware Key Manager
(HWKM) driver.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
Acked-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Add the HWKM master register region base and size to the shared Hoya
architecture config so all Hoya-family targets can reference them, and
enable CFG_QCOM_HWKM by default for the lemans target.

Signed-off-by: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>
Acked-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Standardize RNG driver naming and configuration across
QCOM platforms.Rename prng.c to qcom-csrng.c and consolidate
driver inclusion in the parent qcom/sub.mk with the
unified CFG_QCOM_CSRNG flag.This change simplifies the
driver structure and aligns with platform-agnostic
naming conventions.

The QCOM RNG IPs are confirmed by the hardware team to be
cryptographically secure (CSRNG), so the driver is named
qcom-csrng.c and enabled via CFG_QCOM_CSRNG to explicitly
reflect that the source is safe for key generation.

Update hoya chipset configurations to use the new
CFG_QCOM_CSRNG flag and configure QCOM_RNG_REG_BASE
for PRNG variant support, ensuring backward compatibility
while establishing consistent naming standards across
the codebase.

Force enable CFG_QCOM_CSRNG to use the hardware QRNG driver, and
disable CFG_WITH_SOFTWARE_PRNG whenever CFG_QCOM_CSRNG is enabled to
prevent fallback to the software PRNG.

Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
Configure QCOM_RNG_REG_BASE and enable the consolidated RNG driver
for the Bobcat family (ipq52xx).

Force enable CFG_QCOM_CSRNG to use the hardware QRNG driver, and
disable CFG_WITH_SOFTWARE_PRNG whenever CFG_QCOM_CSRNG is enabled to
prevent fallback to the software PRNG.

When HWRNG_PTA is enabled:
   - Configure HWRNG quality to 1024 bits entropy
   - Set HWRNG rate to 0 (unlimited)

Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
QCOM platforms manage secure watchdog via driver_init() without
framework registration. This is sufficient as QCOM currently
does not require HLOS control over the secure watchdog. The
implementation maintains separation between secure and
non-secure world watchdog management.

The implementation maps the watchdog base (QCOM_WDT_TMR_BASE)
into secure I/O memory, configures bark and bite timeouts
using a 32 KHz clock, registers a bark interrupt handler, and
services the watchdog by writing to the reset register.

Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
CFG_QCOM_SEC_WDOG is enabled in bobcat/arch.mk for
all Bobcat targets, with platform-specific watchdog
base addresses, interrupt IDs, and reset offsets
defined in the respective target_config.h files
(e.g., ipq96xx/ipq54xx and ipq52xx variants).

Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com>
Reviewed-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
The Camera ICP (Imaging Control Processor, PAS ID 33) firmware must be
loaded and authenticated by OP-TEE before CAMX can use the camera
subsystem on SA8775P / Lemans EVK. Without this support the kernel
camera driver fails to bring up the ICP and camera preview is
unavailable.

Add PAS reset ops for the ICP, register the subsystem in the PAS table.

Tested: camera preview use case exercised on Lemans EVK; ICP firmware
loads, authenticates and executes correctly with camera preview
confirmed functional end-to-end.

Signed-off-by: Ignatius Michael Jihan <mignatiu@qti.qualcomm.com>
TZDRAM and the DIAG log are currently protected by TF-A's static XPU
policy, duplicating values OP-TEE already owns. Move ownership to
OP-TEE for a single source of truth.

Add an XPU4 driver: xpu_protect_region() takes a region and access
policy, and resolves the XPU instance and a free resource group
itself. It has no external callers, so it stays static.

Both regions are protected from one service_init() call. DIAG log
protection is skipped when CFG_QCOM_DIAG_LOG is disabled, so an unused
buffer doesn't consume a resource group.

Compiles only when CFG_QCOM_XPUV4 is enabled.

Testing:
XPU resource-group registers matched the expected TZDRAM/DIAG log
ranges and permissions. Non-secure accesses raised XPU violations.

Tested-on: Hermosa (IPQ52xx)
Tested-on: Juhu (IPQ96xx)
Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
Enable OP-TEE-owned XPU protection for TZDRAM and the DIAG log on the
Bobcat family (IPQ52xx, IPQ96xx), replacing TF-A's static policy for
these regions.

Tested-on: Hermosa (IPQ52xx)
Tested-on: Juhu (IPQ96xx)
Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
Align the Qualcomm clock driver filenames with the clk-<vendor> naming
used by the rest of the clk subsystem, and with the clk_qcom.h header
they implement. No functional change.

Update sub.mk in the same commit so the tree still builds; renaming the
sources alone leaves srcs-y pointing at the old paths.

Signed-off-by: Naresh Nunna <nnunna@qti.qualcomm.com>
Assisted-by: Claude:claude-opus-5
Add an API to copy a resource's auxiliary data blob by resource ID.
For ARC resources this is the list of corner levels the rail
supports, which RPMh commands index into rather than accepting a raw
voltage. This is a prerequisite for the QUP SE clock driver's CX/MX
voltage vote, which resolves a rail's supported-corner ordinal list
via this API before voting a corner over RPMh.

copy_aux_data() previously clamped the copy to the caller's buffer and
returned success, so a caller with an undersized buffer received a
silently truncated blob it had no way to detect. Return
TEE_ERROR_SHORT_BUFFER with the required size instead, and copy
nothing. Both existing callers query metadata only (NULL data buffer,
zero length) and never reach this path.

Also fix cmd_db_get_entry_by_res_id() to clear result->len on the
zero-length-entry path, which cmd_db_get_aux() exercises.

Signed-off-by: Naresh Nunna <nnunna@qti.qualcomm.com>
Assisted-by: Claude:claude-opus-5
Register each QUPv3 serial-engine RCG on lemans as a standard struct
clk with no parent (clk_ops: enable/disable/set_rate/get_rate),
consumed on demand by a future TEE-side SPI/I2C driver via
qcom_clk_get_by_name() (this platform has no secure DT, so DT-based
acquire isn't available).

set_rate walks a per-domain frequency-config table (mux/divider/MND/
DFS-index), and votes a CX/MX voltage corner via RPMh around the rate
change -- raise before programming, lower after -- using an aggregate
reference-counted vote model. The voltage vote requires
cmd_db_get_addr()/cmd_db_get_aux() to resolve the rail's RPMh resource
address and its supported-corner ordinal list, so CFG_QCOM_CMD_DB/
CFG_QCOM_RPMH_CLIENT are now force-enabled whenever CFG_QCOM_CLK_BSP=y
(default y).

CX and MX are voted as a pair, so a failure between the two would
leave the rails disagreeing with the cached corner while the
no-change shortcut suppressed the corrective re-vote. Track that
mismatch and re-vote both rails on the next call.

The rail-vote backend lives in clk-qcom-vreg.c behind the
qcom_clk_vreg_vote() contract, so a non-RPMh target can supply its own
without touching the RCG walker. The per-target domain table and
frequency plans live in platform/<flavor>/clk-qcom-bsp.c, keeping the
filename flavor-generic so enabling CFG_QCOM_CLK_BSP on another target
needs no sub.mk change.

Known gap, documented but not blocking: the voltage vote is bypassed
on a hardware-driven DFS switch (it only covers explicit set_rate).

Signed-off-by: Naresh Nunna <nnunna@qti.qualcomm.com>
Assisted-by: Claude:claude-opus-5
The CBCR-to-CMD_RCGR distance used to derive one register's location
from another is not architectural: it is 8 bytes on lemans but not
constant across chipsets (nord's QUP SEs sit 0x10 apart, and nord's
QUPv3 wrapper 3 keeps no fixed relation at all). Deriving offsets that
way silently breaks on any target where the assumption doesn't hold.

Give struct qcom_clk_domain and struct qcom_clk_src_vote a full
physical address per register (cmd_rcgr_addr/cbcr_addr/vote_reg_addr)
instead of one GCC-relative offset each, matching how the reference
clock driver's own HWIO_<reg>_ADDR macros are built. Add struct
qcom_clk_window so a domain names the register window its addresses
fall inside rather than assuming a single global GCC base -- needed
because some targets split their QUP SEs across more than one clock
controller. The walker resolves each address against its domain's
window and bounds-checks it before use.

Convert lemans' header macros from GCC-relative offsets to
(GCC_BASE + offset) form to match, and update its BSP table to the
renamed struct fields. No functional change on lemans: GCC_BASE is
still the only window, and every resolved address is identical to the
previous offset-based one.

Signed-off-by: Naresh Nunna <nnunna@qti.qualcomm.com>
Assisted-by: Claude:claude-opus-5
Add Nord (wildcat) platform support for the QUPv3 serial-engine clock
walker: quadrant-controller GCC bases/windows, cmd_db AOP message-RAM
window and RPMh base, and the SOCCP PAS clock group needed by the
walker's RPMh/cmd_db client path.

Unlike lemans, Nord's cmd_db blob address is not fixed at build time --
AOP publishes it as a pointer word in AOP message RAM, so cmd_db_init()
reads that pointer and maps the blob on the fly via
core_mmu_add_mapping() instead of the static register_phys_mem()/
phys_to_virt() path lemans uses.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
srcs-y += clock-qcom.c
srcs-$(CFG_QCOM_PAS_PTA) += platform/$(PLATFORM_FLAVOR)/clock-qcom-pas.c
srcs-y += clk-qcom.c
srcs-$(CFG_QCOM_PAS_PTA) += platform/$(PLATFORM_FLAVOR)/clk-qcom-pas.c

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we leave clock-qcom-pas.c and not change anything here for PIL?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a file name change to align with optee overall driver naming structure among different vendors.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see pas code for other vendors, so may be let us leave the pass code rename.

* rather than its own CBCR's CLK_ENABLE bit. CLK_OFF is still polled on the
* SE's own CBCR regardless, since the vote register has no status bit.
*/
static TEE_Result qcom_qup_clk_enable(struct clk *clk)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we not having a common cbcr and need a qup clock enable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is due to the optee framework registration, clients gets to use upstream clk enable API which gets linked to our internal qti API. client would get clk handle by using clk_get_by_name and that would return pointer to a structure. with clock details... addr, vote addr and stuff.. this API would internally figure out the voting stuff..for enabling clock based on the data in the structure. the bare enable_cbcr API which is already present only tackles direct cbc address which is being used by pas so did not modify that ...since pas doesn't really use bsp to fit into this framework

return ret < 0 ? TEE_ERROR_TIMEOUT : TEE_SUCCESS;
}

static void qcom_qup_clk_disable(struct clk *clk)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can have a common enable/disable of a clock.

* revisions v1..v4. Offsets are relative to the domain's CMD_RCGR register
* (qcom_clk_domain.cmd_rcgr_offset, itself GCC-relative).
*/
#define QCOM_RCG_CFG_REG_OFFSET 0x4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common offset, so you can make a common offset.

@ldts

Copy link
Copy Markdown
Contributor

how do I test this code?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Nord Qualcomm clock support, including QUPv3 clock domains, voltage voting, DFS, and SOCCP PAS control.

Changes:

  • Adds Nord and Lemans QUP clock BSPs and shared clock-provider logic.
  • Adds RPMh CX/MX voting and command-database auxiliary data access.
  • Adds Nord platform addresses, build configuration, and SOCCP support.

Reviewed changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
core/include/drivers/qcom/cmd_db/cmd_db.h Declares auxiliary-data lookup API.
core/include/drivers/clk_qcom.h Exposes BSP clock APIs and SOCCP group.
core/include/drivers/clk_qcom_bsp.h Defines clock BSP data structures.
core/drivers/qcom/cmd_db/cmd_db.c Supports dynamic database mapping and auxiliary data.
core/drivers/clk/qcom/sub.mk Builds renamed driver and platform backends.
core/drivers/clk/qcom/platform/nord/clock_group_qcom.h Defines Nord clock and reset registers.
core/drivers/clk/qcom/platform/nord/clk-qcom-pas.c Implements SOCCP clock/reset sequencing.
core/drivers/clk/qcom/platform/nord/clk-qcom-bsp.c Provides Nord QUP frequency plans and domains.
core/drivers/clk/qcom/platform/lemans/clock_group_qcom.h Adds Lemans QUP register definitions.
core/drivers/clk/qcom/platform/lemans/clk-qcom-pas.c Implements Lemans PAS clock/reset handling.
core/drivers/clk/qcom/platform/lemans/clk-qcom-bsp.c Provides Lemans QUP clock domains.
core/drivers/clk/qcom/platform/kodiak/clk-qcom-pas.c Moves Kodiak PAS implementation to renamed source.
core/drivers/clk/qcom/clock-qcom.c Removes superseded clock driver source.
core/drivers/clk/qcom/clk-qcom.c Adds shared QUP clock provider and DFS support.
core/drivers/clk/qcom/clk-qcom-vreg.c Implements aggregate RPMh rail voting.
core/drivers/clk/qcom/clk_qcom_vreg.h Declares rail-vote backend contract.
core/arch/arm/plat-qcom/wildcat/nord/target.mk Enables Nord clock dependencies.
core/arch/arm/plat-qcom/wildcat/nord/target_config.h Defines Nord hardware windows.
core/arch/arm/plat-qcom/wildcat/arch_config.h Adds Wildcat AOP and RPMh addresses.
core/arch/arm/plat-qcom/hoya/lemans/target.mk Enables Lemans QUP clock BSP dependencies.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

uint32_t res_hz = 0;
TEE_Result res = TEE_SUCCESS;

res = qcom_domain_set_rate(qup->domain, rate, &qup->corner, &res_hz);
Comment on lines +522 to +524
REG_POLL_TIMEOUT(cbcr, 10 * 1000, 10, &ret, cbcr_branch_on);

return ret < 0 ? TEE_ERROR_TIMEOUT : TEE_SUCCESS;
Comment on lines +572 to +574
qcom_qup_clks = calloc(bsp->n_domains, sizeof(*qcom_qup_clks));
if (!qcom_qup_clks)
return TEE_ERROR_OUT_OF_MEMORY;
Comment on lines +648 to +649
qup = clk->priv;
return qup->domain;
Comment on lines +429 to +444
if (!buf || !len || !*len || *len > UINT32_MAX ||
!is_valid_res_id(res_id))
return TEE_ERROR_BAD_PARAMETERS;

result.len = *len;

mutex_lock(&query_db.lock);

if (!query_db.data) {
mutex_unlock(&query_db.lock);
return TEE_ERROR_BAD_STATE;
}

res = cmd_db_get_entry_by_res_id(res_id, &result, buf);
if (res == TEE_SUCCESS || res == TEE_ERROR_SHORT_BUFFER)
*len = result.len;
@nnunna94

Copy link
Copy Markdown
Contributor Author

how do I test this code?

Jorge A. Ramirez-Ortiz (@ldts) this is similar implementation for SPI driver consumption for serial engine qup clocks. the basic sanity is done but end to end use case would be tested along with SPI driver only. SSG team was asking for this not sure if they have any other dependency so raised the PR.

@ldts

Copy link
Copy Markdown
Contributor

So will you add the SPI driver to this PR?

@nnunna94

Copy link
Copy Markdown
Contributor Author

So will you add the SPI driver to this PR?

Right... the plan is to get this PR merged along with SPI driver. in the meanwhile if SSG really needs this for any other FBC activities, this can serve the requirement. we will work with buses team similar to lemans on Nord as well for getting this support merged.

@ldts

Copy link
Copy Markdown
Contributor

Right... the plan is to get this PR merged along with SPI driver. in the meanwhile if SSG really needs this for any other FBC activities, this can serve the requirement. we will work with buses team similar to lemans on Nord as well for getting this support merged.

sure but we can not merge this code without the driver. both should be proposed as part of the same pull request.

@zelvam95

Copy link
Copy Markdown
Contributor

Right... the plan is to get this PR merged along with SPI driver. in the meanwhile if SSG really needs this for any other FBC activities, this can serve the requirement. we will work with buses team similar to lemans on Nord as well for getting this support merged.

sure but we can not merge this code without the driver. both should be proposed as part of the same pull request.

nnunna94, Would request you to please mark this PR as draft for now & once we have full E2E working piece we can mark it as "ready for review". Meanwhile interested (internal team/POCs) folks can still review the change when its draft and share comments to you.

@nnunna94
nnunna94 marked this pull request as draft August 12, 2026 15:20
@nnunna94

Copy link
Copy Markdown
Contributor Author

Right... the plan is to get this PR merged along with SPI driver. in the meanwhile if SSG really needs this for any other FBC activities, this can serve the requirement. we will work with buses team similar to lemans on Nord as well for getting this support merged.

sure but we can not merge this code without the driver. both should be proposed as part of the same pull request.

nnunna94, Would request you to please mark this PR as draft for now & once we have full E2E working piece we can mark it as "ready for review". Meanwhile interested (internal team/POCs) folks can still review the change when its draft and share comments to you.

done

@ldts

Copy link
Copy Markdown
Contributor

Architectural review (recommendations).

Two robustness issues worth addressing before a consumer lands:

  1. qcom_qup_clk_enable() sets the vote bit (io_setbits32(vote, BIT(dom->vote_bit))) and then returns TEE_ERROR_TIMEOUT if the CBCR never reports on. The clk framework keeps the enable-count at 0 on failure, so no later clk_disable() will clear that vote — the branch stays voted-on with no owner. Recommend clearing the vote bit before the timeout return.

  2. qcom_qup_clocks_register() publishes the qcom_qup_clks global (via calloc) before the per-clk alloc/register loop completes. If any clk_alloc/clk_register fails mid-loop, unfilled entries keep domain == NULL, and qcom_clk_get_by_name() iterates bsp->n_domains dereferencing qcom_qup_clks[i].domain->name. Also, on partial failure the global is left non-NULL, so a re-run hits the if (qcom_qup_clks) return SUCCESS guard and reports success without finishing registration. Recommend building into temp storage and publishing only after full success.

@nnunna94

nnunna94 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Architectural review (recommendations).

Two robustness issues worth addressing before a consumer lands:

  1. qcom_qup_clk_enable() sets the vote bit (io_setbits32(vote, BIT(dom->vote_bit))) and then returns TEE_ERROR_TIMEOUT if the CBCR never reports on. The clk framework keeps the enable-count at 0 on failure, so no later clk_disable() will clear that vote — the branch stays voted-on with no owner. Recommend clearing the vote bit before the timeout return.
  2. qcom_qup_clocks_register() publishes the qcom_qup_clks global (via calloc) before the per-clk alloc/register loop completes. If any clk_alloc/clk_register fails mid-loop, unfilled entries keep domain == NULL, and qcom_clk_get_by_name() iterates bsp->n_domains dereferencing qcom_qup_clks[i].domain->name. Also, on partial failure the global is left non-NULL, so a re-run hits the if (qcom_qup_clks) return SUCCESS guard and reports success without finishing registration. Recommend building into temp storage and publishing only after full success.

This PR needs rebasing and refactoring once PR26 lands Jorge A. Ramirez-Ortiz (@ldts) . Hence this is in Draft.

@b49020
Sumit Garg (b49020) force-pushed the qcom-next branch 2 times, most recently from 1a117cb to dab3efd Compare September 7, 2026 07:43
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.

8 participants