Re: [PATCH v2 04/13] media: sony-starvis2: Parameterise the configuration to allow for other models

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: sony-starvis2: missing hardware init if PM is disabled
--

commit 7eb8011a8d0af06bdae270bac96946f8a554e9b6
Author: Dave Stevenson <dave.stevenson@raspberrypi.com>

media: sony-starvis2: Parameterise the configuration to allow for other models

This commit parameterises various sensor configurations such as the pixel
array size and timings in preparation for supporting other Starvis 2 variants.

> diff --git a/drivers/media/i2c/sony-starvis2.c b/drivers/media/i2c/sony-starvis2.c
> index 20f3489b58d36..f62a957ff1a8c 100644
> --- a/drivers/media/i2c/sony-starvis2.c
> +++ b/drivers/media/i2c/sony-starvis2.c
[ ... ]
> @@ -895,8 +926,12 @@ static int starvis2_write_common(struct starvis2 *starvis2)
>  {
>  	int ret = 0;
>  
> -	cci_multi_reg_write(starvis2->cci, common_regs, ARRAY_SIZE(common_regs),
> -			    &ret);
> +	if (!starvis2->variant)
> +		/* Powered on during probe to do detection. Variant unknown. */
> +		return 0;
> +
> +	cci_multi_reg_write(starvis2->cci, starvis2->variant->common_regs,
> +			    starvis2->variant->num_common_regs, &ret);
>  
>  	cci_write(starvis2->cci, STARVIS2_REG_INCK_SEL, starvis2->inck_sel_val,
>  		  &ret);

[Severity: High]
If CONFIG_PM is disabled or userspace prevents runtime suspend, does this
skip hardware initialization entirely when starting the stream?

In starvis2_probe(), starvis2_power_on() is called before
starvis2_identify_model() populates starvis2->variant:

starvis2_probe() {
	...
	ret = starvis2_power_on(dev);
	...
	ret = starvis2_identify_model(starvis2);
	...
}

Because starvis2->variant is NULL during the initial probe,
starvis2_write_common() returns early here and skips writing the common
configuration.

If the device never runtime suspends after probe, the later call to
pm_runtime_resume_and_get() in starvis2_enable_streams() will return
immediately without invoking the resume callback, bypassing any subsequent
call to starvis2_power_on(). This appears to result in the stream starting
without the sensor's common configuration and clock settings being programmed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-media-starvis2-v2-0-3670722d250a@raspberrypi.com?part=4