[PATCH] media: soc_camera: ov772x: correct setting of banding filter

[PATCH] media: soc_camera: ov772x: correct setting of banding filter

[Akinobu Mita]

The banding filter ON/OFF is controlled via bit 5 of COM8 register.  It
is attempted to be enabled in ov772x_set_params() by the following line.

	ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);

But this unexpectedly results disabling the banding filter, because the
mask and set bits are exclusive.

On the other hand, ov772x_s_ctrl() correctly sets the bit by:

	ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);

The same fix was already applied to non-soc_camera version of ov772x
driver in the commit commit a024ee14cd36 ("media: ov772x: correct setting
of banding filter")

Cc: Jacopo Mondi <jacopo+renesas@jmondi.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/media/i2c/soc_camera/ov772x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/soc_camera/ov772x.c b/drivers/media/i2c/soc_camera/ov772x.c
index 8063835..14377af 100644
--- a/drivers/media/i2c/soc_camera/ov772x.c
+++ b/drivers/media/i2c/soc_camera/ov772x.c
@@ -834,7 +834,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 	 * set COM8
 	 */
 	if (priv->band_filter) {
-		ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
+		ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
 		if (!ret)
 			ret = ov772x_mask_set(client, BDBASE,
 					      0xff, 256 - priv->band_filter);
-- 
2.7.4

Re: [PATCH] media: soc_camera: ov772x: correct setting of banding filter

[jacopo mondi]

[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]

Hi Mita-san,

On Mon, Jun 11, 2018 at 12:42:26AM +0900, Akinobu Mita wrote:
> The banding filter ON/OFF is controlled via bit 5 of COM8 register.  It
> is attempted to be enabled in ov772x_set_params() by the following line.
>
> 	ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
>
> But this unexpectedly results disabling the banding filter, because the
> mask and set bits are exclusive.
>
> On the other hand, ov772x_s_ctrl() correctly sets the bit by:
>
> 	ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
>
> The same fix was already applied to non-soc_camera version of ov772x
> driver in the commit commit a024ee14cd36 ("media: ov772x: correct setting
> of banding filter")

This driver is aimed for removal, but until it's there, as per the
patch you mentioned:
Acked-by: Jacopo Mondi <jacopo+renesas@jmondi.org>

Thanks
   j

>
> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
>  drivers/media/i2c/soc_camera/ov772x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/soc_camera/ov772x.c b/drivers/media/i2c/soc_camera/ov772x.c
> index 8063835..14377af 100644
> --- a/drivers/media/i2c/soc_camera/ov772x.c
> +++ b/drivers/media/i2c/soc_camera/ov772x.c
> @@ -834,7 +834,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
>  	 * set COM8
>  	 */
>  	if (priv->band_filter) {
> -		ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
> +		ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, BNDF_ON_OFF);
>  		if (!ret)
>  			ret = ov772x_mask_set(client, BDBASE,
>  					      0xff, 256 - priv->band_filter);
> --
> 2.7.4
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]