Re: [PATCH 10/13] IR: extend interfaces to support more device settings LIRC: add new IOCTL that enables learning mode (wide band receiver)
Hi Maxim, on 31 Jul 10 at 01:01, Maxim Levitsky wrote: On Fri, 2010-07-30 at 23:22 +0200, Christoph Bartelmus wrote: [...] +#define LIRC_SET_WIDEBAND_RECEIVER _IOW('i', 0x0023, __u32) If you really want this new ioctl, then it should be clarified how it behaves in relation to LIRC_SET_MEASURE_CARRIER_MODE. In my opinion, I won't need the LIRC_SET_MEASURE_CARRIER_MODE, I would just optionally turn that on in learning mode. You disagree, and since that is not important (besides TX and learning features are present only at fraction of ENE devices. The only user I did the debugging with, doesn't seem to want to help debug that code anymore...) But anyway, in current state I want these features to be independent. Driver will enable learning mode if it have to. Please avoid the term learning mode as to you it probably means something different than to me. I'll add the documentation. Do you have to enable the wide-band receiver explicitly before you can enable carrier reports or does enabling carrier reports implicitly switch to the wide-band receiver? I would implicitly switch the learning mode on, untill user turns off the carrier reports. You mean that you'll implicitly switch on the wide-band receiver. Ok. What happens if carrier mode is enabled and you explicitly turn off the wide-band receiver? Wouldn't it be better to have one ioctl for both after all? There may be hardware that allows carrier measurement but does not have a wide-band receiver. And there may be hardware that does have a wide-band receiver but does not allow carrier measurement. irrecord needs to be able to distinguish these cases, so we need separate ioctls. I'd say: carrier reports may switch on the wide-band reciever implicitly. In that case the wide-band receiver cannot be switched off explicitly until carrier reports are disabled again. It just needs to be documented. And while we're at interface stuff: Do we really need LIRC_SETUP_START and LIRC_SETUP_END? It is only used once in lircd during startup. I don't think so. Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/13] IR: extend interfaces to support more device settings LIRC: add new IOCTL that enables learning mode (wide band receiver)
On Sat, 2010-07-31 at 10:10 +0200, Christoph Bartelmus wrote: Hi Maxim, on 31 Jul 10 at 01:01, Maxim Levitsky wrote: On Fri, 2010-07-30 at 23:22 +0200, Christoph Bartelmus wrote: [...] +#define LIRC_SET_WIDEBAND_RECEIVER _IOW('i', 0x0023, __u32) If you really want this new ioctl, then it should be clarified how it behaves in relation to LIRC_SET_MEASURE_CARRIER_MODE. In my opinion, I won't need the LIRC_SET_MEASURE_CARRIER_MODE, I would just optionally turn that on in learning mode. You disagree, and since that is not important (besides TX and learning features are present only at fraction of ENE devices. The only user I did the debugging with, doesn't seem to want to help debug that code anymore...) But anyway, in current state I want these features to be independent. Driver will enable learning mode if it have to. Please avoid the term learning mode as to you it probably means something different than to me. I'll add the documentation. Do you have to enable the wide-band receiver explicitly before you can enable carrier reports or does enabling carrier reports implicitly switch to the wide-band receiver? I would implicitly switch the learning mode on, untill user turns off the carrier reports. You mean that you'll implicitly switch on the wide-band receiver. Ok. What happens if carrier mode is enabled and you explicitly turn off the wide-band receiver? Wouldn't it be better to have one ioctl for both after all? There may be hardware that allows carrier measurement but does not have a wide-band receiver. And there may be hardware that does have a wide-band receiver but does not allow carrier measurement. irrecord needs to be able to distinguish these cases, so we need separate ioctls. I'd say: carrier reports may switch on the wide-band reciever implicitly. In that case the wide-band receiver cannot be switched off explicitly until carrier reports are disabled again. It just needs to be documented. No problem. Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/13] IR: extend interfaces to support more device settings LIRC: add new IOCTL that enables learning mode (wide band receiver)
Still missing features: carrier report timeout reports. Will need to pack these into ir_raw_event Signed-off-by: Maxim Levitsky maximlevit...@gmail.com --- drivers/media/IR/ir-core-priv.h |1 + drivers/media/IR/ir-lirc-codec.c | 112 +++--- include/media/ir-core.h | 14 + include/media/lirc.h |5 ++- 4 files changed, 112 insertions(+), 20 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index 30ff52c..52253b4 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -78,6 +78,7 @@ struct ir_raw_event_ctrl { struct lirc_codec { struct ir_input_dev *ir_dev; struct lirc_driver *drv; + int carrier_low; } lirc; }; diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c index 8ca01fd..5d5150f 100644 --- a/drivers/media/IR/ir-lirc-codec.c +++ b/drivers/media/IR/ir-lirc-codec.c @@ -46,7 +46,6 @@ static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event ev) IR_dprintk(2, LIRC data transfer started (%uus %s)\n, TO_US(ev.duration), TO_STR(ev.pulse)); - sample = ev.duration / 1000; if (ev.pulse) sample |= PULSE_BIT; @@ -96,13 +95,14 @@ out: return ret; } -static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) +static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, + unsigned long __user arg) { struct lirc_codec *lirc; struct ir_input_dev *ir_dev; int ret = 0; void *drv_data; - unsigned long val; + unsigned long val = 0; lirc = lirc_get_pdata(filep); if (!lirc) @@ -114,47 +114,106 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar drv_data = ir_dev-props-priv; - switch (cmd) { - case LIRC_SET_TRANSMITTER_MASK: + if (_IOC_DIR(cmd) _IOC_WRITE) { ret = get_user(val, (unsigned long *)arg); if (ret) return ret; + } + + switch (cmd) { + + /* legacy support */ + case LIRC_GET_SEND_MODE: + val = LIRC_CAN_SEND_PULSE LIRC_CAN_SEND_MASK; + break; + + case LIRC_SET_SEND_MODE: + if (val != (LIRC_MODE_PULSE LIRC_CAN_SEND_MASK)) + return -EINVAL; + break; - if (ir_dev-props ir_dev-props-s_tx_mask) + /* TX settings */ + case LIRC_SET_TRANSMITTER_MASK: + if (ir_dev-props-s_tx_mask) ret = ir_dev-props-s_tx_mask(drv_data, (u32)val); else return -EINVAL; break; case LIRC_SET_SEND_CARRIER: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; - - if (ir_dev-props ir_dev-props-s_tx_carrier) + if (ir_dev-props-s_tx_carrier) ir_dev-props-s_tx_carrier(drv_data, (u32)val); else return -EINVAL; break; - case LIRC_GET_SEND_MODE: - val = LIRC_CAN_SEND_PULSE LIRC_CAN_SEND_MASK; - ret = put_user(val, (unsigned long *)arg); + case LIRC_SET_SEND_DUTY_CYCLE: + if (!ir_dev-props-s_tx_duty_cycle) + return -ENOSYS; + + if (val = 0 || val = 100) + return -EINVAL; + + ir_dev-props-s_tx_duty_cycle(ir_dev-props-priv, val); break; - case LIRC_SET_SEND_MODE: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; + /* RX settings */ + case LIRC_SET_REC_CARRIER: + if (ir_dev-props-s_rx_carrier_range) + ret = ir_dev-props-s_rx_carrier_range( + ir_dev-props-priv, + ir_dev-raw-lirc.carrier_low, val); + else + return -ENOSYS; - if (val != (LIRC_MODE_PULSE LIRC_CAN_SEND_MASK)) + if (!ret) + ir_dev-raw-lirc.carrier_low = 0; + break; + + case LIRC_SET_REC_CARRIER_RANGE: + if (val = 0) + ir_dev-raw-lirc.carrier_low = val; + break; + + + case LIRC_GET_REC_RESOLUTION: + val = ir_dev-props-rx_resolution; + break; + + case LIRC_SET_WIDEBAND_RECEIVER: + if (ir_dev-props-s_learning_mode) + return ir_dev-props-s_learning_mode( + ir_dev-props-priv, !!val); + else + return -ENOSYS; + + /*
Re: [PATCH 10/13] IR: extend interfaces to support more device settings LIRC: add new IOCTL that enables learning mode (wide band receiver)
Hi! Maxim Levitsky maximlevit...@gmail.com wrote: Still missing features: carrier report timeout reports. Will need to pack these into ir_raw_event Hm, this patch changes the LIRC interface but I can't see the according patch to the documentation. [...] * @tx_ir: transmit IR * @s_idle: optional: enable/disable hardware idle mode, upon which, + current * device doesn't interrupt host untill it sees IR data +=== Huh? + device doesn't interrupt host untill it sees IR data + * @s_learning_mode: enable wide band receiver used for learning + patched s/untill/until/ [...] #define LIRC_CAN_MEASURE_CARRIER 0x0200 +#define LIRC_CAN_HAVE_WIDEBAND_RECEIVER 0x0400 LIRC_CAN_USE_WIDEBAND_RECEIVER [...] @@ -145,7 +146,7 @@ * if enabled from the next key press on the driver will send * LIRC_MODE2_FREQUENCY packets */ -#define LIRC_SET_MEASURE_CARRIER_MODE _IOW('i', 0x001d, __u32) +#define LIRC_SET_MEASURE_CARRIER_MODE_IOW('i', 0x001d, __u32) /* * to set a range use @@ -162,4 +163,6 @@ #define LIRC_SETUP_START _IO('i', 0x0021) #define LIRC_SETUP_END _IO('i', 0x0022) +#define LIRC_SET_WIDEBAND_RECEIVER _IOW('i', 0x0023, __u32) If you really want this new ioctl, then it should be clarified how it behaves in relation to LIRC_SET_MEASURE_CARRIER_MODE. Do you have to enable the wide-band receiver explicitly before you can enable carrier reports or does enabling carrier reports implicitly switch to the wide-band receiver? What happens if carrier mode is enabled and you explicitly turn off the wide-band receiver? And while we're at interface stuff: Do we really need LIRC_SETUP_START and LIRC_SETUP_END? It is only used once in lircd during startup. Christoph -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/13] IR: extend interfaces to support more device settings LIRC: add new IOCTL that enables learning mode (wide band receiver)
On Fri, 2010-07-30 at 23:22 +0200, Christoph Bartelmus wrote: Hi! Maxim Levitsky maximlevit...@gmail.com wrote: Still missing features: carrier report timeout reports. Will need to pack these into ir_raw_event Hm, this patch changes the LIRC interface but I can't see the according patch to the documentation. [...] * @tx_ir: transmit IR * @s_idle: optional: enable/disable hardware idle mode, upon which, + current * device doesn't interrupt host untill it sees IR data +=== Huh? :-) + device doesn't interrupt host untill it sees IR data + * @s_learning_mode: enable wide band receiver used for learning + patched s/untill/until/ [...] #define LIRC_CAN_MEASURE_CARRIER 0x0200 +#define LIRC_CAN_HAVE_WIDEBAND_RECEIVER 0x0400 LIRC_CAN_USE_WIDEBAND_RECEIVER OK. [...] @@ -145,7 +146,7 @@ * if enabled from the next key press on the driver will send * LIRC_MODE2_FREQUENCY packets */ -#define LIRC_SET_MEASURE_CARRIER_MODE _IOW('i', 0x001d, __u32) +#define LIRC_SET_MEASURE_CARRIER_MODE _IOW('i', 0x001d, __u32) /* * to set a range use @@ -162,4 +163,6 @@ #define LIRC_SETUP_START _IO('i', 0x0021) #define LIRC_SETUP_END _IO('i', 0x0022) +#define LIRC_SET_WIDEBAND_RECEIVER _IOW('i', 0x0023, __u32) If you really want this new ioctl, then it should be clarified how it behaves in relation to LIRC_SET_MEASURE_CARRIER_MODE. In my opinion, I won't need the LIRC_SET_MEASURE_CARRIER_MODE, I would just optionally turn that on in learning mode. You disagree, and since that is not important (besides TX and learning features are present only at fraction of ENE devices. The only user I did the debugging with, doesn't seem to want to help debug that code anymore...) But anyway, in current state I want these features to be independent. Driver will enable learning mode if it have to. I'll add the documentation. Do you have to enable the wide-band receiver explicitly before you can enable carrier reports or does enabling carrier reports implicitly switch to the wide-band receiver? I would implicitly switch the learning mode on, untill user turns off the carrier reports. What happens if carrier mode is enabled and you explicitly turn off the wide-band receiver? Wouldn't it be better to have one ioctl for both after all? And while we're at interface stuff: Do we really need LIRC_SETUP_START and LIRC_SETUP_END? It is only used once in lircd during startup. I don't think so. Best regards, Maxim Levitsky -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/13] IR: extend interfaces to support more device settings LIRC: add new IOCTL that enables learning mode (wide band receiver)
Still missing features: carrier report timeout reports. Will need to pack these into ir_raw_event Signed-off-by: Maxim Levitsky maximlevit...@gmail.com --- drivers/media/IR/ir-core-priv.h |1 + drivers/media/IR/ir-lirc-codec.c | 112 +++--- include/media/ir-core.h | 14 + include/media/lirc.h |5 ++- 4 files changed, 112 insertions(+), 20 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index 841b76c..068807d 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -78,6 +78,7 @@ struct ir_raw_event_ctrl { struct lirc_codec { struct ir_input_dev *ir_dev; struct lirc_driver *drv; + int carrier_low; } lirc; }; diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c index 8ca01fd..5d5150f 100644 --- a/drivers/media/IR/ir-lirc-codec.c +++ b/drivers/media/IR/ir-lirc-codec.c @@ -46,7 +46,6 @@ static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event ev) IR_dprintk(2, LIRC data transfer started (%uus %s)\n, TO_US(ev.duration), TO_STR(ev.pulse)); - sample = ev.duration / 1000; if (ev.pulse) sample |= PULSE_BIT; @@ -96,13 +95,14 @@ out: return ret; } -static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) +static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, + unsigned long __user arg) { struct lirc_codec *lirc; struct ir_input_dev *ir_dev; int ret = 0; void *drv_data; - unsigned long val; + unsigned long val = 0; lirc = lirc_get_pdata(filep); if (!lirc) @@ -114,47 +114,106 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar drv_data = ir_dev-props-priv; - switch (cmd) { - case LIRC_SET_TRANSMITTER_MASK: + if (_IOC_DIR(cmd) _IOC_WRITE) { ret = get_user(val, (unsigned long *)arg); if (ret) return ret; + } + + switch (cmd) { + + /* legacy support */ + case LIRC_GET_SEND_MODE: + val = LIRC_CAN_SEND_PULSE LIRC_CAN_SEND_MASK; + break; + + case LIRC_SET_SEND_MODE: + if (val != (LIRC_MODE_PULSE LIRC_CAN_SEND_MASK)) + return -EINVAL; + break; - if (ir_dev-props ir_dev-props-s_tx_mask) + /* TX settings */ + case LIRC_SET_TRANSMITTER_MASK: + if (ir_dev-props-s_tx_mask) ret = ir_dev-props-s_tx_mask(drv_data, (u32)val); else return -EINVAL; break; case LIRC_SET_SEND_CARRIER: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; - - if (ir_dev-props ir_dev-props-s_tx_carrier) + if (ir_dev-props-s_tx_carrier) ir_dev-props-s_tx_carrier(drv_data, (u32)val); else return -EINVAL; break; - case LIRC_GET_SEND_MODE: - val = LIRC_CAN_SEND_PULSE LIRC_CAN_SEND_MASK; - ret = put_user(val, (unsigned long *)arg); + case LIRC_SET_SEND_DUTY_CYCLE: + if (!ir_dev-props-s_tx_duty_cycle) + return -ENOSYS; + + if (val = 0 || val = 100) + return -EINVAL; + + ir_dev-props-s_tx_duty_cycle(ir_dev-props-priv, val); break; - case LIRC_SET_SEND_MODE: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; + /* RX settings */ + case LIRC_SET_REC_CARRIER: + if (ir_dev-props-s_rx_carrier_range) + ret = ir_dev-props-s_rx_carrier_range( + ir_dev-props-priv, + ir_dev-raw-lirc.carrier_low, val); + else + return -ENOSYS; - if (val != (LIRC_MODE_PULSE LIRC_CAN_SEND_MASK)) + if (!ret) + ir_dev-raw-lirc.carrier_low = 0; + break; + + case LIRC_SET_REC_CARRIER_RANGE: + if (val = 0) + ir_dev-raw-lirc.carrier_low = val; + break; + + + case LIRC_GET_REC_RESOLUTION: + val = ir_dev-props-rx_resolution; + break; + + case LIRC_SET_WIDEBAND_RECEIVER: + if (ir_dev-props-s_learning_mode) + return ir_dev-props-s_learning_mode( + ir_dev-props-priv, !!val); + else + return -ENOSYS; + + /*