Hi Max On Wed, 27 Sept 2023 at 09:56, Max Schulze <max.schulze@xxxxxxxxx> wrote: > > The ov9281 produced by vision-components does not support > auto-incremented reads, so read the id in 2 separate steps > to circumvent the error: > kernel: ov9282 10-0060: chip id mismatch: 9281!=92ff > kernel: ov9282 10-0060: failed to find sensor: -6 > > > Signed-off-by: Max Schulze <max.schulze@xxxxxxxxx> > Tested-by: Max Schulze <max.schulze@xxxxxxxxx> > Suggested-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > --- > This was tested on rpi cm4 with two ov9281 at the same time, > one from vc, one from inno-maker. > > The rpi kernel supported ov9281 out-of-tree until kernel 6.1. > It carried this change originally made by Dave to support the vc > sensor. Switching to mainline now broke support for it. > > I could not find a single-commit for the original change to which I > could link, only squashed ones, i.e. > > https://github.com/raspberrypi/linux/commit/eb00efc993d8cd835221255b44e9019a31708abe The single commit was on the rpi-5.10.y branch https://github.com/raspberrypi/linux/commit/e19e5fa998c7dfaa9942a494499e37788365ccec > > media: i2c: ov9281: Read chip ID via 2 reads > > Vision Components have made an OV9281 module which blocks reading > back the majority of registers to comply with NDAs, and in doing > so doesn't allow auto-increment register reading as used when > reading the chip ID. I know that Laurent has previously made comments on potentially nicer ways to handle these annoying Vision Components sensors before, but in the absence of anything solid then I have no issues with this patch. It's not going to cause an issue with standard modules, and makes them work with the VC ones. Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > --- > drivers/media/i2c/ov9282.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c > index 068c7449f50e..3b687f6d4581 100644 > --- a/drivers/media/i2c/ov9282.c > +++ b/drivers/media/i2c/ov9282.c > @@ -1078,13 +1078,16 @@ static int ov9282_set_stream(struct v4l2_subdev *sd, int enable) > static int ov9282_detect(struct ov9282 *ov9282) > { > int ret; > - u32 val; > + u32 val = 0, id_msb = 0; > > - ret = ov9282_read_reg(ov9282, OV9282_REG_ID, 2, &val); > - if (ret) > - return ret; > + // some firmware limits auto-increment register writes, so do it separately > + ret = ov9282_read_reg(ov9282, OV9282_REG_ID + 1, 1, &val); > + if (!ret) > + ret = ov9282_read_reg(ov9282, OV9282_REG_ID, 1, &id_msb); > + > + val |= (id_msb << 8); > > - if (val != OV9282_ID) { > + if (ret || val != OV9282_ID) { > dev_err(ov9282->dev, "chip id mismatch: %x!=%x", > OV9282_ID, val); Minor nit: you'll print this error message if one of the transfers failed, whereas previously it would have just returned the error code. Dave > return -ENXIO; > -- > 2.30.2 >