[10/10] rc-core: move timeout and checks to lirc

Message ID 20110428151403.8272.25977.stgit@felix.hardeman.nu (mailing list archive)
State Accepted, archived
Headers

Commit Message

David Härdeman April 28, 2011, 3:14 p.m. UTC
The lirc TX functionality expects the process which writes (TX) data to
the lirc dev to sleep until the actual data has been transmitted by the
hardware.

Since the same timeout calculation is duplicated in more than one driver
(and would have to be duplicated in even more drivers as they gain TX
support), it makes sense to move this timeout calculation to the lirc
layer instead.

At the same time, centralize some of the sanity checks.

Signed-off-by: David Härdeman <david@hardeman.nu>
---
 drivers/media/rc/ir-lirc-codec.c |   33 +++++++++++++++++++++++++++++----
 drivers/media/rc/mceusb.c        |   18 ------------------
 drivers/media/rc/rc-loopback.c   |   12 ------------
 3 files changed, 29 insertions(+), 34 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Comments

Mauro Carvalho Chehab Sept. 21, 2011, 4:40 p.m. UTC | #1
Em 28-04-2011 12:14, David Härdeman escreveu:
> The lirc TX functionality expects the process which writes (TX) data to
> the lirc dev to sleep until the actual data has been transmitted by the
> hardware.
> 
> Since the same timeout calculation is duplicated in more than one driver
> (and would have to be duplicated in even more drivers as they gain TX
> support), it makes sense to move this timeout calculation to the lirc
> layer instead.
> 
> At the same time, centralize some of the sanity checks.

This patch[1] were never applied, nor I'm seeing any comments about it
at the ML.

Jarod, 

would you care to review it?

Thanks,
Mauro

[1] http://patchwork.linuxtv.org/patch/6468/


> 
> Signed-off-by: David Härdeman <david@hardeman.nu>
> ---
>  drivers/media/rc/ir-lirc-codec.c |   33 +++++++++++++++++++++++++++++----
>  drivers/media/rc/mceusb.c        |   18 ------------------
>  drivers/media/rc/rc-loopback.c   |   12 ------------
>  3 files changed, 29 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
> index ac54139..a58c7fe 100644
> --- a/drivers/media/rc/ir-lirc-codec.c
> +++ b/drivers/media/rc/ir-lirc-codec.c
> @@ -106,6 +106,12 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
>  	unsigned int *txbuf; /* buffer with values to transmit */
>  	ssize_t ret = 0;
>  	size_t count;
> +	ktime_t start;
> +	s64 towait;
> +	unsigned int duration = 0; /* signal duration in us */
> +	int i;
> +
> +	start = ktime_get();
>  
>  	lirc = lirc_get_pdata(file);
>  	if (!lirc)
> @@ -128,11 +134,30 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
>  		goto out;
>  	}
>  
> -	if (dev->tx_ir)
> -		ret = dev->tx_ir(dev, txbuf, count);
> +	if (!dev->tx_ir) {
> +		ret = -ENOSYS;
> +		goto out;
> +	}
> +
> +	ret = dev->tx_ir(dev, txbuf, (u32)n);
> +	if (ret < 0)
> +		goto out;
> +
> +	for (i = 0; i < ret; i++)
> +		duration += txbuf[i];
>  
> -	if (ret > 0)
> -		ret *= sizeof(unsigned);
> +	ret *= sizeof(unsigned int);
> +
> +	/*
> +	 * The lircd gap calculation expects the write function to
> +	 * wait for the actual IR signal to be transmitted before
> +	 * returning.
> +	 */
> +	towait = ktime_us_delta(ktime_add_us(start, duration), ktime_get());
> +	if (towait > 0) {
> +		set_current_state(TASK_INTERRUPTIBLE);
> +		schedule_timeout(usecs_to_jiffies(towait));
> +	}
>  
>  out:
>  	kfree(txbuf);
> diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
> index c1cd00d..7b08797 100644
> --- a/drivers/media/rc/mceusb.c
> +++ b/drivers/media/rc/mceusb.c
> @@ -671,10 +671,6 @@ static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
>  	int i, ret = 0;
>  	int cmdcount = 0;
>  	unsigned char *cmdbuf; /* MCE command buffer */
> -	long signal_duration = 0; /* Singnal length in us */
> -	struct timeval start_time, end_time;
> -
> -	do_gettimeofday(&start_time);
>  
>  	cmdbuf = kzalloc(sizeof(unsigned) * MCE_CMDBUF_SIZE, GFP_KERNEL);
>  	if (!cmdbuf)
> @@ -687,7 +683,6 @@ static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
>  
>  	/* Generate mce packet data */
>  	for (i = 0; (i < count) && (cmdcount < MCE_CMDBUF_SIZE); i++) {
> -		signal_duration += txbuf[i];
>  		txbuf[i] = txbuf[i] / MCE_TIME_UNIT;
>  
>  		do { /* loop to support long pulses/spaces > 127*50us=6.35ms */
> @@ -730,19 +725,6 @@ static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
>  	/* Transmit the command to the mce device */
>  	mce_async_out(ir, cmdbuf, cmdcount);
>  
> -	/*
> -	 * The lircd gap calculation expects the write function to
> -	 * wait the time it takes for the ircommand to be sent before
> -	 * it returns.
> -	 */
> -	do_gettimeofday(&end_time);
> -	signal_duration -= (end_time.tv_usec - start_time.tv_usec) +
> -			   (end_time.tv_sec - start_time.tv_sec) * 1000000;
> -
> -	/* delay with the closest number of ticks */
> -	set_current_state(TASK_INTERRUPTIBLE);
> -	schedule_timeout(usecs_to_jiffies(signal_duration));
> -
>  out:
>  	kfree(cmdbuf);
>  	return ret ? ret : count;
> diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
> index fb8ca81..5cb7057 100644
> --- a/drivers/media/rc/rc-loopback.c
> +++ b/drivers/media/rc/rc-loopback.c
> @@ -105,18 +105,9 @@ static int loop_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
>  {
>  	struct loopback_dev *lodev = dev->priv;
>  	u32 rxmask;
> -	unsigned total_duration = 0;
>  	unsigned i;
>  	DEFINE_IR_RAW_EVENT(rawir);
>  
> -	for (i = 0; i < count; i++)
> -		total_duration += abs(txbuf[i]);
> -
> -	if (total_duration == 0) {
> -		dprintk("invalid tx data, total duration zero\n");
> -		return -EINVAL;
> -	}
> -
>  	if (lodev->txcarrier < lodev->rxcarriermin ||
>  	    lodev->txcarrier > lodev->rxcarriermax) {
>  		dprintk("ignoring tx, carrier out of range\n");
> @@ -148,9 +139,6 @@ static int loop_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
>  	ir_raw_event_handle(dev);
>  
>  out:
> -	/* Lirc expects this function to take as long as the total duration */
> -	set_current_state(TASK_INTERRUPTIBLE);
> -	schedule_timeout(usecs_to_jiffies(total_duration));
>  	return count;
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  

Patch

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index ac54139..a58c7fe 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -106,6 +106,12 @@  static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
 	unsigned int *txbuf; /* buffer with values to transmit */
 	ssize_t ret = 0;
 	size_t count;
+	ktime_t start;
+	s64 towait;
+	unsigned int duration = 0; /* signal duration in us */
+	int i;
+
+	start = ktime_get();
 
 	lirc = lirc_get_pdata(file);
 	if (!lirc)
@@ -128,11 +134,30 @@  static ssize_t ir_lirc_transmit_ir(struct file *file, const char *buf,
 		goto out;
 	}
 
-	if (dev->tx_ir)
-		ret = dev->tx_ir(dev, txbuf, count);
+	if (!dev->tx_ir) {
+		ret = -ENOSYS;
+		goto out;
+	}
+
+	ret = dev->tx_ir(dev, txbuf, (u32)n);
+	if (ret < 0)
+		goto out;
+
+	for (i = 0; i < ret; i++)
+		duration += txbuf[i];
 
-	if (ret > 0)
-		ret *= sizeof(unsigned);
+	ret *= sizeof(unsigned int);
+
+	/*
+	 * The lircd gap calculation expects the write function to
+	 * wait for the actual IR signal to be transmitted before
+	 * returning.
+	 */
+	towait = ktime_us_delta(ktime_add_us(start, duration), ktime_get());
+	if (towait > 0) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		schedule_timeout(usecs_to_jiffies(towait));
+	}
 
 out:
 	kfree(txbuf);
diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index c1cd00d..7b08797 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -671,10 +671,6 @@  static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
 	int i, ret = 0;
 	int cmdcount = 0;
 	unsigned char *cmdbuf; /* MCE command buffer */
-	long signal_duration = 0; /* Singnal length in us */
-	struct timeval start_time, end_time;
-
-	do_gettimeofday(&start_time);
 
 	cmdbuf = kzalloc(sizeof(unsigned) * MCE_CMDBUF_SIZE, GFP_KERNEL);
 	if (!cmdbuf)
@@ -687,7 +683,6 @@  static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
 
 	/* Generate mce packet data */
 	for (i = 0; (i < count) && (cmdcount < MCE_CMDBUF_SIZE); i++) {
-		signal_duration += txbuf[i];
 		txbuf[i] = txbuf[i] / MCE_TIME_UNIT;
 
 		do { /* loop to support long pulses/spaces > 127*50us=6.35ms */
@@ -730,19 +725,6 @@  static int mceusb_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
 	/* Transmit the command to the mce device */
 	mce_async_out(ir, cmdbuf, cmdcount);
 
-	/*
-	 * The lircd gap calculation expects the write function to
-	 * wait the time it takes for the ircommand to be sent before
-	 * it returns.
-	 */
-	do_gettimeofday(&end_time);
-	signal_duration -= (end_time.tv_usec - start_time.tv_usec) +
-			   (end_time.tv_sec - start_time.tv_sec) * 1000000;
-
-	/* delay with the closest number of ticks */
-	set_current_state(TASK_INTERRUPTIBLE);
-	schedule_timeout(usecs_to_jiffies(signal_duration));
-
 out:
 	kfree(cmdbuf);
 	return ret ? ret : count;
diff --git a/drivers/media/rc/rc-loopback.c b/drivers/media/rc/rc-loopback.c
index fb8ca81..5cb7057 100644
--- a/drivers/media/rc/rc-loopback.c
+++ b/drivers/media/rc/rc-loopback.c
@@ -105,18 +105,9 @@  static int loop_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
 {
 	struct loopback_dev *lodev = dev->priv;
 	u32 rxmask;
-	unsigned total_duration = 0;
 	unsigned i;
 	DEFINE_IR_RAW_EVENT(rawir);
 
-	for (i = 0; i < count; i++)
-		total_duration += abs(txbuf[i]);
-
-	if (total_duration == 0) {
-		dprintk("invalid tx data, total duration zero\n");
-		return -EINVAL;
-	}
-
 	if (lodev->txcarrier < lodev->rxcarriermin ||
 	    lodev->txcarrier > lodev->rxcarriermax) {
 		dprintk("ignoring tx, carrier out of range\n");
@@ -148,9 +139,6 @@  static int loop_tx_ir(struct rc_dev *dev, unsigned *txbuf, unsigned count)
 	ir_raw_event_handle(dev);
 
 out:
-	/* Lirc expects this function to take as long as the total duration */
-	set_current_state(TASK_INTERRUPTIBLE);
-	schedule_timeout(usecs_to_jiffies(total_duration));
 	return count;
 }