Commit Message
* lirc: Don't propagate reset event to userspace
* lirc: Remove strange logic from lirc that would make first sample always be pulse
* Make TO_US macro actualy print what it should.
Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
drivers/media/IR/ir-core-priv.h | 3 +--
drivers/media/IR/ir-lirc-codec.c | 14 ++++++++------
drivers/media/IR/ir-raw-event.c | 3 +++
3 files changed, 12 insertions(+), 8 deletions(-)
Comments
Em 28-07-2010 12:14, Maxim Levitsky escreveu:
> * lirc: Don't propagate reset event to userspace
> * lirc: Remove strange logic from lirc that would make first sample always be pulse
> * Make TO_US macro actualy print what it should.
>
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
> drivers/media/IR/ir-core-priv.h | 3 +--
> drivers/media/IR/ir-lirc-codec.c | 14 ++++++++------
> drivers/media/IR/ir-raw-event.c | 3 +++
> 3 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
> index babd520..8ce80e4 100644
> --- a/drivers/media/IR/ir-core-priv.h
> +++ b/drivers/media/IR/ir-core-priv.h
> @@ -76,7 +76,6 @@ struct ir_raw_event_ctrl {
> struct lirc_codec {
> struct ir_input_dev *ir_dev;
> struct lirc_driver *drv;
> - int lircdata;
> } lirc;
> };
>
> @@ -104,7 +103,7 @@ static inline void decrease_duration(struct ir_raw_event *ev, unsigned duration)
> ev->duration -= duration;
> }
>
> -#define TO_US(duration) (((duration) + 500) / 1000)
> +#define TO_US(duration) ((duration) / 1000)
It is better to keep rounding the duration to its closest value.
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2010-07-28 at 13:01 -0300, Mauro Carvalho Chehab wrote:
> Em 28-07-2010 12:14, Maxim Levitsky escreveu:
> > * lirc: Don't propagate reset event to userspace
> > * lirc: Remove strange logic from lirc that would make first sample always be pulse
> > * Make TO_US macro actualy print what it should.
> >
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> > ---
> > drivers/media/IR/ir-core-priv.h | 3 +--
> > drivers/media/IR/ir-lirc-codec.c | 14 ++++++++------
> > drivers/media/IR/ir-raw-event.c | 3 +++
> > 3 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
> > index babd520..8ce80e4 100644
> > --- a/drivers/media/IR/ir-core-priv.h
> > +++ b/drivers/media/IR/ir-core-priv.h
> > @@ -76,7 +76,6 @@ struct ir_raw_event_ctrl {
> > struct lirc_codec {
> > struct ir_input_dev *ir_dev;
> > struct lirc_driver *drv;
> > - int lircdata;
> > } lirc;
> > };
> >
> > @@ -104,7 +103,7 @@ static inline void decrease_duration(struct ir_raw_event *ev, unsigned duration)
> > ev->duration -= duration;
> > }
> >
> > -#define TO_US(duration) (((duration) + 500) / 1000)
> > +#define TO_US(duration) ((duration) / 1000)
>
> It is better to keep rounding the duration to its closest value.
But that breaks if duration is already at maximum. At that case,
you see usual signed int wrap from positive to negative, and abnormally
large space...
I didn't notice though that you do rounding here.
I replace that with something better.
Best regards,
Maxim Levitsky
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 28, 2010 at 06:14:04PM +0300, Maxim Levitsky wrote:
> * lirc: Don't propagate reset event to userspace
> * lirc: Remove strange logic from lirc that would make first sample always be pulse
> * Make TO_US macro actualy print what it should.
>
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
For the ir-lirc-codec-specific bits:
Acked-by: Jarod Wilson <jarod@redhat.com>
I'm inclined to pull them into my tree now, and the IR_dprintk and TO_US
portions can be handled separately.
Em 28-07-2010 13:38, Maxim Levitsky escreveu:
> On Wed, 2010-07-28 at 13:01 -0300, Mauro Carvalho Chehab wrote:
>> Em 28-07-2010 12:14, Maxim Levitsky escreveu:
>>> * lirc: Don't propagate reset event to userspace
>>> * lirc: Remove strange logic from lirc that would make first sample always be pulse
>>> * Make TO_US macro actualy print what it should.
>>>
>>> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
>>> ---
>>> drivers/media/IR/ir-core-priv.h | 3 +--
>>> drivers/media/IR/ir-lirc-codec.c | 14 ++++++++------
>>> drivers/media/IR/ir-raw-event.c | 3 +++
>>> 3 files changed, 12 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
>>> index babd520..8ce80e4 100644
>>> --- a/drivers/media/IR/ir-core-priv.h
>>> +++ b/drivers/media/IR/ir-core-priv.h
>>> @@ -76,7 +76,6 @@ struct ir_raw_event_ctrl {
>>> struct lirc_codec {
>>> struct ir_input_dev *ir_dev;
>>> struct lirc_driver *drv;
>>> - int lircdata;
>>> } lirc;
>>> };
>>>
>>> @@ -104,7 +103,7 @@ static inline void decrease_duration(struct ir_raw_event *ev, unsigned duration)
>>> ev->duration -= duration;
>>> }
>>>
>>> -#define TO_US(duration) (((duration) + 500) / 1000)
>>> +#define TO_US(duration) ((duration) / 1000)
>>
>> It is better to keep rounding the duration to its closest value.
>
> But that breaks if duration is already at maximum. At that case,
> you see usual signed int wrap from positive to negative, and abnormally
> large space...
>
> I didn't notice though that you do rounding here.
> I replace that with something better.
There's a function for rounding at kernel. The better is to use it, as it should already
solve the signal wrap.
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -76,7 +76,6 @@ struct ir_raw_event_ctrl {
struct lirc_codec {
struct ir_input_dev *ir_dev;
struct lirc_driver *drv;
- int lircdata;
} lirc;
};
@@ -104,7 +103,7 @@ static inline void decrease_duration(struct ir_raw_event *ev, unsigned duration)
ev->duration -= duration;
}
-#define TO_US(duration) (((duration) + 500) / 1000)
+#define TO_US(duration) ((duration) / 1000)
#define TO_STR(is_pulse) ((is_pulse) ? "pulse" : "space")
#define IS_RESET(ev) (ev.duration == 0)
@@ -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);
+ int sample;
if (!(ir_dev->raw->enabled_protocols & IR_TYPE_LIRC))
return 0;
@@ -39,18 +40,21 @@ 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;
+
IR_dprintk(2, "LIRC data transfer started (%uus %s)\n",
TO_US(ev.duration), TO_STR(ev.pulse));
- ir_dev->raw->lirc.lircdata += ev.duration / 1000;
+
+ sample = ev.duration / 1000;
if (ev.pulse)
- ir_dev->raw->lirc.lircdata |= PULSE_BIT;
+ sample |= PULSE_BIT;
lirc_buffer_write(ir_dev->raw->lirc.drv->rbuf,
- (unsigned char *) &ir_dev->raw->lirc.lircdata);
+ (unsigned char *) &sample);
wake_up(&ir_dev->raw->lirc.drv->rbuf->wait_poll);
- ir_dev->raw->lirc.lircdata = 0;
return 0;
}
@@ -224,8 +228,6 @@ static int ir_lirc_register(struct input_dev *input_dev)
ir_dev->raw->lirc.drv = drv;
ir_dev->raw->lirc.ir_dev = ir_dev;
- ir_dev->raw->lirc.lircdata = PULSE_MASK;
-
return 0;
lirc_register_failed:
@@ -66,6 +66,9 @@ int ir_raw_event_store(struct input_dev *input_dev, struct ir_raw_event *ev)
if (!ir->raw)
return -EINVAL;
+ IR_dprintk(2, "sample: (05%dus %s)\n", TO_US(ev->duration),
+ TO_STR(ev->pulse));
+
if (kfifo_in(&ir->raw->kfifo, ev, sizeof(*ev)) != sizeof(*ev))
return -ENOMEM;