Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices

From: Andi Shyti
Date: Tue Nov 01 2016 - 02:51:19 EST


Hi Sean,

> Andi, it would be good to know what the use-case for the original change is.

the use case is the ir-spi itself which doesn't need the lirc to
perform any waiting on its behalf.

To me it just doesn't look right to simulate a fake transmission
period and wait unnecessary time. Of course, the "over-wait" is not
a big deal and at the end we can decide to drop it.

Otherwise, an alternative could be to add the boolean
'tx_no_wait' in the rc_dev structure. It could be set by the
device driver during the initialization and the we can follow
your approach.

Something like this:

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index c327730..4553d04 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -161,15 +161,19 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, const char __user *buf,

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));
+ if (!dev->tx_no_wait) {
+ /*
+ * 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:
diff --git a/drivers/media/rc/ir-spi.c b/drivers/media/rc/ir-spi.c
index fcda1e4..e44abfa 100644
--- a/drivers/media/rc/ir-spi.c
+++ b/drivers/media/rc/ir-spi.c
@@ -149,6 +149,7 @@ static int ir_spi_probe(struct spi_device *spi)
if (!idata->rc)
return -ENOMEM;

+ idata->rc->tx_no_wait = true;
idata->rc->tx_ir = ir_spi_tx;
idata->rc->s_tx_carrier = ir_spi_set_tx_carrier;
idata->rc->s_tx_duty_cycle = ir_spi_set_duty_cycle;
diff --git a/include/media/rc-core.h b/include/media/rc-core.h
index fe0c9c4..c3ced9b 100644
--- a/include/media/rc-core.h
+++ b/include/media/rc-core.h
@@ -85,6 +85,9 @@ enum rc_filter_type {
* @input_dev: the input child device used to communicate events to userspace
* @driver_type: specifies if protocol decoding is done in hardware or software
* @idle: used to keep track of RX state
+ * @tx_no_wait: decides whether to perform or not a sync write or not. The
+ * device driver setting it to true must make sure to not break the ABI
+ * which requires a sync transfer.
* @allowed_protocols: bitmask with the supported RC_BIT_* protocols
* @enabled_protocols: bitmask with the enabled RC_BIT_* protocols
* @allowed_wakeup_protocols: bitmask with the supported RC_BIT_* wakeup protocols
@@ -147,6 +150,7 @@ struct rc_dev {
struct input_dev *input_dev;
enum rc_driver_type driver_type;
bool idle;
+ bool tx_no_wait;
u64 allowed_protocols;
u64 enabled_protocols;
u64 allowed_wakeup_protocols;

Thanks,
Andi