Re: [RFC PATCH 3/8] media: i2c: ds90ub960: Add 10-bit RGBIr formats

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] media: v4l: UAPI macro collision for new RGBIr formats
- [Low] media: i2c: ds90ub960: dead code due to duplicate format entries
--

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

media: i2c: ds90ub960: Add 10-bit RGBIr formats
    
This commit adds 10-bit RGBIr formats to the ds90ub960 media driver. It defines
new 4x4 RGBIr bayer formats by appending them to the supported formats list.

> diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
> index 7d7ba6d349fc4..c86b18e0b4b33 100644
> --- a/drivers/media/i2c/ds90ub960.c
> +++ b/drivers/media/i2c/ds90ub960.c
> @@ -664,6 +664,24 @@ static const struct ub960_format_info ub960_formats[] = {
>  	{ .code = MEDIA_BUS_FMT_SGBRG12_1X12, .bpp = 12, .datatype = MIPI_CSI2_DT_RAW12, },
>  	{ .code = MEDIA_BUS_FMT_SGRBG12_1X12, .bpp = 12, .datatype = MIPI_CSI2_DT_RAW12, },
>  	{ .code = MEDIA_BUS_FMT_SRGGB12_1X12, .bpp = 12, .datatype = MIPI_CSI2_DT_RAW12, },
> +
> +	{ .code	= MEDIA_BUS_FMT_SRGGB8_1X8, .bpp = 8, .datatype = MIPI_CSI2_DT_RAW8, },
> +	{ .code	= MEDIA_BUS_FMT_SGRBG8_1X8, .bpp = 8, .datatype = MIPI_CSI2_DT_RAW8, },
> +	{ .code	= MEDIA_BUS_FMT_SGBRG8_1X8, .bpp = 8, .datatype = MIPI_CSI2_DT_RAW8, },
> +	{ .code	= MEDIA_BUS_FMT_SBGGR8_1X8, .bpp = 8, .datatype = MIPI_CSI2_DT_RAW8, },
> +	{ .code	= MEDIA_BUS_FMT_SRGGB10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SGRBG10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SGBRG10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SBGGR10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },

[Severity: Low]
Are these format entries unreachable?

Since these 8 standard Bayer 8-bit and 10-bit formats are already present
earlier in the ub960_formats array, appending them here duplicates the exact
same format codes.

Because ub960_find_format() returns the first match it finds sequentially,
these appended duplicates will never be reached, introducing dead code.

> +
> +	{ .code	= MEDIA_BUS_FMT_SRGGI10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SGRIG10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SBGGI10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SGBIG10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },

[Severity: High]
Do these new RGBIr format IDs collide with existing UAPI definitions?

Looking at include/uapi/linux/media-bus-format.h, the newly added 10-bit RGBIr
media bus formats are assigned UAPI values 0x3021 through 0x3024:

include/uapi/linux/media-bus-format.h:
    #define MEDIA_BUS_FMT_SRGGI10_1X10      0x3021
    #define MEDIA_BUS_FMT_SGRIG10_1X10      0x3022
    #define MEDIA_BUS_FMT_SBGGI10_1X10      0x3023
    #define MEDIA_BUS_FMT_SGBIG10_1X10      0x3024

These exact hex values are already assigned to the 20-bit Bayer formats
(MEDIA_BUS_FMT_SBGGR20_1X20 through MEDIA_BUS_FMT_SRGGB20_1X20) further up in
the file.

This causes an unresolvable collision in the public userspace API, where the
kernel and userspace will be unable to distinguish between the 10-bit RGBIr
and 20-bit Bayer formats during format identification.

> +	{ .code	= MEDIA_BUS_FMT_SGIRG10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SIGGR10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SGIBG10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
> +	{ .code	= MEDIA_BUS_FMT_SIGGB10_1X10, .bpp = 10, .datatype = MIPI_CSI2_DT_RAW10, },
>  };
>  
>  static const struct ub960_format_info *ub960_find_format(u32 code)

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