Re: [PATCH v2 6/6] media: i2c: ov9282: Fix device detection

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi,

Am Sonntag, 17. Juli 2022, 16:22:01 CEST schrieb Sakari Ailus:
> Hi Alexander,
> 
> On Fri, Jul 15, 2022 at 09:48:58AM +0200, Alexander Stein wrote:
> > Apparently the Vision Components model (VC MIPI OV9281) does not support
> > address auto-increment, so probe fails with:
> > ov9282 2-0060: chip id mismatch: 9281!=92ff
> > Instead do two 1 byte reads and combine the result.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Changes in v2:
> > * Fix commit message
> > * Add comment about prevented auto-increment
> > * Return early if reading ID register failed
> > * Reorder ID registers, smaller register number first
> > 
> >  drivers/media/i2c/ov9282.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index 263cdffc558f..532791304c3d 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -761,11 +761,24 @@ static int ov9282_set_stream(struct v4l2_subdev *sd,
> > int enable)> 
> >  static int ov9282_detect(struct ov9282 *ov9282)
> >  {
> >  
> >  	int ret;
> > 
> > +	u32 id[2];
> > 
> >  	u32 val;
> > 
> > -	ret = ov9282_read_reg(ov9282, OV9282_REG_ID, 2, &val);
> > +	/*
> > +	 * Some vendors prevent auto-increment, so each register has to
> > +	 * be read separately
> > +	 */
> 
> I suppose it still works for writes I guess? Would be nice to have this in
> the comment.
> 
> > +	ret = ov9282_read_reg(ov9282, OV9282_REG_ID,
> > +			      1, &id[0]);
> 
> Fits on one line. Same below.
> 
> >  	if (ret)
> >  	
> >  		return ret;
> > 
> > +	ret = ov9282_read_reg(ov9282, OV9282_REG_ID + 1,
> > +			      1, &id[1]);
> > +	if (ret)
> > +		return ret;
> > +
> > +	val = id[1];
> > +	val |= (id[0] << 8);
> > 
> >  	if (val != OV9282_ID) {
> >  	
> >  		dev_err(ov9282->dev, "chip id mismatch: %x!=%x",

imx290 is affected by the same problem. This can be handles by using regmap's 
config option use_single_read=true.

Will convert to regmap to address this.

Best regards,
Alexander