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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fvwj4hi4n2uspsggikw5vce7tyxxv2pga5duoqxbqkd46msfjy@ndshmqgxm6ky>
Date: Thu, 6 Mar 2025 10:11:11 +0200
From: Laurentiu Palcu <laurentiu.palcu@....nxp.com>
To: Julien Massot <julien.massot@...labora.com>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, 
	linux-kernel@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH 1/5] media/i2c: max96717: change internal regulator
 voltage

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@....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