Re: [PATCH v4 5/8] media: adv7180: implement g_parm

[Hans Verkuil]

On 08/03/2016 08:03 PM, Steve Longerbeam wrote:
> Implement g_parm to return the current standard's frame period.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>
> Tested-by: Tim Harvey <tharvey@xxxxxxxxxxxxx>
> Acked-by: Tim Harvey <tharvey@xxxxxxxxxxxxx>
> 
> ---
> v4: no changes
> v3: no changes
> v2: no changes
> ---
>  drivers/media/i2c/adv7180.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index b2df181..9705e24 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -764,6 +764,27 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd,
>  	return 0;
>  }
>  
> +static int adv7180_g_parm(struct v4l2_subdev *sd, struct v4l2_streamparm *a)
> +{
> +	struct adv7180_state *state = to_state(sd);
> +	struct v4l2_captureparm *cparm = &a->parm.capture;
> +
> +	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		return -EINVAL;
> +
> +	memset(a, 0, sizeof(*a));
> +	a->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;

Don't memset this, it is the responsibility of the caller to do that,
not of the subdev.

The caller may have set other fields in the struct that the memset would
wipe out.

Regards,

	Hans

> +	if (state->curr_norm & V4L2_STD_525_60) {
> +		cparm->timeperframe.numerator = 1001;
> +		cparm->timeperframe.denominator = 30000;
> +	} else {
> +		cparm->timeperframe.numerator = 1;
> +		cparm->timeperframe.denominator = 25;
> +	}
> +
> +	return 0;
> +}
> +
>  static int adv7180_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *cropcap)
>  {
>  	struct adv7180_state *state = to_state(sd);
> @@ -822,6 +843,7 @@ static int adv7180_subscribe_event(struct v4l2_subdev *sd,
>  static const struct v4l2_subdev_video_ops adv7180_video_ops = {
>  	.s_std = adv7180_s_std,
>  	.g_std = adv7180_g_std,
> +	.g_parm = adv7180_g_parm,
>  	.querystd = adv7180_querystd,
>  	.g_input_status = adv7180_g_input_status,
>  	.s_routing = adv7180_s_routing,
>