Re: [PATCH 1/3] media: i2c: isl7998x: Enable VACTIVE and HDELAY shadow registers

[Michael Tretter <m.tretter@pengutronix.de>]

On Sat, 13 Jul 2024 17:41:54 +0200, Marek Vasut wrote:
> The VACTIVE and HDELAY shadow registers must be enabled for per channel
> NTSC/PAL detection to work, enable them.

I think the description is wrong as the change doesn't affect the
VACTIVE and HDELAY registers. The change sets the standard selection
register to auto detection to always trigger a standard detection during
initialization instead of defaulting to NTSC. querystd explicitly
triggers this behavior.

If the driver always runs standard detection during init, the driver
should also wait for the detection to be finished and update
isl7998x->norm accordingly, because otherwise the standard will be reset
to NTSC.

Maybe we can also get rid of this part from the init sequence and run
the standard detection during the initialization in code?

Michael

> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Michael Tretter <m.tretter@pengutronix.de>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: linux-media@vger.kernel.org
> ---
>  drivers/media/i2c/isl7998x.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/isl7998x.c b/drivers/media/i2c/isl7998x.c
> index c7089035bbc10..5f8f11f7a2c1c 100644
> --- a/drivers/media/i2c/isl7998x.c
> +++ b/drivers/media/i2c/isl7998x.c
> @@ -312,13 +312,13 @@
>  
>  static const struct reg_sequence isl7998x_init_seq_1[] = {
>  	{ ISL7998X_REG_P0_SHORT_DIAG_IRQ_EN, 0xff },
> -	{ ISL7998X_REG_PX_DEC_SDT(0x1), 0x00 },
> +	{ ISL7998X_REG_PX_DEC_SDT(0x1), 0x07 },
>  	{ ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x1), 0x03 },
> -	{ ISL7998X_REG_PX_DEC_SDT(0x2), 0x00 },
> +	{ ISL7998X_REG_PX_DEC_SDT(0x2), 0x07 },
>  	{ ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x2), 0x03 },
> -	{ ISL7998X_REG_PX_DEC_SDT(0x3), 0x00 },
> +	{ ISL7998X_REG_PX_DEC_SDT(0x3), 0x07 },
>  	{ ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x3), 0x03 },
> -	{ ISL7998X_REG_PX_DEC_SDT(0x4), 0x00 },
> +	{ ISL7998X_REG_PX_DEC_SDT(0x4), 0x07 },
>  	{ ISL7998X_REG_PX_DEC_SHORT_DET_CTL_1(0x4), 0x03 },
>  	{ ISL7998X_REG_P5_LI_ENGINE_CTL, 0x00 },
>  	{ ISL7998X_REG_P0_SW_RESET_CTL, 0x1f, 10 },
> -- 
> 2.43.0
> 
>