Re: [PATCH v4 08/14] media: ov772x: support device tree probing

2018-05-03 Thread jacopo mondi 
Akinobu,
a very minor thing, please consider this only if you have to
resend.

On Mon, Apr 30, 2018 at 02:13:07AM +0900, Akinobu Mita wrote:
> The ov772x driver currently only supports legacy platform data probe.
> This change enables device tree probing.
>
> Note that the platform data probe can select auto or manual edge control
> mode, but the device tree probling can only select auto edge control mode
> for now.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Reviewed-by: Jacopo Mondi 
> Signed-off-by: Akinobu Mita 
> ---
> * v4
> - No changes
>
>  drivers/media/i2c/ov772x.c | 64 
> --
>  1 file changed, 45 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index f939e28..621149a 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -749,13 +749,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
>   case V4L2_CID_VFLIP:
>   val = ctrl->val ? VFLIP_IMG : 0x00;
>   priv->flag_vflip = ctrl->val;
> - if (priv->info->flags & OV772X_FLAG_VFLIP)
> + if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
>   val ^= VFLIP_IMG;
>   return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
>   case V4L2_CID_HFLIP:
>   val = ctrl->val ? HFLIP_IMG : 0x00;
>   priv->flag_hflip = ctrl->val;
> - if (priv->info->flags & OV772X_FLAG_HFLIP)
> + if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
>   val ^= HFLIP_IMG;
>   return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
>   case V4L2_CID_BAND_STOP_FILTER:
> @@ -914,19 +914,14 @@ static void ov772x_select_params(const struct 
> v4l2_mbus_framefmt *mf,
>   *win = ov772x_select_win(mf->width, mf->height);
>  }
>
> -static int ov772x_set_params(struct ov772x_priv *priv,
> -  const struct ov772x_color_format *cfmt,
> -  const struct ov772x_win_size *win)
> +static int ov772x_edgectrl(struct ov772x_priv *priv)
>  {
>   struct i2c_client *client = v4l2_get_subdevdata(>subdev);
> - struct v4l2_fract tpf;
>   int ret;
> - u8  val;
>
> - /* Reset hardware. */
> - ov772x_reset(client);
> + if (!priv->info)
> + return 0;
>
> - /* Edge Ctrl. */
>   if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
>   /*
>* Manual Edge Control Mode.
> @@ -937,19 +932,19 @@ static int ov772x_set_params(struct ov772x_priv *priv,
>
>   ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>
>   ret = ov772x_mask_set(client,
> EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK,
> priv->info->edgectrl.threshold);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>
>   ret = ov772x_mask_set(client,
> EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK,
> priv->info->edgectrl.strength);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>
>   } else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
>   /*
> @@ -961,15 +956,35 @@ static int ov772x_set_params(struct ov772x_priv *priv,
> EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
> priv->info->edgectrl.upper);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>
>   ret = ov772x_mask_set(client,
> EDGE_LOWER, OV772X_EDGE_LOWER_MASK,
> priv->info->edgectrl.lower);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>   }
>
> + return 0;
> +}
> +
> +static int ov772x_set_params(struct ov772x_priv *priv,
> +  const struct ov772x_color_format *cfmt,
> +  const struct ov772x_win_size *win)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(>subdev);
> + struct v4l2_fract tpf;
> + int ret;
> + u8  val;
> +
> + /* Reset hardware. */
> + ov772x_reset(client);
> +
> + /* Edge Ctrl. */
> + ret =  ov772x_edgectrl(priv);

You have two spaces before 'ov772x_edgectrl(priv)'

Thanks

[PATCH v4 08/14] media: ov772x: support device tree probing

2018-04-29 Thread Akinobu Mita 
The ov772x driver currently only supports legacy platform data probe.
This change enables device tree probing.

Note that the platform data probe can select auto or manual edge control
mode, but the device tree probling can only select auto edge control mode
for now.

Cc: Jacopo Mondi 
Cc: Laurent Pinchart 
Cc: Hans Verkuil 
Cc: Sakari Ailus 
Cc: Mauro Carvalho Chehab 
Reviewed-by: Jacopo Mondi 
Signed-off-by: Akinobu Mita 
---
* v4
- No changes

 drivers/media/i2c/ov772x.c | 64 --
 1 file changed, 45 insertions(+), 19 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index f939e28..621149a 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -749,13 +749,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
case V4L2_CID_VFLIP:
val = ctrl->val ? VFLIP_IMG : 0x00;
priv->flag_vflip = ctrl->val;
-   if (priv->info->flags & OV772X_FLAG_VFLIP)
+   if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
val ^= VFLIP_IMG;
return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
case V4L2_CID_HFLIP:
val = ctrl->val ? HFLIP_IMG : 0x00;
priv->flag_hflip = ctrl->val;
-   if (priv->info->flags & OV772X_FLAG_HFLIP)
+   if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
val ^= HFLIP_IMG;
return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
case V4L2_CID_BAND_STOP_FILTER:
@@ -914,19 +914,14 @@ static void ov772x_select_params(const struct 
v4l2_mbus_framefmt *mf,
*win = ov772x_select_win(mf->width, mf->height);
 }
 
-static int ov772x_set_params(struct ov772x_priv *priv,
-const struct ov772x_color_format *cfmt,
-const struct ov772x_win_size *win)
+static int ov772x_edgectrl(struct ov772x_priv *priv)
 {
struct i2c_client *client = v4l2_get_subdevdata(>subdev);
-   struct v4l2_fract tpf;
int ret;
-   u8  val;
 
-   /* Reset hardware. */
-   ov772x_reset(client);
+   if (!priv->info)
+   return 0;
 
-   /* Edge Ctrl. */
if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
/*
 * Manual Edge Control Mode.
@@ -937,19 +932,19 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 
ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00);
if (ret < 0)
-   goto ov772x_set_fmt_error;
+   return ret;
 
ret = ov772x_mask_set(client,
  EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK,
  priv->info->edgectrl.threshold);
if (ret < 0)
-   goto ov772x_set_fmt_error;
+   return ret;
 
ret = ov772x_mask_set(client,
  EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK,
  priv->info->edgectrl.strength);
if (ret < 0)
-   goto ov772x_set_fmt_error;
+   return ret;
 
} else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
/*
@@ -961,15 +956,35 @@ static int ov772x_set_params(struct ov772x_priv *priv,
  EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
  priv->info->edgectrl.upper);
if (ret < 0)
-   goto ov772x_set_fmt_error;
+   return ret;
 
ret = ov772x_mask_set(client,
  EDGE_LOWER, OV772X_EDGE_LOWER_MASK,
  priv->info->edgectrl.lower);
if (ret < 0)
-   goto ov772x_set_fmt_error;
+   return ret;
}
 
+   return 0;
+}
+
+static int ov772x_set_params(struct ov772x_priv *priv,
+const struct ov772x_color_format *cfmt,
+const struct ov772x_win_size *win)
+{
+   struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+   struct v4l2_fract tpf;
+   int ret;
+   u8  val;
+
+   /* Reset hardware. */
+   ov772x_reset(client);
+
+   /* Edge Ctrl. */
+   ret =  ov772x_edgectrl(priv);
+   if (ret < 0)
+   return ret;
+
/* Format and window size. */
ret = ov772x_write(client, HSTART, win->rect.left >> 2);
if (ret < 0)
@@ -1020,9 +1035,9 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 
/* Set COM3. */