Re: [PATCH 5/9] IR: extend interfaces to support more device settings

2010-07-29 Thread Christoph Bartelmus
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

2010-07-29 Thread Maxim Levitsky
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

2010-07-29 Thread Christoph Bartelmus
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

2010-07-28 Thread Maxim Levitsky
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

2010-07-28 Thread Mauro Carvalho Chehab
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

2010-07-28 Thread Jarod Wilson
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

2010-07-28 Thread Maxim Levitsky
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:
+