[PATCH V2 0/3] Add timers to en50221 protocol driver

[PATCH V2 0/3] Add timers to en50221 protocol driver

[Jasmin J.]

From: Jasmin Jessich <jasmin@anw.at>

Some (older) CAMs are really slow in accepting data. I got sometimes the 
already known error "CAM tried to send a buffer larger than the ecount 
size". I could track it down to the dvb_ca_en50221_write_data function not 
waiting between sending the data length high/low and data bytes. In fact
the CAM reported a WR error, which triggered later on the mentioned error.
 
The problem is that a simple module parameter can't be used to solve this
by adding timer values, because the protocol handler is used for any CI
interface. A module parameter would be influence all the CAMs on all CI
interfaces. Thus individual timer definitions per CI interface and CAM are
required.
There are two possibilities to implement that, ioctl's and SysFS.
ioctl's require changes in usermode programs and it may take a lot of time
to get this implemented there.
SysFS can be used by simple "cat" and "echo" commands and can be therefore
simply controlled by scripting, which is immediately available.

I decided to go for the SysFS approach, but the required device to add the
SysFS files was not available in the "struct dvb_device". The first patch
of this series adds this device to the structure and also the setting code.

The second patch adds the functions to create the SysFS nodes for all
timers and the new timeouts in the en50221 protocol driver.

The third patch adds the SysFS node documentation.

Jasmin Jessich (3):
  media: dvb-core: Store device structure in dvb_register_device
  media: dvb-core: Added timers for dvb_ca_en50221_write_data
  media: dvb-core: Added documentation for ca sysfs timer nodes

 Documentation/ABI/testing/sysfs-class-ca        |  63 +++++++++++
 Documentation/media/uapi/dvb/ca-sysfs-nodes.rst |  85 +++++++++++++++
 Documentation/media/uapi/dvb/ca.rst             |   1 +
 drivers/media/dvb-core/dvb_ca_en50221.c         | 132 +++++++++++++++++++++++-
 drivers/media/dvb-core/dvbdev.c                 |   1 +
 drivers/media/dvb-core/dvbdev.h                 |   4 +-
 6 files changed, 284 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-ca
 create mode 100644 Documentation/media/uapi/dvb/ca-sysfs-nodes.rst

-- 
2.7.4

[PATCH V2 1/3] media: dvb-core: Store device structure in dvb_register_device

[Jasmin J.]

From: Jasmin Jessich <jasmin@anw.at>

The device created by device_create in dvb_register_device was not
available for DVB device drivers.
Added "struct device *dev" to "struct dvb_device" and store the created
device.

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
Acked-by: Ralph Metzler <rjkm@metzlerbros.de>
---
 drivers/media/dvb-core/dvbdev.c | 1 +
 drivers/media/dvb-core/dvbdev.h | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 060c60d..f55eff1 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -538,6 +538,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
 		       __func__, adap->num, dnames[type], id, PTR_ERR(clsdev));
 		return PTR_ERR(clsdev);
 	}
+	dvbdev->dev = clsdev;
 	dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
 		adap->num, dnames[type], id, minor, minor);
 
diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
index bbc1c20..1f2d2ff 100644
--- a/drivers/media/dvb-core/dvbdev.h
+++ b/drivers/media/dvb-core/dvbdev.h
@@ -147,10 +147,11 @@ struct dvb_adapter {
  * @tsout_num_entities: Number of Transport Stream output entities
  * @tsout_entity: array with MC entities associated to each TS output node
  * @tsout_pads: array with the source pads for each @tsout_entity
+ * @dev:	pointer to struct device that is associated with the dvb device
  *
  * This structure is used by the DVB core (frontend, CA, net, demux) in
  * order to create the device nodes. Usually, driver should not initialize
- * this struct diretly.
+ * this struct directly.
  */
 struct dvb_device {
 	struct list_head list_head;
@@ -183,6 +184,7 @@ struct dvb_device {
 #endif
 
 	void *priv;
+	struct device *dev;
 };
 
 /**
-- 
2.7.4

Re: [PATCH V2 1/3] media: dvb-core: Store device structure in dvb_register_device

[Jasmin J.]

Hi!

Please hold on in merging this series, because I have to investigate a hint
I got related to the buffer size handshake of the protocol driver:
  https://www.linuxtv.org/pipermail/linux-dvb/2007-July/019116.html

BR,
   Jasmin


On 12/21/2017 02:22 PM, Jasmin J. wrote:
> From: Jasmin Jessich <jasmin@anw.at>
> 
> The device created by device_create in dvb_register_device was not
> available for DVB device drivers.
> Added "struct device *dev" to "struct dvb_device" and store the created
> device.
> 
> Signed-off-by: Jasmin Jessich <jasmin@anw.at>
> Acked-by: Ralph Metzler <rjkm@metzlerbros.de>
> ---
>  drivers/media/dvb-core/dvbdev.c | 1 +
>  drivers/media/dvb-core/dvbdev.h | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 060c60d..f55eff1 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -538,6 +538,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
>  		       __func__, adap->num, dnames[type], id, PTR_ERR(clsdev));
>  		return PTR_ERR(clsdev);
>  	}
> +	dvbdev->dev = clsdev;
>  	dprintk("DVB: register adapter%d/%s%d @ minor: %i (0x%02x)\n",
>  		adap->num, dnames[type], id, minor, minor);
>  
> diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
> index bbc1c20..1f2d2ff 100644
> --- a/drivers/media/dvb-core/dvbdev.h
> +++ b/drivers/media/dvb-core/dvbdev.h
> @@ -147,10 +147,11 @@ struct dvb_adapter {
>   * @tsout_num_entities: Number of Transport Stream output entities
>   * @tsout_entity: array with MC entities associated to each TS output node
>   * @tsout_pads: array with the source pads for each @tsout_entity
> + * @dev:	pointer to struct device that is associated with the dvb device
>   *
>   * This structure is used by the DVB core (frontend, CA, net, demux) in
>   * order to create the device nodes. Usually, driver should not initialize
> - * this struct diretly.
> + * this struct directly.
>   */
>  struct dvb_device {
>  	struct list_head list_head;
> @@ -183,6 +184,7 @@ struct dvb_device {
>  #endif
>  
>  	void *priv;
> +	struct device *dev;
>  };
>  
>  /**
> 

[PATCH V2 2/3] media: dvb-core: Added timers for dvb_ca_en50221_write_data

[Jasmin J.]

From: Jasmin Jessich <jasmin@anw.at>

Some (older) CAMs are really slow in accepting data. The CI interface
specification doesn't define a handshake for accepted data. Thus, the
en50221 protocol driver can't control if a data byte has been correctly
written to the CAM.

The current implementation writes the length and the data quick after
each other. Thus, the slow CAMs may generate a WR error, which leads to
the known error logging
   "CAM tried to send a buffer larger than the ecount size".

To solve this issue the en50221 protocol driver needs to wait some CAM
depending time between the different bytes to be written. Because the
time is CAM dependent, an individual value per CAM needs to be set. For
that SysFS is used in favor of ioctl's to allow the control of the timer
values independent from any user space application.

This patch adds the timers and the SysFS nodes to set/get the timeout
values and the timer waiting between the different steps of the CAM write
access. A timer value of 0 (default) means "no timeout".

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
Acked-by: Ralph Metzler <rjkm@metzlerbros.de>
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 132 +++++++++++++++++++++++++++++++-
 1 file changed, 131 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
index a3b2754..9b45d6b 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -86,6 +86,13 @@ MODULE_PARM_DESC(cam_debug, "enable verbose debug messages");
 #define DVB_CA_SLOTSTATE_WAITFR         6
 #define DVB_CA_SLOTSTATE_LINKINIT       7
 
+enum dvb_ca_timers {
+	DVB_CA_TIM_WR_HIGH  /* wait after writing length high */
+,	DVB_CA_TIM_WR_LOW   /* wait after writing length low */
+,	DVB_CA_TIM_WR_DATA  /* wait between data bytes */
+,	DVB_CA_TIM_MAX
+};
+
 /* Information on a CA slot */
 struct dvb_ca_slot {
 	/* current state of the CAM */
@@ -119,6 +126,11 @@ struct dvb_ca_slot {
 	unsigned long timeout;
 };
 
+struct dvb_ca_timer {
+	unsigned long min;
+	unsigned long max;
+};
+
 /* Private CA-interface information */
 struct dvb_ca_private {
 	struct kref refcount;
@@ -161,6 +173,14 @@ struct dvb_ca_private {
 
 	/* mutex serializing ioctls */
 	struct mutex ioctl_mutex;
+
+	struct dvb_ca_timer timers[DVB_CA_TIM_MAX];
+};
+
+static const char dvb_ca_tim_names[DVB_CA_TIM_MAX][15] = {
+	"tim_wr_high"
+,	"tim_wr_low"
+,	"tim_wr_data"
 };
 
 static void dvb_ca_private_free(struct dvb_ca_private *ca)
@@ -223,6 +243,14 @@ static char *findstr(char *haystack, int hlen, char *needle, int nlen)
 	return NULL;
 }
 
+static void dvb_ca_sleep(struct dvb_ca_private *ca, enum dvb_ca_timers tim)
+{
+	unsigned long min = ca->timers[tim].min;
+
+	if (min)
+		usleep_range(min, ca->timers[tim].max);
+}
+
 /* ************************************************************************** */
 /* EN50221 physical interface functions */
 
@@ -869,10 +897,13 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 					    bytes_write >> 8);
 	if (status)
 		goto exit;
+	dvb_ca_sleep(ca, DVB_CA_TIM_WR_HIGH);
+
 	status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_SIZE_LOW,
 					    bytes_write & 0xff);
 	if (status)
 		goto exit;
+	dvb_ca_sleep(ca, DVB_CA_TIM_WR_LOW);
 
 	/* send the buffer */
 	for (i = 0; i < bytes_write; i++) {
@@ -880,6 +911,7 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
 						    buf[i]);
 		if (status)
 			goto exit;
+		dvb_ca_sleep(ca, DVB_CA_TIM_WR_DATA);
 	}
 
 	/* check for write error (WE should now be 0) */
@@ -1834,6 +1866,97 @@ static const struct dvb_device dvbdev_ca = {
 };
 
 /* ************************************************************************** */
+/* EN50221 device attributes (SysFS) */
+
+static int dvb_ca_tim_idx(struct dvb_ca_private *ca, const char *name)
+{
+	int tim_idx;
+
+	for (tim_idx = 0; tim_idx < DVB_CA_TIM_MAX; tim_idx++) {
+		if (!strcmp(dvb_ca_tim_names[tim_idx], name))
+			return tim_idx;
+	}
+	return -1;
+}
+
+static ssize_t dvb_ca_tim_show(struct device *device,
+			       struct device_attribute *attr, char *buf)
+{
+	struct dvb_device *dvbdev = dev_get_drvdata(device);
+	struct dvb_ca_private *ca = dvbdev->priv;
+	int tim_idx = dvb_ca_tim_idx(ca, attr->attr.name);
+
+	if (tim_idx < 0)
+		return -ENXIO;
+
+	return sprintf(buf, "%ld\n", ca->timers[tim_idx].min);
+}
+
+static ssize_t dvb_ca_tim_store(struct device *device,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	struct dvb_device *dvbdev = dev_get_drvdata(device);
+	struct dvb_ca_private *ca = dvbdev->priv;
+	int tim_idx = dvb_ca_tim_idx(ca, attr->attr.name);
+	unsigned long min, max;
+
+	if (tim_idx < 0)
+		return -ENXIO;
+
+	if (sscanf(buf, "%lu\n", &min) != 1)
+		return -EINVAL;
+
+	/* value is in us; 100ms is a good maximum */
+	if (min > (100 * USEC_PER_MSEC))
+		return -EINVAL;
+
+	/* +10% (rounded up) */
+	max = (min * 11 + 5) / 10;
+	ca->timers[tim_idx].min = min;
+	ca->timers[tim_idx].max = max;
+
+	return count;
+}
+
+/* attribute definition with string pointer (see include/linux/sysfs.h) */
+#define DVB_CA_ATTR(_name, _mode, _show, _store) {	\
+	.attr = {.name = _name, .mode = _mode },	\
+	.show	= _show,				\
+	.store	= _store,				\
+}
+
+#define DVB_CA_ATTR_TIM(_tim_idx)					\
+	DVB_CA_ATTR(dvb_ca_tim_names[_tim_idx], 0664, dvb_ca_tim_show,	\
+		    dvb_ca_tim_store)
+
+static const struct device_attribute dvb_ca_attrs[DVB_CA_TIM_MAX] = {
+	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_HIGH)
+,	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_LOW)
+,	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_DATA)
+};
+
+static int dvb_ca_device_attrs_add(struct dvb_ca_private *ca)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dvb_ca_attrs); i++)
+		if (device_create_file(ca->dvbdev->dev, &dvb_ca_attrs[i]))
+			goto fail;
+	return 0;
+fail:
+	return -1;
+}
+
+static void ddb_device_attrs_del(struct dvb_ca_private *ca)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dvb_ca_attrs); i++)
+		device_remove_file(ca->dvbdev->dev, &dvb_ca_attrs[i]);
+}
+
+/* ************************************************************************** */
 /* Initialisation/shutdown functions */
 
 /**
@@ -1903,6 +2026,10 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
 		ret = -EINTR;
 		goto unregister_device;
 	}
+
+	if (dvb_ca_device_attrs_add(ca))
+		goto unregister_device;
+
 	mb();
 
 	/* create a kthread for monitoring this CA device */
@@ -1912,10 +2039,12 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
 		ret = PTR_ERR(ca->thread);
 		pr_err("dvb_ca_init: failed to start kernel_thread (%d)\n",
 		       ret);
-		goto unregister_device;
+		goto delete_attrs;
 	}
 	return 0;
 
+delete_attrs:
+	ddb_device_attrs_del(ca);
 unregister_device:
 	dvb_unregister_device(ca->dvbdev);
 free_slot_info:
@@ -1946,6 +2075,7 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
 	for (i = 0; i < ca->slot_count; i++)
 		dvb_ca_en50221_slot_shutdown(ca, i);
 
+	ddb_device_attrs_del(ca);
 	dvb_remove_device(ca->dvbdev);
 	dvb_ca_private_put(ca);
 	pubca->private = NULL;
-- 
2.7.4

Re: [PATCH V2 2/3] media: dvb-core: Added timers for dvb_ca_en50221_write_data

[Jasmin J.]

Hi!

Please hold on in merging this series, because I have to investigate a hint
I got related to the buffer size handshake of the protocol driver:
  https://www.linuxtv.org/pipermail/linux-dvb/2007-July/019116.html

BR,
   Jasmin


On 12/21/2017 02:22 PM, Jasmin J. wrote:
> From: Jasmin Jessich <jasmin@anw.at>
> 
> Some (older) CAMs are really slow in accepting data. The CI interface
> specification doesn't define a handshake for accepted data. Thus, the
> en50221 protocol driver can't control if a data byte has been correctly
> written to the CAM.
> 
> The current implementation writes the length and the data quick after
> each other. Thus, the slow CAMs may generate a WR error, which leads to
> the known error logging
>    "CAM tried to send a buffer larger than the ecount size".
> 
> To solve this issue the en50221 protocol driver needs to wait some CAM
> depending time between the different bytes to be written. Because the
> time is CAM dependent, an individual value per CAM needs to be set. For
> that SysFS is used in favor of ioctl's to allow the control of the timer
> values independent from any user space application.
> 
> This patch adds the timers and the SysFS nodes to set/get the timeout
> values and the timer waiting between the different steps of the CAM write
> access. A timer value of 0 (default) means "no timeout".
> 
> Signed-off-by: Jasmin Jessich <jasmin@anw.at>
> Acked-by: Ralph Metzler <rjkm@metzlerbros.de>
> ---
>  drivers/media/dvb-core/dvb_ca_en50221.c | 132 +++++++++++++++++++++++++++++++-
>  1 file changed, 131 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
> index a3b2754..9b45d6b 100644
> --- a/drivers/media/dvb-core/dvb_ca_en50221.c
> +++ b/drivers/media/dvb-core/dvb_ca_en50221.c
> @@ -86,6 +86,13 @@ MODULE_PARM_DESC(cam_debug, "enable verbose debug messages");
>  #define DVB_CA_SLOTSTATE_WAITFR         6
>  #define DVB_CA_SLOTSTATE_LINKINIT       7
>  
> +enum dvb_ca_timers {
> +	DVB_CA_TIM_WR_HIGH  /* wait after writing length high */
> +,	DVB_CA_TIM_WR_LOW   /* wait after writing length low */
> +,	DVB_CA_TIM_WR_DATA  /* wait between data bytes */
> +,	DVB_CA_TIM_MAX
> +};
> +
>  /* Information on a CA slot */
>  struct dvb_ca_slot {
>  	/* current state of the CAM */
> @@ -119,6 +126,11 @@ struct dvb_ca_slot {
>  	unsigned long timeout;
>  };
>  
> +struct dvb_ca_timer {
> +	unsigned long min;
> +	unsigned long max;
> +};
> +
>  /* Private CA-interface information */
>  struct dvb_ca_private {
>  	struct kref refcount;
> @@ -161,6 +173,14 @@ struct dvb_ca_private {
>  
>  	/* mutex serializing ioctls */
>  	struct mutex ioctl_mutex;
> +
> +	struct dvb_ca_timer timers[DVB_CA_TIM_MAX];
> +};
> +
> +static const char dvb_ca_tim_names[DVB_CA_TIM_MAX][15] = {
> +	"tim_wr_high"
> +,	"tim_wr_low"
> +,	"tim_wr_data"
>  };
>  
>  static void dvb_ca_private_free(struct dvb_ca_private *ca)
> @@ -223,6 +243,14 @@ static char *findstr(char *haystack, int hlen, char *needle, int nlen)
>  	return NULL;
>  }
>  
> +static void dvb_ca_sleep(struct dvb_ca_private *ca, enum dvb_ca_timers tim)
> +{
> +	unsigned long min = ca->timers[tim].min;
> +
> +	if (min)
> +		usleep_range(min, ca->timers[tim].max);
> +}
> +
>  /* ************************************************************************** */
>  /* EN50221 physical interface functions */
>  
> @@ -869,10 +897,13 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
>  					    bytes_write >> 8);
>  	if (status)
>  		goto exit;
> +	dvb_ca_sleep(ca, DVB_CA_TIM_WR_HIGH);
> +
>  	status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_SIZE_LOW,
>  					    bytes_write & 0xff);
>  	if (status)
>  		goto exit;
> +	dvb_ca_sleep(ca, DVB_CA_TIM_WR_LOW);
>  
>  	/* send the buffer */
>  	for (i = 0; i < bytes_write; i++) {
> @@ -880,6 +911,7 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
>  						    buf[i]);
>  		if (status)
>  			goto exit;
> +		dvb_ca_sleep(ca, DVB_CA_TIM_WR_DATA);
>  	}
>  
>  	/* check for write error (WE should now be 0) */
> @@ -1834,6 +1866,97 @@ static const struct dvb_device dvbdev_ca = {
>  };
>  
>  /* ************************************************************************** */
> +/* EN50221 device attributes (SysFS) */
> +
> +static int dvb_ca_tim_idx(struct dvb_ca_private *ca, const char *name)
> +{
> +	int tim_idx;
> +
> +	for (tim_idx = 0; tim_idx < DVB_CA_TIM_MAX; tim_idx++) {
> +		if (!strcmp(dvb_ca_tim_names[tim_idx], name))
> +			return tim_idx;
> +	}
> +	return -1;
> +}
> +
> +static ssize_t dvb_ca_tim_show(struct device *device,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	struct dvb_device *dvbdev = dev_get_drvdata(device);
> +	struct dvb_ca_private *ca = dvbdev->priv;
> +	int tim_idx = dvb_ca_tim_idx(ca, attr->attr.name);
> +
> +	if (tim_idx < 0)
> +		return -ENXIO;
> +
> +	return sprintf(buf, "%ld\n", ca->timers[tim_idx].min);
> +}
> +
> +static ssize_t dvb_ca_tim_store(struct device *device,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	struct dvb_device *dvbdev = dev_get_drvdata(device);
> +	struct dvb_ca_private *ca = dvbdev->priv;
> +	int tim_idx = dvb_ca_tim_idx(ca, attr->attr.name);
> +	unsigned long min, max;
> +
> +	if (tim_idx < 0)
> +		return -ENXIO;
> +
> +	if (sscanf(buf, "%lu\n", &min) != 1)
> +		return -EINVAL;
> +
> +	/* value is in us; 100ms is a good maximum */
> +	if (min > (100 * USEC_PER_MSEC))
> +		return -EINVAL;
> +
> +	/* +10% (rounded up) */
> +	max = (min * 11 + 5) / 10;
> +	ca->timers[tim_idx].min = min;
> +	ca->timers[tim_idx].max = max;
> +
> +	return count;
> +}
> +
> +/* attribute definition with string pointer (see include/linux/sysfs.h) */
> +#define DVB_CA_ATTR(_name, _mode, _show, _store) {	\
> +	.attr = {.name = _name, .mode = _mode },	\
> +	.show	= _show,				\
> +	.store	= _store,				\
> +}
> +
> +#define DVB_CA_ATTR_TIM(_tim_idx)					\
> +	DVB_CA_ATTR(dvb_ca_tim_names[_tim_idx], 0664, dvb_ca_tim_show,	\
> +		    dvb_ca_tim_store)
> +
> +static const struct device_attribute dvb_ca_attrs[DVB_CA_TIM_MAX] = {
> +	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_HIGH)
> +,	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_LOW)
> +,	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_DATA)
> +};
> +
> +static int dvb_ca_device_attrs_add(struct dvb_ca_private *ca)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(dvb_ca_attrs); i++)
> +		if (device_create_file(ca->dvbdev->dev, &dvb_ca_attrs[i]))
> +			goto fail;
> +	return 0;
> +fail:
> +	return -1;
> +}
> +
> +static void ddb_device_attrs_del(struct dvb_ca_private *ca)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(dvb_ca_attrs); i++)
> +		device_remove_file(ca->dvbdev->dev, &dvb_ca_attrs[i]);
> +}
> +
> +/* ************************************************************************** */
>  /* Initialisation/shutdown functions */
>  
>  /**
> @@ -1903,6 +2026,10 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
>  		ret = -EINTR;
>  		goto unregister_device;
>  	}
> +
> +	if (dvb_ca_device_attrs_add(ca))
> +		goto unregister_device;
> +
>  	mb();
>  
>  	/* create a kthread for monitoring this CA device */
> @@ -1912,10 +2039,12 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
>  		ret = PTR_ERR(ca->thread);
>  		pr_err("dvb_ca_init: failed to start kernel_thread (%d)\n",
>  		       ret);
> -		goto unregister_device;
> +		goto delete_attrs;
>  	}
>  	return 0;
>  
> +delete_attrs:
> +	ddb_device_attrs_del(ca);
>  unregister_device:
>  	dvb_unregister_device(ca->dvbdev);
>  free_slot_info:
> @@ -1946,6 +2075,7 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
>  	for (i = 0; i < ca->slot_count; i++)
>  		dvb_ca_en50221_slot_shutdown(ca, i);
>  
> +	ddb_device_attrs_del(ca);
>  	dvb_remove_device(ca->dvbdev);
>  	dvb_ca_private_put(ca);
>  	pubca->private = NULL;
> 

Re: [PATCH V2 2/3] media: dvb-core: Added timers for dvb_ca_en50221_write_data

[Mauro Carvalho Chehab]

Em Wed, 14 Feb 2018 00:29:43 +0100
"Jasmin J." <jasmin@anw.at> escreveu:

> Hi!
> 
> Please hold on in merging this series, because I have to investigate a hint
> I got related to the buffer size handshake of the protocol driver:
>   https://www.linuxtv.org/pipermail/linux-dvb/2007-July/019116.html

Ok.

I'll mark this series as "Changes requested".

Please resubmit it once you have a new patch series.

> 
> BR,
>    Jasmin
> 
> 
> On 12/21/2017 02:22 PM, Jasmin J. wrote:
> > From: Jasmin Jessich <jasmin@anw.at>
> > 
> > Some (older) CAMs are really slow in accepting data. The CI interface
> > specification doesn't define a handshake for accepted data. Thus, the
> > en50221 protocol driver can't control if a data byte has been correctly
> > written to the CAM.
> > 
> > The current implementation writes the length and the data quick after
> > each other. Thus, the slow CAMs may generate a WR error, which leads to
> > the known error logging
> >    "CAM tried to send a buffer larger than the ecount size".
> > 
> > To solve this issue the en50221 protocol driver needs to wait some CAM
> > depending time between the different bytes to be written. Because the
> > time is CAM dependent, an individual value per CAM needs to be set. For
> > that SysFS is used in favor of ioctl's to allow the control of the timer
> > values independent from any user space application.
> > 
> > This patch adds the timers and the SysFS nodes to set/get the timeout
> > values and the timer waiting between the different steps of the CAM write
> > access. A timer value of 0 (default) means "no timeout".
> > 
> > Signed-off-by: Jasmin Jessich <jasmin@anw.at>
> > Acked-by: Ralph Metzler <rjkm@metzlerbros.de>
> > ---
> >  drivers/media/dvb-core/dvb_ca_en50221.c | 132 +++++++++++++++++++++++++++++++-
> >  1 file changed, 131 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c b/drivers/media/dvb-core/dvb_ca_en50221.c
> > index a3b2754..9b45d6b 100644
> > --- a/drivers/media/dvb-core/dvb_ca_en50221.c
> > +++ b/drivers/media/dvb-core/dvb_ca_en50221.c
> > @@ -86,6 +86,13 @@ MODULE_PARM_DESC(cam_debug, "enable verbose debug messages");
> >  #define DVB_CA_SLOTSTATE_WAITFR         6
> >  #define DVB_CA_SLOTSTATE_LINKINIT       7
> >  
> > +enum dvb_ca_timers {
> > +	DVB_CA_TIM_WR_HIGH  /* wait after writing length high */
> > +,	DVB_CA_TIM_WR_LOW   /* wait after writing length low */
> > +,	DVB_CA_TIM_WR_DATA  /* wait between data bytes */
> > +,	DVB_CA_TIM_MAX
> > +};
> > +
> >  /* Information on a CA slot */
> >  struct dvb_ca_slot {
> >  	/* current state of the CAM */
> > @@ -119,6 +126,11 @@ struct dvb_ca_slot {
> >  	unsigned long timeout;
> >  };
> >  
> > +struct dvb_ca_timer {
> > +	unsigned long min;
> > +	unsigned long max;
> > +};
> > +
> >  /* Private CA-interface information */
> >  struct dvb_ca_private {
> >  	struct kref refcount;
> > @@ -161,6 +173,14 @@ struct dvb_ca_private {
> >  
> >  	/* mutex serializing ioctls */
> >  	struct mutex ioctl_mutex;
> > +
> > +	struct dvb_ca_timer timers[DVB_CA_TIM_MAX];
> > +};
> > +
> > +static const char dvb_ca_tim_names[DVB_CA_TIM_MAX][15] = {
> > +	"tim_wr_high"
> > +,	"tim_wr_low"
> > +,	"tim_wr_data"
> >  };
> >  
> >  static void dvb_ca_private_free(struct dvb_ca_private *ca)
> > @@ -223,6 +243,14 @@ static char *findstr(char *haystack, int hlen, char *needle, int nlen)
> >  	return NULL;
> >  }
> >  
> > +static void dvb_ca_sleep(struct dvb_ca_private *ca, enum dvb_ca_timers tim)
> > +{
> > +	unsigned long min = ca->timers[tim].min;
> > +
> > +	if (min)
> > +		usleep_range(min, ca->timers[tim].max);
> > +}
> > +
> >  /* ************************************************************************** */
> >  /* EN50221 physical interface functions */
> >  
> > @@ -869,10 +897,13 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
> >  					    bytes_write >> 8);
> >  	if (status)
> >  		goto exit;
> > +	dvb_ca_sleep(ca, DVB_CA_TIM_WR_HIGH);
> > +
> >  	status = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_SIZE_LOW,
> >  					    bytes_write & 0xff);
> >  	if (status)
> >  		goto exit;
> > +	dvb_ca_sleep(ca, DVB_CA_TIM_WR_LOW);
> >  
> >  	/* send the buffer */
> >  	for (i = 0; i < bytes_write; i++) {
> > @@ -880,6 +911,7 @@ static int dvb_ca_en50221_write_data(struct dvb_ca_private *ca, int slot,
> >  						    buf[i]);
> >  		if (status)
> >  			goto exit;
> > +		dvb_ca_sleep(ca, DVB_CA_TIM_WR_DATA);
> >  	}
> >  
> >  	/* check for write error (WE should now be 0) */
> > @@ -1834,6 +1866,97 @@ static const struct dvb_device dvbdev_ca = {
> >  };
> >  
> >  /* ************************************************************************** */
> > +/* EN50221 device attributes (SysFS) */
> > +
> > +static int dvb_ca_tim_idx(struct dvb_ca_private *ca, const char *name)
> > +{
> > +	int tim_idx;
> > +
> > +	for (tim_idx = 0; tim_idx < DVB_CA_TIM_MAX; tim_idx++) {
> > +		if (!strcmp(dvb_ca_tim_names[tim_idx], name))
> > +			return tim_idx;
> > +	}
> > +	return -1;
> > +}
> > +
> > +static ssize_t dvb_ca_tim_show(struct device *device,
> > +			       struct device_attribute *attr, char *buf)
> > +{
> > +	struct dvb_device *dvbdev = dev_get_drvdata(device);
> > +	struct dvb_ca_private *ca = dvbdev->priv;
> > +	int tim_idx = dvb_ca_tim_idx(ca, attr->attr.name);
> > +
> > +	if (tim_idx < 0)
> > +		return -ENXIO;
> > +
> > +	return sprintf(buf, "%ld\n", ca->timers[tim_idx].min);
> > +}
> > +
> > +static ssize_t dvb_ca_tim_store(struct device *device,
> > +				struct device_attribute *attr,
> > +				const char *buf, size_t count)
> > +{
> > +	struct dvb_device *dvbdev = dev_get_drvdata(device);
> > +	struct dvb_ca_private *ca = dvbdev->priv;
> > +	int tim_idx = dvb_ca_tim_idx(ca, attr->attr.name);
> > +	unsigned long min, max;
> > +
> > +	if (tim_idx < 0)
> > +		return -ENXIO;
> > +
> > +	if (sscanf(buf, "%lu\n", &min) != 1)
> > +		return -EINVAL;
> > +
> > +	/* value is in us; 100ms is a good maximum */
> > +	if (min > (100 * USEC_PER_MSEC))
> > +		return -EINVAL;
> > +
> > +	/* +10% (rounded up) */
> > +	max = (min * 11 + 5) / 10;
> > +	ca->timers[tim_idx].min = min;
> > +	ca->timers[tim_idx].max = max;
> > +
> > +	return count;
> > +}
> > +
> > +/* attribute definition with string pointer (see include/linux/sysfs.h) */
> > +#define DVB_CA_ATTR(_name, _mode, _show, _store) {	\
> > +	.attr = {.name = _name, .mode = _mode },	\
> > +	.show	= _show,				\
> > +	.store	= _store,				\
> > +}
> > +
> > +#define DVB_CA_ATTR_TIM(_tim_idx)					\
> > +	DVB_CA_ATTR(dvb_ca_tim_names[_tim_idx], 0664, dvb_ca_tim_show,	\
> > +		    dvb_ca_tim_store)
> > +
> > +static const struct device_attribute dvb_ca_attrs[DVB_CA_TIM_MAX] = {
> > +	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_HIGH)
> > +,	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_LOW)
> > +,	DVB_CA_ATTR_TIM(DVB_CA_TIM_WR_DATA)
> > +};
> > +
> > +static int dvb_ca_device_attrs_add(struct dvb_ca_private *ca)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(dvb_ca_attrs); i++)
> > +		if (device_create_file(ca->dvbdev->dev, &dvb_ca_attrs[i]))
> > +			goto fail;
> > +	return 0;
> > +fail:
> > +	return -1;
> > +}
> > +
> > +static void ddb_device_attrs_del(struct dvb_ca_private *ca)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(dvb_ca_attrs); i++)
> > +		device_remove_file(ca->dvbdev->dev, &dvb_ca_attrs[i]);
> > +}
> > +
> > +/* ************************************************************************** */
> >  /* Initialisation/shutdown functions */
> >  
> >  /**
> > @@ -1903,6 +2026,10 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
> >  		ret = -EINTR;
> >  		goto unregister_device;
> >  	}
> > +
> > +	if (dvb_ca_device_attrs_add(ca))
> > +		goto unregister_device;
> > +
> >  	mb();
> >  
> >  	/* create a kthread for monitoring this CA device */
> > @@ -1912,10 +2039,12 @@ int dvb_ca_en50221_init(struct dvb_adapter *dvb_adapter,
> >  		ret = PTR_ERR(ca->thread);
> >  		pr_err("dvb_ca_init: failed to start kernel_thread (%d)\n",
> >  		       ret);
> > -		goto unregister_device;
> > +		goto delete_attrs;
> >  	}
> >  	return 0;
> >  
> > +delete_attrs:
> > +	ddb_device_attrs_del(ca);
> >  unregister_device:
> >  	dvb_unregister_device(ca->dvbdev);
> >  free_slot_info:
> > @@ -1946,6 +2075,7 @@ void dvb_ca_en50221_release(struct dvb_ca_en50221 *pubca)
> >  	for (i = 0; i < ca->slot_count; i++)
> >  		dvb_ca_en50221_slot_shutdown(ca, i);
> >  
> > +	ddb_device_attrs_del(ca);
> >  	dvb_remove_device(ca->dvbdev);
> >  	dvb_ca_private_put(ca);
> >  	pubca->private = NULL;
> >   



Thanks,
Mauro

[PATCH V2 3/3] media: dvb-core: Added documentation for ca sysfs timer nodes

[Jasmin J.]

From: Jasmin Jessich <jasmin@anw.at>

Added the documentation for the new ca? sysfs nodes in
/sys/class/dvb/dvb?/ca?/tim_wr_????.

Signed-off-by: Jasmin Jessich <jasmin@anw.at>
---
 Documentation/ABI/testing/sysfs-class-ca        | 63 ++++++++++++++++++
 Documentation/media/uapi/dvb/ca-sysfs-nodes.rst | 85 +++++++++++++++++++++++++
 Documentation/media/uapi/dvb/ca.rst             |  1 +
 3 files changed, 149 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-class-ca
 create mode 100644 Documentation/media/uapi/dvb/ca-sysfs-nodes.rst

diff --git a/Documentation/ABI/testing/sysfs-class-ca b/Documentation/ABI/testing/sysfs-class-ca
new file mode 100644
index 0000000..7a2a52c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-ca
@@ -0,0 +1,63 @@
+What:		/sys/class/dvb/dvbN/
+Date:		Dec 2017
+KernelVersion:	4.15
+Contact:	Jasmin Jessich <jasmin@anw.at>
+Description:
+        The dvbN/ class sub-directory belongs to the Adapter with the
+        index N. It is created for each found Adapter and depends on
+        the DVB hardware.
+
+What:		/sys/class/dvb/dvbN/caM
+Date:		Dec 2017
+KernelVersion:	4.15
+Contact:	Jasmin Jessich <jasmin@anw.at>
+Description:
+        The dvbN/caM/ class sub-directory belongs to the CA device with
+        the index M on the Adapter with the index N. It is created for
+        each found Conditional Access Interface where M is the number
+        of the CA Interface.
+
+What:		/sys/class/dvb/dvbN/caM/tim_wr_high
+Date:		Dec 2017
+KernelVersion:	4.15
+Contact:	Jasmin Jessich <jasmin@anw.at>
+Description:
+        Reading this file returns the wait time after writing the
+        length high byte to the CAM. The default timeout it '0', which
+        means no 'no timeout'. Any other value specifies the timeout in
+        micro seconds.
+          
+        Writing a value will change the timeout.
+             
+        Write fails with ``EINVAL`` if an invalid value has been written
+        (valid values are 0..100000).
+
+What:		/sys/class/dvb/dvbN/caM/tim_wr_low
+Date:		Dec 2017
+KernelVersion:	4.15
+Contact:	Jasmin Jessich <jasmin@anw.at>
+Description:
+        Reading this file returns the wait time after writing the
+        length low byte to the CAM. The default timeout it '0', which
+        means no 'no timeout'. Any other  value specifies the timeout in
+        micro seconds.
+          
+        Writing a value will change the timeout.
+             
+        Write fails with ``EINVAL`` if an invalid value has been written
+        (valid values are 0..100000).
+
+What:		/sys/class/dvb/dvbN/caM/tim_wr_data
+Date:		Dec 2017
+KernelVersion:	4.15
+Contact:	Jasmin Jessich <jasmin@anw.at>
+Description:
+        Reading this file returns the wait time between data bytes sent
+        to the CAM. The default timeout it '0', which means no 'no timeout'.
+        Any other value specifies the timeout in micro seconds.
+
+        Writing a value will change the timeout.
+             
+        Write fails with ``EINVAL`` if an invalid value has been written
+        (valid values are 0..100000).
+
diff --git a/Documentation/media/uapi/dvb/ca-sysfs-nodes.rst b/Documentation/media/uapi/dvb/ca-sysfs-nodes.rst
new file mode 100644
index 0000000..4a26afd
--- /dev/null
+++ b/Documentation/media/uapi/dvb/ca-sysfs-nodes.rst
@@ -0,0 +1,85 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _ca_sysfs_nodes:
+
+******************************
+Conditional Access sysfs nodes
+******************************
+
+As defined at ``Documentation/ABI/testing/sysfs-class-ca``, those are
+the sysfs nodes that control the en50221 CA driver:
+
+
+.. _sys_class_dvb_dvbN:
+
+/sys/class/dvb/dvbN/
+====================
+
+The ``/sys/class/dvb/dvbN/`` class sub-directory belongs to the Adapter
+with the index N. It is created for each found Adapter and depends on the
+DVB hardware.
+
+
+.. _sys_class_dvb_dvbN_caM:
+
+/sys/class/dvb/dvbN/caM
+=======================
+
+The ``/sys/class/dvb/dvbN/caM`` class sub-directory belongs to the CA device
+with the index M on the Adapter with the index N. It is created for each
+found Conditional Access Interface where M is the number of the CA Interface.
+A Conditional Access Module (CAM) will be inserted into the CI interface. The
+caM device is used to communicate to the CAM.
+
+The communication protocol contains a length field followed by the data bytes.
+The length is written in two parts. First the high byte of the length
+followed by the low byte. The following sysfs nodes define three timeouts
+which may be used to extend the communication to the CAM. Modern CAMs usually
+do not need those timeouts, but older CAMs will produce communication errors,
+when the bytes are written too fast. The underliying hardware has also a big
+impact due to the access speed.
+
+
+.. _sys_class_dvb_dvbN_caM_tim_wr_high:
+
+/sys/class/dvb/dvbN/caM/tim_wr_high
+===================================
+
+Reading this file returns the wait time after writing the length high byte to
+the CAM. The default timeout it '0', which means no 'no timeout'. Any other
+value specifies the timeout in micro seconds.
+
+Writing a value will change the timeout.
+
+Write fails with ``EINVAL`` if an invalid value has been written (valid values
+are 0..100000).
+
+
+.. _sys_class_dvb_dvbN_caM_tim_wr_low:
+
+/sys/class/dvb/dvbN/caM/tim_wr_low
+==================================
+
+Reading this file returns the wait time after writing the length low byte to
+the CAM. The default timeout it '0', which means no 'no timeout'. Any other
+value specifies the timeout in micro seconds.
+
+Writing a value will change the timeout.
+
+Write fails with ``EINVAL`` if an invalid value has been written (valid values
+are 0..100000).
+
+
+.. _sys_class_dvb_dvbN_caM_tim_wr_data:
+
+/sys/class/dvb/dvbN/caM/tim_wr_data
+===================================
+
+Reading this file returns the wait time between data bytes sent to the CAM.
+The default timeout it '0', which means no 'no timeout'. Any other value
+specifies the timeout in micro seconds.
+
+Writing a value will change the timeout.
+
+Write fails with ``EINVAL`` if an invalid value has been written (valid values
+are 0..100000).
diff --git a/Documentation/media/uapi/dvb/ca.rst b/Documentation/media/uapi/dvb/ca.rst
index deac72d..e790d19d 100644
--- a/Documentation/media/uapi/dvb/ca.rst
+++ b/Documentation/media/uapi/dvb/ca.rst
@@ -22,3 +22,4 @@ application.
 
     ca_data_types
     ca_function_calls
+    ca-sysfs-nodes
-- 
2.7.4

Re: [PATCH V2 3/3] media: dvb-core: Added documentation for ca sysfs timer nodes

[Jasmin J.]

Hi!

Please hold on in merging this series, because I have to investigate a hint
I got related to the buffer size handshake of the protocol driver:
  https://www.linuxtv.org/pipermail/linux-dvb/2007-July/019116.html

BR,
   Jasmin


On 12/21/2017 02:22 PM, Jasmin J. wrote:
> From: Jasmin Jessich <jasmin@anw.at>
> 
> Added the documentation for the new ca? sysfs nodes in
> /sys/class/dvb/dvb?/ca?/tim_wr_????.
> 
> Signed-off-by: Jasmin Jessich <jasmin@anw.at>
> ---
>  Documentation/ABI/testing/sysfs-class-ca        | 63 ++++++++++++++++++
>  Documentation/media/uapi/dvb/ca-sysfs-nodes.rst | 85 +++++++++++++++++++++++++
>  Documentation/media/uapi/dvb/ca.rst             |  1 +
>  3 files changed, 149 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-ca
>  create mode 100644 Documentation/media/uapi/dvb/ca-sysfs-nodes.rst
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-ca b/Documentation/ABI/testing/sysfs-class-ca
> new file mode 100644
> index 0000000..7a2a52c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-ca
> @@ -0,0 +1,63 @@
> +What:		/sys/class/dvb/dvbN/
> +Date:		Dec 2017
> +KernelVersion:	4.15
> +Contact:	Jasmin Jessich <jasmin@anw.at>
> +Description:
> +        The dvbN/ class sub-directory belongs to the Adapter with the
> +        index N. It is created for each found Adapter and depends on
> +        the DVB hardware.
> +
> +What:		/sys/class/dvb/dvbN/caM
> +Date:		Dec 2017
> +KernelVersion:	4.15
> +Contact:	Jasmin Jessich <jasmin@anw.at>
> +Description:
> +        The dvbN/caM/ class sub-directory belongs to the CA device with
> +        the index M on the Adapter with the index N. It is created for
> +        each found Conditional Access Interface where M is the number
> +        of the CA Interface.
> +
> +What:		/sys/class/dvb/dvbN/caM/tim_wr_high
> +Date:		Dec 2017
> +KernelVersion:	4.15
> +Contact:	Jasmin Jessich <jasmin@anw.at>
> +Description:
> +        Reading this file returns the wait time after writing the
> +        length high byte to the CAM. The default timeout it '0', which
> +        means no 'no timeout'. Any other value specifies the timeout in
> +        micro seconds.
> +          
> +        Writing a value will change the timeout.
> +             
> +        Write fails with ``EINVAL`` if an invalid value has been written
> +        (valid values are 0..100000).
> +
> +What:		/sys/class/dvb/dvbN/caM/tim_wr_low
> +Date:		Dec 2017
> +KernelVersion:	4.15
> +Contact:	Jasmin Jessich <jasmin@anw.at>
> +Description:
> +        Reading this file returns the wait time after writing the
> +        length low byte to the CAM. The default timeout it '0', which
> +        means no 'no timeout'. Any other  value specifies the timeout in
> +        micro seconds.
> +          
> +        Writing a value will change the timeout.
> +             
> +        Write fails with ``EINVAL`` if an invalid value has been written
> +        (valid values are 0..100000).
> +
> +What:		/sys/class/dvb/dvbN/caM/tim_wr_data
> +Date:		Dec 2017
> +KernelVersion:	4.15
> +Contact:	Jasmin Jessich <jasmin@anw.at>
> +Description:
> +        Reading this file returns the wait time between data bytes sent
> +        to the CAM. The default timeout it '0', which means no 'no timeout'.
> +        Any other value specifies the timeout in micro seconds.
> +
> +        Writing a value will change the timeout.
> +             
> +        Write fails with ``EINVAL`` if an invalid value has been written
> +        (valid values are 0..100000).
> +
> diff --git a/Documentation/media/uapi/dvb/ca-sysfs-nodes.rst b/Documentation/media/uapi/dvb/ca-sysfs-nodes.rst
> new file mode 100644
> index 0000000..4a26afd
> --- /dev/null
> +++ b/Documentation/media/uapi/dvb/ca-sysfs-nodes.rst
> @@ -0,0 +1,85 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _ca_sysfs_nodes:
> +
> +******************************
> +Conditional Access sysfs nodes
> +******************************
> +
> +As defined at ``Documentation/ABI/testing/sysfs-class-ca``, those are
> +the sysfs nodes that control the en50221 CA driver:
> +
> +
> +.. _sys_class_dvb_dvbN:
> +
> +/sys/class/dvb/dvbN/
> +====================
> +
> +The ``/sys/class/dvb/dvbN/`` class sub-directory belongs to the Adapter
> +with the index N. It is created for each found Adapter and depends on the
> +DVB hardware.
> +
> +
> +.. _sys_class_dvb_dvbN_caM:
> +
> +/sys/class/dvb/dvbN/caM
> +=======================
> +
> +The ``/sys/class/dvb/dvbN/caM`` class sub-directory belongs to the CA device
> +with the index M on the Adapter with the index N. It is created for each
> +found Conditional Access Interface where M is the number of the CA Interface.
> +A Conditional Access Module (CAM) will be inserted into the CI interface. The
> +caM device is used to communicate to the CAM.
> +
> +The communication protocol contains a length field followed by the data bytes.
> +The length is written in two parts. First the high byte of the length
> +followed by the low byte. The following sysfs nodes define three timeouts
> +which may be used to extend the communication to the CAM. Modern CAMs usually
> +do not need those timeouts, but older CAMs will produce communication errors,
> +when the bytes are written too fast. The underliying hardware has also a big
> +impact due to the access speed.
> +
> +
> +.. _sys_class_dvb_dvbN_caM_tim_wr_high:
> +
> +/sys/class/dvb/dvbN/caM/tim_wr_high
> +===================================
> +
> +Reading this file returns the wait time after writing the length high byte to
> +the CAM. The default timeout it '0', which means no 'no timeout'. Any other
> +value specifies the timeout in micro seconds.
> +
> +Writing a value will change the timeout.
> +
> +Write fails with ``EINVAL`` if an invalid value has been written (valid values
> +are 0..100000).
> +
> +
> +.. _sys_class_dvb_dvbN_caM_tim_wr_low:
> +
> +/sys/class/dvb/dvbN/caM/tim_wr_low
> +==================================
> +
> +Reading this file returns the wait time after writing the length low byte to
> +the CAM. The default timeout it '0', which means no 'no timeout'. Any other
> +value specifies the timeout in micro seconds.
> +
> +Writing a value will change the timeout.
> +
> +Write fails with ``EINVAL`` if an invalid value has been written (valid values
> +are 0..100000).
> +
> +
> +.. _sys_class_dvb_dvbN_caM_tim_wr_data:
> +
> +/sys/class/dvb/dvbN/caM/tim_wr_data
> +===================================
> +
> +Reading this file returns the wait time between data bytes sent to the CAM.
> +The default timeout it '0', which means no 'no timeout'. Any other value
> +specifies the timeout in micro seconds.
> +
> +Writing a value will change the timeout.
> +
> +Write fails with ``EINVAL`` if an invalid value has been written (valid values
> +are 0..100000).
> diff --git a/Documentation/media/uapi/dvb/ca.rst b/Documentation/media/uapi/dvb/ca.rst
> index deac72d..e790d19d 100644
> --- a/Documentation/media/uapi/dvb/ca.rst
> +++ b/Documentation/media/uapi/dvb/ca.rst
> @@ -22,3 +22,4 @@ application.
>  
>      ca_data_types
>      ca_function_calls
> +    ca-sysfs-nodes
> 

Re: [PATCH V2 0/3] Add timers to en50221 protocol driver

[Jasmin J.]

Hi!

Please hold on in merging this series, because I have to investigate a hint
I got related to the buffer size handshake of the protocol driver:
  https://www.linuxtv.org/pipermail/linux-dvb/2007-July/019116.html

BR,
   Jasmin

****************************************************************************

On 12/21/2017 02:22 PM, Jasmin J. wrote:
> From: Jasmin Jessich <jasmin@anw.at>
> 
> Some (older) CAMs are really slow in accepting data. I got sometimes the 
> already known error "CAM tried to send a buffer larger than the ecount 
> size". I could track it down to the dvb_ca_en50221_write_data function not 
> waiting between sending the data length high/low and data bytes. In fact
> the CAM reported a WR error, which triggered later on the mentioned error.
>  
> The problem is that a simple module parameter can't be used to solve this
> by adding timer values, because the protocol handler is used for any CI
> interface. A module parameter would be influence all the CAMs on all CI
> interfaces. Thus individual timer definitions per CI interface and CAM are
> required.
> There are two possibilities to implement that, ioctl's and SysFS.
> ioctl's require changes in usermode programs and it may take a lot of time
> to get this implemented there.
> SysFS can be used by simple "cat" and "echo" commands and can be therefore
> simply controlled by scripting, which is immediately available.
> 
> I decided to go for the SysFS approach, but the required device to add the
> SysFS files was not available in the "struct dvb_device". The first patch
> of this series adds this device to the structure and also the setting code.
> 
> The second patch adds the functions to create the SysFS nodes for all
> timers and the new timeouts in the en50221 protocol driver.
> 
> The third patch adds the SysFS node documentation.
> 
> Jasmin Jessich (3):
>   media: dvb-core: Store device structure in dvb_register_device
>   media: dvb-core: Added timers for dvb_ca_en50221_write_data
>   media: dvb-core: Added documentation for ca sysfs timer nodes
> 
>  Documentation/ABI/testing/sysfs-class-ca        |  63 +++++++++++
>  Documentation/media/uapi/dvb/ca-sysfs-nodes.rst |  85 +++++++++++++++
>  Documentation/media/uapi/dvb/ca.rst             |   1 +
>  drivers/media/dvb-core/dvb_ca_en50221.c         | 132 +++++++++++++++++++++++-
>  drivers/media/dvb-core/dvbdev.c                 |   1 +
>  drivers/media/dvb-core/dvbdev.h                 |   4 +-
>  6 files changed, 284 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-ca
>  create mode 100644 Documentation/media/uapi/dvb/ca-sysfs-nodes.rst
> 

Re: [PATCH V2 0/3] Add timers to en50221 protocol driver

[Ralph Metzler]

Hi Jasmin,

Jasmin J. writes:
 > Hi!
 > 
 > Please hold on in merging this series, because I have to investigate a hint
 > I got related to the buffer size handshake of the protocol driver:
 >   https://www.linuxtv.org/pipermail/linux-dvb/2007-July/019116.html
 > 
 > BR,
 >    Jasmin


So, there seem to be two bugs:

1. The SW bit is cleared too early during the whole buffer size negotiation.

This should be fixed.


2. IRQEN = CMDREG_DAIE = 0x80 is always set in the command register.

DAIE and FRIE were introduced as recommendation in Cenelec R06-001:1998 and are a requirement for
CI+.

They could cause problems if the IRQ line goes high and the interrupt is enabled but not handled.
They should not cause a problem if the host ignores the interrupt or if the CAM does not support it,
but one never knows with some CAMs ...

So, they should probably only be used if both the host and module say they support it.
R06 does not mention it but CI+ also requires a CIS entry to be present in modules 
supporting this feature.



Regards,
Ralph

Re: [PATCH V2 0/3] Add timers to en50221 protocol driver

[Jasmin J.]

Hallo Ralph!

> 1. The SW bit is cleared too early during the whole buffer size negotiation.
> This should be fixed.
I will look into this when I have time again. Probably end of next week.

> 2. IRQEN = CMDREG_DAIE = 0x80 is always set in the command register.
> So, they should probably only be used if both the host and module say they
> support it.
How can we know that in the driver?
I haven't seen an API for this.
There is the flag "da_irq_supported", which might be used to set the IRQEN
bit it the bit is set.

Any suggestions how to proceed with item 2?

BR,
   Jasmin

Re: [PATCH V2 0/3] Add timers to en50221 protocol driver

[Ralph Metzler]

Hi Jasmin,

Jasmin J. writes:
 > Hallo Ralph!
 > 
 > > 1. The SW bit is cleared too early during the whole buffer size negotiation.
 > > This should be fixed.
 > I will look into this when I have time again. Probably end of next week.
 > 
 > > 2. IRQEN = CMDREG_DAIE = 0x80 is always set in the command register.
 > > So, they should probably only be used if both the host and module say they
 > > support it.
 > How can we know that in the driver?
 > I haven't seen an API for this.

The flags parameter of dvb_ca_en50221_init() allow these values:

#define DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE       1
#define DVB_CA_EN50221_FLAG_IRQ_FR              2
#define DVB_CA_EN50221_FLAG_IRQ_DA              4

but only DVB_CA_EN50221_FLAG_IRQ_CAMCHANGE seems to be used in dvb_ca_en50221.c.
Support for this was seemingly planned but never implemented.

Annex G.2 of
http://www.ci-plus.com/data/ci-plus_specification_v1.3.pdf
contains details about how support is announced in the CIS.


 > There is the flag "da_irq_supported", which might be used to set the IRQEN
 > bit it the bit is set.

Sure, e.g. to store the result from the CIS parsing.
The current use is not much help. It is set if an interrupt occured with DA bit set, which is
a little too late.


 > Any suggestions how to proceed with item 2?

Check the CIS for support by the CAM and ca->flags for support by the host.
If both support it, set CMDREG_FRIE and/or CMDREG_DAIE in command reg.


Regards,
Ralph