Re: [PATCH 1/5] media/i2c: max96717: change internal regulator voltage

[Julien Massot <julien.massot@collabora.com>]

Hi Laurentiu,

Thanks for your patch,
On Fri, 2025-02-07 at 13:29 +0200, Laurentiu Palcu wrote:
> The Programming Notes section of the specifications states:
> 
> """
> MANDATORY REGISTER PROGRAMMING
> Make the following register writes to ensure proper operation of the
> MAX96717F. Without these writes, the operation of the device specified
> in the data sheet cannot be guaranteed.
> Set bits [6:4] = 3'b001 in register 0x302
> """
> 
> Set this register before going on with the chip initialization.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  drivers/media/i2c/max96717.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/media/i2c/max96717.c b/drivers/media/i2c/max96717.c
> index 9259d58ba734e..b1116aade0687 100644
> --- a/drivers/media/i2c/max96717.c
> +++ b/drivers/media/i2c/max96717.c
> @@ -78,6 +78,15 @@
>  #define MAX96717_GPIO_TX_EN       BIT(1)
>  #define MAX96717_GPIO_OUT_DIS     BIT(0)
>  
> +/* CMU */
> +#define MAX96717_CMU_CMU2		CCI_REG8(0x0302)
> +#define MAX96717_PFDDIV_RSHORT_MASK	GENMASK(6, 4)
> +#define MAX96717_PFDDIV_RSHORT_SHIFT	4
> +#define MAX96717_PFDDIV_VREG_1V0	0
> +#define MAX96717_PFDDIV_VREG_1V1	1
> +#define MAX96717_PFDDIV_VREG_0V875	2
> +#define MAX96717_PFDDIV_VREG_0V94	3
> +
>  /* FRONTTOP */
>  /* MAX96717 only have CSI port 'B' */
>  #define MAX96717_FRONTOP0     CCI_REG8(0x308)
> @@ -981,6 +990,14 @@ static int max96717_hw_init(struct max96717_priv *priv)
>  	dev_dbg(dev, "Found %x (rev %lx)\n", (u8)dev_id,
>  		(u8)val & MAX96717_DEV_REV_MASK);
>  
> +	/*
> +	 * According to specs, in the Programming Notes section, there's a mandatory register
> +	 * programming notice that advises to enable the 1.1V internal regulator to guarantee
> proper
> +	 * device operation. Let's do this before any other operations.
> +	 */
Latest MAX96717{,K,F} revision 6 seems not affected by this issue. Can you please make this write
conditional to rev < 6 ? Register 0xe.

> +	cci_write(priv->regmap, MAX96717_CMU_CMU2,
> +		  MAX96717_PFDDIV_VREG_1V1 << MAX96717_PFDDIV_RSHORT_SHIFT, NULL);
> +
>  	ret = cci_read(priv->regmap, MAX96717_MIPI_RX_EXT11, &val, NULL);
>  	if (ret)
>  		return dev_err_probe(dev, ret,
Regards,
-- 
Julien