Re: [PATCH 3/5] media: nxp: imx8-isi: Add 16-bit raw Bayer format support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Crossbar routing artificially restricts sink_stream IDs to the number of available pipelines, breaking valid CSI-2 Virtual Channel routing.
--

commit 7b27c9929d91d2f4cfef36db9983af8a9df1006d
Author: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>

media: nxp: imx8-isi: Add 16-bit raw Bayer format support
    
Add support for 16-bit raw Bayer formats (SBGGR16, SGBRG16, SGRBG16,
SRGGB16) to both the pipeline subdev and video capture interface.

These formats are commonly used by high-end image sensors that output
16-bit raw data, enabling the ISI to process and capture full dynamic
range from such sensors.

> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
> index 2d0843c86534c..e58925d71164d 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c

[ ... ]

While reviewing this series, I noticed an issue in
drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c. This wasn't
introduced by this specific patch, but earlier in the series by the commits
"media: nxp: imx8-isi: Fix stream ID validation bypass in crossbar routing"
and "media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed
streams".

drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:__mxc_isi_crossbar_set_routing() {
        ...
		if (route->sink_stream >= xbar->num_sources) {
			dev_dbg(xbar->isi->dev,
				"sink_stream %u exceeds hardware limit %u\n",
				route->sink_stream, xbar->num_sources - 1);
			return -ENXIO;
		}
        ...
}

[Severity: High]
Does this validation artificially restrict sink_stream IDs to the number of
available pipelines?  On SoCs with a single pipeline (like i.MX8MN where
num_sources=1), this appears to reject valid CSI-2 Virtual Channel routing
for any stream ID other than 0 (e.g. VC1) with -ENXIO.

Additionally, this logic seems to carry over into the stream tracking arrays:

drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:mxc_isi_crossbar_enable_streams() {
        ...
	/* Increment reference count for all requested streams */
	for (stream = 0; stream < xbar->num_sources; stream++) {
		if (!(sink_streams & BIT(stream)))
			continue;

		input->enabled_count[stream]++;
	}
        ...
}

[Severity: High]
Since this loop is bounded by xbar->num_sources, does it fail to track valid
streams that are >= num_sources?

drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c:mxc_isi_stream_counters_alloc() {
        ...
	for (i = 0; i < xbar->num_sinks; ++i) {
		struct mxc_isi_input *input = &xbar->inputs[i];

		input->enabled_count = kcalloc(xbar->num_sources,
					       sizeof(*input->enabled_count),
					       GFP_KERNEL);
        ...
}

[Severity: High]
Is the enabled_count array incorrectly sized here? It allocates based on
xbar->num_sources instead of the maximum supported stream ID. If validation
were bypassed, would this lead to out-of-bounds tracking for streams
>= num_sources, given that the u64 bitmask implies support for up to 64
streams?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-isi-v1-0-deebfdb1b07b@oss.nxp.com?part=3