1. Patchew
  2. linux
  3. [PATCH 0/5] media/i2c: max96717: a few changes
  4. View patch

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

Laurentiu Palcu posted 5 patches 1 year ago
[PATCH 1/5] media/i2c: max96717: change internal regulator voltage
Posted by Laurentiu Palcu 1 year ago 
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.
+	 */
+	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,
-- 
2.34.1
Re: [PATCH 1/5] media/i2c: max96717: change internal regulator voltage
Posted by Julien Massot 11 months, 3 weeks ago 
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
Re: [PATCH 1/5] media/i2c: max96717: change internal regulator voltage
Posted by Laurentiu Palcu 11 months, 1 week ago 
Hi Julien,

Thank you for taking some time to review this series and sorry for the
late reply... :/ I have been involved in other more pressing stuff
lately...

On Tue, Feb 18, 2025 at 02:14:57PM +0100, Julien Massot wrote:
> 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.

Ok, makes sense.

Thanks,
Laurentiu
> 
> > +	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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.