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

["Alessandrelli, Daniele" <daniele.alessandrelli@intel.com>]

On Mon, 2022-07-11 at 10:16 +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 two a 1 byte reads to combine the result.

I think some word is missing from this last statement.

> 
> Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
>  drivers/media/i2c/ov9282.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index c3faf11a99b5..c507d9d4531a 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -761,11 +761,16 @@ 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);
> -       if (ret)
> -               return ret;
> +       ret = ov9282_read_reg(ov9282, OV9282_REG_ID + 1,
> +                             1, &id[1]);

Please add a comment explaining why reading one byte at a time is
needed.

> +       if (!ret)
> +               ret = ov9282_read_reg(ov9282, OV9282_REG_ID,
> +                                     1, &id[0]);

Shouldn't we return in case of error? As the original code was doing?

> +       val = id[1];
> +       val |= (id[0] << 8);
>  
>         if (val != OV9282_ID) {
>                 dev_err(ov9282->dev, "chip id mismatch: %x!=%x",