On Thu, Aug 03, 2023 at 12:48:01PM +0000, Sakari Ailus wrote:
> Hi Hans,
>
> On Thu, Aug 03, 2023 at 11:33:17AM +0200, Hans de Goede wrote:
> > The index into ov2680_hv_flip_bayer_order[] should be 0-3, but
> > ov2680_bayer_order() was using 0 + BIT(2) + (BIT(2) << 1) as
> > max index, while the intention was to use: 0 + 1 + 2 as max index.
> >
> > Fix the index calculation in ov2680_bayer_order(), while at it
> > also just use the ctrl values rather then reading them back using
> > a slow i2c-read transaction.
> >
> > This also allows making the function void, since there now are
> > no more i2c-reads to error check.
> >
> > Note the check for the ctrls being NULL is there to allow
> > adding an ov2680_fill_format() helper later, which will call
> > ov2680_set_bayer_order() during probe() before the ctrls are created.
> >
> > Fixes: 3ee47cad3e69 ("media: ov2680: Add Omnivision OV2680 sensor driver")
> > Reviewed-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
> > Acked-by: Rui Miguel Silva <rmfrfs@xxxxxxxxx>
> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>
> This doesn't seem to compile:
>
> > /home/jenkins/linux/drivers/media/i2c/ov2680.c: In function
> 'ov2680_vflip_disable':
> > /home/jenkins/linux/drivers/media/i2c/ov2680.c:351:9: error: implicit
> declaration of function 'ov2680_bayer_order'; did you mean
> 'ov2680_set_bayer_order'? [-Werror=implicit-function-declaration]
> > 351 | return ov2680_bayer_order(sensor);
> > | ^~~~~~~~~~~~~~~~~~
> > | ov2680_set_bayer_order
> > cc1: some warnings being treated as errors
>
> I guess you missed converting some calls?
Actually the fix was trivial and the code is removed in the next patch
nonetheless. The change I made here was:
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
index 621144f16fdc..60c4e575aa86 100644
--- a/drivers/media/i2c/ov2680.c
+++ b/drivers/media/i2c/ov2680.c
@@ -348,7 +348,8 @@ static int ov2680_vflip_disable(struct ov2680_dev *sensor)
if (ret < 0)
return ret;
- return ov2680_bayer_order(sensor);
+ ov2680_set_bayer_order(sensor);
+ return 0;
}
static int ov2680_hflip_enable(struct ov2680_dev *sensor)
@@ -359,7 +360,8 @@ static int ov2680_hflip_enable(struct ov2680_dev *sensor)
if (ret < 0)
return ret;
- return ov2680_bayer_order(sensor);
+ ov2680_set_bayer_order(sensor);
+ return 0;
}
static int ov2680_hflip_disable(struct ov2680_dev *sensor)
--
Sakari Ailus
