[RFC] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[RFC] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Satendra Singh Thakur]

-For setting one frontend property , one FE_SET_PROPERTY ioctl is called
-Since, size of struct dtv_property is 72 bytes, this ioctl requires
---allocating 72 bytes of memory in user space
---allocating 72 bytes of memory in kernel space
---copying 72 bytes of data from user space to kernel space
-However, for all the properties, only 8 out of 72 bytes are used
 for setting the property
-Four bytes are needed for specifying property type and another 4 for
 property value
-Moreover, there are 2 properties DTV_CLEAR and DTV_TUNE which use
 only 4 bytes for property name
---They don't use property value
-Therefore, we have defined new short variant/forms/version of currently
 used structures for such 8 byte properties.
-This results in 89% (8*100/72) of memory saving in user and kernel space
 each.
-This also results in faster copy (8 bytes as compared to 72 bytes) from
 user to kernel space
-We have added new ioctl FE_SET_PROPERTY_SHORT which utilizes above
 mentioned new property structures
-This ioctl can co-exist with present ioctl FE_SET_PROPERTY
-If the apps wish to use shorter forms they can use
 proposed FE_SET_PROPERTY_SHORT, rest of them can continue to use
 current versions FE_SET_PROPERTY
-We are currently not validating incoming properties in
 function dtv_property_short_process_set because most of
 the frontend drivers in linux source are not using the
 method ops.set_property. Just two drivers are using it
 drivers/media/dvb-frontends/stv0288.c
 driver/media/usb/dvb-usb/friio-fe.c
 -Moreover, stv0288 driver implemments blank function
 for set_property.
-If needed in future, we can define a new
 ops.set_property_short method to support
 struct dtv_property_short.
Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
---
 drivers/media/dvb-core/dvb_frontend.c | 228 +++++++++++++++++++++++++++++++++-
 include/uapi/linux/dvb/frontend.h     |  24 ++++
 2 files changed, 248 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index e3fff8f..e183025 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1914,6 +1914,192 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 	return r;
 }
 
+/**
+ * dtv_property_short_process_set
+ * @fe: Pointer to struct dvb_frontend
+ * @tvp: Pointer to struct dtv_property_short
+ * @file: Pointer to struct file
+ *
+ * helper function for dvb_frontend_ioctl_properties,
+ * which can be used to set dtv property using ioctl
+ * cmd FE_SET_PROPERTY_SHORT.
+ * It assigns property value to corresponding member of
+ * property-cache structure
+ * This func is a variant of the func dtv_property_process_set
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+static int dtv_property_short_process_set(struct dvb_frontend *fe,
+				    struct dtv_property_short *tvp,
+				    struct file *file)
+{
+	int r = 0;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	/* Currently, We do not allow the frontend to validate incoming
+	 * properties, currently, just 2 drivers are using
+	 * ops.set_property method , If required, we can define new
+	 * ops.set_property_short method for this purpose
+	 */
+	switch (tvp->cmd) {
+	case DTV_CLEAR:
+		/*
+		 * Reset a cache of data specific to the frontend here. This
+		 * does not effect hardware.
+		 */
+		dvb_frontend_clear_cache(fe);
+		break;
+	case DTV_TUNE:
+		/* interpret the cache of data, build either a traditional
+		 * frontend tune request so we can pass validation in the
+		 * FE_SET_FRONTEND  ioctl.
+		 */
+		c->state = tvp->cmd;
+		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
+				__func__);
+
+		r = dtv_set_frontend(fe);
+		break;
+	case DTV_FREQUENCY:
+		c->frequency = tvp->data;
+		break;
+	case DTV_MODULATION:
+		c->modulation = tvp->data;
+		break;
+	case DTV_BANDWIDTH_HZ:
+		c->bandwidth_hz = tvp->data;
+		break;
+	case DTV_INVERSION:
+		c->inversion = tvp->data;
+		break;
+	case DTV_SYMBOL_RATE:
+		c->symbol_rate = tvp->data;
+		break;
+	case DTV_INNER_FEC:
+		c->fec_inner = tvp->data;
+		break;
+	case DTV_PILOT:
+		c->pilot = tvp->data;
+		break;
+	case DTV_ROLLOFF:
+		c->rolloff = tvp->data;
+		break;
+	case DTV_DELIVERY_SYSTEM:
+		r = dvbv5_set_delivery_system(fe, tvp->data);
+		break;
+	case DTV_VOLTAGE:
+		c->voltage = tvp->data;
+		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
+			(void *)c->voltage);
+		break;
+	case DTV_TONE:
+		c->sectone = tvp->data;
+		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
+			(void *)c->sectone);
+		break;
+	case DTV_CODE_RATE_HP:
+		c->code_rate_HP = tvp->data;
+		break;
+	case DTV_CODE_RATE_LP:
+		c->code_rate_LP = tvp->data;
+		break;
+	case DTV_GUARD_INTERVAL:
+		c->guard_interval = tvp->data;
+		break;
+	case DTV_TRANSMISSION_MODE:
+		c->transmission_mode = tvp->data;
+		break;
+	case DTV_HIERARCHY:
+		c->hierarchy = tvp->data;
+		break;
+	case DTV_INTERLEAVING:
+		c->interleaving = tvp->data;
+		break;
+
+	/* ISDB-T Support here */
+	case DTV_ISDBT_PARTIAL_RECEPTION:
+		c->isdbt_partial_reception = tvp->data;
+		break;
+	case DTV_ISDBT_SOUND_BROADCASTING:
+		c->isdbt_sb_mode = tvp->data;
+		break;
+	case DTV_ISDBT_SB_SUBCHANNEL_ID:
+		c->isdbt_sb_subchannel = tvp->data;
+		break;
+	case DTV_ISDBT_SB_SEGMENT_IDX:
+		c->isdbt_sb_segment_idx = tvp->data;
+		break;
+	case DTV_ISDBT_SB_SEGMENT_COUNT:
+		c->isdbt_sb_segment_count = tvp->data;
+		break;
+	case DTV_ISDBT_LAYER_ENABLED:
+		c->isdbt_layer_enabled = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERA_FEC:
+		c->layer[0].fec = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERA_MODULATION:
+		c->layer[0].modulation = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
+		c->layer[0].segment_count = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
+		c->layer[0].interleaving = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERB_FEC:
+		c->layer[1].fec = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERB_MODULATION:
+		c->layer[1].modulation = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
+		c->layer[1].segment_count = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
+		c->layer[1].interleaving = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERC_FEC:
+		c->layer[2].fec = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERC_MODULATION:
+		c->layer[2].modulation = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
+		c->layer[2].segment_count = tvp->data;
+		break;
+	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
+		c->layer[2].interleaving = tvp->data;
+		break;
+
+	/* Multistream support */
+	case DTV_STREAM_ID:
+	case DTV_DVBT2_PLP_ID_LEGACY:
+		c->stream_id = tvp->data;
+		break;
+
+	/* ATSC-MH */
+	case DTV_ATSCMH_PARADE_ID:
+		fe->dtv_property_cache.atscmh_parade_id = tvp->data;
+		break;
+	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
+		fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->data;
+		break;
+
+	case DTV_LNA:
+		c->lna = tvp->data;
+		if (fe->ops.set_lna)
+			r = fe->ops.set_lna(fe);
+		if (r < 0)
+			c->lna = LNA_AUTO;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return r;
+}
+
 static int dvb_frontend_ioctl(struct file *file,
 			unsigned int cmd, void *parg)
 {
@@ -1939,7 +2125,8 @@ static int dvb_frontend_ioctl(struct file *file,
 		return -EPERM;
 	}
 
-	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
+	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
+		|| (cmd == FE_SET_PROPERTY_SHORT))
 		err = dvb_frontend_ioctl_properties(file, cmd, parg);
 	else {
 		c->state = DTV_UNDEFINED;
@@ -2026,9 +2213,42 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			err = -EFAULT;
 			goto out;
 		}
-
-	} else
-		err = -EOPNOTSUPP;
+	/* New ioctl for optimizing property set
+	 */
+	} else if (cmd == FE_SET_PROPERTY_SHORT) {
+		struct dtv_property_short *tvp_short = NULL;
+		struct dtv_properties_short *tvps_short = parg;
+
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", \
+		__func__, tvps_short->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", \
+		__func__, tvps_short->props);
+		if ((!tvps_short->num) ||
+		(tvps_short->num > DTV_IOCTL_MAX_MSGS))
+			return -EINVAL;
+		tvp_short = memdup_user(tvps_short->props,
+		tvps_short->num * sizeof(*tvp_short));
+		if (IS_ERR(tvp_short))
+			return PTR_ERR(tvp_short);
+		for (i = 0; i < tvps_short->num; i++) {
+			err = dtv_property_short_process_set(fe, tvp_short + i,\
+				file);
+			if (err < 0) {
+				kfree(tvp_short);
+				return err;
+			}
+			/* Since we are returning when error occurs
+			 * There is no need to store the result as it
+			 * would have been >=0 in case we didn't return
+			 * (tvp + i)->result = err;
+			 */
+		}
+		if (c->state == DTV_TUNE)
+			dev_dbg(fe->dvb->device, "%s: Property cache\
+		is full, tuning\n", __func__);
+		kfree(tvp_short);
+		} else
+			err = -EOPNOTSUPP;
 
 out:
 	kfree(tvp);
diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
index 00a20cd..aa82179 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -476,6 +476,17 @@ struct dtv_property {
 	int result;
 } __attribute__ ((packed));
 
+/**
+ * @struct dtv_property_short
+ * A shorter version of struct dtv_property
+ * @cmd: Property type
+ * @data: Property value
+ */
+struct dtv_property_short {
+	__u32 cmd;
+	__u32 data;
+};
+
 /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
 #define DTV_IOCTL_MAX_MSGS 64
 
@@ -484,6 +495,18 @@ struct dtv_properties {
 	struct dtv_property *props;
 };
 
+/**
+ * @struct dtv_properties_short
+ * A variant of struct dtv_properties
+ * to support struct dtv_property_short
+ * @num: Number of properties
+ * @props: Pointer to struct dtv_property_short
+ */
+struct dtv_properties_short {
+	__u32 num;
+	struct dtv_property_short *props;
+};
+
 #if defined(__DVB_CORE__) || !defined (__KERNEL__)
 
 /*
@@ -565,6 +588,7 @@ struct dvb_frontend_event {
 
 #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
 #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
+#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
 
 /**
  * When set, this flag will disable any zigzagging or other "normal" tuning
-- 
2.7.4

Re: [RFC] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Mauro Carvalho Chehab]

Hi Satendra,

Em Thu, 14 Sep 2017 05:59:27 -0400
Satendra Singh Thakur <satendra.t@samsung.com> escreveu:

> -For setting one frontend property , one FE_SET_PROPERTY ioctl is called
> -Since, size of struct dtv_property is 72 bytes, this ioctl requires
> ---allocating 72 bytes of memory in user space
> ---allocating 72 bytes of memory in kernel space
> ---copying 72 bytes of data from user space to kernel space
> -However, for all the properties, only 8 out of 72 bytes are used
>  for setting the property  

That's true. Yet, for get, the size can be bigger, as ISDB-T can
return statistics per layer, plus a global one.

> -Four bytes are needed for specifying property type and another 4 for
>  property value
> -Moreover, there are 2 properties DTV_CLEAR and DTV_TUNE which use
>  only 4 bytes for property name
> ---They don't use property value
> -Therefore, we have defined new short variant/forms/version of currently
>  used structures for such 8 byte properties.
> -This results in 89% (8*100/72) of memory saving in user and kernel space
>  each.
> -This also results in faster copy (8 bytes as compared to 72 bytes) from
>  user to kernel space
> -We have added new ioctl FE_SET_PROPERTY_SHORT which utilizes above
>  mentioned new property structures
> -This ioctl can co-exist with present ioctl FE_SET_PROPERTY
> -If the apps wish to use shorter forms they can use
>  proposed FE_SET_PROPERTY_SHORT, rest of them can continue to use
>  current versions FE_SET_PROPERTY  

> -We are currently not validating incoming properties in
>  function dtv_property_short_process_set because most of
>  the frontend drivers in linux source are not using the
>  method ops.set_property. Just two drivers are using it
>  drivers/media/dvb-frontends/stv0288.c
>  driver/media/usb/dvb-usb/friio-fe.c
>  -Moreover, stv0288 driver implemments blank function
>  for set_property.
> -If needed in future, we can define a new
>  ops.set_property_short method to support
>  struct dtv_property_short.  

Nah. Better to just get rid of get_property()/set_froperty() for good.

Just sent a RFC patch series doing that.

The only thing is that stv6110 seems to have a dirty hack that may
depend on that. Someone need to double-check if the patch series
I just sent doesn't break anything. If it breaks, then we'll need
to add an extra parameter to stv6110 attach for it to know what
behavior is needed there.


> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> ---
>  drivers/media/dvb-core/dvb_frontend.c | 228 +++++++++++++++++++++++++++++++++-
>  include/uapi/linux/dvb/frontend.h     |  24 ++++
>  2 files changed, 248 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index e3fff8f..e183025 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1914,6 +1914,192 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
>  	return r;
>  }
>  
> +/**
> + * dtv_property_short_process_set
> + * @fe: Pointer to struct dvb_frontend
> + * @tvp: Pointer to struct dtv_property_short
> + * @file: Pointer to struct file
> + *
> + * helper function for dvb_frontend_ioctl_properties,
> + * which can be used to set dtv property using ioctl
> + * cmd FE_SET_PROPERTY_SHORT.
> + * It assigns property value to corresponding member of
> + * property-cache structure
> + * This func is a variant of the func dtv_property_process_set
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +static int dtv_property_short_process_set(struct dvb_frontend *fe,
> +				    struct dtv_property_short *tvp,
> +				    struct file *file)
> +{
> +	int r = 0;
> +	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +	/* Currently, We do not allow the frontend to validate incoming
> +	 * properties, currently, just 2 drivers are using
> +	 * ops.set_property method , If required, we can define new
> +	 * ops.set_property_short method for this purpose
> +	 */
> +	switch (tvp->cmd) {
> +	case DTV_CLEAR:  

Nah. Let's not have multiple validation routines for each variant.

It would be better to change the parameters for dtv_property_process_set
to something like:

static int dtv_property_process_set(struct dvb_frontend *fe,
                                    struct file *file,
				    u32 cmd, u32 data)

And have just one validation routine that would work for both.

If we end by adding some DTV properties that would require more than
4 bytes, only such properties would be implemented on a different
function.

>  static int dvb_frontend_ioctl(struct file *file,
>  			unsigned int cmd, void *parg)
>  {
> @@ -1939,7 +2125,8 @@ static int dvb_frontend_ioctl(struct file *file,
>  		return -EPERM;
>  	}
>  
> -	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
> +	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
> +		|| (cmd == FE_SET_PROPERTY_SHORT))
>  		err = dvb_frontend_ioctl_properties(file, cmd, parg);
>  	else {
>  		c->state = DTV_UNDEFINED;
> @@ -2026,9 +2213,42 @@ static int dvb_frontend_ioctl_properties(struct file *file,
>  			err = -EFAULT;
>  			goto out;
>  		}
> -
> -	} else
> -		err = -EOPNOTSUPP;
> +	/* New ioctl for optimizing property set
> +	 */
> +	} else if (cmd == FE_SET_PROPERTY_SHORT) {
> +		struct dtv_property_short *tvp_short = NULL;
> +		struct dtv_properties_short *tvps_short = parg;
> +
> +		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", \
> +		__func__, tvps_short->num);
> +		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", \
> +		__func__, tvps_short->props);
> +		if ((!tvps_short->num) ||
> +		(tvps_short->num > DTV_IOCTL_MAX_MSGS))
> +			return -EINVAL;
> +		tvp_short = memdup_user(tvps_short->props,
> +		tvps_short->num * sizeof(*tvp_short));
> +		if (IS_ERR(tvp_short))
> +			return PTR_ERR(tvp_short);
> +		for (i = 0; i < tvps_short->num; i++) {
> +			err = dtv_property_short_process_set(fe, tvp_short + i,\
> +				file);
> +			if (err < 0) {
> +				kfree(tvp_short);
> +				return err;
> +			}
> +			/* Since we are returning when error occurs
> +			 * There is no need to store the result as it
> +			 * would have been >=0 in case we didn't return
> +			 * (tvp + i)->result = err;
> +			 */
> +		}
> +		if (c->state == DTV_TUNE)
> +			dev_dbg(fe->dvb->device, "%s: Property cache\
> +		is full, tuning\n", __func__);  

Don't break strings on two lines.

> +		kfree(tvp_short);  

> +		} else
> +			err = -EOPNOTSUPP;  

Indentation here is wrong.

>  
>  out:
>  	kfree(tvp);
> diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
> index 00a20cd..aa82179 100644
> --- a/include/uapi/linux/dvb/frontend.h
> +++ b/include/uapi/linux/dvb/frontend.h
> @@ -476,6 +476,17 @@ struct dtv_property {
>  	int result;
>  } __attribute__ ((packed));
>  
> +/**
> + * @struct dtv_property_short
> + * A shorter version of struct dtv_property
> + * @cmd: Property type
> + * @data: Property value
> + */
> +struct dtv_property_short {
> +	__u32 cmd;
> +	__u32 data;
> +};
> +
>  /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
>  #define DTV_IOCTL_MAX_MSGS 64
>  
> @@ -484,6 +495,18 @@ struct dtv_properties {
>  	struct dtv_property *props;
>  };
>  
> +/**
> + * @struct dtv_properties_short
> + * A variant of struct dtv_properties
> + * to support struct dtv_property_short
> + * @num: Number of properties
> + * @props: Pointer to struct dtv_property_short
> + */
> +struct dtv_properties_short {
> +	__u32 num;
> +	struct dtv_property_short *props;
> +};
> +
>  #if defined(__DVB_CORE__) || !defined (__KERNEL__)
>  
>  /*
> @@ -565,6 +588,7 @@ struct dvb_frontend_event {
>  
>  #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
>  #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
> +#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)  
>  
>  /**
>   * When set, this flag will disable any zigzagging or other "normal" tuning  

Thanks,
Mauro

[PATCH v1] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Satendra Singh Thakur]

Hello  Mr Chehab,
Thanks for reviewing the patch.
I have modified the patch as per your comments.
Please check if it looks fine now.

Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
---
 drivers/media/dvb-core/dvb_frontend.c | 212 +++++++++++++++++++++++++++++++++-
 include/uapi/linux/dvb/frontend.h     |  24 ++++
 2 files changed, 234 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index e3fff8f..bc35a86 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1914,6 +1914,188 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 	return r;
 }
 
+/**
+ * dtv_property_short_process_set
+ * @fe: Pointer to struct dvb_frontend
+ * @file: Pointer to struct file
+ * @cmd: Property name
+ * @data: Property value
+ *
+ * helper function for dvb_frontend_ioctl_properties,
+ * which can be used to set dtv property using ioctl
+ * cmd FE_SET_PROPERTY_SHORT.
+ * It assigns property value to corresponding member of
+ * property-cache structure
+ * This func is a variant of the func dtv_property_process_set
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+static int dtv_property_short_process_set(struct dvb_frontend *fe,
+					struct file *file,
+					u32 cmd, u32 data)
+{
+	int r = 0;
+	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
+	switch (cmd) {
+	case DTV_CLEAR:
+		/*
+		 * Reset a cache of data specific to the frontend here. This
+		 * does not effect hardware.
+		 */
+		dvb_frontend_clear_cache(fe);
+		break;
+	case DTV_TUNE:
+		/* interpret the cache of data, build either a traditional
+		 * frontend tune request so we can pass validation in the
+		 * FE_SET_FRONTEND  ioctl.
+		 */
+		c->state = cmd;
+		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
+				__func__);
+
+		r = dtv_set_frontend(fe);
+		break;
+	case DTV_FREQUENCY:
+		c->frequency = data;
+		break;
+	case DTV_MODULATION:
+		c->modulation = data;
+		break;
+	case DTV_BANDWIDTH_HZ:
+		c->bandwidth_hz = data;
+		break;
+	case DTV_INVERSION:
+		c->inversion = data;
+		break;
+	case DTV_SYMBOL_RATE:
+		c->symbol_rate = data;
+		break;
+	case DTV_INNER_FEC:
+		c->fec_inner = data;
+		break;
+	case DTV_PILOT:
+		c->pilot = data;
+		break;
+	case DTV_ROLLOFF:
+		c->rolloff = data;
+		break;
+	case DTV_DELIVERY_SYSTEM:
+		r = dvbv5_set_delivery_system(fe, data);
+		break;
+	case DTV_VOLTAGE:
+		c->voltage = data;
+		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
+			(void *)c->voltage);
+		break;
+	case DTV_TONE:
+		c->sectone = data;
+		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
+			(void *)c->sectone);
+		break;
+	case DTV_CODE_RATE_HP:
+		c->code_rate_HP = data;
+		break;
+	case DTV_CODE_RATE_LP:
+		c->code_rate_LP = data;
+		break;
+	case DTV_GUARD_INTERVAL:
+		c->guard_interval = data;
+		break;
+	case DTV_TRANSMISSION_MODE:
+		c->transmission_mode = data;
+		break;
+	case DTV_HIERARCHY:
+		c->hierarchy = data;
+		break;
+	case DTV_INTERLEAVING:
+		c->interleaving = data;
+		break;
+
+	/* ISDB-T Support here */
+	case DTV_ISDBT_PARTIAL_RECEPTION:
+		c->isdbt_partial_reception = data;
+		break;
+	case DTV_ISDBT_SOUND_BROADCASTING:
+		c->isdbt_sb_mode = data;
+		break;
+	case DTV_ISDBT_SB_SUBCHANNEL_ID:
+		c->isdbt_sb_subchannel = data;
+		break;
+	case DTV_ISDBT_SB_SEGMENT_IDX:
+		c->isdbt_sb_segment_idx = data;
+		break;
+	case DTV_ISDBT_SB_SEGMENT_COUNT:
+		c->isdbt_sb_segment_count = data;
+		break;
+	case DTV_ISDBT_LAYER_ENABLED:
+		c->isdbt_layer_enabled = data;
+		break;
+	case DTV_ISDBT_LAYERA_FEC:
+		c->layer[0].fec = data;
+		break;
+	case DTV_ISDBT_LAYERA_MODULATION:
+		c->layer[0].modulation = data;
+		break;
+	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
+		c->layer[0].segment_count = data;
+		break;
+	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
+		c->layer[0].interleaving = data;
+		break;
+	case DTV_ISDBT_LAYERB_FEC:
+		c->layer[1].fec = data;
+		break;
+	case DTV_ISDBT_LAYERB_MODULATION:
+		c->layer[1].modulation = data;
+		break;
+	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
+		c->layer[1].segment_count = data;
+		break;
+	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
+		c->layer[1].interleaving = data;
+		break;
+	case DTV_ISDBT_LAYERC_FEC:
+		c->layer[2].fec = data;
+		break;
+	case DTV_ISDBT_LAYERC_MODULATION:
+		c->layer[2].modulation = data;
+		break;
+	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
+		c->layer[2].segment_count = data;
+		break;
+	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
+		c->layer[2].interleaving = data;
+		break;
+
+	/* Multistream support */
+	case DTV_STREAM_ID:
+	case DTV_DVBT2_PLP_ID_LEGACY:
+		c->stream_id = data;
+		break;
+
+	/* ATSC-MH */
+	case DTV_ATSCMH_PARADE_ID:
+		fe->dtv_property_cache.atscmh_parade_id = data;
+		break;
+	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
+		fe->dtv_property_cache.atscmh_rs_frame_ensemble = data;
+		break;
+
+	case DTV_LNA:
+		c->lna = data;
+		if (fe->ops.set_lna)
+			r = fe->ops.set_lna(fe);
+		if (r < 0)
+			c->lna = LNA_AUTO;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	return r;
+}
+
 static int dvb_frontend_ioctl(struct file *file,
 			unsigned int cmd, void *parg)
 {
@@ -1939,7 +2121,8 @@ static int dvb_frontend_ioctl(struct file *file,
 		return -EPERM;
 	}
 
-	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
+	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
+		|| (cmd == FE_SET_PROPERTY_SHORT))
 		err = dvb_frontend_ioctl_properties(file, cmd, parg);
 	else {
 		c->state = DTV_UNDEFINED;
@@ -2026,7 +2209,32 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			err = -EFAULT;
 			goto out;
 		}
-
+	/* New ioctl for optimizing property set
+	 */
+	} else if (cmd == FE_SET_PROPERTY_SHORT) {
+		struct dtv_property_short *tvp_short = NULL;
+		struct dtv_properties_short *tvps_short = parg;
+
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps_short->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps_short->props);
+		if ((!tvps_short->num) ||
+		(tvps_short->num > DTV_IOCTL_MAX_MSGS))
+			return -EINVAL;
+		tvp_short = memdup_user(tvps_short->props,
+		tvps_short->num * sizeof(*tvp_short));
+		if (IS_ERR(tvp_short))
+			return PTR_ERR(tvp_short);
+		for (i = 0; i < tvps_short->num; i++) {
+			err = dtv_property_short_process_set(fe, file,
+			(tvp_short + i)->cmd, (tvp_short + i)->data);
+			if (err < 0) {
+				kfree(tvp_short);
+				return err;
+			}
+		}
+		if (c->state == DTV_TUNE)
+			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n", __func__);
+		kfree(tvp_short);
 	} else
 		err = -EOPNOTSUPP;
 
diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
index 00a20cd..aa82179 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -476,6 +476,17 @@ struct dtv_property {
 	int result;
 } __attribute__ ((packed));
 
+/**
+ * @struct dtv_property_short
+ * A shorter version of struct dtv_property
+ * @cmd: Property type
+ * @data: Property value
+ */
+struct dtv_property_short {
+	__u32 cmd;
+	__u32 data;
+};
+
 /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
 #define DTV_IOCTL_MAX_MSGS 64
 
@@ -484,6 +495,18 @@ struct dtv_properties {
 	struct dtv_property *props;
 };
 
+/**
+ * @struct dtv_properties_short
+ * A variant of struct dtv_properties
+ * to support struct dtv_property_short
+ * @num: Number of properties
+ * @props: Pointer to struct dtv_property_short
+ */
+struct dtv_properties_short {
+	__u32 num;
+	struct dtv_property_short *props;
+};
+
 #if defined(__DVB_CORE__) || !defined (__KERNEL__)
 
 /*
@@ -565,6 +588,7 @@ struct dvb_frontend_event {
 
 #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
 #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
+#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
 
 /**
  * When set, this flag will disable any zigzagging or other "normal" tuning
-- 
2.7.4

Re: [PATCH v1] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Mauro Carvalho Chehab]

Em Fri, 15 Sep 2017 01:50:29 -0400
Satendra Singh Thakur <satendra.t@samsung.com> escreveu:

> Hello  Mr Chehab,
> Thanks for reviewing the patch.
> I have modified the patch as per your comments.
> Please check if it looks fine now.

Forgot to mention, but, for the patch series to be applied, you'll need to
also patch Documentation/media/uapi/dvb.

> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> ---
>  drivers/media/dvb-core/dvb_frontend.c | 212 +++++++++++++++++++++++++++++++++-
>  include/uapi/linux/dvb/frontend.h     |  24 ++++
>  2 files changed, 234 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index e3fff8f..bc35a86 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1914,6 +1914,188 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
>  	return r;
>  }
>  
> +/**
> + * dtv_property_short_process_set
> + * @fe: Pointer to struct dvb_frontend
> + * @file: Pointer to struct file
> + * @cmd: Property name
> + * @data: Property value
> + *
> + * helper function for dvb_frontend_ioctl_properties,
> + * which can be used to set dtv property using ioctl
> + * cmd FE_SET_PROPERTY_SHORT.
> + * It assigns property value to corresponding member of
> + * property-cache structure
> + * This func is a variant of the func dtv_property_process_set
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +static int dtv_property_short_process_set(struct dvb_frontend *fe,
> +					struct file *file,
> +					u32 cmd, u32 data)
> +{
> +	int r = 0;
> +	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> +	switch (cmd) {
> +	case DTV_CLEAR:
> +		/*
> +		 * Reset a cache of data specific to the frontend here. This
> +		 * does not effect hardware.
> +		 */
> +		dvb_frontend_clear_cache(fe);
> +		break;

No, you got me wrong. What I want is to have just *one* switch that
would work for both the normal and the short version of this ioctl.

In other words, I was expecting you to patch the existing
dtv_property_process_set() for it to take u32 cmd and u32 data as
arguments and to be used by both FE_SET_PROPERTY and FE_SET_PROPERTY_SHORT
handling.

> +	case DTV_TUNE:
> +		/* interpret the cache of data, build either a traditional
> +		 * frontend tune request so we can pass validation in the
> +		 * FE_SET_FRONTEND  ioctl.
> +		 */
> +		c->state = cmd;
> +		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
> +				__func__);
> +
> +		r = dtv_set_frontend(fe);
> +		break;
> +	case DTV_FREQUENCY:
> +		c->frequency = data;
> +		break;
> +	case DTV_MODULATION:
> +		c->modulation = data;
> +		break;
> +	case DTV_BANDWIDTH_HZ:
> +		c->bandwidth_hz = data;
> +		break;
> +	case DTV_INVERSION:
> +		c->inversion = data;
> +		break;
> +	case DTV_SYMBOL_RATE:
> +		c->symbol_rate = data;
> +		break;
> +	case DTV_INNER_FEC:
> +		c->fec_inner = data;
> +		break;
> +	case DTV_PILOT:
> +		c->pilot = data;
> +		break;
> +	case DTV_ROLLOFF:
> +		c->rolloff = data;
> +		break;
> +	case DTV_DELIVERY_SYSTEM:
> +		r = dvbv5_set_delivery_system(fe, data);
> +		break;
> +	case DTV_VOLTAGE:
> +		c->voltage = data;
> +		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
> +			(void *)c->voltage);
> +		break;
> +	case DTV_TONE:
> +		c->sectone = data;
> +		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
> +			(void *)c->sectone);
> +		break;
> +	case DTV_CODE_RATE_HP:
> +		c->code_rate_HP = data;
> +		break;
> +	case DTV_CODE_RATE_LP:
> +		c->code_rate_LP = data;
> +		break;
> +	case DTV_GUARD_INTERVAL:
> +		c->guard_interval = data;
> +		break;
> +	case DTV_TRANSMISSION_MODE:
> +		c->transmission_mode = data;
> +		break;
> +	case DTV_HIERARCHY:
> +		c->hierarchy = data;
> +		break;
> +	case DTV_INTERLEAVING:
> +		c->interleaving = data;
> +		break;
> +
> +	/* ISDB-T Support here */
> +	case DTV_ISDBT_PARTIAL_RECEPTION:
> +		c->isdbt_partial_reception = data;
> +		break;
> +	case DTV_ISDBT_SOUND_BROADCASTING:
> +		c->isdbt_sb_mode = data;
> +		break;
> +	case DTV_ISDBT_SB_SUBCHANNEL_ID:
> +		c->isdbt_sb_subchannel = data;
> +		break;
> +	case DTV_ISDBT_SB_SEGMENT_IDX:
> +		c->isdbt_sb_segment_idx = data;
> +		break;
> +	case DTV_ISDBT_SB_SEGMENT_COUNT:
> +		c->isdbt_sb_segment_count = data;
> +		break;
> +	case DTV_ISDBT_LAYER_ENABLED:
> +		c->isdbt_layer_enabled = data;
> +		break;
> +	case DTV_ISDBT_LAYERA_FEC:
> +		c->layer[0].fec = data;
> +		break;
> +	case DTV_ISDBT_LAYERA_MODULATION:
> +		c->layer[0].modulation = data;
> +		break;
> +	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
> +		c->layer[0].segment_count = data;
> +		break;
> +	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
> +		c->layer[0].interleaving = data;
> +		break;
> +	case DTV_ISDBT_LAYERB_FEC:
> +		c->layer[1].fec = data;
> +		break;
> +	case DTV_ISDBT_LAYERB_MODULATION:
> +		c->layer[1].modulation = data;
> +		break;
> +	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
> +		c->layer[1].segment_count = data;
> +		break;
> +	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
> +		c->layer[1].interleaving = data;
> +		break;
> +	case DTV_ISDBT_LAYERC_FEC:
> +		c->layer[2].fec = data;
> +		break;
> +	case DTV_ISDBT_LAYERC_MODULATION:
> +		c->layer[2].modulation = data;
> +		break;
> +	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
> +		c->layer[2].segment_count = data;
> +		break;
> +	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
> +		c->layer[2].interleaving = data;
> +		break;
> +
> +	/* Multistream support */
> +	case DTV_STREAM_ID:
> +	case DTV_DVBT2_PLP_ID_LEGACY:
> +		c->stream_id = data;
> +		break;
> +
> +	/* ATSC-MH */
> +	case DTV_ATSCMH_PARADE_ID:
> +		fe->dtv_property_cache.atscmh_parade_id = data;
> +		break;
> +	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
> +		fe->dtv_property_cache.atscmh_rs_frame_ensemble = data;
> +		break;
> +
> +	case DTV_LNA:
> +		c->lna = data;
> +		if (fe->ops.set_lna)
> +			r = fe->ops.set_lna(fe);
> +		if (r < 0)
> +			c->lna = LNA_AUTO;
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return r;
> +}
> +
>  static int dvb_frontend_ioctl(struct file *file,
>  			unsigned int cmd, void *parg)
>  {
> @@ -1939,7 +2121,8 @@ static int dvb_frontend_ioctl(struct file *file,
>  		return -EPERM;
>  	}
>  
> -	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
> +	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
> +		|| (cmd == FE_SET_PROPERTY_SHORT))
>  		err = dvb_frontend_ioctl_properties(file, cmd, parg);
>  	else {
>  		c->state = DTV_UNDEFINED;
> @@ -2026,7 +2209,32 @@ static int dvb_frontend_ioctl_properties(struct file *file,
>  			err = -EFAULT;
>  			goto out;
>  		}
> -
> +	/* New ioctl for optimizing property set
> +	 */
> +	} else if (cmd == FE_SET_PROPERTY_SHORT) {
> +		struct dtv_property_short *tvp_short = NULL;
> +		struct dtv_properties_short *tvps_short = parg;
> +
> +		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps_short->num);
> +		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps_short->props);
> +		if ((!tvps_short->num) ||
> +		(tvps_short->num > DTV_IOCTL_MAX_MSGS))
> +			return -EINVAL;
> +		tvp_short = memdup_user(tvps_short->props,
> +		tvps_short->num * sizeof(*tvp_short));
> +		if (IS_ERR(tvp_short))
> +			return PTR_ERR(tvp_short);
> +		for (i = 0; i < tvps_short->num; i++) {
> +			err = dtv_property_short_process_set(fe, file,
> +			(tvp_short + i)->cmd, (tvp_short + i)->data);
> +			if (err < 0) {
> +				kfree(tvp_short);
> +				return err;
> +			}
> +		}
> +		if (c->state == DTV_TUNE)
> +			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n", __func__);
> +		kfree(tvp_short);
>  	} else
>  		err = -EOPNOTSUPP;
>  
> diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
> index 00a20cd..aa82179 100644
> --- a/include/uapi/linux/dvb/frontend.h
> +++ b/include/uapi/linux/dvb/frontend.h
> @@ -476,6 +476,17 @@ struct dtv_property {
>  	int result;
>  } __attribute__ ((packed));
>  
> +/**
> + * @struct dtv_property_short
> + * A shorter version of struct dtv_property
> + * @cmd: Property type
> + * @data: Property value
> + */
> +struct dtv_property_short {
> +	__u32 cmd;
> +	__u32 data;
> +};
> +
>  /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
>  #define DTV_IOCTL_MAX_MSGS 64
>  
> @@ -484,6 +495,18 @@ struct dtv_properties {
>  	struct dtv_property *props;
>  };
>  
> +/**
> + * @struct dtv_properties_short
> + * A variant of struct dtv_properties
> + * to support struct dtv_property_short
> + * @num: Number of properties
> + * @props: Pointer to struct dtv_property_short
> + */
> +struct dtv_properties_short {
> +	__u32 num;
> +	struct dtv_property_short *props;
> +};
> +
>  #if defined(__DVB_CORE__) || !defined (__KERNEL__)
>  
>  /*
> @@ -565,6 +588,7 @@ struct dvb_frontend_event {
>  
>  #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
>  #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
> +#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
>  
>  /**
>   * When set, this flag will disable any zigzagging or other "normal" tuning



Thanks,
Mauro

[PATCH v2] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Satendra Singh Thakur]

Hello  Mr Chehab,
Thanks for the comments.
I have modified dtv_property_process_set and
also added documentation.
Please let me know if any further modifications required.

Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
---
 .../media/uapi/dvb/fe-set-property-short.rst       |  60 +++++++++
 Documentation/media/uapi/dvb/frontend_fcalls.rst   |   1 +
 drivers/media/dvb-core/dvb_frontend.c              | 143 +++++++++++++--------
 include/uapi/linux/dvb/frontend.h                  |  24 ++++
 4 files changed, 173 insertions(+), 55 deletions(-)
 create mode 100644 Documentation/media/uapi/dvb/fe-set-property-short.rst

diff --git a/Documentation/media/uapi/dvb/fe-set-property-short.rst b/Documentation/media/uapi/dvb/fe-set-property-short.rst
new file mode 100644
index 0000000..7da1e8d
--- /dev/null
+++ b/Documentation/media/uapi/dvb/fe-set-property-short.rst
@@ -0,0 +1,60 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _FE_SET_PROPERTY_SHORT:
+
+**************************************
+ioctl FE_SET_PROPERTY_SHORT
+**************************************
+
+Name
+====
+
+FE_SET_PROPERTY_SHORT  sets one or more frontend properties.
+
+
+Synopsis
+========
+
+
+.. c:function:: int ioctl( int fd, FE_SET_PROPERTY_SHORT, struct dtv_properties_short *argp )
+    :name: FE_SET_PROPERTY_SHORT
+
+
+Arguments
+=========
+
+``fd``
+    File descriptor returned by :ref:`open() <frontend_f_open>`.
+
+``argp``
+    pointer to struct :c:type:`dtv_properties_short`,
+	which is a shorter variant of struct dtv_properties.
+
+
+Description
+===========
+
+All DVB frontend devices support the ``FE_SET_PROPERTY_SHORT`` ioctl.
+This ioctl is a shorter variant of ioctl FE_SET_PROPERTY.
+The supported properties and statistics depend on the delivery system
+and on the device:
+
+-  ``FE_SET_PROPERTY_SHORT:``
+
+   -  This ioctl is used to set one or more frontend properties.
+
+   -  This is the basic command to request the frontend to tune into
+      some frequency and to start decoding the digital TV signal.
+
+   -  This call requires read/write access to the device.
+
+   -  At return, the values are updated to reflect the actual parameters
+      used.
+
+
+Return Value
+============
+
+On success 0 is returned, on error -1 and the ``errno`` variable is set
+appropriately. The generic error codes are described at the
+:ref:`Generic Error Codes <gen-errors>` chapter.
diff --git a/Documentation/media/uapi/dvb/frontend_fcalls.rst b/Documentation/media/uapi/dvb/frontend_fcalls.rst
index b03f9ca..a1246c6 100644
--- a/Documentation/media/uapi/dvb/frontend_fcalls.rst
+++ b/Documentation/media/uapi/dvb/frontend_fcalls.rst
@@ -14,6 +14,7 @@ Frontend Function Calls
     fe-get-info
     fe-read-status
     fe-get-property
+    fe-set-property-short
     fe-diseqc-reset-overload
     fe-diseqc-send-master-cmd
     fe-diseqc-recv-slave-reply
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index e3fff8f..6616474 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1738,23 +1738,28 @@ static int dvbv3_set_delivery_system(struct dvb_frontend *fe)
 	return emulate_delivery_system(fe, delsys);
 }
 
+/**
+ * dtv_property_process_set
+ * @fe: Pointer to struct dvb_frontend
+ * @file: Pointer to struct file
+ * @cmd: Property name
+ * @data: Property value
+ *
+ * helper function for dvb_frontend_ioctl_properties,
+ * which can be used to set a single dtv property
+ * It assigns property value to corresponding member of
+ * property-cache structure
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
 static int dtv_property_process_set(struct dvb_frontend *fe,
-				    struct dtv_property *tvp,
-				    struct file *file)
+					struct file *file,
+					u32 cmd, u32 data)
 {
 	int r = 0;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
 
-	/* Allow the frontend to validate incoming properties */
-	if (fe->ops.set_property) {
-		r = fe->ops.set_property(fe, tvp);
-		if (r < 0)
-			return r;
-	}
-
-	dtv_property_dump(fe, true, tvp);
-
-	switch(tvp->cmd) {
+	switch (cmd) {
 	case DTV_CLEAR:
 		/*
 		 * Reset a cache of data specific to the frontend here. This does
@@ -1767,140 +1772,140 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 		 * tunerequest so we can pass validation in the FE_SET_FRONTEND
 		 * ioctl.
 		 */
-		c->state = tvp->cmd;
+		c->state = cmd;
 		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
 				__func__);
 
 		r = dtv_set_frontend(fe);
 		break;
 	case DTV_FREQUENCY:
-		c->frequency = tvp->u.data;
+		c->frequency = data;
 		break;
 	case DTV_MODULATION:
-		c->modulation = tvp->u.data;
+		c->modulation = data;
 		break;
 	case DTV_BANDWIDTH_HZ:
-		c->bandwidth_hz = tvp->u.data;
+		c->bandwidth_hz = data;
 		break;
 	case DTV_INVERSION:
-		c->inversion = tvp->u.data;
+		c->inversion = data;
 		break;
 	case DTV_SYMBOL_RATE:
-		c->symbol_rate = tvp->u.data;
+		c->symbol_rate = data;
 		break;
 	case DTV_INNER_FEC:
-		c->fec_inner = tvp->u.data;
+		c->fec_inner = data;
 		break;
 	case DTV_PILOT:
-		c->pilot = tvp->u.data;
+		c->pilot = data;
 		break;
 	case DTV_ROLLOFF:
-		c->rolloff = tvp->u.data;
+		c->rolloff = data;
 		break;
 	case DTV_DELIVERY_SYSTEM:
-		r = dvbv5_set_delivery_system(fe, tvp->u.data);
+		r = dvbv5_set_delivery_system(fe, data);
 		break;
 	case DTV_VOLTAGE:
-		c->voltage = tvp->u.data;
+		c->voltage = data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
 			(void *)c->voltage);
 		break;
 	case DTV_TONE:
-		c->sectone = tvp->u.data;
+		c->sectone = data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
 			(void *)c->sectone);
 		break;
 	case DTV_CODE_RATE_HP:
-		c->code_rate_HP = tvp->u.data;
+		c->code_rate_HP = data;
 		break;
 	case DTV_CODE_RATE_LP:
-		c->code_rate_LP = tvp->u.data;
+		c->code_rate_LP = data;
 		break;
 	case DTV_GUARD_INTERVAL:
-		c->guard_interval = tvp->u.data;
+		c->guard_interval = data;
 		break;
 	case DTV_TRANSMISSION_MODE:
-		c->transmission_mode = tvp->u.data;
+		c->transmission_mode = data;
 		break;
 	case DTV_HIERARCHY:
-		c->hierarchy = tvp->u.data;
+		c->hierarchy = data;
 		break;
 	case DTV_INTERLEAVING:
-		c->interleaving = tvp->u.data;
+		c->interleaving = data;
 		break;
 
 	/* ISDB-T Support here */
 	case DTV_ISDBT_PARTIAL_RECEPTION:
-		c->isdbt_partial_reception = tvp->u.data;
+		c->isdbt_partial_reception = data;
 		break;
 	case DTV_ISDBT_SOUND_BROADCASTING:
-		c->isdbt_sb_mode = tvp->u.data;
+		c->isdbt_sb_mode = data;
 		break;
 	case DTV_ISDBT_SB_SUBCHANNEL_ID:
-		c->isdbt_sb_subchannel = tvp->u.data;
+		c->isdbt_sb_subchannel = data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_IDX:
-		c->isdbt_sb_segment_idx = tvp->u.data;
+		c->isdbt_sb_segment_idx = data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_COUNT:
-		c->isdbt_sb_segment_count = tvp->u.data;
+		c->isdbt_sb_segment_count = data;
 		break;
 	case DTV_ISDBT_LAYER_ENABLED:
-		c->isdbt_layer_enabled = tvp->u.data;
+		c->isdbt_layer_enabled = data;
 		break;
 	case DTV_ISDBT_LAYERA_FEC:
-		c->layer[0].fec = tvp->u.data;
+		c->layer[0].fec = data;
 		break;
 	case DTV_ISDBT_LAYERA_MODULATION:
-		c->layer[0].modulation = tvp->u.data;
+		c->layer[0].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
-		c->layer[0].segment_count = tvp->u.data;
+		c->layer[0].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
-		c->layer[0].interleaving = tvp->u.data;
+		c->layer[0].interleaving = data;
 		break;
 	case DTV_ISDBT_LAYERB_FEC:
-		c->layer[1].fec = tvp->u.data;
+		c->layer[1].fec = data;
 		break;
 	case DTV_ISDBT_LAYERB_MODULATION:
-		c->layer[1].modulation = tvp->u.data;
+		c->layer[1].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
-		c->layer[1].segment_count = tvp->u.data;
+		c->layer[1].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
-		c->layer[1].interleaving = tvp->u.data;
+		c->layer[1].interleaving = data;
 		break;
 	case DTV_ISDBT_LAYERC_FEC:
-		c->layer[2].fec = tvp->u.data;
+		c->layer[2].fec = data;
 		break;
 	case DTV_ISDBT_LAYERC_MODULATION:
-		c->layer[2].modulation = tvp->u.data;
+		c->layer[2].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
-		c->layer[2].segment_count = tvp->u.data;
+		c->layer[2].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
-		c->layer[2].interleaving = tvp->u.data;
+		c->layer[2].interleaving = data;
 		break;
 
 	/* Multistream support */
 	case DTV_STREAM_ID:
 	case DTV_DVBT2_PLP_ID_LEGACY:
-		c->stream_id = tvp->u.data;
+		c->stream_id = data;
 		break;
 
 	/* ATSC-MH */
 	case DTV_ATSCMH_PARADE_ID:
-		fe->dtv_property_cache.atscmh_parade_id = tvp->u.data;
+		fe->dtv_property_cache.atscmh_parade_id = data;
 		break;
 	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
-		fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
+		fe->dtv_property_cache.atscmh_rs_frame_ensemble = data;
 		break;
 
 	case DTV_LNA:
-		c->lna = tvp->u.data;
+		c->lna = data;
 		if (fe->ops.set_lna)
 			r = fe->ops.set_lna(fe);
 		if (r < 0)
@@ -1914,6 +1919,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 	return r;
 }
 
+
 static int dvb_frontend_ioctl(struct file *file,
 			unsigned int cmd, void *parg)
 {
@@ -1939,7 +1945,8 @@ static int dvb_frontend_ioctl(struct file *file,
 		return -EPERM;
 	}
 
-	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
+	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
+		|| (cmd == FE_SET_PROPERTY_SHORT))
 		err = dvb_frontend_ioctl_properties(file, cmd, parg);
 	else {
 		c->state = DTV_UNDEFINED;
@@ -1979,7 +1986,8 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			return PTR_ERR(tvp);
 
 		for (i = 0; i < tvps->num; i++) {
-			err = dtv_property_process_set(fe, tvp + i, file);
+			err = dtv_property_process_set(fe, file,
+			(tvp + i)->cmd, (tvp + i)->u.data);
 			if (err < 0)
 				goto out;
 			(tvp + i)->result = err;
@@ -2026,7 +2034,32 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			err = -EFAULT;
 			goto out;
 		}
-
+	/* New ioctl for optimizing property set
+	 */
+	} else if (cmd == FE_SET_PROPERTY_SHORT) {
+		struct dtv_property_short *tvp_short = NULL;
+		struct dtv_properties_short *tvps_short = parg;
+
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps_short->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps_short->props);
+		if ((!tvps_short->num) ||
+		(tvps_short->num > DTV_IOCTL_MAX_MSGS))
+			return -EINVAL;
+		tvp_short = memdup_user(tvps_short->props,
+		tvps_short->num * sizeof(*tvp_short));
+		if (IS_ERR(tvp_short))
+			return PTR_ERR(tvp_short);
+		for (i = 0; i < tvps_short->num; i++) {
+			err = dtv_property_process_set(fe, file,
+			(tvp_short + i)->cmd, (tvp_short + i)->data);
+			if (err < 0) {
+				kfree(tvp_short);
+				return err;
+			}
+		}
+		if (c->state == DTV_TUNE)
+			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n", __func__);
+		kfree(tvp_short);
 	} else
 		err = -EOPNOTSUPP;
 
diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
index 00a20cd..aa82179 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -476,6 +476,17 @@ struct dtv_property {
 	int result;
 } __attribute__ ((packed));
 
+/**
+ * @struct dtv_property_short
+ * A shorter version of struct dtv_property
+ * @cmd: Property type
+ * @data: Property value
+ */
+struct dtv_property_short {
+	__u32 cmd;
+	__u32 data;
+};
+
 /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
 #define DTV_IOCTL_MAX_MSGS 64
 
@@ -484,6 +495,18 @@ struct dtv_properties {
 	struct dtv_property *props;
 };
 
+/**
+ * @struct dtv_properties_short
+ * A variant of struct dtv_properties
+ * to support struct dtv_property_short
+ * @num: Number of properties
+ * @props: Pointer to struct dtv_property_short
+ */
+struct dtv_properties_short {
+	__u32 num;
+	struct dtv_property_short *props;
+};
+
 #if defined(__DVB_CORE__) || !defined (__KERNEL__)
 
 /*
@@ -565,6 +588,7 @@ struct dvb_frontend_event {
 
 #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
 #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
+#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
 
 /**
  * When set, this flag will disable any zigzagging or other "normal" tuning
-- 
2.7.4

Re: [PATCH v2] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Mauro Carvalho Chehab]

Em Fri, 15 Sep 2017 06:18:20 -0400
Satendra Singh Thakur <satendra.t@samsung.com> escreveu:

> Hello  Mr Chehab,
> Thanks for the comments.
> I have modified dtv_property_process_set and
> also added documentation.
> Please let me know if any further modifications required.

That's a way better. Just a few minor things to modify for it to be
ready for merging.

> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> ---
>  .../media/uapi/dvb/fe-set-property-short.rst       |  60 +++++++++
>  Documentation/media/uapi/dvb/frontend_fcalls.rst   |   1 +
>  drivers/media/dvb-core/dvb_frontend.c              | 143 +++++++++++++--------
>  include/uapi/linux/dvb/frontend.h                  |  24 ++++
>  4 files changed, 173 insertions(+), 55 deletions(-)
>  create mode 100644 Documentation/media/uapi/dvb/fe-set-property-short.rst
> 
> diff --git a/Documentation/media/uapi/dvb/fe-set-property-short.rst b/Documentation/media/uapi/dvb/fe-set-property-short.rst
> new file mode 100644
> index 0000000..7da1e8d
> --- /dev/null
> +++ b/Documentation/media/uapi/dvb/fe-set-property-short.rst
> @@ -0,0 +1,60 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _FE_SET_PROPERTY_SHORT:
> +
> +**************************************
> +ioctl FE_SET_PROPERTY_SHORT
> +**************************************
> +
> +Name
> +====
> +
> +FE_SET_PROPERTY_SHORT  sets one or more frontend properties.
> +
> +
> +Synopsis
> +========
> +
> +
> +.. c:function:: int ioctl( int fd, FE_SET_PROPERTY_SHORT, struct dtv_properties_short *argp )
> +    :name: FE_SET_PROPERTY_SHORT
> +
> +
> +Arguments
> +=========
> +
> +``fd``
> +    File descriptor returned by :ref:`open() <frontend_f_open>`.
> +
> +``argp``
> +    pointer to struct :c:type:`dtv_properties_short`,
> +	which is a shorter variant of struct dtv_properties.
> +
> +
> +Description
> +===========
> +
> +All DVB frontend devices support the ``FE_SET_PROPERTY_SHORT`` ioctl.
> +This ioctl is a shorter variant of ioctl FE_SET_PROPERTY.

Please use, instead:

	:ref:`FE_SET_PROPERTY`

in order to generate cross-reference with the other ioctl.

Also, please add a note at fe-set-property.rst mentioning the
FE_SET_PROPERTY_SHORT variant.

Btw, I'm actually in doubt if the best wouldn't be to just add
this new ioctl to fe-get-property.rst.

> +The supported properties and statistics depend on the delivery system
> +and on the device:
> +
> +-  ``FE_SET_PROPERTY_SHORT:``
> +
> +   -  This ioctl is used to set one or more frontend properties.
> +
> +   -  This is the basic command to request the frontend to tune into
> +      some frequency and to start decoding the digital TV signal.
> +
> +   -  This call requires read/write access to the device.
> +
> +   -  At return, the values are updated to reflect the actual parameters
> +      used.
> +
> +
> +Return Value
> +============
> +
> +On success 0 is returned, on error -1 and the ``errno`` variable is set
> +appropriately. The generic error codes are described at the
> +:ref:`Generic Error Codes <gen-errors>` chapter.
> diff --git a/Documentation/media/uapi/dvb/frontend_fcalls.rst b/Documentation/media/uapi/dvb/frontend_fcalls.rst
> index b03f9ca..a1246c6 100644
> --- a/Documentation/media/uapi/dvb/frontend_fcalls.rst
> +++ b/Documentation/media/uapi/dvb/frontend_fcalls.rst
> @@ -14,6 +14,7 @@ Frontend Function Calls
>      fe-get-info
>      fe-read-status
>      fe-get-property
> +    fe-set-property-short
>      fe-diseqc-reset-overload
>      fe-diseqc-send-master-cmd
>      fe-diseqc-recv-slave-reply
> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index e3fff8f..6616474 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1738,23 +1738,28 @@ static int dvbv3_set_delivery_system(struct dvb_frontend *fe)
>  	return emulate_delivery_system(fe, delsys);
>  }
>  
> +/**
> + * dtv_property_process_set

Nitpick: please add a short description for the function.

> + * @fe: Pointer to struct dvb_frontend
> + * @file: Pointer to struct file
> + * @cmd: Property name
> + * @data: Property value
> + *
> + * helper function for dvb_frontend_ioctl_properties,
> + * which can be used to set a single dtv property
> + * It assigns property value to corresponding member of
> + * property-cache structure
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
>  static int dtv_property_process_set(struct dvb_frontend *fe,
> -				    struct dtv_property *tvp,
> -				    struct file *file)
> +					struct file *file,
> +					u32 cmd, u32 data)
>  {
>  	int r = 0;
>  	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
>  
> -	/* Allow the frontend to validate incoming properties */
> -	if (fe->ops.set_property) {
> -		r = fe->ops.set_property(fe, tvp);
> -		if (r < 0)
> -			return r;
> -	}
> -
> -	dtv_property_dump(fe, true, tvp);
> -

It makes sense to be able to also debug FE_SET_FRONTEND_SHORT. As now
the we don't have a struct dtv_property pointer here anymore, we won't
be able to share this function anymore with FE_GET_FRONTEND.

So, we'll need to rename the existing function to dtv_get_property_dump(),
removing the second argument, and add here something like:

        if (!cmd || cmd > DTV_MAX_COMMAND)
                dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
                         __func__, cmd);
        else
		dev_dbg(fe->dvb->device,
			"%s: SET cmd 0x%08x (%s) to 0x%08x\n",
			__func__, cmd, dtv_cmds[cmd].name, data);

> -	switch(tvp->cmd) {
> +	switch (cmd) {
>  	case DTV_CLEAR:
>  		/*
>  		 * Reset a cache of data specific to the frontend here. This does
> @@ -1767,140 +1772,140 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
>  		 * tunerequest so we can pass validation in the FE_SET_FRONTEND
>  		 * ioctl.
>  		 */
> -		c->state = tvp->cmd;
> +		c->state = cmd;
>  		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
>  				__func__);
>  
>  		r = dtv_set_frontend(fe);
>  		break;
>  	case DTV_FREQUENCY:
> -		c->frequency = tvp->u.data;
> +		c->frequency = data;
>  		break;
>  	case DTV_MODULATION:
> -		c->modulation = tvp->u.data;
> +		c->modulation = data;
>  		break;
>  	case DTV_BANDWIDTH_HZ:
> -		c->bandwidth_hz = tvp->u.data;
> +		c->bandwidth_hz = data;
>  		break;
>  	case DTV_INVERSION:
> -		c->inversion = tvp->u.data;
> +		c->inversion = data;
>  		break;
>  	case DTV_SYMBOL_RATE:
> -		c->symbol_rate = tvp->u.data;
> +		c->symbol_rate = data;
>  		break;
>  	case DTV_INNER_FEC:
> -		c->fec_inner = tvp->u.data;
> +		c->fec_inner = data;
>  		break;
>  	case DTV_PILOT:
> -		c->pilot = tvp->u.data;
> +		c->pilot = data;
>  		break;
>  	case DTV_ROLLOFF:
> -		c->rolloff = tvp->u.data;
> +		c->rolloff = data;
>  		break;
>  	case DTV_DELIVERY_SYSTEM:
> -		r = dvbv5_set_delivery_system(fe, tvp->u.data);
> +		r = dvbv5_set_delivery_system(fe, data);
>  		break;
>  	case DTV_VOLTAGE:
> -		c->voltage = tvp->u.data;
> +		c->voltage = data;
>  		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
>  			(void *)c->voltage);
>  		break;
>  	case DTV_TONE:
> -		c->sectone = tvp->u.data;
> +		c->sectone = data;
>  		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
>  			(void *)c->sectone);
>  		break;
>  	case DTV_CODE_RATE_HP:
> -		c->code_rate_HP = tvp->u.data;
> +		c->code_rate_HP = data;
>  		break;
>  	case DTV_CODE_RATE_LP:
> -		c->code_rate_LP = tvp->u.data;
> +		c->code_rate_LP = data;
>  		break;
>  	case DTV_GUARD_INTERVAL:
> -		c->guard_interval = tvp->u.data;
> +		c->guard_interval = data;
>  		break;
>  	case DTV_TRANSMISSION_MODE:
> -		c->transmission_mode = tvp->u.data;
> +		c->transmission_mode = data;
>  		break;
>  	case DTV_HIERARCHY:
> -		c->hierarchy = tvp->u.data;
> +		c->hierarchy = data;
>  		break;
>  	case DTV_INTERLEAVING:
> -		c->interleaving = tvp->u.data;
> +		c->interleaving = data;
>  		break;
>  
>  	/* ISDB-T Support here */
>  	case DTV_ISDBT_PARTIAL_RECEPTION:
> -		c->isdbt_partial_reception = tvp->u.data;
> +		c->isdbt_partial_reception = data;
>  		break;
>  	case DTV_ISDBT_SOUND_BROADCASTING:
> -		c->isdbt_sb_mode = tvp->u.data;
> +		c->isdbt_sb_mode = data;
>  		break;
>  	case DTV_ISDBT_SB_SUBCHANNEL_ID:
> -		c->isdbt_sb_subchannel = tvp->u.data;
> +		c->isdbt_sb_subchannel = data;
>  		break;
>  	case DTV_ISDBT_SB_SEGMENT_IDX:
> -		c->isdbt_sb_segment_idx = tvp->u.data;
> +		c->isdbt_sb_segment_idx = data;
>  		break;
>  	case DTV_ISDBT_SB_SEGMENT_COUNT:
> -		c->isdbt_sb_segment_count = tvp->u.data;
> +		c->isdbt_sb_segment_count = data;
>  		break;
>  	case DTV_ISDBT_LAYER_ENABLED:
> -		c->isdbt_layer_enabled = tvp->u.data;
> +		c->isdbt_layer_enabled = data;
>  		break;
>  	case DTV_ISDBT_LAYERA_FEC:
> -		c->layer[0].fec = tvp->u.data;
> +		c->layer[0].fec = data;
>  		break;
>  	case DTV_ISDBT_LAYERA_MODULATION:
> -		c->layer[0].modulation = tvp->u.data;
> +		c->layer[0].modulation = data;
>  		break;
>  	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
> -		c->layer[0].segment_count = tvp->u.data;
> +		c->layer[0].segment_count = data;
>  		break;
>  	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
> -		c->layer[0].interleaving = tvp->u.data;
> +		c->layer[0].interleaving = data;
>  		break;
>  	case DTV_ISDBT_LAYERB_FEC:
> -		c->layer[1].fec = tvp->u.data;
> +		c->layer[1].fec = data;
>  		break;
>  	case DTV_ISDBT_LAYERB_MODULATION:
> -		c->layer[1].modulation = tvp->u.data;
> +		c->layer[1].modulation = data;
>  		break;
>  	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
> -		c->layer[1].segment_count = tvp->u.data;
> +		c->layer[1].segment_count = data;
>  		break;
>  	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
> -		c->layer[1].interleaving = tvp->u.data;
> +		c->layer[1].interleaving = data;
>  		break;
>  	case DTV_ISDBT_LAYERC_FEC:
> -		c->layer[2].fec = tvp->u.data;
> +		c->layer[2].fec = data;
>  		break;
>  	case DTV_ISDBT_LAYERC_MODULATION:
> -		c->layer[2].modulation = tvp->u.data;
> +		c->layer[2].modulation = data;
>  		break;
>  	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
> -		c->layer[2].segment_count = tvp->u.data;
> +		c->layer[2].segment_count = data;
>  		break;
>  	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
> -		c->layer[2].interleaving = tvp->u.data;
> +		c->layer[2].interleaving = data;
>  		break;
>  
>  	/* Multistream support */
>  	case DTV_STREAM_ID:
>  	case DTV_DVBT2_PLP_ID_LEGACY:
> -		c->stream_id = tvp->u.data;
> +		c->stream_id = data;
>  		break;
>  
>  	/* ATSC-MH */
>  	case DTV_ATSCMH_PARADE_ID:
> -		fe->dtv_property_cache.atscmh_parade_id = tvp->u.data;
> +		fe->dtv_property_cache.atscmh_parade_id = data;
>  		break;
>  	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
> -		fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
> +		fe->dtv_property_cache.atscmh_rs_frame_ensemble = data;
>  		break;
>  
>  	case DTV_LNA:
> -		c->lna = tvp->u.data;
> +		c->lna = data;
>  		if (fe->ops.set_lna)
>  			r = fe->ops.set_lna(fe);
>  		if (r < 0)


> @@ -1914,6 +1919,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
>  	return r;
>  }
>  
> +
>  static int dvb_frontend_ioctl(struct file *file,
>  			unsigned int cmd, void *parg)
>  {

Nitpick: please remove this hunk. No need to add an extra blank line here.

> @@ -1939,7 +1945,8 @@ static int dvb_frontend_ioctl(struct file *file,
>  		return -EPERM;
>  	}
>  
> -	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
> +	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
> +		|| (cmd == FE_SET_PROPERTY_SHORT))

Nitpick: adjust alignment for the second line to:

	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
	    || (cmd == FE_SET_PROPERTY_SHORT))


>  		err = dvb_frontend_ioctl_properties(file, cmd, parg);
>  	else {
>  		c->state = DTV_UNDEFINED;
> @@ -1979,7 +1986,8 @@ static int dvb_frontend_ioctl_properties(struct file *file,
>  			return PTR_ERR(tvp);
>  
>  		for (i = 0; i < tvps->num; i++) {
> -			err = dtv_property_process_set(fe, tvp + i, file);
> +			err = dtv_property_process_set(fe, file,
> +			(tvp + i)->cmd, (tvp + i)->u.data);

Nitpick: adjust alignment:

			err = dtv_property_process_set(fe, file,
						       (tvp + i)->cmd,
						       (tvp + i)->u.data);


>  			if (err < 0)
>  				goto out;
>  			(tvp + i)->result = err;
> @@ -2026,7 +2034,32 @@ static int dvb_frontend_ioctl_properties(struct file *file,
>  			err = -EFAULT;
>  			goto out;
>  		}
> -
> +	/* New ioctl for optimizing property set
> +	 */

Just use a single line for the comment. The above violates Kernel 
coding style, e. g.:

	/* New ioctl for optimizing property set */

> +	} else if (cmd == FE_SET_PROPERTY_SHORT) {
> +		struct dtv_property_short *tvp_short = NULL;
> +		struct dtv_properties_short *tvps_short = parg;
> +
> +		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps_short->num);
> +		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps_short->props);

Nitpick: avoid lines with more than 80 columns. In this specific case:

		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
			__func__, tvps_short->num);
		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
			__func__, tvps_short->props);


> +		if ((!tvps_short->num) ||
> +		(tvps_short->num > DTV_IOCTL_MAX_MSGS))

Adjust alignment.

> +			return -EINVAL;
> +		tvp_short = memdup_user(tvps_short->props,
> +		tvps_short->num * sizeof(*tvp_short));
> +		if (IS_ERR(tvp_short))
> +			return PTR_ERR(tvp_short);
> +		for (i = 0; i < tvps_short->num; i++) {
> +			err = dtv_property_process_set(fe, file,
> +			(tvp_short + i)->cmd, (tvp_short + i)->data);

Adjust alignment.

> +			if (err < 0) {
> +				kfree(tvp_short);
> +				return err;
> +			}
> +		}
> +		if (c->state == DTV_TUNE)
> +			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n", __func__);
> +		kfree(tvp_short);
>  	} else
>  		err = -EOPNOTSUPP;
>  
> diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
> index 00a20cd..aa82179 100644
> --- a/include/uapi/linux/dvb/frontend.h
> +++ b/include/uapi/linux/dvb/frontend.h
> @@ -476,6 +476,17 @@ struct dtv_property {
>  	int result;
>  } __attribute__ ((packed));
>  
> +/**
> + * @struct dtv_property_short
> + * A shorter version of struct dtv_property

Use &struct dtv_property, in order to generate cross-references at html/pdf
output. Also, in order to keep it coherent with other descriptions, please
an hyphen and add a blank line after description.

> + * @cmd: Property type
> + * @data: Property value

Please use the same terms as defined at struct dtv_property description.

E. g. the kernel-doc markup should be something like:

/**
 * @struct dtv_property_short - A shorter version of &struct dtv_property
 *
 * @cmd:	Digital TV command.
 * @data:	An unsigned 32-bits number.
 */

PS.: Don't forget to test if the produced output is ok with:

	make htmldocs

you'll need to install Sphinx for it to work. If you're writing the 
patches against the latest Linus tree, if you call it without having
Sphinx, a helper script will be called and will print the
commands to install it.

The output for those kernel-doc markups will be at:

	Documentation/output/media/uapi/dvb/frontend-header.html

> + */
> +struct dtv_property_short {
> +	__u32 cmd;
> +	__u32 data;
> +};
> +
>  /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
>  #define DTV_IOCTL_MAX_MSGS 64
>  
> @@ -484,6 +495,18 @@ struct dtv_properties {
>  	struct dtv_property *props;
>  };
>  
> +/**
> + * @struct dtv_properties_short
> + * A variant of struct dtv_properties
> + * to support struct dtv_property_short
> + * @num: Number of properties
> + * @props: Pointer to struct dtv_property_short
> + */

Same comments I did for the previous kernel-doc tags apply here.

> +struct dtv_properties_short {
> +	__u32 num;
> +	struct dtv_property_short *props;
> +};
> +
>  #if defined(__DVB_CORE__) || !defined (__KERNEL__)
>  
>  /*
> @@ -565,6 +588,7 @@ struct dvb_frontend_event {
>  
>  #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
>  #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
> +#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
>  
>  /**
>   * When set, this flag will disable any zigzagging or other "normal" tuning



Thanks,
Mauro

[PATCH v3] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Satendra Singh Thakur]

Hello  Mr Chehab,
Thanks for the review.
I have modified the code and documentation as per your comments.
Please let me know if this patch is ready for merging.

Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
---
 Documentation/media/uapi/dvb/fe-get-property.rst |  24 ++-
 drivers/media/dvb-core/dvb_frontend.c            | 191 ++++++++++++++---------
 include/uapi/linux/dvb/frontend.h                |  24 +++
 3 files changed, 158 insertions(+), 81 deletions(-)

diff --git a/Documentation/media/uapi/dvb/fe-get-property.rst b/Documentation/media/uapi/dvb/fe-get-property.rst
index 015d4db..b63a588 100644
--- a/Documentation/media/uapi/dvb/fe-get-property.rst
+++ b/Documentation/media/uapi/dvb/fe-get-property.rst
@@ -2,14 +2,14 @@
 
 .. _FE_GET_PROPERTY:
 
-**************************************
-ioctl FE_SET_PROPERTY, FE_GET_PROPERTY
-**************************************
+**************************************************************
+ioctl FE_SET_PROPERTY, FE_GET_PROPERTY, FE_SET_PROPERTY_SHORT
+**************************************************************
 
 Name
 ====
 
-FE_SET_PROPERTY - FE_GET_PROPERTY - FE_SET_PROPERTY sets one or more frontend properties. - FE_GET_PROPERTY returns one or more frontend properties.
+FE_SET_PROPERTY and FE_SET_PROPERTY_SHORT set one or more frontend properties. FE_GET_PROPERTY returns one or more frontend properties.
 
 
 Synopsis
@@ -21,6 +21,8 @@ Synopsis
 .. c:function:: int ioctl( int fd, FE_SET_PROPERTY, struct dtv_properties *argp )
     :name: FE_SET_PROPERTY
 
+.. c:function:: int ioctl( int fd, FE_SET_PROPERTY_SHORT, struct dtv_properties_short *argp )
+    :name: FE_SET_PROPERTY_SHORT
 
 Arguments
 =========
@@ -29,15 +31,16 @@ Arguments
     File descriptor returned by :ref:`open() <frontend_f_open>`.
 
 ``argp``
-    pointer to struct :c:type:`dtv_properties`
+    Pointer to struct :c:type:`dtv_properties` or
+	struct :c:type:`dtv_properties_short`.
 
 
 Description
 ===========
 
-All DVB frontend devices support the ``FE_SET_PROPERTY`` and
-``FE_GET_PROPERTY`` ioctls. The supported properties and statistics
-depends on the delivery system and on the device:
+All DVB frontend devices support the ``FE_SET_PROPERTY``, ``FE_GET_PROPERTY``
+and ``FE_SET_PROPERTY_SHORT`` ioctls. The supported  properties and
+statistics depends on the delivery system and on the device:
 
 -  ``FE_SET_PROPERTY:``
 
@@ -60,6 +63,11 @@ depends on the delivery system and on the device:
 
    -  This call only requires read-only access to the device.
 
+-  ``FE_SET_PROPERTY_SHORT:``
+
+   -  This ioctl is similar to FE_SET_PROPERTY ioctl mentioned above
+      except that the arguments of the former utilize a struct :c:type:`dtv_property_short`
+      which is smaller in size.
 
 Return Value
 ============
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index e3fff8f..3e49127 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1081,28 +1081,25 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = {
 	_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
 };
 
-static void dtv_property_dump(struct dvb_frontend *fe,
-			      bool is_set,
+static void dtv_get_property_dump(struct dvb_frontend *fe,
 			      struct dtv_property *tvp)
 {
 	int i;
 
 	if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
-		dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
-				__func__,
-				is_set ? "SET" : "GET",
-				tvp->cmd);
+		dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
+				 , __func__,
+				 tvp->cmd);
 		return;
 	}
 
-	dev_dbg(fe->dvb->device, "%s: %s tvp.cmd    = 0x%08x (%s)\n", __func__,
-		is_set ? "SET" : "GET",
-		tvp->cmd,
-		dtv_cmds[tvp->cmd].name);
+	dev_dbg(fe->dvb->device, "%s: GET tvp.cmd    = 0x%08x (%s)\n", __func__,
+		   	tvp->cmd,
+		   	dtv_cmds[tvp->cmd].name);
 
 	if (dtv_cmds[tvp->cmd].buffer) {
 		dev_dbg(fe->dvb->device, "%s: tvp.u.buffer.len = 0x%02x\n",
-			__func__, tvp->u.buffer.len);
+				__func__, tvp->u.buffer.len);
 
 		for(i = 0; i < tvp->u.buffer.len; i++)
 			dev_dbg(fe->dvb->device,
@@ -1515,7 +1512,7 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 			return r;
 	}
 
-	dtv_property_dump(fe, false, tvp);
+	dtv_get_property_dump(fe, tvp);
 
 	return 0;
 }
@@ -1738,23 +1735,35 @@ static int dvbv3_set_delivery_system(struct dvb_frontend *fe)
 	return emulate_delivery_system(fe, delsys);
 }
 
+/**
+ * dtv_property_process_set -  Sets a single DTV property
+ * @fe:		Pointer to &struct dvb_frontend
+ * @file:	Pointer to &struct file
+ * @cmd:	Digital TV command
+ * @data:	An unsigned 32-bits number
+ *
+ * This routine assigns the property
+ * value to the corresponding member of
+ * &struct dtv_frontend_properties
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
 static int dtv_property_process_set(struct dvb_frontend *fe,
-				    struct dtv_property *tvp,
-				    struct file *file)
+					struct file *file,
+					u32 cmd, u32 data)
 {
 	int r = 0;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
-
-	/* Allow the frontend to validate incoming properties */
-	if (fe->ops.set_property) {
-		r = fe->ops.set_property(fe, tvp);
-		if (r < 0)
-			return r;
-	}
-
-	dtv_property_dump(fe, true, tvp);
-
-	switch(tvp->cmd) {
+	/** Dump DTV command name and value*/
+	if (!cmd || cmd > DTV_MAX_COMMAND)
+		dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
+				 __func__, cmd);
+	else
+		dev_dbg(fe->dvb->device,
+				"%s: SET cmd 0x%08x (%s) to 0x%08x\n",
+				__func__, cmd, dtv_cmds[cmd].name, data);
+	switch (cmd) {
 	case DTV_CLEAR:
 		/*
 		 * Reset a cache of data specific to the frontend here. This does
@@ -1767,140 +1776,140 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 		 * tunerequest so we can pass validation in the FE_SET_FRONTEND
 		 * ioctl.
 		 */
-		c->state = tvp->cmd;
+		c->state = cmd;
 		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
 				__func__);
 
 		r = dtv_set_frontend(fe);
 		break;
 	case DTV_FREQUENCY:
-		c->frequency = tvp->u.data;
+		c->frequency = data;
 		break;
 	case DTV_MODULATION:
-		c->modulation = tvp->u.data;
+		c->modulation = data;
 		break;
 	case DTV_BANDWIDTH_HZ:
-		c->bandwidth_hz = tvp->u.data;
+		c->bandwidth_hz = data;
 		break;
 	case DTV_INVERSION:
-		c->inversion = tvp->u.data;
+		c->inversion = data;
 		break;
 	case DTV_SYMBOL_RATE:
-		c->symbol_rate = tvp->u.data;
+		c->symbol_rate = data;
 		break;
 	case DTV_INNER_FEC:
-		c->fec_inner = tvp->u.data;
+		c->fec_inner = data;
 		break;
 	case DTV_PILOT:
-		c->pilot = tvp->u.data;
+		c->pilot = data;
 		break;
 	case DTV_ROLLOFF:
-		c->rolloff = tvp->u.data;
+		c->rolloff = data;
 		break;
 	case DTV_DELIVERY_SYSTEM:
-		r = dvbv5_set_delivery_system(fe, tvp->u.data);
+		r = dvbv5_set_delivery_system(fe, data);
 		break;
 	case DTV_VOLTAGE:
-		c->voltage = tvp->u.data;
+		c->voltage = data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
 			(void *)c->voltage);
 		break;
 	case DTV_TONE:
-		c->sectone = tvp->u.data;
+		c->sectone = data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
 			(void *)c->sectone);
 		break;
 	case DTV_CODE_RATE_HP:
-		c->code_rate_HP = tvp->u.data;
+		c->code_rate_HP = data;
 		break;
 	case DTV_CODE_RATE_LP:
-		c->code_rate_LP = tvp->u.data;
+		c->code_rate_LP = data;
 		break;
 	case DTV_GUARD_INTERVAL:
-		c->guard_interval = tvp->u.data;
+		c->guard_interval = data;
 		break;
 	case DTV_TRANSMISSION_MODE:
-		c->transmission_mode = tvp->u.data;
+		c->transmission_mode = data;
 		break;
 	case DTV_HIERARCHY:
-		c->hierarchy = tvp->u.data;
+		c->hierarchy = data;
 		break;
 	case DTV_INTERLEAVING:
-		c->interleaving = tvp->u.data;
+		c->interleaving = data;
 		break;
 
 	/* ISDB-T Support here */
 	case DTV_ISDBT_PARTIAL_RECEPTION:
-		c->isdbt_partial_reception = tvp->u.data;
+		c->isdbt_partial_reception = data;
 		break;
 	case DTV_ISDBT_SOUND_BROADCASTING:
-		c->isdbt_sb_mode = tvp->u.data;
+		c->isdbt_sb_mode = data;
 		break;
 	case DTV_ISDBT_SB_SUBCHANNEL_ID:
-		c->isdbt_sb_subchannel = tvp->u.data;
+		c->isdbt_sb_subchannel = data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_IDX:
-		c->isdbt_sb_segment_idx = tvp->u.data;
+		c->isdbt_sb_segment_idx = data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_COUNT:
-		c->isdbt_sb_segment_count = tvp->u.data;
+		c->isdbt_sb_segment_count = data;
 		break;
 	case DTV_ISDBT_LAYER_ENABLED:
-		c->isdbt_layer_enabled = tvp->u.data;
+		c->isdbt_layer_enabled = data;
 		break;
 	case DTV_ISDBT_LAYERA_FEC:
-		c->layer[0].fec = tvp->u.data;
+		c->layer[0].fec = data;
 		break;
 	case DTV_ISDBT_LAYERA_MODULATION:
-		c->layer[0].modulation = tvp->u.data;
+		c->layer[0].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
-		c->layer[0].segment_count = tvp->u.data;
+		c->layer[0].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
-		c->layer[0].interleaving = tvp->u.data;
+		c->layer[0].interleaving = data;
 		break;
 	case DTV_ISDBT_LAYERB_FEC:
-		c->layer[1].fec = tvp->u.data;
+		c->layer[1].fec = data;
 		break;
 	case DTV_ISDBT_LAYERB_MODULATION:
-		c->layer[1].modulation = tvp->u.data;
+		c->layer[1].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
-		c->layer[1].segment_count = tvp->u.data;
+		c->layer[1].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
-		c->layer[1].interleaving = tvp->u.data;
+		c->layer[1].interleaving = data;
 		break;
 	case DTV_ISDBT_LAYERC_FEC:
-		c->layer[2].fec = tvp->u.data;
+		c->layer[2].fec = data;
 		break;
 	case DTV_ISDBT_LAYERC_MODULATION:
-		c->layer[2].modulation = tvp->u.data;
+		c->layer[2].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
-		c->layer[2].segment_count = tvp->u.data;
+		c->layer[2].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
-		c->layer[2].interleaving = tvp->u.data;
+		c->layer[2].interleaving = data;
 		break;
 
 	/* Multistream support */
 	case DTV_STREAM_ID:
 	case DTV_DVBT2_PLP_ID_LEGACY:
-		c->stream_id = tvp->u.data;
+		c->stream_id = data;
 		break;
 
 	/* ATSC-MH */
 	case DTV_ATSCMH_PARADE_ID:
-		fe->dtv_property_cache.atscmh_parade_id = tvp->u.data;
+		fe->dtv_property_cache.atscmh_parade_id = data;
 		break;
 	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
-		fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
+		fe->dtv_property_cache.atscmh_rs_frame_ensemble = data;
 		break;
 
 	case DTV_LNA:
-		c->lna = tvp->u.data;
+		c->lna = data;
 		if (fe->ops.set_lna)
 			r = fe->ops.set_lna(fe);
 		if (r < 0)
@@ -1939,7 +1948,8 @@ static int dvb_frontend_ioctl(struct file *file,
 		return -EPERM;
 	}
 
-	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
+	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
+	    || (cmd == FE_SET_PROPERTY_SHORT))
 		err = dvb_frontend_ioctl_properties(file, cmd, parg);
 	else {
 		c->state = DTV_UNDEFINED;
@@ -1966,8 +1976,10 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 	dev_dbg(fe->dvb->device, "%s:\n", __func__);
 
 	if (cmd == FE_SET_PROPERTY) {
-		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps->num);
-		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps->props);
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
+				__func__, tvps->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
+				__func__, tvps->props);
 
 		/* Put an arbitrary limit on the number of messages that can
 		 * be sent at once */
@@ -1979,20 +1991,25 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			return PTR_ERR(tvp);
 
 		for (i = 0; i < tvps->num; i++) {
-			err = dtv_property_process_set(fe, tvp + i, file);
+			err = dtv_property_process_set(fe, file,
+									   (tvp + i)->cmd,
+									   (tvp + i)->u.data);
 			if (err < 0)
 				goto out;
 			(tvp + i)->result = err;
 		}
 
 		if (c->state == DTV_TUNE)
-			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n", __func__);
+			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n",
+					__func__);
 
 	} else if (cmd == FE_GET_PROPERTY) {
 		struct dtv_frontend_properties getp = fe->dtv_property_cache;
 
-		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps->num);
-		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps->props);
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
+				__func__, tvps->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
+				__func__, tvps->props);
 
 		/* Put an arbitrary limit on the number of messages that can
 		 * be sent at once */
@@ -2026,7 +2043,35 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			err = -EFAULT;
 			goto out;
 		}
-
+	/* New ioctl for optimizing property set*/
+	} else if (cmd == FE_SET_PROPERTY_SHORT) {
+		struct dtv_property_short *tvp_short = NULL;
+		struct dtv_properties_short *tvps_short = parg;
+
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
+				__func__, tvps_short->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
+				__func__, tvps_short->props);
+		if ((!tvps_short->num) ||
+		    (tvps_short->num > DTV_IOCTL_MAX_MSGS))
+			return -EINVAL;
+		tvp_short = memdup_user(tvps_short->props,
+		tvps_short->num * sizeof(*tvp_short));
+		if (IS_ERR(tvp_short))
+			return PTR_ERR(tvp_short);
+		for (i = 0; i < tvps_short->num; i++) {
+			err = dtv_property_process_set(fe, file,
+									   (tvp_short + i)->cmd,
+									   (tvp_short + i)->data);
+			if (err < 0) {
+				kfree(tvp_short);
+				return err;
+			}
+		}
+		if (c->state == DTV_TUNE)
+			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n",
+					__func__);
+		kfree(tvp_short);
 	} else
 		err = -EOPNOTSUPP;
 
diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
index 00a20cd..9d96dab 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -476,6 +476,17 @@ struct dtv_property {
 	int result;
 } __attribute__ ((packed));
 
+/**
+ * struct dtv_property_short - A shorter version of &struct dtv_property
+ *
+ * @cmd:	Digital TV command.
+ * @data:	An unsigned 32-bits number.
+ */
+struct dtv_property_short {
+	__u32 cmd;
+	__u32 data;
+};
+
 /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
 #define DTV_IOCTL_MAX_MSGS 64
 
@@ -484,6 +495,18 @@ struct dtv_properties {
 	struct dtv_property *props;
 };
 
+/**
+ * struct dtv_properties_short - A variant of &struct dtv_properties
+ * to support &struct dtv_property_short
+ *
+ * @num:	Number of properties
+ * @props:	Pointer to &struct dtv_property_short
+ */
+struct dtv_properties_short {
+	__u32 num;
+	struct dtv_property_short *props;
+};
+
 #if defined(__DVB_CORE__) || !defined (__KERNEL__)
 
 /*
@@ -565,6 +588,7 @@ struct dvb_frontend_event {
 
 #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
 #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
+#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
 
 /**
  * When set, this flag will disable any zigzagging or other "normal" tuning
-- 
2.7.4

[PATCH v4] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Satendra Singh Thakur]

Hello  Mr Chehab,
It seems that there is a mismatch among tab spacing
in local patch on my PC, the patch in email
and the patch in lkml site.
This is causing alignment problem. Even if I fix alignment problem
in my PC, alignment is different in lkml and email.
Anyway, I have run checkpatch and got 0 err and warnings
Please review it.
Thanks for the patience.

Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
---
 Documentation/media/uapi/dvb/fe-get-property.rst |  24 ++-
 drivers/media/dvb-core/dvb_frontend.c            | 191 ++++++++++++++---------
 include/uapi/linux/dvb/frontend.h                |  24 +++
 3 files changed, 158 insertions(+), 81 deletions(-)

diff --git a/Documentation/media/uapi/dvb/fe-get-property.rst b/Documentation/media/uapi/dvb/fe-get-property.rst
index 015d4db..b63a588 100644
--- a/Documentation/media/uapi/dvb/fe-get-property.rst
+++ b/Documentation/media/uapi/dvb/fe-get-property.rst
@@ -2,14 +2,14 @@
 
 .. _FE_GET_PROPERTY:
 
-**************************************
-ioctl FE_SET_PROPERTY, FE_GET_PROPERTY
-**************************************
+**************************************************************
+ioctl FE_SET_PROPERTY, FE_GET_PROPERTY, FE_SET_PROPERTY_SHORT
+**************************************************************
 
 Name
 ====
 
-FE_SET_PROPERTY - FE_GET_PROPERTY - FE_SET_PROPERTY sets one or more frontend properties. - FE_GET_PROPERTY returns one or more frontend properties.
+FE_SET_PROPERTY and FE_SET_PROPERTY_SHORT set one or more frontend properties. FE_GET_PROPERTY returns one or more frontend properties.
 
 
 Synopsis
@@ -21,6 +21,8 @@ Synopsis
 .. c:function:: int ioctl( int fd, FE_SET_PROPERTY, struct dtv_properties *argp )
     :name: FE_SET_PROPERTY
 
+.. c:function:: int ioctl( int fd, FE_SET_PROPERTY_SHORT, struct dtv_properties_short *argp )
+    :name: FE_SET_PROPERTY_SHORT
 
 Arguments
 =========
@@ -29,15 +31,16 @@ Arguments
     File descriptor returned by :ref:`open() <frontend_f_open>`.
 
 ``argp``
-    pointer to struct :c:type:`dtv_properties`
+    Pointer to struct :c:type:`dtv_properties` or
+	struct :c:type:`dtv_properties_short`.
 
 
 Description
 ===========
 
-All DVB frontend devices support the ``FE_SET_PROPERTY`` and
-``FE_GET_PROPERTY`` ioctls. The supported properties and statistics
-depends on the delivery system and on the device:
+All DVB frontend devices support the ``FE_SET_PROPERTY``, ``FE_GET_PROPERTY``
+and ``FE_SET_PROPERTY_SHORT`` ioctls. The supported  properties and
+statistics depends on the delivery system and on the device:
 
 -  ``FE_SET_PROPERTY:``
 
@@ -60,6 +63,11 @@ depends on the delivery system and on the device:
 
    -  This call only requires read-only access to the device.
 
+-  ``FE_SET_PROPERTY_SHORT:``
+
+   -  This ioctl is similar to FE_SET_PROPERTY ioctl mentioned above
+      except that the arguments of the former utilize a struct :c:type:`dtv_property_short`
+      which is smaller in size.
 
 Return Value
 ============
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index e3fff8f..7c96197 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1081,28 +1081,25 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = {
 	_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
 };
 
-static void dtv_property_dump(struct dvb_frontend *fe,
-			      bool is_set,
+static void dtv_get_property_dump(struct dvb_frontend *fe,
 			      struct dtv_property *tvp)
 {
 	int i;
 
 	if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
-		dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
-				__func__,
-				is_set ? "SET" : "GET",
-				tvp->cmd);
+		dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
+				 , __func__,
+				 tvp->cmd);
 		return;
 	}
 
-	dev_dbg(fe->dvb->device, "%s: %s tvp.cmd    = 0x%08x (%s)\n", __func__,
-		is_set ? "SET" : "GET",
-		tvp->cmd,
-		dtv_cmds[tvp->cmd].name);
+	dev_dbg(fe->dvb->device, "%s: GET tvp.cmd    = 0x%08x (%s)\n", __func__,
+			tvp->cmd,
+			dtv_cmds[tvp->cmd].name);
 
 	if (dtv_cmds[tvp->cmd].buffer) {
 		dev_dbg(fe->dvb->device, "%s: tvp.u.buffer.len = 0x%02x\n",
-			__func__, tvp->u.buffer.len);
+				__func__, tvp->u.buffer.len);
 
 		for(i = 0; i < tvp->u.buffer.len; i++)
 			dev_dbg(fe->dvb->device,
@@ -1515,7 +1512,7 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 			return r;
 	}
 
-	dtv_property_dump(fe, false, tvp);
+	dtv_get_property_dump(fe, tvp);
 
 	return 0;
 }
@@ -1738,23 +1735,35 @@ static int dvbv3_set_delivery_system(struct dvb_frontend *fe)
 	return emulate_delivery_system(fe, delsys);
 }
 
+/**
+ * dtv_property_process_set -  Sets a single DTV property
+ * @fe:		Pointer to &struct dvb_frontend
+ * @file:	Pointer to &struct file
+ * @cmd:	Digital TV command
+ * @data:	An unsigned 32-bits number
+ *
+ * This routine assigns the property
+ * value to the corresponding member of
+ * &struct dtv_frontend_properties
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
 static int dtv_property_process_set(struct dvb_frontend *fe,
-				    struct dtv_property *tvp,
-				    struct file *file)
+					struct file *file,
+					u32 cmd, u32 data)
 {
 	int r = 0;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
-
-	/* Allow the frontend to validate incoming properties */
-	if (fe->ops.set_property) {
-		r = fe->ops.set_property(fe, tvp);
-		if (r < 0)
-			return r;
-	}
-
-	dtv_property_dump(fe, true, tvp);
-
-	switch(tvp->cmd) {
+	/** Dump DTV command name and value*/
+	if (!cmd || cmd > DTV_MAX_COMMAND)
+		dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
+				 __func__, cmd);
+	else
+		dev_dbg(fe->dvb->device,
+				"%s: SET cmd 0x%08x (%s) to 0x%08x\n",
+				__func__, cmd, dtv_cmds[cmd].name, data);
+	switch (cmd) {
 	case DTV_CLEAR:
 		/*
 		 * Reset a cache of data specific to the frontend here. This does
@@ -1767,140 +1776,140 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 		 * tunerequest so we can pass validation in the FE_SET_FRONTEND
 		 * ioctl.
 		 */
-		c->state = tvp->cmd;
+		c->state = cmd;
 		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
 				__func__);
 
 		r = dtv_set_frontend(fe);
 		break;
 	case DTV_FREQUENCY:
-		c->frequency = tvp->u.data;
+		c->frequency = data;
 		break;
 	case DTV_MODULATION:
-		c->modulation = tvp->u.data;
+		c->modulation = data;
 		break;
 	case DTV_BANDWIDTH_HZ:
-		c->bandwidth_hz = tvp->u.data;
+		c->bandwidth_hz = data;
 		break;
 	case DTV_INVERSION:
-		c->inversion = tvp->u.data;
+		c->inversion = data;
 		break;
 	case DTV_SYMBOL_RATE:
-		c->symbol_rate = tvp->u.data;
+		c->symbol_rate = data;
 		break;
 	case DTV_INNER_FEC:
-		c->fec_inner = tvp->u.data;
+		c->fec_inner = data;
 		break;
 	case DTV_PILOT:
-		c->pilot = tvp->u.data;
+		c->pilot = data;
 		break;
 	case DTV_ROLLOFF:
-		c->rolloff = tvp->u.data;
+		c->rolloff = data;
 		break;
 	case DTV_DELIVERY_SYSTEM:
-		r = dvbv5_set_delivery_system(fe, tvp->u.data);
+		r = dvbv5_set_delivery_system(fe, data);
 		break;
 	case DTV_VOLTAGE:
-		c->voltage = tvp->u.data;
+		c->voltage = data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
 			(void *)c->voltage);
 		break;
 	case DTV_TONE:
-		c->sectone = tvp->u.data;
+		c->sectone = data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
 			(void *)c->sectone);
 		break;
 	case DTV_CODE_RATE_HP:
-		c->code_rate_HP = tvp->u.data;
+		c->code_rate_HP = data;
 		break;
 	case DTV_CODE_RATE_LP:
-		c->code_rate_LP = tvp->u.data;
+		c->code_rate_LP = data;
 		break;
 	case DTV_GUARD_INTERVAL:
-		c->guard_interval = tvp->u.data;
+		c->guard_interval = data;
 		break;
 	case DTV_TRANSMISSION_MODE:
-		c->transmission_mode = tvp->u.data;
+		c->transmission_mode = data;
 		break;
 	case DTV_HIERARCHY:
-		c->hierarchy = tvp->u.data;
+		c->hierarchy = data;
 		break;
 	case DTV_INTERLEAVING:
-		c->interleaving = tvp->u.data;
+		c->interleaving = data;
 		break;
 
 	/* ISDB-T Support here */
 	case DTV_ISDBT_PARTIAL_RECEPTION:
-		c->isdbt_partial_reception = tvp->u.data;
+		c->isdbt_partial_reception = data;
 		break;
 	case DTV_ISDBT_SOUND_BROADCASTING:
-		c->isdbt_sb_mode = tvp->u.data;
+		c->isdbt_sb_mode = data;
 		break;
 	case DTV_ISDBT_SB_SUBCHANNEL_ID:
-		c->isdbt_sb_subchannel = tvp->u.data;
+		c->isdbt_sb_subchannel = data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_IDX:
-		c->isdbt_sb_segment_idx = tvp->u.data;
+		c->isdbt_sb_segment_idx = data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_COUNT:
-		c->isdbt_sb_segment_count = tvp->u.data;
+		c->isdbt_sb_segment_count = data;
 		break;
 	case DTV_ISDBT_LAYER_ENABLED:
-		c->isdbt_layer_enabled = tvp->u.data;
+		c->isdbt_layer_enabled = data;
 		break;
 	case DTV_ISDBT_LAYERA_FEC:
-		c->layer[0].fec = tvp->u.data;
+		c->layer[0].fec = data;
 		break;
 	case DTV_ISDBT_LAYERA_MODULATION:
-		c->layer[0].modulation = tvp->u.data;
+		c->layer[0].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
-		c->layer[0].segment_count = tvp->u.data;
+		c->layer[0].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
-		c->layer[0].interleaving = tvp->u.data;
+		c->layer[0].interleaving = data;
 		break;
 	case DTV_ISDBT_LAYERB_FEC:
-		c->layer[1].fec = tvp->u.data;
+		c->layer[1].fec = data;
 		break;
 	case DTV_ISDBT_LAYERB_MODULATION:
-		c->layer[1].modulation = tvp->u.data;
+		c->layer[1].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
-		c->layer[1].segment_count = tvp->u.data;
+		c->layer[1].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
-		c->layer[1].interleaving = tvp->u.data;
+		c->layer[1].interleaving = data;
 		break;
 	case DTV_ISDBT_LAYERC_FEC:
-		c->layer[2].fec = tvp->u.data;
+		c->layer[2].fec = data;
 		break;
 	case DTV_ISDBT_LAYERC_MODULATION:
-		c->layer[2].modulation = tvp->u.data;
+		c->layer[2].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
-		c->layer[2].segment_count = tvp->u.data;
+		c->layer[2].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
-		c->layer[2].interleaving = tvp->u.data;
+		c->layer[2].interleaving = data;
 		break;
 
 	/* Multistream support */
 	case DTV_STREAM_ID:
 	case DTV_DVBT2_PLP_ID_LEGACY:
-		c->stream_id = tvp->u.data;
+		c->stream_id = data;
 		break;
 
 	/* ATSC-MH */
 	case DTV_ATSCMH_PARADE_ID:
-		fe->dtv_property_cache.atscmh_parade_id = tvp->u.data;
+		fe->dtv_property_cache.atscmh_parade_id = data;
 		break;
 	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
-		fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
+		fe->dtv_property_cache.atscmh_rs_frame_ensemble = data;
 		break;
 
 	case DTV_LNA:
-		c->lna = tvp->u.data;
+		c->lna = data;
 		if (fe->ops.set_lna)
 			r = fe->ops.set_lna(fe);
 		if (r < 0)
@@ -1939,7 +1948,8 @@ static int dvb_frontend_ioctl(struct file *file,
 		return -EPERM;
 	}
 
-	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
+	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
+	    || (cmd == FE_SET_PROPERTY_SHORT))
 		err = dvb_frontend_ioctl_properties(file, cmd, parg);
 	else {
 		c->state = DTV_UNDEFINED;
@@ -1966,8 +1976,10 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 	dev_dbg(fe->dvb->device, "%s:\n", __func__);
 
 	if (cmd == FE_SET_PROPERTY) {
-		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps->num);
-		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps->props);
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
+				__func__, tvps->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
+				__func__, tvps->props);
 
 		/* Put an arbitrary limit on the number of messages that can
 		 * be sent at once */
@@ -1979,20 +1991,25 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			return PTR_ERR(tvp);
 
 		for (i = 0; i < tvps->num; i++) {
-			err = dtv_property_process_set(fe, tvp + i, file);
+			err = dtv_property_process_set(fe, file,
+					(tvp + i)->cmd,
+					(tvp + i)->u.data);
 			if (err < 0)
 				goto out;
 			(tvp + i)->result = err;
 		}
 
 		if (c->state == DTV_TUNE)
-			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n", __func__);
+			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n",
+					__func__);
 
 	} else if (cmd == FE_GET_PROPERTY) {
 		struct dtv_frontend_properties getp = fe->dtv_property_cache;
 
-		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps->num);
-		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps->props);
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
+				__func__, tvps->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
+				__func__, tvps->props);
 
 		/* Put an arbitrary limit on the number of messages that can
 		 * be sent at once */
@@ -2026,7 +2043,35 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			err = -EFAULT;
 			goto out;
 		}
-
+	/* New ioctl for optimizing property set*/
+	} else if (cmd == FE_SET_PROPERTY_SHORT) {
+		struct dtv_property_short *tvp_short = NULL;
+		struct dtv_properties_short *tvps_short = parg;
+
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
+				__func__, tvps_short->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
+				__func__, tvps_short->props);
+		if ((!tvps_short->num) ||
+		    (tvps_short->num > DTV_IOCTL_MAX_MSGS))
+			return -EINVAL;
+		tvp_short = memdup_user(tvps_short->props,
+		tvps_short->num * sizeof(*tvp_short));
+		if (IS_ERR(tvp_short))
+			return PTR_ERR(tvp_short);
+		for (i = 0; i < tvps_short->num; i++) {
+			err = dtv_property_process_set(fe, file,
+					(tvp_short + i)->cmd,
+					(tvp_short + i)->data);
+			if (err < 0) {
+				kfree(tvp_short);
+				return err;
+			}
+		}
+		if (c->state == DTV_TUNE)
+			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n",
+					__func__);
+		kfree(tvp_short);
 	} else
 		err = -EOPNOTSUPP;
 
diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
index 00a20cd..9d96dab 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -476,6 +476,17 @@ struct dtv_property {
 	int result;
 } __attribute__ ((packed));
 
+/**
+ * struct dtv_property_short - A shorter version of &struct dtv_property
+ *
+ * @cmd:	Digital TV command.
+ * @data:	An unsigned 32-bits number.
+ */
+struct dtv_property_short {
+	__u32 cmd;
+	__u32 data;
+};
+
 /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
 #define DTV_IOCTL_MAX_MSGS 64
 
@@ -484,6 +495,18 @@ struct dtv_properties {
 	struct dtv_property *props;
 };
 
+/**
+ * struct dtv_properties_short - A variant of &struct dtv_properties
+ * to support &struct dtv_property_short
+ *
+ * @num:	Number of properties
+ * @props:	Pointer to &struct dtv_property_short
+ */
+struct dtv_properties_short {
+	__u32 num;
+	struct dtv_property_short *props;
+};
+
 #if defined(__DVB_CORE__) || !defined (__KERNEL__)
 
 /*
@@ -565,6 +588,7 @@ struct dvb_frontend_event {
 
 #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
 #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
+#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
 
 /**
  * When set, this flag will disable any zigzagging or other "normal" tuning
-- 
2.7.4

Re: [PATCH v4] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Mauro Carvalho Chehab]

Em Mon, 18 Sep 2017 05:35:45 -0400
Satendra Singh Thakur <satendra.t@samsung.com> escreveu:

> Hello  Mr Chehab,
> It seems that there is a mismatch among tab spacing
> in local patch on my PC, the patch in email
> and the patch in lkml site.
> This is causing alignment problem. Even if I fix alignment problem
> in my PC, alignment is different in lkml and email.
> Anyway, I have run checkpatch and got 0 err and warnings
> Please review it.
> Thanks for the patience.

Please, always put patch description here.

> 
> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> ---

If you have any comments to maintainers/reviewers, add here,
after the "---" line.

>  Documentation/media/uapi/dvb/fe-get-property.rst |  24 ++-
>  drivers/media/dvb-core/dvb_frontend.c            | 191 ++++++++++++++---------
>  include/uapi/linux/dvb/frontend.h                |  24 +++
>  3 files changed, 158 insertions(+), 81 deletions(-)
> 
> diff --git a/Documentation/media/uapi/dvb/fe-get-property.rst b/Documentation/media/uapi/dvb/fe-get-property.rst
> index 015d4db..b63a588 100644
> --- a/Documentation/media/uapi/dvb/fe-get-property.rst
> +++ b/Documentation/media/uapi/dvb/fe-get-property.rst
> @@ -2,14 +2,14 @@
>  
>  .. _FE_GET_PROPERTY:
>  
> -**************************************
> -ioctl FE_SET_PROPERTY, FE_GET_PROPERTY
> -**************************************
> +**************************************************************
> +ioctl FE_SET_PROPERTY, FE_GET_PROPERTY, FE_SET_PROPERTY_SHORT
> +**************************************************************
>  
>  Name
>  ====
>  
> -FE_SET_PROPERTY - FE_GET_PROPERTY - FE_SET_PROPERTY sets one or more frontend properties. - FE_GET_PROPERTY returns one or more frontend properties.
> +FE_SET_PROPERTY and FE_SET_PROPERTY_SHORT set one or more frontend properties. FE_GET_PROPERTY returns one or more frontend properties.
>  
>  
>  Synopsis
> @@ -21,6 +21,8 @@ Synopsis
>  .. c:function:: int ioctl( int fd, FE_SET_PROPERTY, struct dtv_properties *argp )
>      :name: FE_SET_PROPERTY
>  
> +.. c:function:: int ioctl( int fd, FE_SET_PROPERTY_SHORT, struct dtv_properties_short *argp )
> +    :name: FE_SET_PROPERTY_SHORT
>  
>  Arguments
>  =========
> @@ -29,15 +31,16 @@ Arguments
>      File descriptor returned by :ref:`open() <frontend_f_open>`.
>  
>  ``argp``
> -    pointer to struct :c:type:`dtv_properties`
> +    Pointer to struct :c:type:`dtv_properties` or
> +	struct :c:type:`dtv_properties_short`.
>  
>  
>  Description
>  ===========
>  
> -All DVB frontend devices support the ``FE_SET_PROPERTY`` and
> -``FE_GET_PROPERTY`` ioctls. The supported properties and statistics
> -depends on the delivery system and on the device:
> +All DVB frontend devices support the ``FE_SET_PROPERTY``, ``FE_GET_PROPERTY``
> +and ``FE_SET_PROPERTY_SHORT`` ioctls. The supported  properties and
> +statistics depends on the delivery system and on the device:
>  
>  -  ``FE_SET_PROPERTY:``
>  
> @@ -60,6 +63,11 @@ depends on the delivery system and on the device:
>  
>     -  This call only requires read-only access to the device.
>  
> +-  ``FE_SET_PROPERTY_SHORT:``
> +
> +   -  This ioctl is similar to FE_SET_PROPERTY ioctl mentioned above
> +      except that the arguments of the former utilize a struct :c:type:`dtv_property_short`
> +      which is smaller in size.
>  
>  Return Value
>  ============

I'm still in doubt about something: what's the real requirement from userspace
to justify a new ioctl?

Userspace will still need to allocate the same memory in order to get back
the tuner parameters with FE_GET_PROPERTY and to get statistics. Also, tuning
happens just once per channel, so hardly this would bring *any* real 
performance improvement while tuning into a channel.

> diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
> index e3fff8f..7c96197 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1081,28 +1081,25 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = {
>  	_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
>  };
>  
> -static void dtv_property_dump(struct dvb_frontend *fe,
> -			      bool is_set,
> +static void dtv_get_property_dump(struct dvb_frontend *fe,
>  			      struct dtv_property *tvp)
>  {
>  	int i;
>  
>  	if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
> -		dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
> -				__func__,
> -				is_set ? "SET" : "GET",
> -				tvp->cmd);
> +		dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
> +				 , __func__,
> +				 tvp->cmd);
>  		return;
>  	}
>  
> -	dev_dbg(fe->dvb->device, "%s: %s tvp.cmd    = 0x%08x (%s)\n", __func__,
> -		is_set ? "SET" : "GET",
> -		tvp->cmd,
> -		dtv_cmds[tvp->cmd].name);
> +	dev_dbg(fe->dvb->device, "%s: GET tvp.cmd    = 0x%08x (%s)\n", __func__,
> +			tvp->cmd,
> +			dtv_cmds[tvp->cmd].name);
>  
>  	if (dtv_cmds[tvp->cmd].buffer) {
>  		dev_dbg(fe->dvb->device, "%s: tvp.u.buffer.len = 0x%02x\n",
> -			__func__, tvp->u.buffer.len);
> +				__func__, tvp->u.buffer.len);
>  
>  		for(i = 0; i < tvp->u.buffer.len; i++)
>  			dev_dbg(fe->dvb->device,
> @@ -1515,7 +1512,7 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
>  			return r;
>  	}
>  
> -	dtv_property_dump(fe, false, tvp);
> +	dtv_get_property_dump(fe, tvp);
>  
>  	return 0;
>  }
> @@ -1738,23 +1735,35 @@ static int dvbv3_set_delivery_system(struct dvb_frontend *fe)
>  	return emulate_delivery_system(fe, delsys);
>  }
>  
> +/**
> + * dtv_property_process_set -  Sets a single DTV property
> + * @fe:		Pointer to &struct dvb_frontend
> + * @file:	Pointer to &struct file
> + * @cmd:	Digital TV command
> + * @data:	An unsigned 32-bits number
> + *
> + * This routine assigns the property
> + * value to the corresponding member of
> + * &struct dtv_frontend_properties
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
>  static int dtv_property_process_set(struct dvb_frontend *fe,
> -				    struct dtv_property *tvp,
> -				    struct file *file)
> +					struct file *file,
> +					u32 cmd, u32 data)
>  {
>  	int r = 0;
>  	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
> -
> -	/* Allow the frontend to validate incoming properties */
> -	if (fe->ops.set_property) {
> -		r = fe->ops.set_property(fe, tvp);
> -		if (r < 0)
> -			return r;
> -	}
> -
> -	dtv_property_dump(fe, true, tvp);
> -
> -	switch(tvp->cmd) {
> +	/** Dump DTV command name and value*/
> +	if (!cmd || cmd > DTV_MAX_COMMAND)
> +		dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
> +				 __func__, cmd);
> +	else
> +		dev_dbg(fe->dvb->device,
> +				"%s: SET cmd 0x%08x (%s) to 0x%08x\n",
> +				__func__, cmd, dtv_cmds[cmd].name, data);
> +	switch (cmd) {
>  	case DTV_CLEAR:
>  		/*
>  		 * Reset a cache of data specific to the frontend here. This does
> @@ -1767,140 +1776,140 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
>  		 * tunerequest so we can pass validation in the FE_SET_FRONTEND
>  		 * ioctl.
>  		 */
> -		c->state = tvp->cmd;
> +		c->state = cmd;
>  		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
>  				__func__);
>  
>  		r = dtv_set_frontend(fe);
>  		break;
>  	case DTV_FREQUENCY:
> -		c->frequency = tvp->u.data;
> +		c->frequency = data;
>  		break;
>  	case DTV_MODULATION:
> -		c->modulation = tvp->u.data;
> +		c->modulation = data;
>  		break;
>  	case DTV_BANDWIDTH_HZ:
> -		c->bandwidth_hz = tvp->u.data;
> +		c->bandwidth_hz = data;
>  		break;
>  	case DTV_INVERSION:
> -		c->inversion = tvp->u.data;
> +		c->inversion = data;
>  		break;
>  	case DTV_SYMBOL_RATE:
> -		c->symbol_rate = tvp->u.data;
> +		c->symbol_rate = data;
>  		break;
>  	case DTV_INNER_FEC:
> -		c->fec_inner = tvp->u.data;
> +		c->fec_inner = data;
>  		break;
>  	case DTV_PILOT:
> -		c->pilot = tvp->u.data;
> +		c->pilot = data;
>  		break;
>  	case DTV_ROLLOFF:
> -		c->rolloff = tvp->u.data;
> +		c->rolloff = data;
>  		break;
>  	case DTV_DELIVERY_SYSTEM:
> -		r = dvbv5_set_delivery_system(fe, tvp->u.data);
> +		r = dvbv5_set_delivery_system(fe, data);
>  		break;
>  	case DTV_VOLTAGE:
> -		c->voltage = tvp->u.data;
> +		c->voltage = data;
>  		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
>  			(void *)c->voltage);
>  		break;
>  	case DTV_TONE:
> -		c->sectone = tvp->u.data;
> +		c->sectone = data;
>  		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
>  			(void *)c->sectone);
>  		break;
>  	case DTV_CODE_RATE_HP:
> -		c->code_rate_HP = tvp->u.data;
> +		c->code_rate_HP = data;
>  		break;
>  	case DTV_CODE_RATE_LP:
> -		c->code_rate_LP = tvp->u.data;
> +		c->code_rate_LP = data;
>  		break;
>  	case DTV_GUARD_INTERVAL:
> -		c->guard_interval = tvp->u.data;
> +		c->guard_interval = data;
>  		break;
>  	case DTV_TRANSMISSION_MODE:
> -		c->transmission_mode = tvp->u.data;
> +		c->transmission_mode = data;
>  		break;
>  	case DTV_HIERARCHY:
> -		c->hierarchy = tvp->u.data;
> +		c->hierarchy = data;
>  		break;
>  	case DTV_INTERLEAVING:
> -		c->interleaving = tvp->u.data;
> +		c->interleaving = data;
>  		break;
>  
>  	/* ISDB-T Support here */
>  	case DTV_ISDBT_PARTIAL_RECEPTION:
> -		c->isdbt_partial_reception = tvp->u.data;
> +		c->isdbt_partial_reception = data;
>  		break;
>  	case DTV_ISDBT_SOUND_BROADCASTING:
> -		c->isdbt_sb_mode = tvp->u.data;
> +		c->isdbt_sb_mode = data;
>  		break;
>  	case DTV_ISDBT_SB_SUBCHANNEL_ID:
> -		c->isdbt_sb_subchannel = tvp->u.data;
> +		c->isdbt_sb_subchannel = data;
>  		break;
>  	case DTV_ISDBT_SB_SEGMENT_IDX:
> -		c->isdbt_sb_segment_idx = tvp->u.data;
> +		c->isdbt_sb_segment_idx = data;
>  		break;
>  	case DTV_ISDBT_SB_SEGMENT_COUNT:
> -		c->isdbt_sb_segment_count = tvp->u.data;
> +		c->isdbt_sb_segment_count = data;
>  		break;
>  	case DTV_ISDBT_LAYER_ENABLED:
> -		c->isdbt_layer_enabled = tvp->u.data;
> +		c->isdbt_layer_enabled = data;
>  		break;
>  	case DTV_ISDBT_LAYERA_FEC:
> -		c->layer[0].fec = tvp->u.data;
> +		c->layer[0].fec = data;
>  		break;
>  	case DTV_ISDBT_LAYERA_MODULATION:
> -		c->layer[0].modulation = tvp->u.data;
> +		c->layer[0].modulation = data;
>  		break;
>  	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
> -		c->layer[0].segment_count = tvp->u.data;
> +		c->layer[0].segment_count = data;
>  		break;
>  	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
> -		c->layer[0].interleaving = tvp->u.data;
> +		c->layer[0].interleaving = data;
>  		break;
>  	case DTV_ISDBT_LAYERB_FEC:
> -		c->layer[1].fec = tvp->u.data;
> +		c->layer[1].fec = data;
>  		break;
>  	case DTV_ISDBT_LAYERB_MODULATION:
> -		c->layer[1].modulation = tvp->u.data;
> +		c->layer[1].modulation = data;
>  		break;
>  	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
> -		c->layer[1].segment_count = tvp->u.data;
> +		c->layer[1].segment_count = data;
>  		break;
>  	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
> -		c->layer[1].interleaving = tvp->u.data;
> +		c->layer[1].interleaving = data;
>  		break;
>  	case DTV_ISDBT_LAYERC_FEC:
> -		c->layer[2].fec = tvp->u.data;
> +		c->layer[2].fec = data;
>  		break;
>  	case DTV_ISDBT_LAYERC_MODULATION:
> -		c->layer[2].modulation = tvp->u.data;
> +		c->layer[2].modulation = data;
>  		break;
>  	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
> -		c->layer[2].segment_count = tvp->u.data;
> +		c->layer[2].segment_count = data;
>  		break;
>  	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
> -		c->layer[2].interleaving = tvp->u.data;
> +		c->layer[2].interleaving = data;
>  		break;
>  
>  	/* Multistream support */
>  	case DTV_STREAM_ID:
>  	case DTV_DVBT2_PLP_ID_LEGACY:
> -		c->stream_id = tvp->u.data;
> +		c->stream_id = data;
>  		break;
>  
>  	/* ATSC-MH */
>  	case DTV_ATSCMH_PARADE_ID:
> -		fe->dtv_property_cache.atscmh_parade_id = tvp->u.data;
> +		fe->dtv_property_cache.atscmh_parade_id = data;
>  		break;
>  	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
> -		fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
> +		fe->dtv_property_cache.atscmh_rs_frame_ensemble = data;
>  		break;
>  
>  	case DTV_LNA:
> -		c->lna = tvp->u.data;
> +		c->lna = data;
>  		if (fe->ops.set_lna)
>  			r = fe->ops.set_lna(fe);
>  		if (r < 0)
> @@ -1939,7 +1948,8 @@ static int dvb_frontend_ioctl(struct file *file,
>  		return -EPERM;
>  	}

The above optimization seem OK. Still, it is better to place it on
a separate patch.

>  
> -	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
> +	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
> +	    || (cmd == FE_SET_PROPERTY_SHORT))
>  		err = dvb_frontend_ioctl_properties(file, cmd, parg);
>  	else {
>  		c->state = DTV_UNDEFINED;

Assuming that you provide a good reason why we need this ioctl, this
should be on the second patch with the new ioctl.

> @@ -1966,8 +1976,10 @@ static int dvb_frontend_ioctl_properties(struct file *file,
>  	dev_dbg(fe->dvb->device, "%s:\n", __func__);
>  
>  	if (cmd == FE_SET_PROPERTY) {
> -		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps->num);
> -		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps->props);
> +		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
> +				__func__, tvps->num);
> +		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
> +				__func__, tvps->props);
>  
>  		/* Put an arbitrary limit on the number of messages that can
>  		 * be sent at once */
> @@ -1979,20 +1991,25 @@ static int dvb_frontend_ioctl_properties(struct file *file,
>  			return PTR_ERR(tvp);
>  
>  		for (i = 0; i < tvps->num; i++) {
> -			err = dtv_property_process_set(fe, tvp + i, file);
> +			err = dtv_property_process_set(fe, file,
> +					(tvp + i)->cmd,
> +					(tvp + i)->u.data);
>  			if (err < 0)
>  				goto out;
>  			(tvp + i)->result = err;
>  		}
>  
>  		if (c->state == DTV_TUNE)
> -			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n", __func__);
> +			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n",
> +					__func__);
>  
>  	} else if (cmd == FE_GET_PROPERTY) {
>  		struct dtv_frontend_properties getp = fe->dtv_property_cache;
>  
> -		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n", __func__, tvps->num);
> -		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n", __func__, tvps->props);
> +		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
> +				__func__, tvps->num);
> +		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
> +				__func__, tvps->props);
>  
>  		/* Put an arbitrary limit on the number of messages that can
>  		 * be sent at once */
> @@ -2026,7 +2043,35 @@ static int dvb_frontend_ioctl_properties(struct file *file,
>  			err = -EFAULT;
>  			goto out;
>  		}

Indentation is still wrong. Are you setting tab to 8?

> -
> +	/* New ioctl for optimizing property set*/
> +	} else if (cmd == FE_SET_PROPERTY_SHORT) {
> +		struct dtv_property_short *tvp_short = NULL;
> +		struct dtv_properties_short *tvps_short = parg;
> +
> +		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
> +				__func__, tvps_short->num);
> +		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
> +				__func__, tvps_short->props);
> +		if ((!tvps_short->num) ||
> +		    (tvps_short->num > DTV_IOCTL_MAX_MSGS))
> +			return -EINVAL;
> +		tvp_short = memdup_user(tvps_short->props,
> +		tvps_short->num * sizeof(*tvp_short));
> +		if (IS_ERR(tvp_short))
> +			return PTR_ERR(tvp_short);
> +		for (i = 0; i < tvps_short->num; i++) {
> +			err = dtv_property_process_set(fe, file,
> +					(tvp_short + i)->cmd,
> +					(tvp_short + i)->data);
> +			if (err < 0) {
> +				kfree(tvp_short);
> +				return err;
> +			}
> +		}
> +		if (c->state == DTV_TUNE)
> +			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n",
> +					__func__);
> +		kfree(tvp_short);

Place the new ioctl code on the second patch.

>  	} else
>  		err = -EOPNOTSUPP;
>  
> diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
> index 00a20cd..9d96dab 100644
> --- a/include/uapi/linux/dvb/frontend.h
> +++ b/include/uapi/linux/dvb/frontend.h
> @@ -476,6 +476,17 @@ struct dtv_property {
>  	int result;
>  } __attribute__ ((packed));
>  
> +/**
> + * struct dtv_property_short - A shorter version of &struct dtv_property
> + *
> + * @cmd:	Digital TV command.
> + * @data:	An unsigned 32-bits number.
> + */
> +struct dtv_property_short {
> +	__u32 cmd;
> +	__u32 data;
> +};
> +
>  /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
>  #define DTV_IOCTL_MAX_MSGS 64
>  
> @@ -484,6 +495,18 @@ struct dtv_properties {
>  	struct dtv_property *props;
>  };
>  
> +/**
> + * struct dtv_properties_short - A variant of &struct dtv_properties
> + * to support &struct dtv_property_short
> + *
> + * @num:	Number of properties
> + * @props:	Pointer to &struct dtv_property_short
> + */
> +struct dtv_properties_short {
> +	__u32 num;
> +	struct dtv_property_short *props;
> +};
> +
>  #if defined(__DVB_CORE__) || !defined (__KERNEL__)
>  
>  /*
> @@ -565,6 +588,7 @@ struct dvb_frontend_event {
>  
>  #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
>  #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
> +#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
>  
>  /**
>   * When set, this flag will disable any zigzagging or other "normal" tuning

Place the new ioctl code on the second patch.


Thanks,
Mauro

[PATCH v5] [DVB][FRONTEND] Modified the code related to FE_SET_PROPERTY ioctl

[Satendra Singh Thakur]

-Since all the properties in the func dtv_property_process_set
 are 4 bytes, We have added 2 new arguments u32 cmd and u32 data;
 in place of older argument struct dtv_property *tvp.
-We have removed property validation in function
 dtv_property_process_set because most of the frontend drivers in
 linux source are not using the method ops.set_property.
-In place of dtv_property_dump, we have added own logic
 to dump cmd and data of the incoming property
-We have also re-named dtv_property_dump to dtv_get_property_dump.
-We have modified logs in this func to be suitable only for getting
 properties.
Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
---
 drivers/media/dvb-core/dvb_frontend.c | 135 ++++++++++++++++++----------------
 1 file changed, 73 insertions(+), 62 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 2fcba16..013476e 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1092,22 +1092,19 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] = {
 	_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
 };
 
-static void dtv_property_dump(struct dvb_frontend *fe,
-			      bool is_set,
+static void dtv_get_property_dump(struct dvb_frontend *fe,
 			      struct dtv_property *tvp)
 {
 	int i;
 
 	if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
-		dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
-				__func__,
-				is_set ? "SET" : "GET",
+		dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
+				, __func__,
 				tvp->cmd);
 		return;
 	}
 
-	dev_dbg(fe->dvb->device, "%s: %s tvp.cmd    = 0x%08x (%s)\n", __func__,
-		is_set ? "SET" : "GET",
+	dev_dbg(fe->dvb->device, "%s: GET tvp.cmd    = 0x%08x (%s)\n", __func__,
 		tvp->cmd,
 		dtv_cmds[tvp->cmd].name);
 
@@ -1526,7 +1523,7 @@ static int dtv_property_process_get(struct dvb_frontend *fe,
 			return r;
 	}
 
-	dtv_property_dump(fe, false, tvp);
+	dtv_get_property_dump(fe, tvp);
 
 	return 0;
 }
@@ -1749,23 +1746,35 @@ static int dvbv3_set_delivery_system(struct dvb_frontend *fe)
 	return emulate_delivery_system(fe, delsys);
 }
 
+/**
+ * dtv_property_process_set -  Sets a single DTV property
+ * @fe:		Pointer to &struct dvb_frontend
+ * @file:	Pointer to &struct file
+ * @cmd:	Digital TV command
+ * @data:	An unsigned 32-bits number
+ *
+ * This routine assigns the property
+ * value to the corresponding member of
+ * &struct dtv_frontend_properties
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
 static int dtv_property_process_set(struct dvb_frontend *fe,
-				    struct dtv_property *tvp,
-				    struct file *file)
+					struct file *file,
+					u32 cmd, u32 data)
 {
 	int r = 0;
 	struct dtv_frontend_properties *c = &fe->dtv_property_cache;
-
-	/* Allow the frontend to validate incoming properties */
-	if (fe->ops.set_property) {
-		r = fe->ops.set_property(fe, tvp);
-		if (r < 0)
-			return r;
-	}
-
-	dtv_property_dump(fe, true, tvp);
-
-	switch(tvp->cmd) {
+	/** Dump DTV command name and value*/
+	if (!cmd || cmd > DTV_MAX_COMMAND)
+		dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
+				 __func__, cmd);
+	else
+		dev_dbg(fe->dvb->device,
+				"%s: SET cmd 0x%08x (%s) to 0x%08x\n",
+				__func__, cmd, dtv_cmds[cmd].name, data);
+	switch (cmd) {
 	case DTV_CLEAR:
 		/*
 		 * Reset a cache of data specific to the frontend here. This does
@@ -1778,140 +1787,140 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 		 * tunerequest so we can pass validation in the FE_SET_FRONTEND
 		 * ioctl.
 		 */
-		c->state = tvp->cmd;
+		c->state = cmd;
 		dev_dbg(fe->dvb->device, "%s: Finalised property cache\n",
 				__func__);
 
 		r = dtv_set_frontend(fe);
 		break;
 	case DTV_FREQUENCY:
-		c->frequency = tvp->u.data;
+		c->frequency = data;
 		break;
 	case DTV_MODULATION:
-		c->modulation = tvp->u.data;
+		c->modulation = data;
 		break;
 	case DTV_BANDWIDTH_HZ:
-		c->bandwidth_hz = tvp->u.data;
+		c->bandwidth_hz = data;
 		break;
 	case DTV_INVERSION:
-		c->inversion = tvp->u.data;
+		c->inversion = data;
 		break;
 	case DTV_SYMBOL_RATE:
-		c->symbol_rate = tvp->u.data;
+		c->symbol_rate = data;
 		break;
 	case DTV_INNER_FEC:
-		c->fec_inner = tvp->u.data;
+		c->fec_inner = data;
 		break;
 	case DTV_PILOT:
-		c->pilot = tvp->u.data;
+		c->pilot = data;
 		break;
 	case DTV_ROLLOFF:
-		c->rolloff = tvp->u.data;
+		c->rolloff = data;
 		break;
 	case DTV_DELIVERY_SYSTEM:
-		r = dvbv5_set_delivery_system(fe, tvp->u.data);
+		r = dvbv5_set_delivery_system(fe, data);
 		break;
 	case DTV_VOLTAGE:
-		c->voltage = tvp->u.data;
+		c->voltage = data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_VOLTAGE,
 			(void *)c->voltage);
 		break;
 	case DTV_TONE:
-		c->sectone = tvp->u.data;
+		c->sectone = data;
 		r = dvb_frontend_ioctl_legacy(file, FE_SET_TONE,
 			(void *)c->sectone);
 		break;
 	case DTV_CODE_RATE_HP:
-		c->code_rate_HP = tvp->u.data;
+		c->code_rate_HP = data;
 		break;
 	case DTV_CODE_RATE_LP:
-		c->code_rate_LP = tvp->u.data;
+		c->code_rate_LP = data;
 		break;
 	case DTV_GUARD_INTERVAL:
-		c->guard_interval = tvp->u.data;
+		c->guard_interval = data;
 		break;
 	case DTV_TRANSMISSION_MODE:
-		c->transmission_mode = tvp->u.data;
+		c->transmission_mode = data;
 		break;
 	case DTV_HIERARCHY:
-		c->hierarchy = tvp->u.data;
+		c->hierarchy = data;
 		break;
 	case DTV_INTERLEAVING:
-		c->interleaving = tvp->u.data;
+		c->interleaving = data;
 		break;
 
 	/* ISDB-T Support here */
 	case DTV_ISDBT_PARTIAL_RECEPTION:
-		c->isdbt_partial_reception = tvp->u.data;
+		c->isdbt_partial_reception = data;
 		break;
 	case DTV_ISDBT_SOUND_BROADCASTING:
-		c->isdbt_sb_mode = tvp->u.data;
+		c->isdbt_sb_mode = data;
 		break;
 	case DTV_ISDBT_SB_SUBCHANNEL_ID:
-		c->isdbt_sb_subchannel = tvp->u.data;
+		c->isdbt_sb_subchannel = data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_IDX:
-		c->isdbt_sb_segment_idx = tvp->u.data;
+		c->isdbt_sb_segment_idx = data;
 		break;
 	case DTV_ISDBT_SB_SEGMENT_COUNT:
-		c->isdbt_sb_segment_count = tvp->u.data;
+		c->isdbt_sb_segment_count = data;
 		break;
 	case DTV_ISDBT_LAYER_ENABLED:
-		c->isdbt_layer_enabled = tvp->u.data;
+		c->isdbt_layer_enabled = data;
 		break;
 	case DTV_ISDBT_LAYERA_FEC:
-		c->layer[0].fec = tvp->u.data;
+		c->layer[0].fec = data;
 		break;
 	case DTV_ISDBT_LAYERA_MODULATION:
-		c->layer[0].modulation = tvp->u.data;
+		c->layer[0].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERA_SEGMENT_COUNT:
-		c->layer[0].segment_count = tvp->u.data;
+		c->layer[0].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERA_TIME_INTERLEAVING:
-		c->layer[0].interleaving = tvp->u.data;
+		c->layer[0].interleaving = data;
 		break;
 	case DTV_ISDBT_LAYERB_FEC:
-		c->layer[1].fec = tvp->u.data;
+		c->layer[1].fec = data;
 		break;
 	case DTV_ISDBT_LAYERB_MODULATION:
-		c->layer[1].modulation = tvp->u.data;
+		c->layer[1].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERB_SEGMENT_COUNT:
-		c->layer[1].segment_count = tvp->u.data;
+		c->layer[1].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERB_TIME_INTERLEAVING:
-		c->layer[1].interleaving = tvp->u.data;
+		c->layer[1].interleaving = data;
 		break;
 	case DTV_ISDBT_LAYERC_FEC:
-		c->layer[2].fec = tvp->u.data;
+		c->layer[2].fec = data;
 		break;
 	case DTV_ISDBT_LAYERC_MODULATION:
-		c->layer[2].modulation = tvp->u.data;
+		c->layer[2].modulation = data;
 		break;
 	case DTV_ISDBT_LAYERC_SEGMENT_COUNT:
-		c->layer[2].segment_count = tvp->u.data;
+		c->layer[2].segment_count = data;
 		break;
 	case DTV_ISDBT_LAYERC_TIME_INTERLEAVING:
-		c->layer[2].interleaving = tvp->u.data;
+		c->layer[2].interleaving = data;
 		break;
 
 	/* Multistream support */
 	case DTV_STREAM_ID:
 	case DTV_DVBT2_PLP_ID_LEGACY:
-		c->stream_id = tvp->u.data;
+		c->stream_id = data;
 		break;
 
 	/* ATSC-MH */
 	case DTV_ATSCMH_PARADE_ID:
-		fe->dtv_property_cache.atscmh_parade_id = tvp->u.data;
+		fe->dtv_property_cache.atscmh_parade_id = data;
 		break;
 	case DTV_ATSCMH_RS_FRAME_ENSEMBLE:
-		fe->dtv_property_cache.atscmh_rs_frame_ensemble = tvp->u.data;
+		fe->dtv_property_cache.atscmh_rs_frame_ensemble = data;
 		break;
 
 	case DTV_LNA:
-		c->lna = tvp->u.data;
+		c->lna = data;
 		if (fe->ops.set_lna)
 			r = fe->ops.set_lna(fe);
 		if (r < 0)
@@ -1990,7 +1999,9 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			return PTR_ERR(tvp);
 
 		for (i = 0; i < tvps->num; i++) {
-			err = dtv_property_process_set(fe, tvp + i, file);
+			err = dtv_property_process_set(fe, file,
+							(tvp + i)->cmd,
+							(tvp + i)->u.data);
 			if (err < 0)
 				goto out;
 			(tvp + i)->result = err;
-- 
2.7.4

[PATCH v5 2/2] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Satendra Singh Thakur]

-For setting one frontend property , one FE_SET_PROPERTY ioctl is called
-Since, size of struct dtv_property is 72 bytes, this ioctl requires
---allocating 72 bytes of memory in user space
---allocating 72 bytes of memory in kernel space
---copying 72 bytes of data from user space to kernel space
-However, for all the properties, only 8 out of 72 bytes are used
 for setting the property
-Four bytes are needed for specifying property type and another 4 for
 property value
-Moreover, there are 2 properties DTV_CLEAR and DTV_TUNE which use
 only 4 bytes for property name
---They don't use property value
-Therefore, we have defined new short variant/forms/version of currently
 used structures for such 8 byte properties.
-This results in 89% (8*100/72) of memory saving in user and kernel space
 each.
-This also results in faster copy (8 bytes as compared to 72 bytes) from
 user to kernel space
-We have added new ioctl FE_SET_PROPERTY_SHORT which utilizes above
 mentioned new property structures
-This ioctl can co-exist with present ioctl FE_SET_PROPERTY
-If the apps wish to use shorter forms they can use
 proposed FE_SET_PROPERTY_SHORT, rest of them can continue to use
 current versions FE_SET_PROPERTY

Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
---
 Documentation/media/uapi/dvb/fe-get-property.rst | 24 +++++++++++------
 drivers/media/dvb-core/dvb_frontend.c            | 33 +++++++++++++++++++++++-
 include/uapi/linux/dvb/frontend.h                | 24 +++++++++++++++++
 3 files changed, 72 insertions(+), 9 deletions(-)

diff --git a/Documentation/media/uapi/dvb/fe-get-property.rst b/Documentation/media/uapi/dvb/fe-get-property.rst
index 948d2ba..efe95af 100644
--- a/Documentation/media/uapi/dvb/fe-get-property.rst
+++ b/Documentation/media/uapi/dvb/fe-get-property.rst
@@ -2,14 +2,14 @@
 
 .. _FE_GET_PROPERTY:
 
-**************************************
-ioctl FE_SET_PROPERTY, FE_GET_PROPERTY
-**************************************
+**************************************************************
+ioctl FE_SET_PROPERTY, FE_GET_PROPERTY, FE_SET_PROPERTY_SHORT
+**************************************************************
 
 Name
 ====
 
-FE_SET_PROPERTY - FE_GET_PROPERTY - FE_SET_PROPERTY sets one or more frontend properties. - FE_GET_PROPERTY returns one or more frontend properties.
+FE_SET_PROPERTY and FE_SET_PROPERTY_SHORT set one or more frontend properties. FE_GET_PROPERTY returns one or more frontend properties.
 
 
 Synopsis
@@ -21,6 +21,8 @@ Synopsis
 .. c:function:: int ioctl( int fd, FE_SET_PROPERTY, struct dtv_properties *argp )
     :name: FE_SET_PROPERTY
 
+.. c:function:: int ioctl( int fd, FE_SET_PROPERTY_SHORT, struct dtv_properties_short *argp )
+    :name: FE_SET_PROPERTY_SHORT
 
 Arguments
 =========
@@ -29,15 +31,16 @@ Arguments
     File descriptor returned by :ref:`open() <frontend_f_open>`.
 
 ``argp``
-    Pointer to struct :c:type:`dtv_properties`.
+    Pointer to struct :c:type:`dtv_properties` or
+	struct :c:type:`dtv_properties_short`.
 
 
 Description
 ===========
 
-All Digital TV frontend devices support the ``FE_SET_PROPERTY`` and
-``FE_GET_PROPERTY`` ioctls. The supported properties and statistics
-depends on the delivery system and on the device:
+All Digital TV frontend devices support the ``FE_SET_PROPERTY``, ``FE_GET_PROPERTY``
+and ``FE_SET_PROPERTY_SHORT`` ioctls. The supported  properties and
+statistics depends on the delivery system and on the device:
 
 -  ``FE_SET_PROPERTY:``
 
@@ -60,6 +63,11 @@ depends on the delivery system and on the device:
 
    -  This call only requires read-only access to the device.
 
+-  ``FE_SET_PROPERTY_SHORT:``
+
+   -  This ioctl is similar to FE_SET_PROPERTY ioctl mentioned above
+      except that the arguments of the former utilize a struct :c:type:`dtv_property_short`
+      which is smaller in size.
 
 Return Value
 ============
diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 2fcba16..a6a5340 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1925,6 +1925,7 @@ static int dtv_property_process_set(struct dvb_frontend *fe,
 	return r;
 }
 
+
 static int dvb_frontend_ioctl(struct file *file,
 			unsigned int cmd, void *parg)
 {
@@ -1950,7 +1951,8 @@ static int dvb_frontend_ioctl(struct file *file,
 		return -EPERM;
 	}
 
-	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY))
+	if ((cmd == FE_SET_PROPERTY) || (cmd == FE_GET_PROPERTY)
+	    || (cmd == FE_SET_PROPERTY_SHORT))
 		err = dvb_frontend_ioctl_properties(file, cmd, parg);
 	else {
 		c->state = DTV_UNDEFINED;
@@ -2037,6 +2039,35 @@ static int dvb_frontend_ioctl_properties(struct file *file,
 			err = -EFAULT;
 			goto out;
 		}
+	/* New ioctl for optimizing property set*/
+	} else if (cmd == FE_SET_PROPERTY_SHORT) {
+		struct dtv_property_short *tvp_short = NULL;
+		struct dtv_properties_short *tvps_short = parg;
+
+		dev_dbg(fe->dvb->device, "%s: properties.num = %d\n",
+				__func__, tvps_short->num);
+		dev_dbg(fe->dvb->device, "%s: properties.props = %p\n",
+				__func__, tvps_short->props);
+		if ((!tvps_short->num) ||
+		    (tvps_short->num > DTV_IOCTL_MAX_MSGS))
+			return -EINVAL;
+		tvp_short = memdup_user(tvps_short->props,
+		tvps_short->num * sizeof(*tvp_short));
+		if (IS_ERR(tvp_short))
+			return PTR_ERR(tvp_short);
+		for (i = 0; i < tvps_short->num; i++) {
+			err = dtv_property_process_set(fe, file,
+					(tvp_short + i)->cmd,
+					(tvp_short + i)->data);
+			if (err < 0) {
+				kfree(tvp_short);
+				return err;
+			}
+		}
+		if (c->state == DTV_TUNE)
+			dev_dbg(fe->dvb->device, "%s: Property cache is full, tuning\n",
+					__func__);
+		kfree(tvp_short);
 
 	} else
 		err = -EOPNOTSUPP;
diff --git a/include/uapi/linux/dvb/frontend.h b/include/uapi/linux/dvb/frontend.h
index 861cacd..dedee0d 100644
--- a/include/uapi/linux/dvb/frontend.h
+++ b/include/uapi/linux/dvb/frontend.h
@@ -857,6 +857,17 @@ struct dtv_property {
 	int result;
 } __attribute__ ((packed));
 
+/**
+ * struct dtv_property_short - A shorter version of &struct dtv_property
+ *
+ * @cmd:	Digital TV command.
+ * @data:	An unsigned 32-bits number.
+ */
+struct dtv_property_short {
+	__u32 cmd;
+	__u32 data;
+};
+
 /* num of properties cannot exceed DTV_IOCTL_MAX_MSGS per ioctl */
 #define DTV_IOCTL_MAX_MSGS 64
 
@@ -871,6 +882,18 @@ struct dtv_properties {
 	struct dtv_property *props;
 };
 
+/**
+ * struct dtv_properties_short - A variant of &struct dtv_properties
+ * to support &struct dtv_property_short
+ *
+ * @num:	Number of properties
+ * @props:	Pointer to &struct dtv_property_short
+ */
+struct dtv_properties_short {
+	__u32 num;
+	struct dtv_property_short *props;
+};
+
 /*
  * When set, this flag will disable any zigzagging or other "normal" tuning
  * behavior. Additionally, there will be no automatic monitoring of the lock
@@ -906,6 +929,7 @@ struct dtv_properties {
 
 #define FE_SET_PROPERTY		   _IOW('o', 82, struct dtv_properties)
 #define FE_GET_PROPERTY		   _IOR('o', 83, struct dtv_properties)
+#define FE_SET_PROPERTY_SHORT	   _IOW('o', 84, struct dtv_properties_short)
 
 #if defined(__DVB_CORE__) || !defined(__KERNEL__)
 
-- 
2.7.4

Re: [RFC] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Honza Petrouš]

2017-09-14 22:50 GMT+02:00 Mauro Carvalho Chehab <mchehab@s-opensource.com>:
> Hi Satendra,
>
> Em Thu, 14 Sep 2017 05:59:27 -0400
> Satendra Singh Thakur <satendra.t@samsung.com> escreveu:
>
>> -For setting one frontend property , one FE_SET_PROPERTY ioctl is called
>> -Since, size of struct dtv_property is 72 bytes, this ioctl requires
>> ---allocating 72 bytes of memory in user space
>> ---allocating 72 bytes of memory in kernel space
>> ---copying 72 bytes of data from user space to kernel space
>> -However, for all the properties, only 8 out of 72 bytes are used
>>  for setting the property
>
> That's true. Yet, for get, the size can be bigger, as ISDB-T can
> return statistics per layer, plus a global one.
>
>> -Four bytes are needed for specifying property type and another 4 for
>>  property value
>> -Moreover, there are 2 properties DTV_CLEAR and DTV_TUNE which use
>>  only 4 bytes for property name
>> ---They don't use property value
>> -Therefore, we have defined new short variant/forms/version of currently
>>  used structures for such 8 byte properties.
>> -This results in 89% (8*100/72) of memory saving in user and kernel space
>>  each.
>> -This also results in faster copy (8 bytes as compared to 72 bytes) from
>>  user to kernel space
>> -We have added new ioctl FE_SET_PROPERTY_SHORT which utilizes above
>>  mentioned new property structures
>> -This ioctl can co-exist with present ioctl FE_SET_PROPERTY
>> -If the apps wish to use shorter forms they can use
>>  proposed FE_SET_PROPERTY_SHORT, rest of them can continue to use
>>  current versions FE_SET_PROPERTY
>
>> -We are currently not validating incoming properties in
>>  function dtv_property_short_process_set because most of
>>  the frontend drivers in linux source are not using the
>>  method ops.set_property. Just two drivers are using it
>>  drivers/media/dvb-frontends/stv0288.c
>>  driver/media/usb/dvb-usb/friio-fe.c
>>  -Moreover, stv0288 driver implemments blank function
>>  for set_property.
>> -If needed in future, we can define a new
>>  ops.set_property_short method to support
>>  struct dtv_property_short.
>
> Nah. Better to just get rid of get_property()/set_froperty() for good.
>
> Just sent a RFC patch series doing that.
>
> The only thing is that stv6110 seems to have a dirty hack that may
> depend on that. Someone need to double-check if the patch series
> I just sent doesn't break anything. If it breaks, then we'll need
> to add an extra parameter to stv6110 attach for it to know what
> behavior is needed there.

Do you mean in stv6110_set_frequency()?

I must say I was shocked by the beginning of it.
Can somebody explain me the reason for such strange
srate computation?

See the head of function:

static int stv6110_set_frequency(struct dvb_frontend *fe, u32 frequency)
{
        struct stv6110_priv *priv = fe->tuner_priv;
        struct dtv_frontend_properties *c = &fe->dtv_property_cache;
        u8 ret = 0x04;
        u32 divider, ref, p, presc, i, result_freq, vco_freq;
        s32 p_calc, p_calc_opt = 1000, r_div, r_div_opt = 0, p_val;
        s32 srate;

        dprintk("%s, freq=%d kHz, mclk=%d Hz\n", __func__,
                                                frequency, priv->mclk);

        /* K = (Reference / 1000000) - 16 */
        priv->regs[RSTV6110_CTRL1] &= ~(0x1f << 3);
        priv->regs[RSTV6110_CTRL1] |=
                                ((((priv->mclk / 1000000) - 16) & 0x1f) << 3);

        /* BB_GAIN = db/2 */
        if (fe->ops.set_property && fe->ops.get_property) {
                srate = c->symbol_rate;
                dprintk("%s: Get Frontend parameters: srate=%d\n",
                                                        __func__, srate);
        } else
                srate = 15000000;

^^^^ here I would like to note, there there is NO MORE
anything dependant on srate. It looks like some dead code for me.

And the condition sentence looks even more funny - is it
for real to check of retrieval of srate only in case
if some other function pointers are not null?

/Honza

PS: Don't forget that we have duplicated drivers for STV6110,
stv6110 by Igor and stv6110x by Manu.