Re: [PATCH] read ov9281 sensor-id in 2-steps in case the firmware locks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux