Repository navigation
media: i2c: vd55g1, ov13858: consistent power/resource ownership on failure and removal - #89
Merged
Merged
Conversation
…ailure and removal Post-merge audit of #73 found five ownership defects; this fixes all five across both drivers: 1. Failed starts now roll back to software standby before the PM reference drops (vd55g1_enable_streams err_rollback ladder; ov13858_enable_streams best-effort stop_streaming), so a sensor whose START reached hardware is never left streaming unowned. Rollback failures are logged alongside the original error. 2. vd55g1 power_on/out preserve and propagate shutdown errors: the regulator unwind keeps the acquisition error as the result and latches a new power_fault flag; power_off returns the regulator error and latches too; PM suspend refuses transitions while faulted so runtime PM never reports suspended against possibly-enabled rails. 3. ov13858's fault latch no longer abandons ownership: power_on's failed bulk enable best-effort disables partially-enabled rails before latching (managed handles released later do not warn about outstanding enables), and a faulted power_off still frees the known-enabled clock before refusing. 4. Both remove paths drop the stream's runtime PM usage reference before the final power off (retained failed stops never issue their put), so unbind/rebind cannot inherit a reference that blocks the next instance's runtime suspend; suspended state is only reflected when power off actually succeeded. 5. vd55g1_remove now unregisters the subdev BEFORE quiescing: no new stream or control operation can start once unpublished, closing the window where a concurrent disable could restart hardware between the stop and state destruction. vd55g1_subdev_cleanup no longer double-unregisters. Built on Surface Pro 11 (arm64 gcc 15.2, 7.3.0-jg-1sp11v4 headers); no new warnings (the ov13858 MODULE_DEVICE_TABLE -Wattributes warnings are pre-existing in this build configuration). Signed-off-by: Leon Silcott <lnsilcott@gmail.com>
…back, brace Review of #89 found the ov13858 half of the changes had been lost to a stale working tree plus a new brace error, and two real vd55g1 gaps: - ov13858: reapply the failed-start rollback to standby (P1), the partial bulk-enable cleanup and faulted power_off clock release (P1), and the remove-path PM reference drop (P1); close the truncated ov13858_remove() (P0 compile error). - vd55g1: track physical resource ownership with rails_on/xclk_on flags. power_off is now ownership-gated and idempotent (a failed runtime suspend that already released the clock is no longer double-released by remove, P2), and remove attempts physical cleanup whenever anything may still be held even if runtime PM reports suspended after a faulted resume unwind (P1). Built on Surface Pro 11 (arm64 gcc 15.2), no new warnings. Signed-off-by: Leon Silcott <lnsilcott@gmail.com>
Round-2 review found the aggregate rails_on flag and bulk-disable cleanup could not safely recover partial regulator rollback: bulk disable against already-released consumers produces unbalanced disables and stops before the still-owned lower-index rails. Track ownership per consumer as a bitmap (rails_on) plus xclk_on: - vd55g1_release_rails()/ov13858_release_rails() release only rails whose bit is set, one regulator at a time, clearing each bit on success; the first failure keeps the remaining bits for a later attempt (remove) and latches power_fault. - power_on fills the bitmap only after bulk enable succeeds (bulk enable already unwinds its own partial failures; the extra bulk-disable cleanup in ov13858 could itself produce unbalanced disables and is removed). - ov13858 power_off is now ownership-gated and idempotent: a faulted device still gets remaining owned resources released exactly once, never double-released, then reports failure. - Both remove paths also attempt cleanup when any ownership bit is still set regardless of PM status. Built on Surface Pro 11 (arm64 gcc 15.2), no new warnings. Signed-off-by: Leon Silcott <lnsilcott@gmail.com>
…rror Round-3 review fixes: - vd55g1 power_on unwind no longer overwrites the acquisition error with the rail-release result: successful cleanup after a failed start now correctly propagates the original failure. - rails_on bitmaps are initialized with GENMASK(n-1, 0) instead of bitmap_fill(), which set the whole unsigned long and left phantom high bits after release; remove's ownership test now reports ownership accurately. - Document the regulator-core accounting limit on both release helpers: consumer use counts drop before late disable stages can fail and bulk-enable rollback errors are ignored, so the bitmap is best driver knowledge, not a core guarantee. Built on Surface Pro 11 (arm64 gcc 15.2), no new warnings. Signed-off-by: Leon Silcott <lnsilcott@gmail.com>
regulator_bulk_enable() rollback ignores rollback-disable errors, so a failed enable can leave an unrecorded enabled rail. ov13858 already latches power_fault on that path; vd55g1 now does the same instead of returning a plain error that a later resume could 'recover' into unbalanced ownership. Built on Surface Pro 11 (arm64 gcc 15.2), no warnings. Signed-off-by: Leon Silcott <lnsilcott@gmail.com>
ooaklee
added a commit
that referenced
this pull request
Nov 14, 2026
…lind disable Review round 4 of #90: the per-supply uncertain disable loop returned success even when a real disable failed (clearing QMP_HOLD_REGS and quarantine, forgetting a stuck supply), and its -EINVAL tolerance was wrong: this tree's _regulator_disable WARNs and returns -EIO on a zero count, while -EINVAL means a notifier vetoed a real disable (ownership retained). No consumer enable-count query exists, so the two cases are indistinguishable from the driver. Take the conservative terminal position instead: refuse the release, keep QMP_HOLD_REGS set and the acquire-failure quarantine latched, and leave the supplies to device teardown — mirroring the documented best-knowledge limitation accepted for the camera drivers in #89. Built on Surface Pro 11 (arm64 gcc 15.2), no warnings. Signed-off-by: Leon Silcott <lnsilcott@gmail.com>
ooaklee
added a commit
that referenced
this pull request
Nov 14, 2026
Eight-agent fleet sweep (2026-11-14). PMIC is SPMI PON logging (no glink gap); battery confirms MSHW0146/SAM dual transport and MAX34417 I2C meters; audio confirms QCOM internal codec; front camera confirms retained-stop and mirrored-unwind power ownership matching #89; UCSI analysis blocked on a Ghidra decompiler-open failure to fix next. Signed-off-by: Leon Silcott <lnsilcott@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Post-merge audit of #73 (camera stop/ownership) found five P1/P2 defects in the failed-start and removal paths of both sensor drivers. This PR fixes all five:
err_rpm_put.power_faultlatch blocks PM transitions against uncertain rails (power_on,power_off, PM suspend/resume).pm_runtime_set_suspendedonly when power truly went down.Test plan