Re: [PATCH v5 3/3] arm64: dts: qcom: sm8250: Add JPEG encoder node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition in stream teardown can disable interrupts for active streams and cause hardware deadlocks.
--

--- Patch [3]: [PATCH v5 3/3] arm64: dts: qcom: sm8250: Add JPEG encoder node ---
commit 37f9191db4e80eea5cca3dd9978def555946b6b5
Author: Atanas Filipov <atanas.filipov@oss.qualcomm.com>

arm64: dts: qcom: sm8250: Add JPEG encoder node

Add the JPEG encoder hardware node to the SM8250 device tree so the
qcom-jpeg V4L2 encoder driver can bind and operate on this platform.

> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index 7076720413ab2..98935f3fcd05a 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
[ ... ]
> @@ -4469,6 +4470,79 @@ cci1_i2c1: i2c-bus@1 {
>  			};
>  		};
>  
> +		jpeg-encoder@ac53000 {

[Severity: High]
Does enabling this device expose a race condition in the driver's stream
teardown sequence?

Looking at bop_jpeg_vb2_stop_streaming() in
drivers/media/platform/qcom/jpeg/qcom_jenc_v4l2.c, the lock is dropped
before calling hw_release():

	mutex_unlock(&jenc->dev_mutex);

	/*
	 * hw_release() calls jpeg_deinit() which issues HW_STOP and waits for
	 * the STOP_ACK interrupt completion.  It must be called outside
	 * dev_mutex to avoid a deadlock: the threaded IRQ handler acquires
	 * dev_mutex before signalling the completion.
	 */
	if (do_release)
		hw->hw_release(ectx, q);

Since dev_mutex is explicitly dropped here, could a concurrent stream
acquisition via start_streaming() acquire dev_mutex and see the lockless
ref_count transition from 0 to 1, thus initializing the hardware and
enabling the IRQ?

If the preempted stop_streaming() path then resumes and executes
op_jpeg_release() in drivers/media/platform/qcom/jpeg/qcom_jenc_ops.c:

	ref = atomic_dec_if_positive(&jenc->ref_count);
	if (ref < 0) {
		WARN_ON_ONCE(1);
		return 0;
	}

	if (!ref) {
		rc = jpeg_deinit(jenc);
		// ...
		disable_irq(jenc->irq);

could this destroy the hardware state and disable interrupts for the newly
active stream, potentially leading to deadlocks or panics from unclocked
register access?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728082548.2511694-1-atanas.filipov@oss.qualcomm.com?part=3