Re: [PATCH 7/8] IR: extend ir_raw_event and do refactoring

2010-10-14 Thread Jarod Wilson 
On Tue, Sep 07, 2010 at 12:26:12AM +0300, Maxim Levitsky wrote:
 Add new event types for timeout  carrier report
 Move timeout handling from ir_raw_event_store_with_filter to
 ir-lirc-codec, where it is really needed.
 Now lirc bridge ensures proper gap handling.
 Extend lirc bridge for carrier  timeout reports
 
 Note: all new ir_raw_event variables now should be initialized
 like that: DEFINE_IR_RAW_EVENT(ev);
 
 To clean an existing event, use init_ir_raw_event(ev);
 
 Signed-off-by: Maxim Levitsky maximlevit...@gmail.com

Okay, so I think the end result of discussion on this patch was that we're
all pretty much fine with it going in, even the as-yet-unused duty_cycle
bit, as there *are* drivers that can use it sooner than later.

Acked-by: Jarod Wilson ja...@redhat.com


-- 
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

Re: [PATCH 7/8] IR: extend ir_raw_event and do refactoring

2010-09-08 Thread Jarod Wilson 
On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky maximlevit...@gmail.com wrote:
 Add new event types for timeout  carrier report
 Move timeout handling from ir_raw_event_store_with_filter to
 ir-lirc-codec, where it is really needed.
 Now lirc bridge ensures proper gap handling.
 Extend lirc bridge for carrier  timeout reports

 Note: all new ir_raw_event variables now should be initialized
 like that: DEFINE_IR_RAW_EVENT(ev);

 To clean an existing event, use init_ir_raw_event(ev);

 Signed-off-by: Maxim Levitsky maximlevit...@gmail.com
 ---
  drivers/media/IR/ene_ir.c          |    4 +-
  drivers/media/IR/ir-core-priv.h    |   13 ++-
  drivers/media/IR/ir-jvc-decoder.c  |    5 +-
  drivers/media/IR/ir-lirc-codec.c   |   78 
 +++-
  drivers/media/IR/ir-nec-decoder.c  |    5 +-
  drivers/media/IR/ir-raw-event.c    |   45 +++-
  drivers/media/IR/ir-rc5-decoder.c  |    5 +-
  drivers/media/IR/ir-rc6-decoder.c  |    5 +-
  drivers/media/IR/ir-sony-decoder.c |    5 +-
  drivers/media/IR/mceusb.c          |    3 +-
  drivers/media/IR/streamzap.c       |    8 ++-
  include/media/ir-core.h            |   40 ---
  12 files changed, 153 insertions(+), 63 deletions(-)
...
 @@ -162,22 +164,48 @@ u32 ir_g_keycode_from_table(struct input_dev 
 *input_dev, u32 scancode);
  /* From ir-raw-event.c */

  struct ir_raw_event {
 -       unsigned                        pulse:1;
 -       unsigned                        duration:31;
 +       union {
 +               u32             duration;
 +
 +               struct {
 +                       u32     carrier;
 +                       u8      duty_cycle;
 +               };
 +       };
 +
 +       unsigned                pulse:1;
 +       unsigned                reset:1;
 +       unsigned                timeout:1;
 +       unsigned                carrier_report:1;
  };

I'm generally good with this entire patch, but the union usage looks a
bit odd, as the members aren't of the same size, which is generally
what I've come to expect looking at other code. I'd be inclined to
simply move duty_cycle out of the union and leave just duration and
carrier in it. However, as discussed on irc and upon looking at the
code, we don't actually do anything useful with duty_cycle yet, so
perhaps just cut it out entirely for the moment, and add it later when
its of use.

-- 
Jarod Wilson
ja...@wilsonet.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

Re: [PATCH 7/8] IR: extend ir_raw_event and do refactoring

2010-09-08 Thread Andy Walls 
On Wed, 2010-09-08 at 11:26 -0400, Jarod Wilson wrote:
 On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky maximlevit...@gmail.com 
 wrote:
  Add new event types for timeout  carrier report
  Move timeout handling from ir_raw_event_store_with_filter to
  ir-lirc-codec, where it is really needed.
  Now lirc bridge ensures proper gap handling.
  Extend lirc bridge for carrier  timeout reports
 
  Note: all new ir_raw_event variables now should be initialized
  like that: DEFINE_IR_RAW_EVENT(ev);
 
  To clean an existing event, use init_ir_raw_event(ev);
 
  Signed-off-by: Maxim Levitsky maximlevit...@gmail.com
  ---
   drivers/media/IR/ene_ir.c  |4 +-
   drivers/media/IR/ir-core-priv.h|   13 ++-
   drivers/media/IR/ir-jvc-decoder.c  |5 +-
   drivers/media/IR/ir-lirc-codec.c   |   78 
  +++-
   drivers/media/IR/ir-nec-decoder.c  |5 +-
   drivers/media/IR/ir-raw-event.c|   45 +++-
   drivers/media/IR/ir-rc5-decoder.c  |5 +-
   drivers/media/IR/ir-rc6-decoder.c  |5 +-
   drivers/media/IR/ir-sony-decoder.c |5 +-
   drivers/media/IR/mceusb.c  |3 +-
   drivers/media/IR/streamzap.c   |8 ++-
   include/media/ir-core.h|   40 ---
   12 files changed, 153 insertions(+), 63 deletions(-)
 ...
  @@ -162,22 +164,48 @@ u32 ir_g_keycode_from_table(struct input_dev 
  *input_dev, u32 scancode);
   /* From ir-raw-event.c */
 
   struct ir_raw_event {
  -   unsignedpulse:1;
  -   unsignedduration:31;
  +   union {
  +   u32 duration;
  +
  +   struct {
  +   u32 carrier;
  +   u8  duty_cycle;
  +   };
  +   };
  +
  +   unsignedpulse:1;
  +   unsignedreset:1;
  +   unsignedtimeout:1;
  +   unsignedcarrier_report:1;
   };
 
 I'm generally good with this entire patch, but the union usage looks a
 bit odd, as the members aren't of the same size, which is generally
 what I've come to expect looking at other code.

Having a union with different sized members is perfectly valid C code. 

Just look at the definition of the XEvent in Xlib.h.  The int type; is
smaller than everything, and the XAnyEvent is smaller than the other
event types.

The size of the union will be the size of its largest member.



  I'd be inclined to
 simply move duty_cycle out of the union and leave just duration and
 carrier in it.

That's not necessary and it could be confusing depending on where you
put duty_cycle.

Regards,
Andy

  However, as discussed on irc and upon looking at the
 code, we don't actually do anything useful with duty_cycle yet, so
 perhaps just cut it out entirely for the moment, and add it later when
 its of use.
 


--
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 7/8] IR: extend ir_raw_event and do refactoring

2010-09-08 Thread Mauro Carvalho Chehab 
Em 06-09-2010 18:26, Maxim Levitsky escreveu:
 Add new event types for timeout  carrier report
 Move timeout handling from ir_raw_event_store_with_filter to
 ir-lirc-codec, where it is really needed.
 Now lirc bridge ensures proper gap handling.
 Extend lirc bridge for carrier  timeout reports
 
 Note: all new ir_raw_event variables now should be initialized
 like that: DEFINE_IR_RAW_EVENT(ev);
 
 To clean an existing event, use init_ir_raw_event(ev);
 
 Signed-off-by: Maxim Levitsky maximlevit...@gmail.com
 ---
  drivers/media/IR/ene_ir.c  |4 +-
  drivers/media/IR/ir-core-priv.h|   13 ++-
  drivers/media/IR/ir-jvc-decoder.c  |5 +-
  drivers/media/IR/ir-lirc-codec.c   |   78 
 +++-
  drivers/media/IR/ir-nec-decoder.c  |5 +-
  drivers/media/IR/ir-raw-event.c|   45 +++-
  drivers/media/IR/ir-rc5-decoder.c  |5 +-
  drivers/media/IR/ir-rc6-decoder.c  |5 +-
  drivers/media/IR/ir-sony-decoder.c |5 +-
  drivers/media/IR/mceusb.c  |3 +-
  drivers/media/IR/streamzap.c   |8 ++-
  include/media/ir-core.h|   40 ---
  12 files changed, 153 insertions(+), 63 deletions(-)
 
 diff --git a/drivers/media/IR/ene_ir.c b/drivers/media/IR/ene_ir.c
 index 7880d9c..dc32509 100644
 --- a/drivers/media/IR/ene_ir.c
 +++ b/drivers/media/IR/ene_ir.c
 @@ -701,7 +701,7 @@ static irqreturn_t ene_isr(int irq, void *data)
   unsigned long flags;
   irqreturn_t retval = IRQ_NONE;
   struct ene_device *dev = (struct ene_device *)data;
 - struct ir_raw_event ev;
 + DEFINE_IR_RAW_EVENT(ev);
  
   spin_lock_irqsave(dev-hw_lock, flags);
  
 @@ -904,7 +904,7 @@ static int ene_set_learning_mode(void *data, int enable)
  }
  
  /* outside interface: enable or disable idle mode */
 -static void ene_rx_set_idle(void *data, int idle)
 +static void ene_rx_set_idle(void *data, bool idle)
  {
   struct ene_device *dev = (struct ene_device *)data;
  
 diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
 index 5d7e08f..2559e72 100644
 --- a/drivers/media/IR/ir-core-priv.h
 +++ b/drivers/media/IR/ir-core-priv.h
 @@ -82,6 +82,12 @@ struct ir_raw_event_ctrl {
   struct ir_input_dev *ir_dev;
   struct lirc_driver *drv;
   int carrier_low;
 +
 + ktime_t gap_start;
 + u64 gap_duration;
 + bool gap;
 + bool send_timeout_reports;
 +
   } lirc;
  };
  
 @@ -109,9 +115,14 @@ static inline void decrease_duration(struct ir_raw_event 
 *ev, unsigned duration)
   ev-duration -= duration;
  }
  
 +/* Returns true if event is normal pulse/space event */
 +static inline bool is_timing_event(struct ir_raw_event ev)
 +{
 + return !ev.carrier_report  !ev.reset;
 +}
 +
  #define TO_US(duration)  DIV_ROUND_CLOSEST((duration), 
 1000)
  #define TO_STR(is_pulse) ((is_pulse) ? pulse : space)
 -#define IS_RESET(ev) (ev.duration == 0)
  /*
   * Routines from ir-sysfs.c - Meant to be called only internally inside
   * ir-core
 diff --git a/drivers/media/IR/ir-jvc-decoder.c 
 b/drivers/media/IR/ir-jvc-decoder.c
 index 77a89c4..63dca6e 100644
 --- a/drivers/media/IR/ir-jvc-decoder.c
 +++ b/drivers/media/IR/ir-jvc-decoder.c
 @@ -50,8 +50,9 @@ static int ir_jvc_decode(struct input_dev *input_dev, 
 struct ir_raw_event ev)
   if (!(ir_dev-raw-enabled_protocols  IR_TYPE_JVC))
   return 0;
  
 - if (IS_RESET(ev)) {
 - data-state = STATE_INACTIVE;
 + if (!is_timing_event(ev)) {
 + if (ev.reset)
 + data-state = STATE_INACTIVE;
   return 0;
   }
  
 diff --git a/drivers/media/IR/ir-lirc-codec.c 
 b/drivers/media/IR/ir-lirc-codec.c
 index e63f757..0f40bc2 100644
 --- a/drivers/media/IR/ir-lirc-codec.c
 +++ b/drivers/media/IR/ir-lirc-codec.c
 @@ -32,6 +32,7 @@
  static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event 
 ev)
  {
   struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 + struct lirc_codec *lirc = ir_dev-raw-lirc;
   int sample;
  
   if (!(ir_dev-raw-enabled_protocols  IR_TYPE_LIRC))
 @@ -40,21 +41,57 @@ 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))
 + /* Packet start */
 + if (ev.reset)
   return 0;
  
 - IR_dprintk(2, LIRC data transfer started (%uus %s)\n,
 -TO_US(ev.duration), TO_STR(ev.pulse));
 + /* Carrier reports */
 + if (ev.carrier_report) {
 + sample = LIRC_FREQUENCY(ev.carrier);
 +
 + /* Packet end */
 + } else if (ev.timeout) {
 +
 + if (lirc-gap)
 + return 0;
 +
 + lirc-gap_start = ktime_get();
 + lirc-gap = true;
 +

Re: [PATCH 7/8] IR: extend ir_raw_event and do refactoring

2010-09-08 Thread David Härdeman 
On Wed, Sep 08, 2010 at 07:42:04PM -0300, Mauro Carvalho Chehab wrote:
 Em 06-09-2010 18:26, Maxim Levitsky escreveu:
  Add new event types for timeout  carrier report
  Move timeout handling from ir_raw_event_store_with_filter to
  ir-lirc-codec, where it is really needed.
  Now lirc bridge ensures proper gap handling.
  Extend lirc bridge for carrier  timeout reports
  
  Note: all new ir_raw_event variables now should be initialized
  like that: DEFINE_IR_RAW_EVENT(ev);
  
  To clean an existing event, use init_ir_raw_event(ev);
  
  Signed-off-by: Maxim Levitsky maximlevit...@gmail.com
  ---
   drivers/media/IR/ene_ir.c  |4 +-
   drivers/media/IR/ir-core-priv.h|   13 ++-
   drivers/media/IR/ir-jvc-decoder.c  |5 +-
   drivers/media/IR/ir-lirc-codec.c   |   78 
  +++-
   drivers/media/IR/ir-nec-decoder.c  |5 +-
   drivers/media/IR/ir-raw-event.c|   45 +++-
   drivers/media/IR/ir-rc5-decoder.c  |5 +-
   drivers/media/IR/ir-rc6-decoder.c  |5 +-
   drivers/media/IR/ir-sony-decoder.c |5 +-
   drivers/media/IR/mceusb.c  |3 +-
   drivers/media/IR/streamzap.c   |8 ++-
   include/media/ir-core.h|   40 ---
   12 files changed, 153 insertions(+), 63 deletions(-)
  
  diff --git a/drivers/media/IR/ene_ir.c b/drivers/media/IR/ene_ir.c
  index 7880d9c..dc32509 100644
  --- a/drivers/media/IR/ene_ir.c
  +++ b/drivers/media/IR/ene_ir.c
  @@ -701,7 +701,7 @@ static irqreturn_t ene_isr(int irq, void *data)
  unsigned long flags;
  irqreturn_t retval = IRQ_NONE;
  struct ene_device *dev = (struct ene_device *)data;
  -   struct ir_raw_event ev;
  +   DEFINE_IR_RAW_EVENT(ev);
   
  spin_lock_irqsave(dev-hw_lock, flags);
   
  @@ -904,7 +904,7 @@ static int ene_set_learning_mode(void *data, int enable)
   }
   
   /* outside interface: enable or disable idle mode */
  -static void ene_rx_set_idle(void *data, int idle)
  +static void ene_rx_set_idle(void *data, bool idle)
   {
  struct ene_device *dev = (struct ene_device *)data;
   
  diff --git a/drivers/media/IR/ir-core-priv.h 
  b/drivers/media/IR/ir-core-priv.h
  index 5d7e08f..2559e72 100644
  --- a/drivers/media/IR/ir-core-priv.h
  +++ b/drivers/media/IR/ir-core-priv.h
  @@ -82,6 +82,12 @@ struct ir_raw_event_ctrl {
  struct ir_input_dev *ir_dev;
  struct lirc_driver *drv;
  int carrier_low;
  +
  +   ktime_t gap_start;
  +   u64 gap_duration;
  +   bool gap;
  +   bool send_timeout_reports;
  +
  } lirc;
   };
   
  @@ -109,9 +115,14 @@ static inline void decrease_duration(struct 
  ir_raw_event *ev, unsigned duration)
  ev-duration -= duration;
   }
   
  +/* Returns true if event is normal pulse/space event */
  +static inline bool is_timing_event(struct ir_raw_event ev)
  +{
  +   return !ev.carrier_report  !ev.reset;
  +}
  +
   #define TO_US(duration)DIV_ROUND_CLOSEST((duration), 
  1000)
   #define TO_STR(is_pulse)   ((is_pulse) ? pulse : space)
  -#define IS_RESET(ev)   (ev.duration == 0)
   /*
* Routines from ir-sysfs.c - Meant to be called only internally inside
* ir-core
  diff --git a/drivers/media/IR/ir-jvc-decoder.c 
  b/drivers/media/IR/ir-jvc-decoder.c
  index 77a89c4..63dca6e 100644
  --- a/drivers/media/IR/ir-jvc-decoder.c
  +++ b/drivers/media/IR/ir-jvc-decoder.c
  @@ -50,8 +50,9 @@ static int ir_jvc_decode(struct input_dev *input_dev, 
  struct ir_raw_event ev)
  if (!(ir_dev-raw-enabled_protocols  IR_TYPE_JVC))
  return 0;
   
  -   if (IS_RESET(ev)) {
  -   data-state = STATE_INACTIVE;
  +   if (!is_timing_event(ev)) {
  +   if (ev.reset)
  +   data-state = STATE_INACTIVE;
  return 0;
  }
   
  diff --git a/drivers/media/IR/ir-lirc-codec.c 
  b/drivers/media/IR/ir-lirc-codec.c
  index e63f757..0f40bc2 100644
  --- a/drivers/media/IR/ir-lirc-codec.c
  +++ b/drivers/media/IR/ir-lirc-codec.c
  @@ -32,6 +32,7 @@
   static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event 
  ev)
   {
  struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
  +   struct lirc_codec *lirc = ir_dev-raw-lirc;
  int sample;
   
  if (!(ir_dev-raw-enabled_protocols  IR_TYPE_LIRC))
  @@ -40,21 +41,57 @@ 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))
  +   /* Packet start */
  +   if (ev.reset)
  return 0;
   
  -   IR_dprintk(2, LIRC data transfer started (%uus %s)\n,
  -  TO_US(ev.duration), TO_STR(ev.pulse));
  +   /* Carrier reports */
  +   if (ev.carrier_report) {
  +   sample = LIRC_FREQUENCY(ev.carrier);
  +
  +   /* Packet end */
  +   } else if (ev.timeout) {
  +
  +   if (lirc-gap)

Re: [PATCH 7/8] IR: extend ir_raw_event and do refactoring

2010-09-08 Thread Mauro Carvalho Chehab 
Em 08-09-2010 19:49, David Härdeman escreveu:
 On Wed, Sep 08, 2010 at 07:42:04PM -0300, Mauro Carvalho Chehab wrote:
 Em 06-09-2010 18:26, Maxim Levitsky escreveu:
 diff --git a/drivers/media/IR/ir-rc6-decoder.c 
 b/drivers/media/IR/ir-rc6-decoder.c
 index f1624b8..d25da91 100644
 --- a/drivers/media/IR/ir-rc6-decoder.c
 +++ b/drivers/media/IR/ir-rc6-decoder.c
 @@ -85,8 +85,9 @@ static int ir_rc6_decode(struct input_dev *input_dev, 
 struct ir_raw_event ev)
 if (!(ir_dev-raw-enabled_protocols  IR_TYPE_RC6))
 return 0;
  
 -   if (IS_RESET(ev)) {
 -   data-state = STATE_INACTIVE;
 +   if (!is_timing_event(ev)) {
 +   if (ev.reset)

 Again, why do you need to test first for !is_timing_event()?
 
 Because the decoder should return early if the event is not a timing 
 event (the return 0 two lines below)...think carrier report event...

Yeah, I saw that. I was supposed to remove all the comments about that, but I
forgot to remove the last one ;)

Cheers,
Mauro.
--
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 7/8] IR: extend ir_raw_event and do refactoring

2010-09-08 Thread Andy Walls 
On Wed, 2010-09-08 at 13:27 -0400, Jarod Wilson wrote:
 On Wed, Sep 08, 2010 at 12:50:46PM -0400, Andy Walls wrote:
  On Wed, 2010-09-08 at 11:26 -0400, Jarod Wilson wrote:
   On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky maximlevit...@gmail.com 
   wrote:

   
   I'm generally good with this entire patch, but the union usage looks a
   bit odd, as the members aren't of the same size, which is generally
   what I've come to expect looking at other code.
  
  Having a union with different sized members is perfectly valid C code. 
  

 Yeah, no, I know that it'll work, just that most of the unions I've
 actually paid any attention to had members all of the same size. Seemed
 like sort of an unwritten rule for in-kernel use. But its probably just
 fine.

Well if it's an unwritten rule, not everyone is following it. :)
There are numerous counter-examples in include/linux/*.h .  Here are a
few easy to see ones:

include/linux/input.h:
union in struct ff_effect: ff_rumble vs. ff_periodic   

include/linux/i2c.h
union i2c_smbus_data: byte vs. word vs. block[]

include/linux/kfifo.h
DECLARE_KFIFO



I'd be inclined to
   simply move duty_cycle out of the union and leave just duration and
   carrier in it.
  
  That's not necessary and it could be confusing depending on where you
  put duty_cycle.
 
 There's that. But without having code that actually uses duty_cycle in a
 meaningful way yet, its hard to say for sure. If carrier and duty_cycle
 were only being sent out in their own events, you might actually want a
 union of duration, carrier and duty_cycle. Though I suspect we'll probably
 want to pass along carrier and duty_cycle at the same time.

I suspect you're right on that.  I don't have any experience with
hardware that can actually estimate carrier freq or duty cycle.  I
suspect they can be measured together using edge detection on both
rising and falling edges.

Regards,
Andy




--
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 7/8] IR: extend ir_raw_event and do refactoring

2010-09-08 Thread Mauro Carvalho Chehab 
Em 08-09-2010 20:02, Andy Walls escreveu:
 On Wed, 2010-09-08 at 13:27 -0400, Jarod Wilson wrote:
 
  I'd be inclined to
 simply move duty_cycle out of the union and leave just duration and
 carrier in it.

 That's not necessary and it could be confusing depending on where you
 put duty_cycle.

 There's that. But without having code that actually uses duty_cycle in a
 meaningful way yet, its hard to say for sure. If carrier and duty_cycle
 were only being sent out in their own events, you might actually want a
 union of duration, carrier and duty_cycle. Though I suspect we'll probably
 want to pass along carrier and duty_cycle at the same time.
 
 I suspect you're right on that.  I don't have any experience with
 hardware that can actually estimate carrier freq or duty cycle.  I
 suspect they can be measured together using edge detection on both
 rising and falling edges.

As duty cycle is not currently used, the better is to just remove it from
the struct, adding it on a separate patch, together with a code that will
need it.

Cheers,
Mauro
--
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 7/8] IR: extend ir_raw_event and do refactoring

2010-09-06 Thread Maxim Levitsky 
Add new event types for timeout  carrier report
Move timeout handling from ir_raw_event_store_with_filter to
ir-lirc-codec, where it is really needed.
Now lirc bridge ensures proper gap handling.
Extend lirc bridge for carrier  timeout reports

Note: all new ir_raw_event variables now should be initialized
like that: DEFINE_IR_RAW_EVENT(ev);

To clean an existing event, use init_ir_raw_event(ev);

Signed-off-by: Maxim Levitsky maximlevit...@gmail.com
---
 drivers/media/IR/ene_ir.c  |4 +-
 drivers/media/IR/ir-core-priv.h|   13 ++-
 drivers/media/IR/ir-jvc-decoder.c  |5 +-
 drivers/media/IR/ir-lirc-codec.c   |   78 +++-
 drivers/media/IR/ir-nec-decoder.c  |5 +-
 drivers/media/IR/ir-raw-event.c|   45 +++-
 drivers/media/IR/ir-rc5-decoder.c  |5 +-
 drivers/media/IR/ir-rc6-decoder.c  |5 +-
 drivers/media/IR/ir-sony-decoder.c |5 +-
 drivers/media/IR/mceusb.c  |3 +-
 drivers/media/IR/streamzap.c   |8 ++-
 include/media/ir-core.h|   40 ---
 12 files changed, 153 insertions(+), 63 deletions(-)

diff --git a/drivers/media/IR/ene_ir.c b/drivers/media/IR/ene_ir.c
index 7880d9c..dc32509 100644
--- a/drivers/media/IR/ene_ir.c
+++ b/drivers/media/IR/ene_ir.c
@@ -701,7 +701,7 @@ static irqreturn_t ene_isr(int irq, void *data)
unsigned long flags;
irqreturn_t retval = IRQ_NONE;
struct ene_device *dev = (struct ene_device *)data;
-   struct ir_raw_event ev;
+   DEFINE_IR_RAW_EVENT(ev);
 
spin_lock_irqsave(dev-hw_lock, flags);
 
@@ -904,7 +904,7 @@ static int ene_set_learning_mode(void *data, int enable)
 }
 
 /* outside interface: enable or disable idle mode */
-static void ene_rx_set_idle(void *data, int idle)
+static void ene_rx_set_idle(void *data, bool idle)
 {
struct ene_device *dev = (struct ene_device *)data;
 
diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
index 5d7e08f..2559e72 100644
--- a/drivers/media/IR/ir-core-priv.h
+++ b/drivers/media/IR/ir-core-priv.h
@@ -82,6 +82,12 @@ struct ir_raw_event_ctrl {
struct ir_input_dev *ir_dev;
struct lirc_driver *drv;
int carrier_low;
+
+   ktime_t gap_start;
+   u64 gap_duration;
+   bool gap;
+   bool send_timeout_reports;
+
} lirc;
 };
 
@@ -109,9 +115,14 @@ static inline void decrease_duration(struct ir_raw_event 
*ev, unsigned duration)
ev-duration -= duration;
 }
 
+/* Returns true if event is normal pulse/space event */
+static inline bool is_timing_event(struct ir_raw_event ev)
+{
+   return !ev.carrier_report  !ev.reset;
+}
+
 #define TO_US(duration)DIV_ROUND_CLOSEST((duration), 
1000)
 #define TO_STR(is_pulse)   ((is_pulse) ? pulse : space)
-#define IS_RESET(ev)   (ev.duration == 0)
 /*
  * Routines from ir-sysfs.c - Meant to be called only internally inside
  * ir-core
diff --git a/drivers/media/IR/ir-jvc-decoder.c 
b/drivers/media/IR/ir-jvc-decoder.c
index 77a89c4..63dca6e 100644
--- a/drivers/media/IR/ir-jvc-decoder.c
+++ b/drivers/media/IR/ir-jvc-decoder.c
@@ -50,8 +50,9 @@ static int ir_jvc_decode(struct input_dev *input_dev, struct 
ir_raw_event ev)
if (!(ir_dev-raw-enabled_protocols  IR_TYPE_JVC))
return 0;
 
-   if (IS_RESET(ev)) {
-   data-state = STATE_INACTIVE;
+   if (!is_timing_event(ev)) {
+   if (ev.reset)
+   data-state = STATE_INACTIVE;
return 0;
}
 
diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c
index e63f757..0f40bc2 100644
--- a/drivers/media/IR/ir-lirc-codec.c
+++ b/drivers/media/IR/ir-lirc-codec.c
@@ -32,6 +32,7 @@
 static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 {
struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+   struct lirc_codec *lirc = ir_dev-raw-lirc;
int sample;
 
if (!(ir_dev-raw-enabled_protocols  IR_TYPE_LIRC))
@@ -40,21 +41,57 @@ 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))
+   /* Packet start */
+   if (ev.reset)
return 0;
 
-   IR_dprintk(2, LIRC data transfer started (%uus %s)\n,
-  TO_US(ev.duration), TO_STR(ev.pulse));
+   /* Carrier reports */
+   if (ev.carrier_report) {
+   sample = LIRC_FREQUENCY(ev.carrier);
+
+   /* Packet end */
+   } else if (ev.timeout) {
+
+   if (lirc-gap)
+   return 0;
+
+   lirc-gap_start = ktime_get();
+   lirc-gap = true;
+   lirc-gap_duration = ev.duration;
+
+   if (!lirc-send_timeout_reports)
+

Re: [PATCH 7/8] IR: extend ir_raw_event and do refactoring

2010-09-05 Thread David Härdeman 
Comments inline...

On Sun, Sep 05, 2010 at 02:23:02AM +0300, Maxim Levitsky wrote:
 Add new event types for timeout  carrier report
 Move timeout handling from ir_raw_event_store_with_filter to
 ir-lirc-codec, where it is really needed.
 Now lirc bridge ensures proper gap handling.
 Extend lirc bridge for carrier  timeout reports
 
 Note: all new ir_raw_event variables now should be initialized
 like that: DEFINE_RC_RAW_EVENT(ev);
 
 To clean an existing event, use init_ir_raw_event(ev);
 
 Signed-off-by: Maxim Levitsky maximlevit...@gmail.com
 ---
  drivers/media/IR/ene_ir.c  |2 +-
  drivers/media/IR/ir-core-priv.h|   11 +-
  drivers/media/IR/ir-jvc-decoder.c  |5 +-
  drivers/media/IR/ir-lirc-codec.c   |   78 
 +++-
  drivers/media/IR/ir-nec-decoder.c  |5 +-
  drivers/media/IR/ir-raw-event.c|   43 +++-
  drivers/media/IR/ir-rc5-decoder.c  |5 +-
  drivers/media/IR/ir-rc6-decoder.c  |5 +-
  drivers/media/IR/ir-sony-decoder.c |5 +-
  drivers/media/IR/mceusb.c  |3 +-
  drivers/media/IR/streamzap.c   |8 ++-
  include/media/ir-core.h|   35 ++--
  12 files changed, 146 insertions(+), 59 deletions(-)
 
 diff --git a/drivers/media/IR/ene_ir.c b/drivers/media/IR/ene_ir.c
 index 7880d9c..5ebafb5 100644
 --- a/drivers/media/IR/ene_ir.c
 +++ b/drivers/media/IR/ene_ir.c
 @@ -701,7 +701,7 @@ static irqreturn_t ene_isr(int irq, void *data)
   unsigned long flags;
   irqreturn_t retval = IRQ_NONE;
   struct ene_device *dev = (struct ene_device *)data;
 - struct ir_raw_event ev;
 + DEFINE_RC_RAW_EVENT(ev);
  
   spin_lock_irqsave(dev-hw_lock, flags);
  
 diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
 index 5d7e08f..a287373 100644
 --- a/drivers/media/IR/ir-core-priv.h
 +++ b/drivers/media/IR/ir-core-priv.h
 @@ -82,6 +82,10 @@ struct ir_raw_event_ctrl {
   struct ir_input_dev *ir_dev;
   struct lirc_driver *drv;
   int carrier_low;
 + ktime_t timeout_start;
 + bool timeout;
 + bool send_timeout_reports;
 +
   } lirc;
  };
  
 @@ -109,9 +113,14 @@ static inline void decrease_duration(struct ir_raw_event 
 *ev, unsigned duration)
   ev-duration -= duration;
  }
  
 +/* Returns true if event is normal pulse/space event */
 +static inline bool is_timing_event(struct ir_raw_event ev)
 +{
 + return !ev.carrier_report  !ev.reset;
 +}
 +
  #define TO_US(duration)  DIV_ROUND_CLOSEST((duration), 
 1000)
  #define TO_STR(is_pulse) ((is_pulse) ? pulse : space)
 -#define IS_RESET(ev) (ev.duration == 0)
  /*
   * Routines from ir-sysfs.c - Meant to be called only internally inside
   * ir-core
 diff --git a/drivers/media/IR/ir-jvc-decoder.c 
 b/drivers/media/IR/ir-jvc-decoder.c
 index 77a89c4..63dca6e 100644
 --- a/drivers/media/IR/ir-jvc-decoder.c
 +++ b/drivers/media/IR/ir-jvc-decoder.c
 @@ -50,8 +50,9 @@ static int ir_jvc_decode(struct input_dev *input_dev, 
 struct ir_raw_event ev)
   if (!(ir_dev-raw-enabled_protocols  IR_TYPE_JVC))
   return 0;
  
 - if (IS_RESET(ev)) {
 - data-state = STATE_INACTIVE;
 + if (!is_timing_event(ev)) {
 + if (ev.reset)
 + data-state = STATE_INACTIVE;
   return 0;
   }
  
 diff --git a/drivers/media/IR/ir-lirc-codec.c 
 b/drivers/media/IR/ir-lirc-codec.c
 index e63f757..15112db 100644
 --- a/drivers/media/IR/ir-lirc-codec.c
 +++ b/drivers/media/IR/ir-lirc-codec.c
 @@ -32,6 +32,7 @@
  static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event 
 ev)
  {
   struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 + struct lirc_codec *lirc = ir_dev-raw-lirc;
   int sample;
  
   if (!(ir_dev-raw-enabled_protocols  IR_TYPE_LIRC))
 @@ -40,21 +41,57 @@ 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))
 + /* Packet start */
 + if (ev.reset)
   return 0;
  
 - IR_dprintk(2, LIRC data transfer started (%uus %s)\n,
 -TO_US(ev.duration), TO_STR(ev.pulse));
 + /* Carrier reports */
 + if (ev.carrier_report) {
 + sample = LIRC_FREQUENCY(ev.carrier);
 +
 + /* Packet end */
 + } else if (ev.timeout) {
 +
 + if (lirc-timeout)
 + return 0;
 +
 + lirc-timeout_start = ktime_get();
 + lirc-timeout = true;
 +
 + if (!lirc-send_timeout_reports)
 + return 0;
 +
 + sample = LIRC_TIMEOUT(ev.duration / 1000);
  
 - sample = ev.duration / 1000;
 - if (ev.pulse)
 - sample |= PULSE_BIT;
 + /* Normal sample */
 + } else {
 +
 + if

Re: [PATCH 7/8] IR: extend ir_raw_event and do refactoring

2010-09-05 Thread Maxim Levitsky 
On Sun, 2010-09-05 at 23:23 +0200, David Härdeman wrote: 
 Comments inline...
 
 On Sun, Sep 05, 2010 at 02:23:02AM +0300, Maxim Levitsky wrote:
  Add new event types for timeout  carrier report
  Move timeout handling from ir_raw_event_store_with_filter to
  ir-lirc-codec, where it is really needed.
  Now lirc bridge ensures proper gap handling.
  Extend lirc bridge for carrier  timeout reports
  
  Note: all new ir_raw_event variables now should be initialized
  like that: DEFINE_RC_RAW_EVENT(ev);
  
  To clean an existing event, use init_ir_raw_event(ev);
  
  Signed-off-by: Maxim Levitsky maximlevit...@gmail.com
  ---
   drivers/media/IR/ene_ir.c  |2 +-
   drivers/media/IR/ir-core-priv.h|   11 +-
   drivers/media/IR/ir-jvc-decoder.c  |5 +-
   drivers/media/IR/ir-lirc-codec.c   |   78 
  +++-
   drivers/media/IR/ir-nec-decoder.c  |5 +-
   drivers/media/IR/ir-raw-event.c|   43 +++-
   drivers/media/IR/ir-rc5-decoder.c  |5 +-
   drivers/media/IR/ir-rc6-decoder.c  |5 +-
   drivers/media/IR/ir-sony-decoder.c |5 +-
   drivers/media/IR/mceusb.c  |3 +-
   drivers/media/IR/streamzap.c   |8 ++-
   include/media/ir-core.h|   35 ++--
   12 files changed, 146 insertions(+), 59 deletions(-)
  
  diff --git a/drivers/media/IR/ene_ir.c b/drivers/media/IR/ene_ir.c
  index 7880d9c..5ebafb5 100644
  --- a/drivers/media/IR/ene_ir.c
  +++ b/drivers/media/IR/ene_ir.c
  @@ -701,7 +701,7 @@ static irqreturn_t ene_isr(int irq, void *data)
  unsigned long flags;
  irqreturn_t retval = IRQ_NONE;
  struct ene_device *dev = (struct ene_device *)data;
  -   struct ir_raw_event ev;
  +   DEFINE_RC_RAW_EVENT(ev);
   
  spin_lock_irqsave(dev-hw_lock, flags);
   
  diff --git a/drivers/media/IR/ir-core-priv.h 
  b/drivers/media/IR/ir-core-priv.h
  index 5d7e08f..a287373 100644
  --- a/drivers/media/IR/ir-core-priv.h
  +++ b/drivers/media/IR/ir-core-priv.h
  @@ -82,6 +82,10 @@ struct ir_raw_event_ctrl {
  struct ir_input_dev *ir_dev;
  struct lirc_driver *drv;
  int carrier_low;
  +   ktime_t timeout_start;
  +   bool timeout;
  +   bool send_timeout_reports;
  +
  } lirc;
   };
   
  @@ -109,9 +113,14 @@ static inline void decrease_duration(struct 
  ir_raw_event *ev, unsigned duration)
  ev-duration -= duration;
   }
   
  +/* Returns true if event is normal pulse/space event */
  +static inline bool is_timing_event(struct ir_raw_event ev)
  +{
  +   return !ev.carrier_report  !ev.reset;
  +}
  +
   #define TO_US(duration)DIV_ROUND_CLOSEST((duration), 
  1000)
   #define TO_STR(is_pulse)   ((is_pulse) ? pulse : space)
  -#define IS_RESET(ev)   (ev.duration == 0)
   /*
* Routines from ir-sysfs.c - Meant to be called only internally inside
* ir-core
  diff --git a/drivers/media/IR/ir-jvc-decoder.c 
  b/drivers/media/IR/ir-jvc-decoder.c
  index 77a89c4..63dca6e 100644
  --- a/drivers/media/IR/ir-jvc-decoder.c
  +++ b/drivers/media/IR/ir-jvc-decoder.c
  @@ -50,8 +50,9 @@ static int ir_jvc_decode(struct input_dev *input_dev, 
  struct ir_raw_event ev)
  if (!(ir_dev-raw-enabled_protocols  IR_TYPE_JVC))
  return 0;
   
  -   if (IS_RESET(ev)) {
  -   data-state = STATE_INACTIVE;
  +   if (!is_timing_event(ev)) {
  +   if (ev.reset)
  +   data-state = STATE_INACTIVE;
  return 0;
  }
   
  diff --git a/drivers/media/IR/ir-lirc-codec.c 
  b/drivers/media/IR/ir-lirc-codec.c
  index e63f757..15112db 100644
  --- a/drivers/media/IR/ir-lirc-codec.c
  +++ b/drivers/media/IR/ir-lirc-codec.c
  @@ -32,6 +32,7 @@
   static int ir_lirc_decode(struct input_dev *input_dev, struct ir_raw_event 
  ev)
   {
  struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
  +   struct lirc_codec *lirc = ir_dev-raw-lirc;
  int sample;
   
  if (!(ir_dev-raw-enabled_protocols  IR_TYPE_LIRC))
  @@ -40,21 +41,57 @@ 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))
  +   /* Packet start */
  +   if (ev.reset)
  return 0;
   
  -   IR_dprintk(2, LIRC data transfer started (%uus %s)\n,
  -  TO_US(ev.duration), TO_STR(ev.pulse));
  +   /* Carrier reports */
  +   if (ev.carrier_report) {
  +   sample = LIRC_FREQUENCY(ev.carrier);
  +
  +   /* Packet end */
  +   } else if (ev.timeout) {
  +
  +   if (lirc-timeout)
  +   return 0;
  +
  +   lirc-timeout_start = ktime_get();
  +   lirc-timeout = true;
  +
  +   if (!lirc-send_timeout_reports)
  +   return 0;
  +
  +   sample = LIRC_TIMEOUT(ev.duration / 1000);
   
  -   sample = ev.duration / 1000;
  -   if (ev.pulse)