drivers: qcom: add GENI I2C driver and enable i2c config - #58
VeshalaAnilKumar wants to merge 18 commits into
Conversation
ca169e2 to
1a117cb
Compare
…files Nord was the only Wildcat chip in the tree, so its GIC base addresses and DARE-TZ TZDRAM region settings were placed in the shared Wildcat architecture layer. Adding a second Wildcat chip with different addresses makes the architecture layer the wrong home for them. Move GICD_BASE and GICR_BASE from arch_config.h to nord/target_config.h, and move the DARE-TZ TZDRAM region configuration from qcom-arch.mk to nord/target.mk, so the shared Wildcat layer stays chip-agnostic. Add SPDX licence identifier to qcom-arch.mk while there. Signed-off-by: Pawan Rai <pawarai@qti.qualcomm.com> Reviewed-by: Harshal Dev <harshal.dev@oss.qualcomm.com> Tested-by: Harshal Dev <harshal.dev@oss.qualcomm.com> Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Cacao is a Qualcomm XR chipset in the Wildcat architecture family, featuring an octa-core Oryon CPU, a GICv4 interrupt controller, and DARE-TZ in-line memory encryption managed by the TME root-of-trust. OP-TEE runs in a DARE-TZ protected DRAM region and does not need a separate DARE driver. Testing: Tested on Rumi. Signed-off-by: Pawan Rai <pawarai@qti.qualcomm.com> Reviewed-by: Harshal Dev <harshal.dev@oss.qualcomm.com> Tested-by: Harshal Dev <harshal.dev@oss.qualcomm.com> Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Add PLATFORM=qcom-cacao build to the CI. Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> Signed-off-by: Pawan Rai <pawarai@qti.qualcomm.com>
Shikra is a Qualcomm IoT chipset in the Bruin architecture family, featuring a quad-core Cortex-A55 CPU and a GICv3 interrupt controller. Tested optee boot-up on Shikra board. Signed-off-by: Pawan Rai <pawarai@qti.qualcomm.com> Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> Reviewed-by: Harshal Dev <harshal.dev@oss.qualcomm.com> Tested-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
Add PLATFORM=qcom-shikra build to the CI. Signed-off-by: Pawan Rai <pawarai@qti.qualcomm.com> Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
1a117cb to
dab3efd
Compare
|
Please rebase this PR to tip of qcom-next. |
1009b32 to
d4661f6
Compare
Fix the address mappings for DRAM on Nord IQ-10. DRAM0 has a size of 0x8000 0000 while DRAM1 starts at 0x8 8000 0000 with a size of 0x7 8000 0000. Also, there exists a DRAM2 region which the Linux kernel maps as HIGHMEM for allocating memory for userspace applications. Since this region starting at 0x88 0000 0000 is not mapped by OP-TEE, TEE_IOC_SHM_REGISTER ioctl fails because OP-TEE doesn't recognize it as Non-secure memory. Validated by running xtest 1002 after building OP-TEE with CFG_TEE_CORE_EMBED_INTERNAL_TESTS=y which was failing otherwise. Signed-off-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
Add a pseudo-TA for the Qualcomm Inline Crypto Engine (ICE) that lets the
kernel dm-crypt/inline-crypt path program raw software keys into ICE key
slots for inline storage encryption.
Two commands are exposed:
- PTA_CMD_ICE_INVALIDATE_KEY: wipe a key slot with random data.
- PTA_CMD_ICE_SET_CONFIG_KEY: program key/salt, cipher mode and
data-unit size for AES-XTS-128/256 and AES-CBC-128/256.
Only the REE kernel may open a session on this PTA. The PTA is gated by
CFG_ICE_FS_ENC_PTA and is not built unless a platform enables it.
Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com>
Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com>
Reviewed-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
Turn on CFG_ICE_FS_ENC_PTA for ipq52xx and ipq96xx, which carry the SDCC ICE block used for eMMC inline encryption, and point the generic ICE_LUT_KEYS at the SDCC LUT-keys register region. Testing: Booted to the kernel, dispatched an ICE software-key config from the kernel to this PTA, then wrote and read back a file on the encrypted filesystem and confirmed matching md5sums. Tested-on: IPQ52xx, IPQ96xx Signed-off-by: Harikrishna <hart@qti.qualcomm.com> Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Reviewed-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
This file already lives under drivers/clk/qcom/, so repeating qcom in its own name is redundant, and inconsistent with how other vendor subdirectories (sam/, stm32) name their own files. Signed-off-by: Naresh Nunna <nnunna@qti.qualcomm.com> Assisted-by: Claude:opus-5 Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Reviewed-by: Vinod Kumar Amanaganti <vinoda@qti.qualcomm.com>
The QUP SE clock driver's CX/MX voltage vote needs a rail's supported corner ordinals, which for ARC resources live in the auxiliary data blob RPMh commands index into rather than a raw voltage. Add cmd_db_get_aux() to fetch it by resource ID. Signed-off-by: Naresh Nunna <nnunna@qti.qualcomm.com> Assisted-by: Claude:opus-5 Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Reviewed-by: Vinod Kumar Amanaganti <vinoda@qti.qualcomm.com>
Some clock rates need a higher CX/MX voltage corner than others, and a corner may be shared by multiple RCGs; rail_vote() refcounts per corner over RPMh so each rail is only raised for as long as some caller actually needs it, and CX/MX are resolved independently since they don't necessarily share the same hlvl encoding. Signed-off-by: Naresh Nunna <nnunna@qti.qualcomm.com> Assisted-by: Claude:opus-5 Assisted-by: Claude:sonnet-5 Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Reviewed-by: Vinod Kumar Amanaganti <vinoda@qti.qualcomm.com>
Lemans has no secure DT, so register each QUPv3 SE clock as a plain struct clk, modeling the PLL/RCG/branch as separate objects so the generic framework's own refcounting governs each PLL vote. Signed-off-by: Naresh Nunna <nnunna@qti.qualcomm.com> Assisted-by: Claude:opus-5 Assisted-by: Claude:sonnet-5 Reviewed-by: Jorge Ramirez-Ortiz <jorge.ramirez@oss.qualcomm.com> Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com> Reviewed-by: Vinod Kumar Amanaganti <vinoda@qti.qualcomm.com>
The RPMh command MSGID encodes a MSG_LENGTH field describing the payload length, in bytes, of the command. This field was hardcoded to 1 which is leading to unpredictable behavior on the AOP side (including the command never being acknowledged, causing timeouts). Changing it to 8 (the correct length for the single 32-bit data word every RPMh command carries). Using MSGID_WRITE since it's a write command. Fixes: b6ff325 (drivers: qcom: rpmh: add RPMH client driver) Signed-off-by: Shivam Sanjay <shivsanj@qti.qualcomm.com> Reviewed-by: Dinesh Choudhary <idinesh@qti.qualcomm.com> Reviewed-by: Selvam Sathappan Periakaruppan <speriaka@qti.qualcomm.com>
Add PAS support for the ipq96xx CDSP (Turing/NSP) remote processor: load the firmware and its DTB, program the QDSP6 boot registers, run the two-stage boot FSM, and handle shutdown/reset via GCC. Also adds the Turing clock bring-up. ipq96xx gates these windows to secure-only accesses via XPU, so a new qcom_pas_data::secure flag maps them MEM_AREA_IO_SEC instead of MEM_AREA_IO_NSEC; other platforms are unaffected. Testing: Built for PLATFORM_FLAVOR=ipq96xx and kodiak with aarch64-linux-gnu-. CDSP boot verified on ipq96xx hardware. Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
Enable the PAS PTA and Qualcomm clock driver on ipq96xx, add the CDSP (Turing/NSP) register window bases (Turing, GCC, MPM2, TCSR), and register the PAS pseudo TA as an in-tree early TA. Testing: Built for PLATFORM_FLAVOR=ipq96xx with aarch64-linux-gnu-; the CDSP PAS and clock objects link into the image and the PAS early TA is signed. Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
|
VeshalaAnilKumar how has this PR been tested on Lemans EVK? Is there any I2C bus assigned to TZ/OP-TEE? |
There was a problem hiding this comment.
🟡 Changes recommended
The new driver currently references missing clock APIs (build/link blocker) and contains a few confirmed correctness/configuration issues that should be resolved before it can be safely merged.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a Qualcomm QUPv3 GENI-based I2C controller driver for OP-TEE, intended to let the lemans platform use I2C via the generic i2c_ctrl_ops interface without devicetree integration.
Changes:
- Add a new GENI I2C driver (
qcom_geni_i2c.c) with polling-based FIFO transfers and optional firmware loading/pinmux setup. - Add lemans platform configuration for QUP GENI I2C instances (register mappings, clocks, pin groups, and an embedded firmware blob).
- Wire the driver into the build and enable
CFG_DRIVERS_I2C/CFG_QCOM_GENI_I2Cfor lemans.
File summaries
| File | Description |
|---|---|
| core/include/drivers/qcom_geni_i2c.h | New public interface and platform config structures for the GENI I2C driver |
| core/drivers/i2c/sub.mk | Adds Qualcomm I2C subdirectory to the build |
| core/drivers/i2c/qcom/sub.mk | Adds the GENI I2C driver + platform subdir when enabled |
| core/drivers/i2c/qcom/qcom_geni_i2c.c | New polling-mode GENI I2C controller implementation |
| core/drivers/i2c/qcom/platform/sub.mk | Selects per-flavor platform config subdir |
| core/drivers/i2c/qcom/platform/lemans/sub.mk | Adds lemans GENI I2C config source |
| core/drivers/i2c/qcom/platform/lemans/qcom_geni_i2c_config.c | Lemans register mappings, pinmux groups, clocks, and embedded I2C firmware blob |
| core/arch/arm/plat-qcom/hoya/lemans/target.mk | Enables I2C + GENI I2C driver for lemans |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| res = qcom_clk_get_by_name(qi->se_clock_name, &qi->se_clk); | ||
| if (res) { | ||
| EMSG("QUP I2C: cannot get clock %s: %#" PRIx32, | ||
| qi->se_clock_name, res); | ||
| return res; |
| #include <mm/core_mmu.h> | ||
| #include <util.h> | ||
|
|
||
| #define CFG_QUP2_SE2_I2C_EN |
There was a problem hiding this comment.
i will check and update
| EMSG("QUP I2C %u: no SCL timing table for a %lu Hz SE clock (need 19.2 or 32 MHz)", | ||
| qi->id, qi->clk_hz); |
|
|
||
| #define CFG_QUP2_SE2_I2C_EN | ||
|
|
||
| const uint8_t i2c_qup_fw[] = |
| * (geni_i2c_clk_map_idx() keys off clk_get_rate(gi2c->se.clk), not off a | ||
| * config value). Trusting the platform-cfg number instead would silently | ||
| * mis-time SCL by whatever ratio the real rate differs by, so read it | ||
| * back from the clock and only fall back to the cfg value if the clock | ||
| * framework cannot report one. |
| res = qcom_clk_enable_dfs(qi->se_clk); | ||
| if (res) { | ||
| EMSG("QUP SPI: enable DFS on %s failed: %#" PRIx32, | ||
| qi->se_clock_name, res); | ||
| qi->se_clk = NULL; | ||
| return res; | ||
| } |
there is no POR use case for I2C on Lemans EVK, implemented in general. |
d4661f6 to
ee04ac4
Compare
Sumit Garg (b49020)
left a comment
There was a problem hiding this comment.
End-to-end use-case missing, please clarify how this driver is going to be used for a particular target.
ee04ac4 to
1785341
Compare
|
what pins have you used on the external expansion port to validate the driver? going to integrate the SE05x crypto device on this platform so there is at least a user for this driver - you can see how it is done on other boards via the glue layer: https://github.com/OP-TEE/optee_os/tree/master/core/drivers/crypto/se050/glue - I expect this wont be much different: I just need to know what pins to use from the LS expansion - I cant find a valid map, I assume you tested some external I2C device? |
There is no use case on Leman's, just picked some random gpio's and tested with prodigy slave, i want to make sure driver is properly working or not. |
I dont understand what you mean - I do have a use case in Lemans (so there is at least this one). I need to integrate the a hardware security module via the LS expansion port for which I need the pinout. My question is how have you tested - what pins correspond to which controller. I will use that to test the stability of the driver and then continue with the review. |
tested CFG_QUP2_SE2_I2C_EN Instance GPIO86, GPIO87, whiich i did rework(blue wiring) to connect external prodigy, |
then I could enable the SE05x with GENI support and we would have a client for the driver. so please do let me know VeshalaAnilKumar |
please go ahead, it should work. se05 -- gpio52 and gpio53 ? if yes please enable below config |
|
AFAICS this driver it is not functional. |
| size_t count = 0; | ||
| size_t i = 0; | ||
| TEE_Result res = TEE_SUCCESS; | ||
|
|
There was a problem hiding this comment.
Should we need NULL checks for qi ?
|
VeshalaAnilKumar could you add this commit : ldts/optee_os@2621a4f to your I2C series please? Then we have a consumer for I2C Notice that it needs some of these fixes |
Add a Qualcomm GENI I2C driver implementing i2c_ctrl_ops for a GENI Serial Engine in FIFO transfer mode, driven by polling rather than interrupts. Not yet enabled for any platform. Signed-off-by: Anil Veshala Veshala <anil.veshala@oss.qualcomm.com>
Enable CFG_DRIVERS_I2C and CFG_QCOM_GENI_I2C for lemans and add the platform SE table, with the corresponding qup_i2c_config[] entries. Signed-off-by: Anil Veshala Veshala <anil.veshala@oss.qualcomm.com>
Add a boot-time self-test for the QUP GENI I2C driver. I2C has no internal digital loopback path (SDA/SCL are open-drain and need a real device to see anything at all), so this exercises a real peripheral wired to the SE; its outcome is reported but never fails the boot. Enable with CFG_QUP_I2C_TEST=y and CFG_QUP_I2C_TEST_EXTERNAL=y. Signed-off-by: Anil Veshala Veshala <anil.veshala@oss.qualcomm.com>
1785341 to
34d584d
Compare
5fde2c5 to
9b756a1
Compare
Add a Qualcomm I2C geni driver implementing i2c_ctrl_ops for a GENI Serial Engine in FIFO transfer mode, driven by polling rather than interrupts. Enabled CFG_QCOM_GENI_I2C in lemans platform, and add the corresponding qup config settings in qup_i2c_config[] table.