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@xxxxxxxxxxx>
---
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
