Repository navigation
media: i2c: retain ownership on failed stops and quiesce on remove - #82
Merged
ooaklee merged 4 commits intoNov 13, 2026
Merged
Conversation
ov13858 discarded the standby-write error in its stop path: the sensor could be powered down mid-stream while the caller was told the stop succeeded. Capture the error, keep the runtime-PM reference on failure, and opt into V4L2_SUBDEV_FL_STOP_RETRY so the framework preserves the enabled state and retries. vd55g1 returned -EINVAL from the failed-start path, discarding the real transport error; return it instead. Its remove path unregistered the subdev and cut power without stopping a live stream: quiesce a still- streaming sensor to software standby first (saving the exposure cluster for AE coldstart), mirroring disable_streams without the PM put.
V4L2_SUBDEV_FL_STOP_RETRY is only honored by call_s_stream() when .s_stream is v4l2_subdev_s_stream_helper. With the legacy .s_stream callback the flag is dead: a failed stop is suppressed and the driver keeps its PM runtime reference, leaving the usage counter unbalanced. Convert the driver to the streams API so the helper is used and the failed-stop error, together with the retained PM reference, is propagated: - replace .open() with an .init_state() internal op that initializes the pad-0 format to the highest-resolution default mode - add .enable_streams()/.disable_streams() pad ops carrying over the power-fault check, PM get/put, start/stop logic and the failed-stop error return with retained PM reference - set .s_stream to v4l2_subdev_s_stream_helper - call v4l2_subdev_init_finalize() in probe and v4l2_subdev_cleanup() in the probe error paths and remove() No behavior change on the success paths.
v4l2_subdev_is_streaming() must be called with the active-state lock held, and the quiesce sequence it guards can race with a concurrent stream or control operation. Take the active-state lock before the check and release it only after the sensor has been quiesced, before vd55g1_subdev_cleanup() frees the state. No behavior change on the success paths.
Owner
Author
|
Addressed pre-merge review: (1) ov13858 converted to the streams API — |
With the conversion to the streams API the subdev core holds sd->state_lock (ov13858->mutex) while calling the pad ops, stream ops and init_state. Taking the mutex again in get_fmt, set_fmt and the enable/disable_streams callbacks self-deadlocks; remove the redundant locking. Also unwind a failed pm_runtime_set_active() through error_subdev_cleanup so v4l2_subdev_cleanup() releases the allocated active state.
Owner
Author
|
Third round: removed the self-deadlocking mutex takes from the state-locked callbacks (get_fmt/set_fmt/enable_streams/disable_streams — the core holds sd->state_lock, which is ov13858->mutex) and fixed the pm_runtime_set_active failure path to unwind through error_subdev_cleanup (active-state leak). checkpatch --strict clean. |
4 tasks
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
Follow-up to #73 (merged as c6836ff): addresses the known residual blocker "failed-start and removal paths still need consistent power/resource ownership and recovery."
ov13858
ov13858_stop_streaming()error: the sensor could be powered down mid-stream while the caller was told stop succeeded. Now: capture the error, keep the runtime-PM reference on failure, and return the error.V4L2_SUBDEV_FL_STOP_RETRY(same pattern as vd55g1 in this tree) so the framework preserves the enabled state and retries the stop.vd55g1
enable_streamsfailed-start path returned-EINVAL, discarding the real transport/poll error — return the original error for diagnosis.remove()unregistered the subdev and cut power without stopping a live stream (unregister does not stop streaming). Now: if still streaming, quiesce to software standby first (saving the exposure cluster for AE coldstart), mirroringdisable_streamswithout the PM put.Windows reference: the Qualcomm CamZ sensor driver (camfrontsensor8380.sys) archives StreamOff as a tracked operation before per-resource power teardown — same stop-before-power-off ordering.
Cross-reviewed by GLM-5.3 and Codex GPT-6.1 peer passes (findings #1/#6/#7 of the ownership audit).
Validation