| Message ID | 20220715074858.875808-7-alexander.stein@ew.tq-group.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | OV9281 support | expand |
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",
Hello Sakari, thanks for the feedback. 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. Yes, I didn't notice any problems regarding writes. Will address this in the comment. > > + ret = ov9282_read_reg(ov9282, OV9282_REG_ID, > > + 1, &id[0]); > > Fits on one line. Same below. Sure, will be changed. Thanks and best regards, Alexander > > 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",
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
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 + */ + ret = ov9282_read_reg(ov9282, OV9282_REG_ID, + 1, &id[0]); 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",
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(-)