Re: [PATCH 10/13] IR: extend interfaces to support more device settings LIRC: add new IOCTL that enables learning mode (wide band receiver)

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

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

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

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

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

2010-07-29 Thread Maxim Levitsky
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;
+
+   /*