Conversation
4a0e7d9 to
1d46e73
Compare
There was a problem hiding this comment.
Rather than calling it a TLMM driver, use the standard pinctrl naming conventions. Follow naming conventions similar to Linux: drivers/pinctrl/qcom/pinctrl*
|
Can you please clarify the intended use case for the TLMM/Pinctrl driver in OP-TEE? From these two commits, I don't see any actual GPIO/TLMM configuration being performed. The TLMM initialization appears to only register the driver and set up the mappings, but no pins are configured or exercised. Do we expect the public TLMM APIs introduced here to be consumed by platform-specific code in a future change, or is there an existing use case that I may have missed? |
690842e to
a4d253d
Compare
|
Rebase to tip of qcom-next |
1d46e73 to
24bd49f
Compare
There was a problem hiding this comment.
Pull request overview
Adds a Qualcomm TLMM (Top Level Mode Multiplexer) GPIO + pinctrl driver using a non-device-tree model, and wires it into the QCOM build for the lemans platform flavor.
Changes:
- Introduces a new TLMM driver under
core/drivers/qcom/tlmm/implementing GPIO ops and a pinctrl-state builder/applier. - Adds a public TLMM API header and a lemans-specific TLMM hardware descriptor (
tlmm_soc_desc). - Enables the driver in the QCOM and lemans build system (new subdir +
CFG_QCOM_TLMM).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| core/include/drivers/qcom/tlmm/tlmm.h | Public TLMM API (GPIO chip access, pin ownership, non-DT pinctrl state helpers). |
| core/drivers/qcom/tlmm/tlmm_private.h | Internal register definitions, ownership bookkeeping, and helper prototypes. |
| core/drivers/qcom/tlmm/sub.mk | Adds TLMM GPIO + pinctrl sources to the build when enabled. |
| core/drivers/qcom/tlmm/pinctrl.c | Implements pinctrl ops and helpers to build/apply/free TLMM pin states. |
| core/drivers/qcom/tlmm/gpio.c | Implements GPIO ops, register access helpers, ownership API, and driver init/mapping. |
| core/drivers/qcom/sub.mk | Hooks the new tlmm/ driver directory behind CFG_QCOM_TLMM. |
| core/arch/arm/plat-qcom/sub.mk | Adds platform-specific TLMM SoC data compilation when enabled. |
| core/arch/arm/plat-qcom/hoya/lemans/tlmm_soc_data.c | Defines the lemans TLMM base/stride/tiles/gpio count descriptor. |
| core/arch/arm/plat-qcom/hoya/lemans/target.mk | Enables CFG_QCOM_TLMM for lemans. |
| core/arch/arm/plat-qcom/hoya/lemans/target_config.h | Adds TLMM base/size address definitions for lemans. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
24bd49f to
491c940
Compare
491c940 to
edafb8e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (1)
core/drivers/qcom/tlmm/gpio.c:199
- The GPIO direction operation leaves
TLMM_CFG_FUNC_MASKunchanged. If earlier firmware left the pad on an alternate function,tlmm_request_pin()followed by the GPIO API only toggles OE and never muxes the pad to GPIO function 0, so reads/writes may not control the pin. Clear the function field when selecting either GPIO direction.
if (dir == GPIO_DIR_OUT)
tlmm_write_cfg_unlocked(tc, pin, 0, TLMM_CFG_OE);
else
tlmm_write_cfg_unlocked(tc, pin, TLMM_CFG_OE, 0);
edafb8e to
f111d3e
Compare
ca169e2 to
1a117cb
Compare
|
Please rebase to tip of qcom-next |
1a117cb to
dab3efd
Compare
f111d3e to
645e47d
Compare
| .get_value = tlmm_get_value, | ||
| .set_value = tlmm_set_value, | ||
| }; | ||
| DECLARE_KEEP_PAGER(tlmm_gpio_ops); |
There was a problem hiding this comment.
Pager isn't supported on Qcom SoCs, drop these declarations here and in other places.
|
siva-potu please create an upstream PR for this TLMM/pinctrl driver since further review can happen there. After addressing one comment I have above we should be good to merge this in |
There was a problem hiding this comment.
🟡 Changes recommended
GPIO requests do not select GPIO muxing, and invalid pin configurations are silently transformed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Balanced
| EMSG("TLMM: pin %u already owned", pin); | ||
| return TEE_ERROR_BUSY; | ||
| } | ||
| tlmm.pin_owners[pin / 32] |= BIT(pin % 32); |
| if (!pins || !pin_count || !out_conf) | ||
| return TEE_ERROR_BAD_PARAMETERS; | ||
|
|
||
| if (!tlmm.desc) | ||
| return TEE_ERROR_BAD_STATE; |
Add a Qualcomm TLMM GPIO and pinctrl driver implementing the OP-TEE gpio_ops and pinctrl_ops interfaces. Signed-off-by: Siva Rama Krishna Potu <spotu@qti.qualcomm.com>
Enable CFG_QCOM_TLMM for lemans and add the SoC hardware descriptor. Signed-off-by: Siva Rama Krishna Potu <spotu@qti.qualcomm.com>
645e47d to
ed42f52
Compare
Adds a Qualcomm TLMM GPIO and pinctrl driver (non-DT model) and enables it for lemans.
Supersedes #18 — rebased on qcom-next, comments cleaned up across all files.