Re: [PATCH 6/6] media: ov772x: support device tree probing

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

 



2018-04-09 18:27 GMT+09:00 jacopo mondi <jacopo@xxxxxxxxxx>:
> Hi Akinobu,
>
> On Sun, Apr 08, 2018 at 12:48:10AM +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 <jacopo+renesas@xxxxxxxxxx>
>> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>> Cc: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
>> ---
>>  drivers/media/i2c/ov772x.c | 60 ++++++++++++++++++++++++++++++----------------
>>  1 file changed, 40 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
>> index 5e91fa1..e67ec37 100644
>> --- a/drivers/media/i2c/ov772x.c
>> +++ b/drivers/media/i2c/ov772x.c
>> @@ -763,13 +763,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:
>> @@ -928,19 +928,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(&priv->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.
>> @@ -951,19 +946,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) {
>>               /*
>> @@ -975,15 +970,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(&priv->subdev);
>> +     struct v4l2_fract tpf;
>> +     int ret;
>> +     u8  val;
>> +
>> +     /* Reset hardware. */
>> +     ov772x_reset(client);
>> +
>> +     /* Edge Ctrl. */
>> +     ret =  ov772x_edgectrl(priv);
>> +     if (ret < 0)
>> +             goto ov772x_set_fmt_error;
>> +
>>       /* Format and window size. */
>>       ret = ov772x_write(client, HSTART, win->rect.left >> 2);
>>       if (ret < 0)
>> @@ -1284,11 +1299,6 @@ static int ov772x_probe(struct i2c_client *client,
>>       struct i2c_adapter      *adapter = client->adapter;
>>       int                     ret;
>>
>> -     if (!client->dev.platform_data) {
>
> Not sure this has to be removed completely. I'm debated, as in
> general, for mainline code, we should make sure every user of this
> driver shall provide platform_data during review, and this check is
> not requried. But in general, I still think it may have value.
>
> What about:
>         if (!client->dev.of_node && !client->dev.platform_data) {
>                dev_err(&client->dev, "Missing ov772x platform data\n");
>                return -EINVAL;
>         }

OK. I'll take this.



[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