drivers: qcom: add GENI I2C driver and enable i2c config - #58
VeshalaAnilKumar wants to merge 1 commit into
Conversation
1a117cb to
dab3efd
Compare
|
Please rebase this PR to tip of qcom-next. |
1009b32 to
d4661f6
Compare
|
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 |
| 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
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.
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. Signed-off-by: Anil Veshala Veshala <anil.veshala@oss.qualcomm.com>
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 |
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.