Re: [PATCH v13 11/17] media: cadence: csi2rx: add multistream support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The hardware reference counter `csi2rx->count` counts API calls instead of enabled streams, leading to premature hardware shutdown or resource leaks.
--

commit dc92c5b7cb43fb07195719c19a138fea74234e1c
Author: Jai Luthra <jai.luthra@linux.dev>

media: cadence: csi2rx: add multistream support

This commit adds multi-stream support to the Cadence CSI-2 bridge IP,
allowing for 1-to-1 mapping of virtual channels from sink to source pads.

> diff --git a/drivers/media/platform/cadence/cdns-csi2rx.c b/drivers/media/platform/cadence/cdns-csi2rx.c
> index bba0e4c0a34da..8931e824c597c 100644
> --- a/drivers/media/platform/cadence/cdns-csi2rx.c
> +++ b/drivers/media/platform/cadence/cdns-csi2rx.c
[ ... ]
> @@ -471,18 +499,57 @@ static int csi2rx_log_status(struct v4l2_subdev *sd)
[ ... ]
>  static int csi2rx_enable_streams(struct v4l2_subdev *subdev,
>  				 struct v4l2_subdev_state *state, u32 pad,
>  				 u64 streams_mask)
>  {
>  	struct csi2rx_priv *csi2rx = v4l2_subdev_to_csi2rx(subdev);
> +	u64 sink_streams;
>  	int ret;
>  
> +	sink_streams = v4l2_subdev_state_xlate_streams(state, pad,
> +						       CSI2RX_PAD_SINK,
> +						       &streams_mask);
> +
>  	/*
>  	 * If we're not the first users, there's no need to
>  	 * enable the whole controller.
>  	 */
>  	if (!csi2rx->count) {
> +		csi2rx_update_vc_select(csi2rx, state);
>  		ret = csi2rx_start(csi2rx);
>  		if (ret)
>  			return ret;
> @@ -490,10 +557,11 @@ static int csi2rx_enable_streams(struct v4l2_subdev *subdev,
>  
>  	/* Start streaming on the source */
>  	ret = v4l2_subdev_enable_streams(csi2rx->source_subdev, csi2rx->source_pad,
> -					 BIT_U64(0));
> +					 sink_streams);

[Severity: High]
Since the V4L2 streams API allows enabling or disabling multiple streams
simultaneously using the streams_mask bitmask, does this create a tracking
issue for the hardware reference counter?

If a caller enables multiple streams in a single call (for example,
streams_mask = BIT(0) | BIT(1)), csi2rx->count will only increment by 1
because it counts API calls rather than the actual number of enabled streams.

If the caller subsequently disables those streams sequentially in separate
calls (for example, disable_streams(BIT(0)) followed by disable_streams(BIT(1))),
the first call will decrement csi2rx->count to 0.

Would this trigger csi2rx_stop(csi2rx) and shut down the entire CSI-2
receiver hardware prematurely, while the second stream is still actively
streaming?

Could the driver track the actual number of enabled streams using
hweight64() on the active streams mask instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520120022.539913-1-r-donadkar@ti.com?part=11