Re: [PATCH 5/9] IR: extend interfaces to support more device settings
Hi! Maxim Levitsky maximlevit...@gmail.com wrote: Also reuse LIRC_SET_MEASURE_CARRIER_MODE as LIRC_SET_LEARN_MODE (LIRC_SET_LEARN_MODE will start carrier reports if possible, and tune receiver to wide band mode) I don't like the rename of the ioctl. The ioctl should enable carrier reports. Anything else is hardware specific. Learn mode gives a somewhat wrong association to me. irrecord always has been using learn mode without ever using this ioctl. 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 5/9] IR: extend interfaces to support more device settings
On Thu, 2010-07-29 at 09:25 +0200, Christoph Bartelmus wrote: Hi! Maxim Levitsky maximlevit...@gmail.com wrote: Also reuse LIRC_SET_MEASURE_CARRIER_MODE as LIRC_SET_LEARN_MODE (LIRC_SET_LEARN_MODE will start carrier reports if possible, and tune receiver to wide band mode) I don't like the rename of the ioctl. The ioctl should enable carrier reports. Anything else is hardware specific. Learn mode gives a somewhat wrong association to me. irrecord always has been using learn mode without ever using this ioctl. Why? Carrier measure (if supported by hardware I think should always be enabled, because it can help in-kernel decoders). (Which raises seperate question on how to do so. I guess I will need to make ir_raw_event 64 bit after all...) Another thing is reporting these results to lirc. By default lirc shouldn't get carrier reports, but as soon as irrecord starts, it can place device in special mode that allows it to capture input better, and optionally do carrier reports. Do you think carrier reports are needed by lircd? 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
Re: [PATCH 5/9] IR: extend interfaces to support more device settings
Hi Maxim, on 29 Jul 10 at 18:27, Maxim Levitsky wrote: On Thu, 2010-07-29 at 09:25 +0200, Christoph Bartelmus wrote: Hi! Maxim Levitsky maximlevit...@gmail.com wrote: Also reuse LIRC_SET_MEASURE_CARRIER_MODE as LIRC_SET_LEARN_MODE (LIRC_SET_LEARN_MODE will start carrier reports if possible, and tune receiver to wide band mode) I don't like the rename of the ioctl. The ioctl should enable carrier reports. Anything else is hardware specific. Learn mode gives a somewhat wrong association to me. irrecord always has been using learn mode without ever using this ioctl. Why? If an ioctl enables/disables measuring of the carrier, then call it LIRC_SET_MEASURE_CARRIER_MODE and not LIRC_SET_LEARN_MODE. Whether we need a LIRC_ENABLE_WIDE_BAND_RECEIVER ioctl is another question. Carrier measure (if supported by hardware I think should always be enabled, because it can help in-kernel decoders). That does not work in the real-world scenario. All receivers with a high range demodulate the signal and you won't get the carrier. [...] Another thing is reporting these results to lirc. By default lirc shouldn't get carrier reports, but as soon as irrecord starts, it can place device in special mode that allows it to capture input better, and optionally do carrier reports. And that's what LIRC_SET_MEASURE_CARRIER_MODE is made for. Do you think carrier reports are needed by lircd? No. 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
[PATCH 5/9] IR: extend interfaces to support more device settings
Also reuse LIRC_SET_MEASURE_CARRIER_MODE as LIRC_SET_LEARN_MODE (LIRC_SET_LEARN_MODE will start carrier reports if possible, and tune receiver to wide band mode) This IOCTL isn't yet used by lirc, so this won't break userspace. Signed-off-by: Maxim Levitsky maximlevit...@gmail.com --- drivers/media/IR/ir-core-priv.h |2 + drivers/media/IR/ir-lirc-codec.c | 100 ++ include/media/ir-core.h | 11 include/media/lirc.h |4 +- 4 files changed, 105 insertions(+), 12 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index 3eafdb7..4ed170d 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -77,6 +77,8 @@ struct ir_raw_event_ctrl { struct lirc_codec { struct ir_input_dev *ir_dev; struct lirc_driver *drv; + int timeout_report; + int carrier_low; } lirc; }; diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c index 8ca01fd..0f3969c 100644 --- a/drivers/media/IR/ir-lirc-codec.c +++ b/drivers/media/IR/ir-lirc-codec.c @@ -96,13 +96,13 @@ 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) @@ -116,10 +116,21 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar switch (cmd) { case LIRC_SET_TRANSMITTER_MASK: + case LIRC_SET_SEND_CARRIER: + case LIRC_SET_SEND_MODE: + case LIRC_SET_REC_TIMEOUT: + case LIRC_SET_REC_TIMEOUT_REPORTS: + case LIRC_SET_LEARN_MODE: + case LIRC_SET_REC_CARRIER: + case LIRC_SET_REC_CARRIER_RANGE: + case LIRC_SET_SEND_DUTY_CYCLE: ret = get_user(val, (unsigned long *)arg); if (ret) return ret; + } + switch (cmd) { + case LIRC_SET_TRANSMITTER_MASK: if (ir_dev-props ir_dev-props-s_tx_mask) ret = ir_dev-props-s_tx_mask(drv_data, (u32)val); else @@ -127,10 +138,6 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar 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) ir_dev-props-s_tx_carrier(drv_data, (u32)val); else @@ -143,14 +150,75 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar break; case LIRC_SET_SEND_MODE: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; - if (val != (LIRC_MODE_PULSE LIRC_CAN_SEND_MASK)) return -EINVAL; break; + case LIRC_GET_REC_RESOLUTION: + val = ir_dev-props-rx_resolution; + ret = put_user(val, (unsigned long *)arg); + break; + + case LIRC_SET_REC_TIMEOUT: + if (val ir_dev-props-min_timeout || + val ir_dev-props-max_timeout) + return -EINVAL; + + ir_dev-props-timeout = val; + break; + + case LIRC_SET_REC_TIMEOUT_REPORTS: + ir_dev-raw-lirc.timeout_report = !!val; + return 0; + + case LIRC_GET_MIN_TIMEOUT: + + if (!ir_dev-props-max_timeout) + return -ENOSYS; + + ret = put_user(ir_dev-props-min_timeout, (unsigned long *)arg); + break; + case LIRC_GET_MAX_TIMEOUT: + if (!ir_dev-props-max_timeout) + return -ENOSYS; + + ret = put_user(ir_dev-props-max_timeout, (unsigned long *)arg); + break; + + case LIRC_SET_LEARN_MODE: + if (ir_dev-props-s_learning_mode) + return ir_dev-props-s_learning_mode( + ir_dev-props-priv, !!val); + else + return -ENOSYS; + + 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 (!ret) +
Re: [PATCH 5/9] IR: extend interfaces to support more device settings
Em 28-07-2010 12:14, Maxim Levitsky escreveu: Also reuse LIRC_SET_MEASURE_CARRIER_MODE as LIRC_SET_LEARN_MODE (LIRC_SET_LEARN_MODE will start carrier reports if possible, and tune receiver to wide band mode) This IOCTL isn't yet used by lirc, so this won't break userspace. Signed-off-by: Maxim Levitsky maximlevit...@gmail.com --- drivers/media/IR/ir-core-priv.h |2 + drivers/media/IR/ir-lirc-codec.c | 100 ++ include/media/ir-core.h | 11 include/media/lirc.h |4 +- 4 files changed, 105 insertions(+), 12 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index 3eafdb7..4ed170d 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -77,6 +77,8 @@ struct ir_raw_event_ctrl { struct lirc_codec { struct ir_input_dev *ir_dev; struct lirc_driver *drv; + int timeout_report; + int carrier_low; } lirc; }; diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c index 8ca01fd..0f3969c 100644 --- a/drivers/media/IR/ir-lirc-codec.c +++ b/drivers/media/IR/ir-lirc-codec.c @@ -96,13 +96,13 @@ 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) @@ -116,10 +116,21 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar switch (cmd) { case LIRC_SET_TRANSMITTER_MASK: + case LIRC_SET_SEND_CARRIER: + case LIRC_SET_SEND_MODE: + case LIRC_SET_REC_TIMEOUT: + case LIRC_SET_REC_TIMEOUT_REPORTS: + case LIRC_SET_LEARN_MODE: + case LIRC_SET_REC_CARRIER: + case LIRC_SET_REC_CARRIER_RANGE: + case LIRC_SET_SEND_DUTY_CYCLE: ret = get_user(val, (unsigned long *)arg); if (ret) return ret; + } As, in all cases, the argument is an __u32, you can just use this, to get the arguments for all LIRC_SET_* cases: if (_IOC_DIR(cmd) _IOC_WRITE) { ret = get_user(val, (unsigned long *)arg); if (ret) return ret; } + switch (cmd) { + case LIRC_SET_TRANSMITTER_MASK: if (ir_dev-props ir_dev-props-s_tx_mask) ret = ir_dev-props-s_tx_mask(drv_data, (u32)val); else @@ -127,10 +138,6 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar 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) ir_dev-props-s_tx_carrier(drv_data, (u32)val); else @@ -143,14 +150,75 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar break; case LIRC_SET_SEND_MODE: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; - if (val != (LIRC_MODE_PULSE LIRC_CAN_SEND_MASK)) return -EINVAL; break; + case LIRC_GET_REC_RESOLUTION: + val = ir_dev-props-rx_resolution; + ret = put_user(val, (unsigned long *)arg); + break; You can use something like this, to handle the LIRC_GET* cases: switch (cmd) { ... case LIRC_GET_REC_RESOLUTION: val = ir_dev-props-rx_resolution; break; ... } if (_IOC_DIR(cmd) _IOC_READ) { ret = put_user(val, (unsigned long *)arg); if (ret) return ret; } + + case LIRC_SET_REC_TIMEOUT: + if (val ir_dev-props-min_timeout || + val ir_dev-props-max_timeout) + return -EINVAL; + + ir_dev-props-timeout = val; + break; + + case LIRC_SET_REC_TIMEOUT_REPORTS: + ir_dev-raw-lirc.timeout_report = !!val; + return 0; + + case LIRC_GET_MIN_TIMEOUT: + + if (!ir_dev-props-max_timeout) + return -ENOSYS; + + ret = put_user(ir_dev-props-min_timeout, (unsigned long *)arg); + break; + case LIRC_GET_MAX_TIMEOUT: + if (!ir_dev-props-max_timeout) + return -ENOSYS; + + ret =
Re: [PATCH 5/9] IR: extend interfaces to support more device settings
On Wed, Jul 28, 2010 at 06:14:07PM +0300, Maxim Levitsky wrote: Also reuse LIRC_SET_MEASURE_CARRIER_MODE as LIRC_SET_LEARN_MODE (LIRC_SET_LEARN_MODE will start carrier reports if possible, and tune receiver to wide band mode) This IOCTL isn't yet used by lirc, so this won't break userspace. Plus, once lirc 0.8.7 is released (Real Soon Now), we'll start working on lirc 0.9.0 with the express goal of it being built against lirc.h as provided by the kernel. These all generally look good and sane to me, and I'll make use of the LEARN_MODE bits for mceusb after something along these lines is committed. I like the simplifications Mauro suggested for the ioctl handling. In addition to those, there's a bit of whitespace damage in lirc.h that I'd like to see cleaned up for v2. -- Jarod Wilson ja...@redhat.com -- 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 5/9] IR: extend interfaces to support more device settings
Also reuse LIRC_SET_MEASURE_CARRIER_MODE as LIRC_SET_LEARN_MODE (LIRC_SET_LEARN_MODE will start carrier reports if possible, and tune receiver to wide band mode) This IOCTL isn't yet used by lirc, so this won't break userspace. Note that drivers currently can't report carrier, because raw event doesn't have space to indicate that. Signed-off-by: Maxim Levitsky maximlevit...@gmail.com --- drivers/media/IR/ir-core-priv.h |2 + drivers/media/IR/ir-lirc-codec.c | 119 +++--- drivers/media/IR/ir-raw-event.c | 13 ++--- include/media/ir-core.h | 11 include/media/lirc.h |4 +- 5 files changed, 117 insertions(+), 32 deletions(-) diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h index d6ec4fe..9c594af 100644 --- a/drivers/media/IR/ir-core-priv.h +++ b/drivers/media/IR/ir-core-priv.h @@ -77,6 +77,8 @@ struct ir_raw_event_ctrl { struct lirc_codec { struct ir_input_dev *ir_dev; struct lirc_driver *drv; + int timeout_report; + int carrier_low; } lirc; }; diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c index 8ca01fd..70c299f 100644 --- a/drivers/media/IR/ir-lirc-codec.c +++ b/drivers/media/IR/ir-lirc-codec.c @@ -40,16 +40,24 @@ static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event ev) if (!ir_dev-raw-lirc.drv || !ir_dev-raw-lirc.drv-rbuf) return -EINVAL; - if (IS_RESET(ev)) - return 0; + if (IS_RESET(ev)) { + + if (ir_dev-raw-lirc.timeout_report) + sample = LIRC_TIMEOUT(0); + else + return 0; - IR_dprintk(2, LIRC data transfer started (%uus %s)\n, - TO_US(ev.duration), TO_STR(ev.pulse)); + IR_dprintk(2, LIRC: Sending timeout packet\n); + } else { + sample = ev.duration / 1000; + if (ev.pulse) + sample |= PULSE_BIT; + + 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; lirc_buffer_write(ir_dev-raw-lirc.drv-rbuf, (unsigned char *) sample); @@ -96,13 +104,13 @@ 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,24 +122,22 @@ 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; + } - if (ir_dev-props ir_dev-props-s_tx_mask) + switch (cmd) { + 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; @@ -139,22 +145,79 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd, unsigned long ar case LIRC_GET_SEND_MODE: val = LIRC_CAN_SEND_PULSE LIRC_CAN_SEND_MASK; - ret = put_user(val, (unsigned long *)arg); break; case LIRC_SET_SEND_MODE: - ret = get_user(val, (unsigned long *)arg); - if (ret) - return ret; - if (val != (LIRC_MODE_PULSE LIRC_CAN_SEND_MASK)) return -EINVAL; break; + case LIRC_GET_REC_RESOLUTION: + val = ir_dev-props-rx_resolution; + break; + + case LIRC_SET_REC_TIMEOUT: + if (val ir_dev-props-min_timeout || + val ir_dev-props-max_timeout) + return -EINVAL; + ir_dev-props-timeout = val; + break; + + case LIRC_SET_REC_TIMEOUT_REPORTS: +