Re: [PATCH 2/6] media: qcom: camss: csid: Add support for CSID 900

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: qcom: camss: Data race on aup_update and rup_update masks
--

commit d62dff88807e9c3cde57315a677507f8c380eeee
Author: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>

media: qcom: camss: csid: Add support for CSID 900

Add a CSID driver for the "900" revision on the Nord SoC. It shares the
CSI-2 RX helpers and the split RUP/AUP command model of the gen3/980
lineage, but uses a single register map for both the full and lite
instances, so add a separate file rather than a variant of an existing
CSID.

> diff --git a/drivers/media/platform/qcom/camss/camss-csid-900.c b/drivers/media/platform/qcom/camss/camss-csid-900.c
> new file mode 100644
> index 0000000000000..921ddb7aca37f
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss/camss-csid-900.c
[ ... ]
> +static void __csid_aup_update(struct csid_device *csid, int port_id)
> +{
> +	csid->aup_update |= CSID_RUP_AUP_RDI(port_id);
> +	writel(csid->aup_update, csid->base + CSID_AUP_CMD);
> +
> +	__csid_aup_rup_trigger(csid);
> +}
> +
> +static void __csid_rup_update(struct csid_device *csid, int port_id)
> +{
> +	csid->rup_update |= CSID_RUP_AUP_RDI(port_id);
> +	writel(csid->rup_update, csid->base + CSID_RUP_CMD);
> +
> +	__csid_aup_rup_trigger(csid);
> +}

[Severity: High]
Is there a potential data race on the aup_update and rup_update masks here?

These variables are modified using non-atomic read-modify-write operations
in process context during stream start and buffer queueing, without holding
a lock or disabling interrupts.

> +
> +static void __csid_aup_rup_clear(struct csid_device *csid, int port_id)
> +{
> +	/* Hardware clears the registers upon consuming the settings */
> +	csid->aup_update &= ~CSID_RUP_AUP_RDI(port_id);
> +	csid->rup_update &= ~CSID_RUP_AUP_RDI(port_id);
> +}

[Severity: High]
Can these modifications concurrently race against the updates in
__csid_aup_update() and __csid_rup_update()?

__csid_aup_rup_clear() is called from hardirq context via csid_isr() when
clearing the INFO_RUP_DONE bit, which also does not appear to hold a lock.

If process context (such as a userspace ioctl queuing new buffers) races
against the hardware interrupt continuously generating buffer-done interrupts
during streaming, could the driver state become corrupted? This might lead to
lost or spurious hardware register updates, which could hang the V4L2
streaming pipeline or cause dropped buffers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915-nord-v1-0-20db41a7f1ff@oss.qualcomm.com?part=2