Re: [PATCH v5 16/16] media: sony-starvis2: Add support for IMX585

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] media: sony-starvis2: starvis2_write_common() writes 0x4e3c on IMX585
- [Medium] media: sony-starvis2: starvis2_identify_model() reads 0x4d18 on IMX585
--

commit 760bcd8df1ec1dd08aab2a18c904b0c265aacbd2
Author: Dave Stevenson <dave.stevenson@raspberrypi.com>

media: sony-starvis2: Add support for IMX585

This commit adds register sequences, variant definitions, and device tree
match entries to support the Sony IMX585 8MPix Starvis 2 sensor.

> diff --git a/drivers/media/i2c/sony-starvis2.c b/drivers/media/i2c/sony-starvis2.c
> index b9ade0887b6bc..e89aa1a7d5eb8 100644
> --- a/drivers/media/i2c/sony-starvis2.c
> +++ b/drivers/media/i2c/sony-starvis2.c
> @@ -1090,6 +1092,286 @@ static const struct starvis2_variant imx832_variant_def = {
>  	.shr_min = 4,
>  };
>  
> +static const struct cci_reg_sequence imx585_common_regs[] = {

[ ... ]

> +	{ CCI_REG8(0x4d12), 0x1f },
> +	{ CCI_REG8(0x4d13), 0x1e },
> +	{ CCI_REG8(0x4d26), 0x33 },
> +	{ CCI_REG8(0x4e0e), 0x59 },
> +	{ CCI_REG8(0x4e14), 0x55 },
> +	{ CCI_REG8(0x4e16), 0x59 },
> +	{ CCI_REG8(0x4e1e), 0x3b },
> +	{ CCI_REG8(0x4e20), 0x47 },
> +	{ CCI_REG8(0x4e22), 0x54 },
> +	{ CCI_REG8(0x4e26), 0x81 },
> +	{ CCI_REG8(0x4e2c), 0x7d },
> +	{ CCI_REG8(0x4e2e), 0x81 },
> +	{ CCI_REG8(0x4e36), 0x63 },
> +	{ CCI_REG8(0x4e38), 0x6f },
> +	{ CCI_REG8(0x4e3a), 0x7c },

[ ... ]

> +	{ CCI_REG8(0x5222), 0x91 },
> +	{ CCI_REG8(0x5224), 0x87 },
> +	{ CCI_REG8(0x5226), 0x82 },
> +	{ CCI_REG8(0x5b3c), 0x07 },
> +
> +	{ STARVIS2_REG_WDMODE, 0x00 },
> +	{ STARVIS2_REG_MDBIT, 0x01 },
> +	{ STARVIS2_REG_XXS_DRV, 0x00 },
> +};

[Severity: Medium]
On IMX678, register 0x4e3c (STARVIS2_REG_INTERFACE_SEL) selects the 2-lane or
4-lane CSI-2 interface (0x07). On IMX585, the register banks above 0x34xx
extend through 0x4dxx..0x52xx and the identification and interface selector
blocks are shifted by +0x0d00, so imx585_common_regs programs the interface
select register at 0x5b3c (0x4e3c + 0x0d00) while 0x4e0e..0x4e3a are internal
timing registers.

However, when starvis2_power_on() or starvis2_enable_streams() calls
starvis2_write_common(), it still unconditionally writes to
STARVIS2_REG_INTERFACE_SEL (0x4e3c) after writing common_regs:

drivers/media/i2c/sony-starvis2.c:starvis2_write_common() {
	...
	cci_multi_reg_write(starvis2->cci, starvis2->variant->common_regs,
			    starvis2->variant->num_common_regs, &ret);
	...
	cci_write(starvis2->cci, STARVIS2_REG_INTERFACE_SEL,
		  STARVIS2_INTERFACE_2L_4L, &ret);
	...
}

Will this unintentionally overwrite register 0x4e3c in IMX585's internal
0x4exx timing bank whenever the sensor is powered on or streaming is enabled?

[ ... ]

> +static const struct starvis2_variant imx585_variant_def = {
> +	.name = "imx585",
> +	.id_reg = CCI_REG16_LE(0x5a1d),
> +	.id_mask = 0x3ff,
> +	.id_value = 0x259,

[Severity: Medium]
On IMX678, IMX662, IMX675, and IMX832, the sensor OTP identification registers
reside in the 0x4dxx range (with id_reg at 0x4d12 or 0x4d1c, and
STARVIS2_REG_MONOCHROME at 0x4d18).

On IMX585, 0x4dxx is a normal control register bank (imx585_common_regs writes
0x4d12, 0x4d13, and 0x4d26), and the identification block is shifted to 0x5axx
(with id_reg at 0x5a1d).

During starvis2_probe() -> starvis2_identify_model(), the driver reads the
variant's id_reg, but then unconditionally reads the hardcoded
STARVIS2_REG_MONOCHROME (0x4d18) for all variants:

drivers/media/i2c/sony-starvis2.c:starvis2_identify_model() {
	...
	cci_read(starvis2->cci, variant->id_reg, &val, &ret);
	...
	cci_read(starvis2->cci, STARVIS2_REG_MONOCHROME, &val, &ret);
	...
	detected = val & STARVIS2_TYPE;
}

Does reading 0x4d18 on IMX585 cause broken color/monochrome autodetection when
probing with the "sony,imx585" compatible, as well as false sensor-type
mismatch errors in dmesg when probing with "sony,imx585-aamj" or
"sony,imx585-aaqj"?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-media-starvis2-v5-0-eef935347ce9@raspberrypi.com?part=16