Skip to content

soc: qcom: pmic_glink: add client-state generation API - #83

Merged
ooaklee merged 1 commit into
sp11/ubuntu-qcom-x1e-7.3.yfrom
x/pr72-pmic-glink-generation
Nov 14, 2026
Merged

ooaklee merged 1 commit into
sp11/ubuntu-qcom-x1e-7.3.yfrom
x/pr72-pmic-glink-generation

Conversation

@ooaklee

@ooaklee ooaklee commented Nov 13, 2026

Copy link
Copy Markdown
Owner

Summary

First bounded extraction from the #72 recovery overlay: the PMIC GLINK core/header generation API, called out in #72 as "a useful next bounded foundation".

Problem being solved: clients (ucsi_glink, altmode) observe client state in a receive/state callback, build a request from that state, then send. If the service restarts in between, the request is built on stale state and the reply (or its absence) is misattributed.

Design (ported from overlay head 507356d, adapted to the current base):

  • struct pmic_glink gains atomic64_t state_generation, incremented inside pmic_glink_state_notify_clients() (under state_lock + client_lock) on every actual client-state transition.
  • pmic_glink_get_generation() — snapshot from callbacks.
  • pmic_glink_send_generation() — under state_lock: rejects -ECONNRESET if the endpoint is gone, the service is not UP, or the generation changed since sampling; otherwise sends with the existing retry loop.
  • lockdep_assert_held(&pg->state_lock) added to the notify helper.

Not ported from the overlay: its probe-time register_rpmsg_driver() restructure — the current base already carries the equivalent as upstream commit 1452aab ("hack: avoid platform/rpmsg probe race"), making that hunk a no-op/conflict.

pmic_glink_send() is byte-identical (loop duplicated deliberately to minimise review surface).

Validation

  • checkpatch --strict clean.
  • AArch64 .o build attempted on target hardware; blocked (ngrok tunnel dropped mid-session). Requesting CI/hermetic build validation.

Relation to #72

This does not merge #72; it lands its first bounded piece. Remaining after this: altmode state-machine review, the eight DP files, QMP reconciliation.

Split the first bounded foundation out of the #72 recovery overlay.
Clients frequently observe client state in a receive or state callback
and then send a request built from that state. If the service restarts
in between, the request is built on stale state and the reply (or its
absence) is misattributed.

Add an opaque generation token that increments on every client-state
transition. pmic_glink_get_generation() snapshots it;
pmic_glink_send_generation() re-checks it together with endpoint
availability and the UP state under state_lock, so a send cannot slip
between a transition and its notifications.

The send loop is deliberately duplicated rather than refactored into a
helper: keeping pmic_glink_send() byte-identical minimises the review
surface of this extraction from the historical overlay.
ooaklee added a commit that referenced this pull request Nov 13, 2026
Second bounded extraction from the #72 recovery overlay, building on the
pmic_glink generation API (#83).

The historical driver kept per-port firmware state in struct fields
written by receive callbacks and read by workers with no consistency
between them, waited for unattributable PAN ACKs, and published HPD on
work scheduled from any state:

- Replace per-port state fields with a bounded kfifo of snapshot events
  (PMIC_GLINK_MAX_EVENTS=64); a full queue is reported and dropped, never
  overwritten, so firmware cannot drive unbounded memory use.
- Tag every queued event and request with the pmic_glink generation;
stale events are discarded and in-flight requests rejected across a
service restart (pmic_glink_altmode_current()).
- Split locking: event_lock (spinlock, queues/state only, never held
over providers or waits) and lifecycle_lock (mutex, serialises
provider/HPD application and reconciliation, never ACK waits). The old
single-lock request path serialized unrelated ports behind ACK waits.
- pdr_notify reconciles a new generation under event_lock: queues are
reset, waiters are woken for cancellation (not as ACKs), and
enable_worker re-registers PAN_EN once for the new generation.
- HPD bridge notifications now track last-published state so a stale
generation cannot leave a connector stuck connected; a service
transition forces disconnected before reopening the barrier.
- SC8180X legacy notifications no longer infer SC8280 mux state
(has_mux_ctrl=false) and are only used for orientation/SVID/HPD.

Adaptations vs overlay head 507356d: keep the base's corrected
usbc_sc8280x_tbt_data size (res[7]) and usbc_notify trailing reserved
word (upstream b255c6f), and keep drm_aux_hpd_bridge_notify_extra /
system_freezable_wq usage already in this base.
@ooaklee

ooaklee commented Nov 14, 2026

Copy link
Copy Markdown
Owner Author

Build verification (Surface Pro 11 native, arm64)

Both objects compile clean on the target with the shipped kernel source (7.3.0-jg-1sp11v4) + /boot config:

$ make ARCH=arm64 olddefconfig   # OK
$ make ARCH=arm64 -j12 drivers/soc/qcom/pmic_glink.o drivers/soc/qcom/pmic_glink_altmode.o
  CC      drivers/soc/qcom/pmic_glink.o
  CC      drivers/soc/qcom/pmic_glink_altmode.o   # rc=0, 0 warnings

Rebuilt with KCFLAGS="-Wall -Wextra": 0 warnings, 0 errors. (Note: this branch alone also builds since the altmode consumer of the new symbols is on the follow-up branch; the two were verified stacked, as intended for merge order.)

@ooaklee
ooaklee marked this pull request as ready for review November 14, 2026 05:42
@ooaklee
ooaklee merged commit 9551441 into sp11/ubuntu-qcom-x1e-7.3.y Nov 14, 2026
ooaklee added a commit that referenced this pull request Nov 14, 2026
Second bounded extraction from the #72 recovery overlay, building on the
pmic_glink generation API (#83).

The historical driver kept per-port firmware state in struct fields
written by receive callbacks and read by workers with no consistency
between them, waited for unattributable PAN ACKs, and published HPD on
work scheduled from any state:

- Replace per-port state fields with a bounded kfifo of snapshot events
  (PMIC_GLINK_MAX_EVENTS=64); a full queue is reported and dropped, never
  overwritten, so firmware cannot drive unbounded memory use.
- Tag every queued event and request with the pmic_glink generation;
stale events are discarded and in-flight requests rejected across a
service restart (pmic_glink_altmode_current()).
- Split locking: event_lock (spinlock, queues/state only, never held
over providers or waits) and lifecycle_lock (mutex, serialises
provider/HPD application and reconciliation, never ACK waits). The old
single-lock request path serialized unrelated ports behind ACK waits.
- pdr_notify reconciles a new generation under event_lock: queues are
reset, waiters are woken for cancellation (not as ACKs), and
enable_worker re-registers PAN_EN once for the new generation.
- HPD bridge notifications now track last-published state so a stale
generation cannot leave a connector stuck connected; a service
transition forces disconnected before reopening the barrier.
- SC8180X legacy notifications no longer infer SC8280 mux state
(has_mux_ctrl=false) and are only used for orientation/SVID/HPD.

Adaptations vs overlay head 507356d: keep the base's corrected
usbc_sc8280x_tbt_data size (res[7]) and usbc_notify trailing reserved
word (upstream b255c6f), and keep drm_aux_hpd_bridge_notify_extra /
system_freezable_wq usage already in this base.
ooaklee added a commit that referenced this pull request Nov 14, 2026
…blish final disconnect

Address peer-review findings on the generation-based event lifecycle:

1. Late-ACK fencing: WRITE_REQ acks carry no request identity, so after
   a timed-out request the next one could be satisfied by the stale ACK.
   Track ack_outstanding under event_lock; a new request refuses with
   -EAGAIN until the previous ACK drains (or a generation change resets
   the state, since old ACKs can no longer be attributed either way).

2. FIFO overflow recovery: a rejected event may be the final disconnect,
   leaving published HPD high indefinitely. On overflow, mark the
   generation unreconciled and schedule reconciliation, which re-derives
   connector state instead of replaying a lost event.

3. stop() publishes the final HPD disconnect: queued disconnects can be
   discarded during drain; after workers are cancelled and while the
   bridges are still alive, publish disconnect for every port still
   reporting hpd_connected.

Also rebased onto the merged generation API (#83) so the stack ancestry
is correct for merge.

Built on Surface Pro 11 (arm64 gcc 15.2, 7.3.0-jg-1sp11v4 headers),
zero warnings.

Signed-off-by: Leon Silcott <lnsilcott@gmail.com>
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.

1 participant