drivers: qcom: add TLMM GPIO and pinctrl driver for lemans by siva-potu · Pull Request #23 · qualcomm-linux/optee_os

siva-potu · 2026-07-14T14:01:48Z

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.

Sumit Garg (b49020) · 2026-07-17T12:24:20Z

Rather than calling it a TLMM driver, use the standard pinctrl naming conventions. Follow naming conventions similar to Linux: drivers/pinctrl/qcom/pinctrl*

Sumit Garg (b49020) · 2026-07-17T12:25:33Z

Rather move this file as a SoC specific file in the drivers directory.

Selvam Sathappan (zelvam95) · 2026-07-21T06:39:23Z

+	if (pin >= TLMM_MAX_GPIOS || pin >= tlmm.desc->num_gpios)
+		return TEE_ERROR_BAD_PARAMETERS;
+
+	exceptions = cpu_spin_lock_xsave(&tlmm.lock);


Can we use simple mutex here instead of CPU spin locks which are expensive?

The critical section here is limited to a bitmap update and a register read-modify-write. Given the small scope of the protected region, a mutex may introduce unnecessary overhead, and a spinlock seems more appropriate here.

CPU spin locks in OP-TEE are expensive based on previous discussion with upstream maintainers. Would request to drop to mutex unless there is some real requirement to use spinlock.

sure, will change to mutex

Selvam Sathappan (zelvam95) · 2026-07-21T06:39:41Z

+
+	tlmm_restore_lp(&tlmm, pin);
+
+	exceptions = cpu_spin_lock_xsave(&tlmm.lock);


Same comment; Can we use simple mutex instead of this?

same as above

Responded to above comment.

Selvam Sathappan (zelvam95) · 2026-07-21T06:41:40Z

+	tlmm.desc = desc;
+	tlmm.gpio_chip.ops = &tlmm_gpio_ops;
+
+	DMSG("TLMM: mapped base 0x%"PRIxPA" -> va 0x%"PRIxVA


Same print in 2 different log levels? Can we keep the logging minimal and not overly verbose and only keep logs which will really help during debugging/errors/etc.?

removed the unnecessary logging.

Selvam Sathappan (zelvam95) · 2026-07-21T06:42:27Z

+ */
+
+#ifndef __QCOM_TLMM_PRIVATE_H
+#define __QCOM_TLMM_PRIVATE_H


Can we call tlmm_internal maybe instead of tlmm_private? I believe thats the naming used by other existing drivers/vendors in upstream?

I couldn't find any _internal references under core/drivers/ and we've used _priv in our own existing driver (qcom/qfprom/qfprom_priv.h).

Keeping it as tlmm_private.h. If you'd prefer consistency with our qfprom driver, I can change it to tlmm_priv.h

Makes sense, given for a qcom driver we've already used that naming convention; We can align to that here as well;

Selvam Sathappan (zelvam95) · 2026-07-21T06:52:40Z

+	if (drive_ma <= 8)
+		return 3;
+	if (drive_ma <= 10)
+		return 4;


if (drive_ma <= 2)
return 0;

return MIN(DIV_ROUND_UP(drive_ma - 2, 2), 7U);

Function can be simplified as above?

updated as per the suggestion

Selvam Sathappan (zelvam95) · 2026-07-21T07:02:05Z

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?

Sumit Garg (b49020) · 2026-07-21T13:33:19Z

Rebase to tip of qcom-next

Selvam Sathappan (zelvam95) · 2026-08-02T17:15:24Z

+	       + reg_off;
+}
+
+void tlmm_write_cfg(struct tlmm_chip *chip, unsigned int pin,


🟡 core/drivers/qcom/tlmm/gpio.c:57-70 tlmm_write_cfg() and pinctrl.c:47-74 tlmm_conf_apply() — hardware read-modify-write on the shared per-pin TLMM_GPIO_CFG register is not serialized against concurrent updates on the same pin

tlmm_write_cfg is a plain read-modify-write with no locking:

void tlmm_write_cfg(struct tlmm_chip *chip, unsigned int pin, uint32_t clear_mask, uint32_t set_mask) { vaddr_t reg = TLMM_GPIO_CFG(chip, pin); uint32_t val = io_read32(reg); ... val = (val & ~clear_mask) | set_mask; io_write32(reg, val); }

Callers include:

tlmm_set_direction (gpio.c:147-161) — no lock

tlmm_conf_apply (pinctrl.c:70-74) — drops the spinlock at
pinctrl.c:52 before iterating tlmm_write_cfg

Per-pin registers don't share a 32-bit word (the SoC descriptor sets
pin_reg_width = 0x1000, so each pin has its own dedicated
TLMM_REG_CFG at a page-aligned offset — verified via
tlmm_soc_data.c:15). So updates to different pins can't race at the
hardware level, and there's no cross-pin RMW hazard.

The remaining hazard is same-pin concurrent updates: e.g. a TA/driver
calling gpio_ops.set_direction on pin N while another thread has an
outstanding tlmm_conf_apply for a pin group including N. The
ownership bitmap will reject the second one via
tlmm_pin_is_owned in tlmm_set_direction, but tlmm_pin_is_owned
reads pin_owners without holding the lock (gpio.c:83-89), so
ownership state is racy against a concurrent request_pin / pinctrl
conf_apply in flight.

Suggest: either take chip->lock around the RMW inside
tlmm_write_cfg itself (simplest and most defensive), or document a
"caller must hold chip->lock" contract on tlmm_write_cfg and
ensure tlmm_conf_apply retains the lock across the hardware writes,
not just across the ownership grant.

Selvam Sathappan (zelvam95) · 2026-08-02T17:15:48Z

+
+	clear_mask &= ~TLMM_CFG_PRESERVE_MASK;
+
+	if (chip->desc->has_egpio && (val & TLMM_CFG_EGPIO_PRESENT))


🟡 core/drivers/qcom/tlmm/gpio.c:65-66 tlmm_write_cfg() — TLMM_CFG_EGPIO_ENABLE is unconditionally forced on for any pin where TLMM_CFG_EGPIO_PRESENT is set; this is a behavioural change vs the previous CFG value

if (chip->desc->has_egpio && (val & TLMM_CFG_EGPIO_PRESENT)) set_mask |= TLMM_CFG_EGPIO_ENABLE;

Once this driver runs, any pin the SoC labels as "EGPIO-capable" gets
TLMM_CFG_EGPIO_ENABLE set on the very next tlmm_write_cfg() call —
regardless of what the caller intended. If the pin was previously being
driven in non-EGPIO mode (e.g. a plain SPI SE pin muxed via the FUNC
field), enabling EGPIO mode routes the pin to a different peripheral
group (typically EMAC / secondary MDIO / etc. on Qualcomm SoCs). That's a
non-obvious behavioural change that:

can not be undone by clearing clear_mask because
clear_mask &= ~TLMM_CFG_PRESERVE_MASK explicitly filters HIHYS
only, not EGPIO_ENABLE;

silently overrides whatever the SPI/UART/etc. driver in PR drivers: qcom: add GENI SPI driver and enable spi config #33 or
future users expect by writing FUNC alone.

If the intent is "EGPIO-capable pins default to EGPIO mode when
touched by TLMM," that should be documented and configurable per pin,
not a global "on if PRESENT." If the intent is "preserve whatever
EGPIO_ENABLE bit was already there," this is the wrong pattern — the
existing bit should be included in the clear mask so the caller
controls the final value, or the value should be preserved with a
set |= val & TLMM_CFG_EGPIO_ENABLE pattern.

Suggest: confirm the intended semantics of EGPIO_ENABLE for the
TLMM driver's callers (pinctrl users generally want FUNC-mux, not
EGPIO passthrough). Then either:

Preserve the previous value of the bit (set |= val & TLMM_CFG_EGPIO_ENABLE), or

Expose EGPIO enable/disable via the pinconf group struct so the
caller can choose per group.

Addressed. The EGPIO handling is now an explicit tlmm_claim_egpio_unlocked() helper instead of being folded into the CFG write's set_mask.

EGPIO_PRESENT is read-only and set by hardware for pads shared with an Island TLMM (LPASS/SSC). EGPIO_ENABLE selects ownership: 1 gives the pad to the chip TLMM, while 0 leaves it with the Island domain. Since the bit resets to 0, shared pads start out Island-owned and CFG/IN_OUT programming has no effect until ownership is claimed. For that reason, ownership is claimed unconditionally for EGPIO_PRESENT pins rather than preserving the current value.

Selvam Sathappan (zelvam95) · 2026-08-02T17:16:45Z

+			return TEE_ERROR_BUSY;
+		}
+	}
+	for (i = 0; i < pc->pin_count; i++) {


🟡 core/drivers/qcom/tlmm/pinctrl.c:47-77 tlmm_conf_apply() — sets pc->applied = true before the hardware writes; a subsequent failure would leave the state inconsistent

... cpu_spin_unlock_xrestore(&chip->lock, exceptions); pc->applied = true; /* :54 */ clear = TLMM_CFG_PULL_MASK | ... ... for (i = 0; i < pc->pin_count; i++) { DMSG(...); tlmm_write_cfg(chip, pc->pins[i], clear, set); /* :73 */ }

tlmm_write_cfg currently returns void and has no failure path, so
the flag is functionally accurate today. But if any of the register
writes were changed to be error-returning in the future (e.g. adding a
"probe if register access faulted" recovery, or an io_write32_checked),
the applied = true at line 54 would already commit conf_free to
call tlmm_restore_lp on pins that never had their hardware programmed.
Move pc->applied = true to after the hardware-write loop so the
invariant "applied ↔ hardware programmed" is preserved.

Also, note that pc->applied is written without holding chip->lock,
while conf_free reads it also without holding the lock. Not a data
race today (single writer per conf) but worth being explicit that
applied is protected by the "no concurrent free while apply is in
flight" contract.

Addressed - applied = true now happens inside the lock alongside the ownership claim that it mirrors.

applied tracks ownership being claimed, not hardware programming completion. It is used by conf_free() to determine whether the pins need to be released. Moving it after the write loop could leave the pins claimed but applied == false on an early-return path, resulting in the ownership never being released. restore_lp() on a pin that was never programmed is a no-op unless LP_CFG_APPLIED is set, so there is no downside to setting applied earlier.

Added a comment in the code to clarify this behavior. The write is now performed under the lock as noted; the read in conf_free() remains unlocked.

Selvam Sathappan (zelvam95) · 2026-08-02T17:17:15Z

+	uint32_t exceptions = 0;
+
+	if (pc->applied) {
+		exceptions = cpu_spin_lock_xsave(&chip->lock);


Also, can't we use regular mutex that's avail in OP-TEE here? Do we really need cpu_spin_lock_xsave/unlock_xrestore here?

same as above

Selvam Sathappan (zelvam95) · 2026-08-02T17:17:52Z

+	return TEE_SUCCESS;
+}
+
+void tlmm_release_pin(unsigned int pin)


🟡 core/drivers/qcom/tlmm/gpio.c:123-135 tlmm_release_pin() — calls tlmm_restore_lp() (hardware register writes) before acquiring tlmm.lock

void tlmm_release_pin(unsigned int pin) { ... tlmm_restore_lp(&tlmm, pin); /* :130 */ exceptions = cpu_spin_lock_xsave(&tlmm.lock); tlmm.pin_owners[pin / 32] &= ~BIT(pin % 32); /* :133 */ cpu_spin_unlock_xrestore(&tlmm.lock, exceptions); }

Between tlmm_restore_lp() and the ownership clear, another thread
that observes ownership still set can issue a set_direction /
set_value and its tlmm_write_cfg RMW races against the restore's
own RMW on the same pin's CFG register (item 1 above). If the ordering
"restore hardware → release ownership" is intentional (so callers can
still observe ownership while the register is being restored), the
whole sequence should be inside the lock — or better, the ownership
bit should be cleared first (blocking new callers) and the hardware
restored after.

Symmetric in tlmm_conf_free (pinctrl.c:79-98): the LP restore
happens inside the lock but the free(conf) happens outside — fine.

Fixed - the ownership check, LP restore and ownership clear are now all inside tlmm.lock. set_direction/set_value also take the lock now, so both sides of the CFG RMW are serialised.

Selvam Sathappan (zelvam95) · 2026-08-02T17:18:58Z

+
+static const struct gpio_ops tlmm_gpio_ops;
+
+uint32_t tlmm_tile_offset(const struct tlmm_chip *chip, unsigned int pin)


🔵 core/drivers/qcom/tlmm/gpio.c:26-46 tlmm_tile_offset() — every register access probes all tiles at O(num_tiles) cost; cache the per-pin tile assignment at init

for (t = 0; t < d->num_tiles; t++) { id_reg = chip->base + d->tile_offsets[t] + (vaddr_t)d->pin_reg_width * pin + TLMM_REG_ID_STATUS; if (io_read32(id_reg) & TLMM_ID_STATUS_PRESENT) return d->tile_offsets[t]; }

tlmm_tile_offset is called from tlmm_pin_reg (line 48-55), which
in turn is called by every TLMM_GPIO_CFG / TLMM_GPIO_IN_OUT /
TLMM_GPIO_LP_CFG macro. On a multi-tile SoC (e.g. up to 5 tiles per
TLMM_MAX_TILES), every GPIO op does up to 5 register reads before
the actual read/write. set_direction alone is: (5 probes + 1 RMW-read

1 write). At bring-up speed this doesn't matter, but for a driver
that a real SPI or UART may hit thousands of times per second per pin,
this adds up.

Lemans as configured has num_tiles = 1 so the early return at
gpio.c:32-33 short-circuits — no impact today. But future multi-tile
SoCs will pay for every access. Suggest: cache the tile index per
pin at driver init (small array indexed by pin, filled by probing each
TLMM_REG_ID_STATUS once).

This is an intentional design choice.

The tile probe reads GPIO_ID_STATUS from the pin's register page. TLMM pin registers are XPU-protected at per-pin granularity (pin_reg_width), and not every pin is necessarily accessible from the secure side. Pins may be assigned to HYP or another domain, and probing them can trigger an XPU violation.

I hit exactly this issue in the boot-image TLMM driver, where an eager tile-index cache populated during initialization faulted in UEFI because some pins were HYP-owned and not readable from TZ. As a result, I intentionally avoided an init-time probe of all pins here as well.

With the current approach, we only probe pins that a consumer actually requests, avoiding accesses to unrelated XPU-protected pin pages.

Selvam Sathappan (zelvam95) · 2026-08-02T17:20:41Z

+	return TEE_SUCCESS;
+}
+
+void tlmm_free_pin_state(struct pinctrl_state *state)


tlmm_free_pin_state() and tlmm_apply_pin_state() — no NULL check on the state pointer

Both dereference state->confs[i] / state->conf_count
unconditionally. A caller with a stashed pointer that failed to be
built (tlmm_make_pin_state on error returns via return res but sets
*out_state only on success — good, so callers with a properly-checked
return value don't hit this) — but defensive if (!state) return; is
cheap and prevents future misuse.

Addressed:
tlmm_apply_pin_state() returns TEE_ERROR_BAD_PARAMETERS on NULL and tlmm_free_pin_state() returns early

Selvam Sathappan (zelvam95) · 2026-08-02T17:22:42Z

 CFG_DRIVERS_CLK ?= y
 CFG_DRIVERS_QCOM_CLK ?= y

+CFG_QCOM_TLMM ?= y


— CFG_QCOM_TLMM ?= y places the enable in target.mk, but the source list for tlmm_soc_data.c is added in core/arch/arm/plat-qcom/sub.mk:10, not per-target

The registration is:

# plat-qcom/sub.mk srcs-$(CFG_QCOM_TLMM) += $(QCOM_ARCH_FAMILY)/$(PLATFORM_FLAVOR)/tlmm_soc_data.c

This assumes every target that enables CFG_QCOM_TLMM has a
tlmm_soc_data.c at that path. If a future target enables TLMM
without providing the SoC descriptor, the build fails with an obscure
"file not found" from make. Suggest: either move the srcs-y line
into the target's own sub.mk (per-target opt-in), or add a $(call force,CFG_QCOM_TLMM,n,...) when the required file is absent, or a
$(if $(wildcard ...),..., $(error ...)) guard with a clearer message.

moved the 'srcs-y' registration into core/drivers/qcom/tlmm/sub.mk

Selvam Sathappan (zelvam95) · 2026-08-02T17:23:08Z

+	.size            = TLMM_BASE_SIZE,
+	.pin_reg_width   = 0x1000,
+	.num_tiles       = 1,
+	.tile_offsets    = { 0x100000, 0, 0, 0, 0 },


Lemans is declared with num_tiles = 1 and tile_offsets = { 0x100000, 0, 0, 0, 0 }; the trailing zeros are non-load-bearing but noisy

The tlmm_tile_offset code returns tile_offsets[0] for num_tiles <= 1, so entries [1..4] are never touched. Zeroing them for
readability is fine — but consider using a designated initializer like
.tile_offsets = { [0] = 0x100000 } and let the array's implicit
zero-init do the rest.

Copilot

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.

+		if (pins[i] >= tlmm.desc->num_gpios) {
+			EMSG("TLMM: pin %u out of range (num_gpios %u)",
+			     pins[i], tlmm.desc->num_gpios);
+			return TEE_ERROR_BAD_PARAMETERS;
+		}


+	uint32_t exceptions = 0;
+
+	if (pin >= TLMM_MAX_GPIOS || pin >= tlmm.desc->num_gpios)
+		return;
+
+	tlmm_restore_lp(&tlmm, pin);
+
+	exceptions = cpu_spin_lock_xsave(&tlmm.lock);
+	tlmm.pin_owners[pin / 32] &= ~BIT(pin % 32);
+	cpu_spin_unlock_xrestore(&tlmm.lock, exceptions);


+	const struct tlmm_desc *desc = &tlmm_soc_desc;
+
+	DMSG("TLMM: init start, mapping base 0x%"PRIxPA" size 0x%zx",
+	     desc->base, desc->size);


+TEE_Result tlmm_make_pin_state(const struct tlmm_pin_group *groups,
+			       unsigned int group_count,
+			       struct pinctrl_state **out_state);
+TEE_Result tlmm_apply_pin_state(struct pinctrl_state *state);
+void tlmm_free_pin_state(struct pinctrl_state *state);


Copilot

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_MASK unchanged. 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);

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>

siva-potu mentioned this pull request Jul 14, 2026

tlmm: qcom: add gpio and pinctrl driver #18

Closed

siva-potu force-pushed the qcom-tlmm-gpio-pinctrl branch 3 times, most recently from 4a0e7d9 to 1d46e73 Compare July 15, 2026 05:22

nnunna94 requested review from Sumit Garg (b49020) and Jorge A. Ramirez-Ortiz (ldts) July 15, 2026 08:51

Sumit Garg (b49020) reviewed Jul 17, 2026

View reviewed changes

Selvam Sathappan (zelvam95) reviewed Jul 21, 2026

View reviewed changes

Sumit Garg (b49020) force-pushed the qcom-next branch from 690842e to a4d253d Compare July 21, 2026 13:26

siva-potu force-pushed the qcom-tlmm-gpio-pinctrl branch from 1d46e73 to 24bd49f Compare July 24, 2026 16:44

Selvam Sathappan (zelvam95) reviewed Aug 2, 2026

View reviewed changes

Sumit Garg (b49020) requested a review from Copilot August 3, 2026 04:49

Copilot started reviewing on behalf of Sumit Garg (b49020) August 3, 2026 04:49 View session

Copilot AI reviewed Aug 3, 2026

View reviewed changes

siva-potu force-pushed the qcom-tlmm-gpio-pinctrl branch from 24bd49f to 491c940 Compare August 3, 2026 05:34

siva-potu force-pushed the qcom-tlmm-gpio-pinctrl branch from 491c940 to edafb8e Compare August 6, 2026 10:27

Jorge A. Ramirez-Ortiz (ldts) requested a balanced review from Copilot August 10, 2026 10:56

Copilot started reviewing on behalf of Jorge A. Ramirez-Ortiz (ldts) August 10, 2026 10:56 View session

Copilot AI reviewed Aug 10, 2026

View reviewed changes

siva-potu added 2 commits August 13, 2026 14:05

drivers: qcom: add TLMM GPIO and pinctrl driver

5ea5e74

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>

drivers: qcom: tlmm: enable TLMM driver for lemans

f111d3e

Enable CFG_QCOM_TLMM for lemans and add the SoC hardware descriptor. Signed-off-by: Siva Rama Krishna Potu <spotu@qti.qualcomm.com>

siva-potu force-pushed the qcom-tlmm-gpio-pinctrl branch from edafb8e to f111d3e Compare August 13, 2026 08:37


		tlmm_restore_lp(&tlmm, pin);

		exceptions = cpu_spin_lock_xsave(&tlmm.lock);


		clear_mask &= ~TLMM_CFG_PRESERVE_MASK;

		if (chip->desc->has_egpio && (val & TLMM_CFG_EGPIO_PRESENT))


		static const struct gpio_ops tlmm_gpio_ops;

		uint32_t tlmm_tile_offset(const struct tlmm_chip *chip, unsigned int pin)

Uh oh!

Conversation

siva-potu commented Jul 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Selvam Sathappan (zelvam95) commented Jul 21, 2026

Uh oh!

Sumit Garg (b49020) commented Jul 21, 2026

Uh oh!

Choose a reason for hiding this comment

🟡 core/drivers/qcom/tlmm/gpio.c:57-70 tlmm_write_cfg() and pinctrl.c:47-74 tlmm_conf_apply() — hardware read-modify-write on the shared per-pin TLMM_GPIO_CFG register is not serialized against concurrent updates on the same pin

Uh oh!

Choose a reason for hiding this comment

🟡 core/drivers/qcom/tlmm/gpio.c:65-66 tlmm_write_cfg() — TLMM_CFG_EGPIO_ENABLE is unconditionally forced on for any pin where TLMM_CFG_EGPIO_PRESENT is set; this is a behavioural change vs the previous CFG value

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

🟡 core/drivers/qcom/tlmm/pinctrl.c:47-77 tlmm_conf_apply() — sets pc->applied = true before the hardware writes; a subsequent failure would leave the state inconsistent

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

🟡 core/drivers/qcom/tlmm/gpio.c:123-135 tlmm_release_pin() — calls tlmm_restore_lp() (hardware register writes) before acquiring tlmm.lock

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

🔵 core/drivers/qcom/tlmm/gpio.c:26-46 tlmm_tile_offset() — every register access probes all tiles at O(num_tiles) cost; cache the per-pin tile assignment at init

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

🟡 `core/drivers/qcom/tlmm/gpio.c:57-70` `tlmm_write_cfg()` and `pinctrl.c:47-74` `tlmm_conf_apply()` — hardware read-modify-write on the shared per-pin `TLMM_GPIO_CFG` register is not serialized against concurrent updates on the same pin

🟡 `core/drivers/qcom/tlmm/gpio.c:65-66` `tlmm_write_cfg()` — `TLMM_CFG_EGPIO_ENABLE` is unconditionally forced on for any pin where `TLMM_CFG_EGPIO_PRESENT` is set; this is a behavioural change vs the previous CFG value

🟡 `core/drivers/qcom/tlmm/pinctrl.c:47-77` `tlmm_conf_apply()` — sets `pc->applied = true` before the hardware writes; a subsequent failure would leave the state inconsistent

🟡 `core/drivers/qcom/tlmm/gpio.c:123-135` `tlmm_release_pin()` — calls `tlmm_restore_lp()` (hardware register writes) before acquiring `tlmm.lock`

🔵 `core/drivers/qcom/tlmm/gpio.c:26-46` `tlmm_tile_offset()` — every register access probes all tiles at O(num_tiles) cost; cache the per-pin tile assignment at init