[PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Maxim Levitsky]

Hi,

Here is full overview of my patches:

Patch #1 fixes races in ir thread.
It fixes the case when ktherad_stop waits forever for the thread.
This happens on module unload and therefore it never finishes.
Sorry for introducing this bug.

Patch #2, fixes a crash on my module load.
It happens because ir core initializes the input device a bit early,
therefore it could be accessed while still not set up.

Patch #3 fixes a small typo in lirc code that makes it impossible to use tx duty cycle setting.

Patch #4 fixes a problem seen on my system that results in stuck down forever key.

Patch #5 adds few keys to MCE keymap that were found on laptop of an user I tested this driver with

Patch #6, is a combined update ti my driver. It contains lot of refactoring thanks to docs I have now,
and lot of fixes, and supports latest version of firmware (and I have 4 users asking for that)
It is quite huge, but it would be a tedios job to break it up. This can't introduce regressions
because the ene_ir was never released. In addition to that it was tested by me and another two users.

Patch #7 the really only patch that touches drivers I don't have does touch the ir-core.
It is quite small, and it adds a proper solution to dilema about what to do with huge space between keypresses.
Now this space is just truncated by the driver with timeout flag.
The lirc codec then ensures that right sample is send to the lircd.
Please review and test it.

Patch #8 is very simple. It just builds on top of patch #7 and adds carrier reports to ene driver.


Best regards,
	Maxim Levitsky

[PATCH 1/8] IR: plug races in IR raw thread.

[Maxim Levitsky]

Unfortunelly (my fault) the kernel thread that now handles IR processing
has classical races in regard to wakeup and stop.
This patch hopefully closes them all.
Tested with module reload running in a loop, while receiver is blasted
with IR data for 10 minutes.

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/IR/ir-core-priv.h |    2 ++
 drivers/media/IR/ir-raw-event.c |   34 +++++++++++++++++++++++++---------
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
index a85a8c7..761e7f4 100644
--- a/drivers/media/IR/ir-core-priv.h
+++ b/drivers/media/IR/ir-core-priv.h
@@ -17,6 +17,7 @@
 #define _IR_RAW_EVENT
 
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <media/ir-core.h>
 
 struct ir_raw_handler {
@@ -33,6 +34,7 @@ struct ir_raw_handler {
 struct ir_raw_event_ctrl {
 	struct list_head		list;		/* to keep track of raw clients */
 	struct task_struct		*thread;
+	spinlock_t			lock;
 	struct kfifo			kfifo;		/* fifo for the pulse/space durations */
 	ktime_t				last_event;	/* when last event occurred */
 	enum raw_event_type		last_type;	/* last event type */
diff --git a/drivers/media/IR/ir-raw-event.c b/drivers/media/IR/ir-raw-event.c
index 43094e7..56797be 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -39,22 +39,34 @@ static int ir_raw_event_thread(void *data)
 	struct ir_raw_event ev;
 	struct ir_raw_handler *handler;
 	struct ir_raw_event_ctrl *raw = (struct ir_raw_event_ctrl *)data;
+	int retval;
 
 	while (!kthread_should_stop()) {
-		try_to_freeze();
 
-		mutex_lock(&ir_raw_handler_lock);
+		spin_lock_irq(&raw->lock);
+		retval = kfifo_out(&raw->kfifo, &ev, sizeof(ev));
+
+		if (!retval) {
+			set_current_state(TASK_INTERRUPTIBLE);
 
-		while (kfifo_out(&raw->kfifo, &ev, sizeof(ev)) == sizeof(ev)) {
-			list_for_each_entry(handler, &ir_raw_handler_list, list)
-				handler->decode(raw->input_dev, ev);
-			raw->prev_ev = ev;
+			if (kthread_should_stop())
+				set_current_state(TASK_RUNNING);
+
+			spin_unlock_irq(&raw->lock);
+			schedule();
+			continue;
 		}
 
-		mutex_unlock(&ir_raw_handler_lock);
+		spin_unlock_irq(&raw->lock);
 
-		set_current_state(TASK_INTERRUPTIBLE);
-		schedule();
+
+		BUG_ON(retval != sizeof(ev));
+
+		mutex_lock(&ir_raw_handler_lock);
+		list_for_each_entry(handler, &ir_raw_handler_list, list)
+			handler->decode(raw->input_dev, ev);
+		raw->prev_ev = ev;
+		mutex_unlock(&ir_raw_handler_lock);
 	}
 
 	return 0;
@@ -232,11 +244,14 @@ EXPORT_SYMBOL_GPL(ir_raw_event_set_idle);
 void ir_raw_event_handle(struct input_dev *input_dev)
 {
 	struct ir_input_dev *ir = input_get_drvdata(input_dev);
+	unsigned long flags;
 
 	if (!ir->raw)
 		return;
 
+	spin_lock_irqsave(&ir->raw->lock, flags);
 	wake_up_process(ir->raw->thread);
+	spin_unlock_irqrestore(&ir->raw->lock, flags);
 }
 EXPORT_SYMBOL_GPL(ir_raw_event_handle);
 
@@ -275,6 +290,7 @@ int ir_raw_event_register(struct input_dev *input_dev)
 		return rc;
 	}
 
+	spin_lock_init(&ir->raw->lock);
 	ir->raw->thread = kthread_run(ir_raw_event_thread, ir->raw,
 			"rc%u",  (unsigned int)ir->devno);
 
-- 
1.7.0.4

Re: [PATCH 1/8] IR: plug races in IR raw thread.

[Jarod Wilson]

On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> Unfortunelly (my fault) the kernel thread that now handles IR processing
> has classical races in regard to wakeup and stop.
> This patch hopefully closes them all.
> Tested with module reload running in a loop, while receiver is blasted
> with IR data for 10 minutes.
>
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

Took a while to unwind everything in ir_raw_event_thread() in my head,
but now that I think I have it sorted out, yeah, that looks like the
processing logic all remains the same, with the addition of locking
that should prevent the race (also heavily supported by your testing).

Acked-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [1/8] IR: plug races in IR raw thread.

[Jarod Wilson]

On Mon, Sep 06, 2010 at 09:26:06PM -0000, Maxim Levitsky wrote:
> Unfortunelly (my fault) the kernel thread that now handles IR processing
> has classical races in regard to wakeup and stop.
> This patch hopefully closes them all.
> Tested with module reload running in a loop, while receiver is blasted
> with IR data for 10 minutes.
> 
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

Carrying over ack from earlier(?) version of the patch:

Took a while to unwind everything in ir_raw_event_thread() in my head,
but now that I think I have it sorted out, yeah, that looks like the
processing logic all remains the same, with the addition of locking
that should prevent the race (also heavily supported by your testing).

Acked-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@redhat.com

[PATCH 2/8] IR: make sure we register the input device when it is safe to do so.

[Maxim Levitsky]

As soon as input device is registered, it might be accessed (and it is)
This can trigger a hardware interrupt that can access
not yet initialized ir->raw, (by sending a sample)

This can be reproduced by holding down a remote button and reloading the module.
And this always crashes the systems where hardware decides to send an interrupt
right at the moment it is enabled.

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/IR/ir-core-priv.h |    1 +
 drivers/media/IR/ir-keytable.c  |    2 ++
 drivers/media/IR/ir-sysfs.c     |   27 +++++++++++++++++----------
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/media/IR/ir-core-priv.h b/drivers/media/IR/ir-core-priv.h
index 761e7f4..5d7e08f 100644
--- a/drivers/media/IR/ir-core-priv.h
+++ b/drivers/media/IR/ir-core-priv.h
@@ -116,6 +116,7 @@ static inline void decrease_duration(struct ir_raw_event *ev, unsigned duration)
  * Routines from ir-sysfs.c - Meant to be called only internally inside
  * ir-core
  */
+int ir_register_input(struct input_dev *input_dev);
 
 int ir_register_class(struct input_dev *input_dev);
 void ir_unregister_class(struct input_dev *input_dev);
diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c
index 51dbc67..0917535 100644
--- a/drivers/media/IR/ir-keytable.c
+++ b/drivers/media/IR/ir-keytable.c
@@ -505,6 +505,8 @@ int __ir_input_register(struct input_dev *input_dev,
 				goto out_event;
 		}
 
+	rc = ir_register_input(input_dev);
+
 	IR_dprintk(1, "Registered input device on %s for %s remote%s.\n",
 		   driver_name, rc_tab->name,
 		   (ir_dev->props && ir_dev->props->driver_type == RC_DRIVER_IR_RAW) ?
diff --git a/drivers/media/IR/ir-sysfs.c b/drivers/media/IR/ir-sysfs.c
index 96dafc4..c0075f1 100644
--- a/drivers/media/IR/ir-sysfs.c
+++ b/drivers/media/IR/ir-sysfs.c
@@ -251,8 +251,6 @@ static struct device_type rc_dev_type = {
  */
 int ir_register_class(struct input_dev *input_dev)
 {
-	int rc;
-	const char *path;
 	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
 	int devno = find_first_zero_bit(&ir_core_dev_number,
 					IRRCV_NUM_DEVICES);
@@ -261,17 +259,28 @@ int ir_register_class(struct input_dev *input_dev)
 		return devno;
 
 	ir_dev->dev.type = &rc_dev_type;
+	ir_dev->devno = devno;
 
 	ir_dev->dev.class = &ir_input_class;
 	ir_dev->dev.parent = input_dev->dev.parent;
+	input_dev->dev.parent = &ir_dev->dev;
 	dev_set_name(&ir_dev->dev, "rc%d", devno);
 	dev_set_drvdata(&ir_dev->dev, ir_dev);
-	rc = device_register(&ir_dev->dev);
-	if (rc)
-		return rc;
+	return  device_register(&ir_dev->dev);
+};
+
+/**
+ * ir_register_input - registers ir input device with input subsystem
+ * @input_dev:	the struct input_dev descriptor of the device
+ */
+
+int ir_register_input(struct input_dev *input_dev)
+{
+	struct ir_input_dev *ir_dev = input_get_drvdata(input_dev);
+	int rc;
+	const char *path;
 
 
-	input_dev->dev.parent = &ir_dev->dev;
 	rc = input_register_device(input_dev);
 	if (rc < 0) {
 		device_del(&ir_dev->dev);
@@ -287,11 +296,9 @@ int ir_register_class(struct input_dev *input_dev)
 		path ? path : "N/A");
 	kfree(path);
 
-	ir_dev->devno = devno;
-	set_bit(devno, &ir_core_dev_number);
-
+	set_bit(ir_dev->devno, &ir_core_dev_number);
 	return 0;
-};
+}
 
 /**
  * ir_unregister_class() - removes the sysfs for sysfs for
-- 
1.7.0.4

Re: [PATCH 2/8] IR: make sure we register the input device when it is safe to do so.

[Jarod Wilson]

On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> As soon as input device is registered, it might be accessed (and it is)
> This can trigger a hardware interrupt that can access
> not yet initialized ir->raw, (by sending a sample)
>
> This can be reproduced by holding down a remote button and reloading the module.
> And this always crashes the systems where hardware decides to send an interrupt
> right at the moment it is enabled.
>
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

Finally got my head wrapped around this one too, and I do see the
problem, and this fix looks good to me.

Acked-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [2/8] IR: make sure we register the input device when it is safe to do so.

[Jarod Wilson]

On Mon, Sep 06, 2010 at 09:26:07PM -0000, Maxim Levitsky wrote:
> As soon as input device is registered, it might be accessed (and it is)
> This can trigger a hardware interrupt that can access
> not yet initialized ir->raw, (by sending a sample)
> 
> This can be reproduced by holding down a remote button and reloading the module.
> And this always crashes the systems where hardware decides to send an interrupt
> right at the moment it is enabled.
> 
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

Another one I thought I'd acked, but I don't see the ack in patchwork, so
it may have been from an earlier/superseded version...

Finally got my head wrapped around this one too, and I do see the
problem, and this fix looks good to me.

Acked-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@redhat.com

[PATCH 3/8] IR: fix duty cycle capability

[Maxim Levitsky]

Due to typo lirc bridge enabled wrong capability.

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/IR/ir-lirc-codec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/IR/ir-lirc-codec.c b/drivers/media/IR/ir-lirc-codec.c
index 77b5946..e63f757 100644
--- a/drivers/media/IR/ir-lirc-codec.c
+++ b/drivers/media/IR/ir-lirc-codec.c
@@ -267,7 +267,7 @@ static int ir_lirc_register(struct input_dev *input_dev)
 			features |= LIRC_CAN_SET_SEND_CARRIER;
 
 		if (ir_dev->props->s_tx_duty_cycle)
-			features |= LIRC_CAN_SET_REC_DUTY_CYCLE;
+			features |= LIRC_CAN_SET_SEND_DUTY_CYCLE;
 	}
 
 	if (ir_dev->props->s_rx_carrier_range)
-- 
1.7.0.4

Re: [PATCH 3/8] IR: fix duty cycle capability

[Jarod Wilson]

On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> Due to typo lirc bridge enabled wrong capability.
>
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
>  drivers/media/IR/ir-lirc-codec.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Acked-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@wilsonet.com

[PATCH 4/8] IR: fix keys beeing stuck down forever.

[Maxim Levitsky]

The logic in ir_timer_keyup was inverted.

In case that values aren't equal,
the meaning of the time_is_after_eq_jiffies(ir->keyup_jiffies) is that
ir->keyup_jiffies is after the the jiffies or equally that
that jiffies are before the the ir->keyup_jiffies which is
exactly the situation we want to avoid (that the timeout is in the future)
Confusing Eh?

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/IR/ir-keytable.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/IR/ir-keytable.c b/drivers/media/IR/ir-keytable.c
index 0917535..a8fd777 100644
--- a/drivers/media/IR/ir-keytable.c
+++ b/drivers/media/IR/ir-keytable.c
@@ -319,7 +319,7 @@ static void ir_timer_keyup(unsigned long cookie)
 	 * a keyup event might follow immediately after the keydown.
 	 */
 	spin_lock_irqsave(&ir->keylock, flags);
-	if (time_is_after_eq_jiffies(ir->keyup_jiffies))
+	if (time_is_before_eq_jiffies(ir->keyup_jiffies))
 		ir_keyup(ir);
 	spin_unlock_irqrestore(&ir->keylock, flags);
 }
-- 
1.7.0.4

Re: [PATCH 4/8] IR: fix keys beeing stuck down forever.

[Jarod Wilson]

On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> The logic in ir_timer_keyup was inverted.
>
> In case that values aren't equal,
> the meaning of the time_is_after_eq_jiffies(ir->keyup_jiffies) is that
> ir->keyup_jiffies is after the the jiffies or equally that
> that jiffies are before the the ir->keyup_jiffies which is
> exactly the situation we want to avoid (that the timeout is in the future)
> Confusing Eh?

Yeah, seen time_is_{before,after}_jiffies use accidentally inverted a
couple of times... Kinda hints that we could use better names and/or
descriptions of the functions, but maybe people just need to read more
carefully (dunno, haven't looked to see what's there for usage
descriptions already)... Anyway.

> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
>  drivers/media/IR/ir-keytable.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Acked-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@wilsonet.com

[PATCH 5/8] IR: extend MCE keymap.

[Maxim Levitsky]

These keys are found on remote bundled with
Toshiba Qosmio F50-10q.

Found and tested by, Sami R <maesesami@gmail.com>

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/IR/keymaps/rc-rc6-mce.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/media/IR/keymaps/rc-rc6-mce.c b/drivers/media/IR/keymaps/rc-rc6-mce.c
index 64264f7..39557ad 100644
--- a/drivers/media/IR/keymaps/rc-rc6-mce.c
+++ b/drivers/media/IR/keymaps/rc-rc6-mce.c
@@ -19,6 +19,7 @@ static struct ir_scancode rc6_mce[] = {
 
 	{ 0x800f0416, KEY_PLAY },
 	{ 0x800f0418, KEY_PAUSE },
+	{ 0x800f046e, KEY_PLAYPAUSE },
 	{ 0x800f0419, KEY_STOP },
 	{ 0x800f0417, KEY_RECORD },
 
@@ -37,6 +38,8 @@ static struct ir_scancode rc6_mce[] = {
 	{ 0x800f0411, KEY_VOLUMEDOWN },
 	{ 0x800f0412, KEY_CHANNELUP },
 	{ 0x800f0413, KEY_CHANNELDOWN },
+	{ 0x800f043a, KEY_BRIGHTNESSUP },
+	{ 0x800f0480, KEY_BRIGHTNESSDOWN },
 
 	{ 0x800f0401, KEY_NUMERIC_1 },
 	{ 0x800f0402, KEY_NUMERIC_2 },
-- 
1.7.0.4

Re: [PATCH 5/8] IR: extend MCE keymap.

[Jarod Wilson]

On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> These keys are found on remote bundled with
> Toshiba Qosmio F50-10q.
>
> Found and tested by, Sami R <maesesami@gmail.com>
>
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
>  drivers/media/IR/keymaps/rc-rc6-mce.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Acked-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [PATCH 5/8] IR: extend MCE keymap.

[Jarod Wilson]

On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> These keys are found on remote bundled with
> Toshiba Qosmio F50-10q.
>
> Found and tested by, Sami R <maesesami@gmail.com>
>
> Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> ---
>  drivers/media/IR/keymaps/rc-rc6-mce.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Acked-by: Jarod Wilson <jarod@redhat.com>

-- 
Jarod Wilson
jarod@wilsonet.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] IR: extend MCE keymap.

[Maxim Levitsky]

On Wed, 2010-09-08 at 10:47 -0400, Jarod Wilson wrote: 
> On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > These keys are found on remote bundled with
> > Toshiba Qosmio F50-10q.
> >
> > Found and tested by, Sami R <maesesami@gmail.com>
> >
> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> > ---
> >  drivers/media/IR/keymaps/rc-rc6-mce.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)

Tommorow I will resend that patch with even more scancodes.

Best regards,
Maxim Levitsky

Re: [PATCH 5/8] IR: extend MCE keymap.

[Jarod Wilson]

On Thu, Sep 9, 2010 at 8:40 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> On Wed, 2010-09-08 at 10:47 -0400, Jarod Wilson wrote:
>> On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
>> > These keys are found on remote bundled with
>> > Toshiba Qosmio F50-10q.
>> >
>> > Found and tested by, Sami R <maesesami@gmail.com>
>> >
>> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
>> > ---
>> >  drivers/media/IR/keymaps/rc-rc6-mce.c |    3 +++
>> >  1 files changed, 3 insertions(+), 0 deletions(-)
>
> Tommorow I will resend that patch with even more scancodes.

Saw the discussion on irc. Feel your pain big-time on the X server
limitation on keycodes. Its put a big damper on efforts to add native
support to mythtv. Peter Hutterer's libXi2 cookbook tutorials talk a
good game about how libXi2 supports 32-bit keycodes, but neglects to
mention that the X server still gobbles up anything above 248 or 255
or whatever it is, and remedying that is no small task. :(

I think for mythtv, we're going to end up having a daemon process with
elevated privs that reads directly from input devices to get around
this annoyance, until such time as the annoyance is gone.

-- 
Jarod Wilson
jarod@wilsonet.com

Re: [PATCH 5/8] IR: extend MCE keymap.

[Jarod Wilson]

On Thu, Sep 9, 2010 at 8:40 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> On Wed, 2010-09-08 at 10:47 -0400, Jarod Wilson wrote:
>> On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
>> > These keys are found on remote bundled with
>> > Toshiba Qosmio F50-10q.
>> >
>> > Found and tested by, Sami R <maesesami@gmail.com>
>> >
>> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
>> > ---
>> >  drivers/media/IR/keymaps/rc-rc6-mce.c |    3 +++
>> >  1 files changed, 3 insertions(+), 0 deletions(-)
>
> Tommorow I will resend that patch with even more scancodes.

Saw the discussion on irc. Feel your pain big-time on the X server
limitation on keycodes. Its put a big damper on efforts to add native
support to mythtv. Peter Hutterer's libXi2 cookbook tutorials talk a
good game about how libXi2 supports 32-bit keycodes, but neglects to
mention that the X server still gobbles up anything above 248 or 255
or whatever it is, and remedying that is no small task. :(

I think for mythtv, we're going to end up having a daemon process with
elevated privs that reads directly from input devices to get around
this annoyance, until such time as the annoyance is gone.

-- 
Jarod Wilson
jarod@wilsonet.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] IR: extend MCE keymap.

[David Härdeman]

On Fri, September 10, 2010 03:37, Jarod Wilson wrote:
> I think for mythtv, we're going to end up having a daemon process with
> elevated privs that reads directly from input devices to get around
> this annoyance, until such time as the annoyance is gone.

A similar approach could work for XBMC since it already has input plugins
(e.g. for using a Nintendo Wii controller, etc).

-- 
David Härdeman

Re: [PATCH 5/8] IR: extend MCE keymap.

[Jarod Wilson]

On Fri, Sep 10, 2010 at 10:27:02AM +0200, David Härdeman wrote:
> On Fri, September 10, 2010 03:37, Jarod Wilson wrote:
> > I think for mythtv, we're going to end up having a daemon process with
> > elevated privs that reads directly from input devices to get around
> > this annoyance, until such time as the annoyance is gone.
> 
> A similar approach could work for XBMC since it already has input plugins
> (e.g. for using a Nintendo Wii controller, etc).

While attempting to drift off to sleep last night, it occurred to me that
lircd already has support for attaching to input devices, so it might be
an option to extend lircd and the lirc client library to have a "pass raw
keycodes" mode. Both xmbc and mythtv already have options to build in lirc
client support, so it'd just be minor extension of its use in both cases,
at least in theory...

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH 5/8] IR: extend MCE keymap.

[Jarod Wilson]

On Fri, Sep 10, 2010 at 10:27:02AM +0200, David Härdeman wrote:
> On Fri, September 10, 2010 03:37, Jarod Wilson wrote:
> > I think for mythtv, we're going to end up having a daemon process with
> > elevated privs that reads directly from input devices to get around
> > this annoyance, until such time as the annoyance is gone.
> 
> A similar approach could work for XBMC since it already has input plugins
> (e.g. for using a Nintendo Wii controller, etc).

While attempting to drift off to sleep last night, it occurred to me that
lircd already has support for attaching to input devices, so it might be
an option to extend lircd and the lirc client library to have a "pass raw
keycodes" mode. Both xmbc and mythtv already have options to build in lirc
client support, so it'd just be minor extension of its use in both cases,
at least in theory...

-- 
Jarod Wilson
jarod@redhat.com

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/8] IR: extend MCE keymap.

[Maxim Levitsky]

On Thu, 2010-09-09 at 21:37 -0400, Jarod Wilson wrote: 
> On Thu, Sep 9, 2010 at 8:40 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > On Wed, 2010-09-08 at 10:47 -0400, Jarod Wilson wrote:
> >> On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> >> > These keys are found on remote bundled with
> >> > Toshiba Qosmio F50-10q.
> >> >
> >> > Found and tested by, Sami R <maesesami@gmail.com>
> >> >
> >> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> >> > ---
> >> >  drivers/media/IR/keymaps/rc-rc6-mce.c |    3 +++
> >> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > Tommorow I will resend that patch with even more scancodes.
> 
> Saw the discussion on irc. Feel your pain big-time on the X server
> limitation on keycodes. Its put a big damper on efforts to add native
> support to mythtv. Peter Hutterer's libXi2 cookbook tutorials talk a
> good game about how libXi2 supports 32-bit keycodes, but neglects to
> mention that the X server still gobbles up anything above 248 or 255
> or whatever it is, and remedying that is no small task. :(
> 
> I think for mythtv, we're going to end up having a daemon process with
> elevated privs that reads directly from input devices to get around
> this annoyance, until such time as the annoyance is gone.


Btw, indeed Xi2 still doesn't pass > 248 keycodes, just tested that with
-git versions of X stack from about 2 months ago.
However this can be fixed relatively easily.
Maybe even I could do that.

The big problem is however about ability to map extended keycodes to
actions, thing that should be provided by XKB2, which we will see
probably when DNF is released on Phantom console...
Also this will need lots of changes in toolkits.
Thats the problem I don't have resources to fix.

Best regards,
Maxim Levitsky

Re: [PATCH 5/8] IR: extend MCE keymap.

[Jarod Wilson]

On Fri, Sep 10, 2010 at 11:40:31AM +0300, Maxim Levitsky wrote:
> On Thu, 2010-09-09 at 21:37 -0400, Jarod Wilson wrote: 
> > On Thu, Sep 9, 2010 at 8:40 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > > On Wed, 2010-09-08 at 10:47 -0400, Jarod Wilson wrote:
> > >> On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> > >> > These keys are found on remote bundled with
> > >> > Toshiba Qosmio F50-10q.
> > >> >
> > >> > Found and tested by, Sami R <maesesami@gmail.com>
> > >> >
> > >> > Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
> > >> > ---
> > >> >  drivers/media/IR/keymaps/rc-rc6-mce.c |    3 +++
> > >> >  1 files changed, 3 insertions(+), 0 deletions(-)
> > >
> > > Tommorow I will resend that patch with even more scancodes.
> > 
> > Saw the discussion on irc. Feel your pain big-time on the X server
> > limitation on keycodes. Its put a big damper on efforts to add native
> > support to mythtv. Peter Hutterer's libXi2 cookbook tutorials talk a
> > good game about how libXi2 supports 32-bit keycodes, but neglects to
> > mention that the X server still gobbles up anything above 248 or 255
> > or whatever it is, and remedying that is no small task. :(
> > 
> > I think for mythtv, we're going to end up having a daemon process with
> > elevated privs that reads directly from input devices to get around
> > this annoyance, until such time as the annoyance is gone.
> 
> Btw, indeed Xi2 still doesn't pass > 248 keycodes, just tested that with
> -git versions of X stack from about 2 months ago.
> However this can be fixed relatively easily.
> Maybe even I could do that.
> 
> The big problem is however about ability to map extended keycodes to
> actions, thing that should be provided by XKB2, which we will see
> probably when DNF is released on Phantom console...
> Also this will need lots of changes in toolkits.
> Thats the problem I don't have resources to fix.

Yeah, that's exactly what I got from Peter when I was asking him about
this issue on irc -- X server can be modified fairly easily, but there's
the arduous and undesirable task of making all the toolkits and whatnot
behave again, and nobody in their right mind really *wants* to dig into
xkb. :)

-- 
Jarod Wilson
jarod@redhat.com

[PATCH 6/8] IR: ene_ir: updates

[Maxim Levitsky]

* Add support for newer firmware version that uses different
buffer format. Makes hardware work for many users.

* Register name updates

* Lot of refactoring

* Lots of fixes as a result of full testing

* Idle mode is done now by resetting the device, and this eliminates
the ugly sample_period = 75 hack.

Every feature of the driver is now well tested.

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/IR/Kconfig  |    6 +-
 drivers/media/IR/ene_ir.c |  978 ++++++++++++++++++++++++++-------------------
 drivers/media/IR/ene_ir.h |  273 +++++++------
 3 files changed, 719 insertions(+), 538 deletions(-)

diff --git a/drivers/media/IR/Kconfig b/drivers/media/IR/Kconfig
index 490c57c..e9bfdca 100644
--- a/drivers/media/IR/Kconfig
+++ b/drivers/media/IR/Kconfig
@@ -114,15 +114,15 @@ config IR_MCEUSB
 	   module will be called mceusb.
 
 config IR_ENE
-	tristate "ENE eHome Receiver/Transciever (pnp id: ENE0100/ENE02xxx)"
+	tristate "ENE eHome Receiver/Transceiver (pnp id: ENE0100/ENE02xxx)"
 	depends on PNP
 	depends on IR_CORE
 	---help---
 	   Say Y here to enable support for integrated infrared receiver
-	   /transciever made by ENE.
+	   /transceiver made by ENE.
 
 	   You can see if you have it by looking at lspnp output.
-	   Output should include ENE0100 ENE0200 or something similiar.
+	   Output should include ENE0100 ENE0200 or something similar.
 
 	   To compile this driver as a module, choose M here: the
 	   module will be called ene_ir.
diff --git a/drivers/media/IR/ene_ir.c b/drivers/media/IR/ene_ir.c
index 5447750..7880d9c 100644
--- a/drivers/media/IR/ene_ir.c
+++ b/drivers/media/IR/ene_ir.c
@@ -1,5 +1,5 @@
 /*
- * driver for ENE KB3926 B/C/D CIR (pnp id: ENE0XXX)
+ * driver for ENE KB3926 B/C/D/E/F CIR (pnp id: ENE0XXX)
  *
  * Copyright (C) 2010 Maxim Levitsky <maximlevitsky@gmail.com>
  *
@@ -17,6 +17,17 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
  * USA
+ *
+ * Special thanks to:
+ *   Sami R. <maesesami@gmail.com> for lot of help in debugging and therefore
+ *    bringing to life support for transmission & learning mode.
+ *
+ *   Charlie Andrews <charliethepilot@googlemail.com> for lots of help in
+ *   bringing up the support of new firmware buffer that is popular
+ *   on latest notebooks
+ *
+ *   ENE for partial device documentation
+ *
  */
 
 #include <linux/kernel.h>
@@ -31,51 +42,59 @@
 #include <media/ir-common.h>
 #include "ene_ir.h"
 
-
-static int sample_period = -1;
-static int enable_idle = 1;
-static int input = 1;
+static int sample_period;
+static bool learning_mode;
 static int debug;
-static int txsim;
+static bool txsim;
 
-static int ene_irq_status(struct ene_device *dev);
+static void ene_set_reg_addr(struct ene_device *dev, u16 reg)
+{
+	outb(reg >> 8, dev->hw_io + ENE_ADDR_HI);
+	outb(reg & 0xFF, dev->hw_io + ENE_ADDR_LO);
+}
 
 /* read a hardware register */
-static u8 ene_hw_read_reg(struct ene_device *dev, u16 reg)
+static u8 ene_read_reg(struct ene_device *dev, u16 reg)
 {
 	u8 retval;
-	outb(reg >> 8, dev->hw_io + ENE_ADDR_HI);
-	outb(reg & 0xFF, dev->hw_io + ENE_ADDR_LO);
+	ene_set_reg_addr(dev, reg);
 	retval = inb(dev->hw_io + ENE_IO);
-
-	ene_dbg_verbose("reg %04x == %02x", reg, retval);
+	dbg_regs("reg %04x == %02x", reg, retval);
 	return retval;
 }
 
 /* write a hardware register */
-static void ene_hw_write_reg(struct ene_device *dev, u16 reg, u8 value)
+static void ene_write_reg(struct ene_device *dev, u16 reg, u8 value)
 {
-	outb(reg >> 8, dev->hw_io + ENE_ADDR_HI);
-	outb(reg & 0xFF, dev->hw_io + ENE_ADDR_LO);
+	dbg_regs("reg %04x <- %02x", reg, value);
+	ene_set_reg_addr(dev, reg);
 	outb(value, dev->hw_io + ENE_IO);
-
-	ene_dbg_verbose("reg %04x <- %02x", reg, value);
 }
 
-/* change specific bits in hardware register */
-static void ene_hw_write_reg_mask(struct ene_device *dev,
-				  u16 reg, u8 value, u8 mask)
+/* Set bits in hardware register */
+static void ene_set_reg_mask(struct ene_device *dev, u16 reg, u8 mask)
 {
-	u8 regvalue;
-
-	outb(reg >> 8, dev->hw_io + ENE_ADDR_HI);
-	outb(reg & 0xFF, dev->hw_io + ENE_ADDR_LO);
+	dbg_regs("reg %04x |= %02x", reg, mask);
+	ene_set_reg_addr(dev, reg);
+	outb(inb(dev->hw_io + ENE_IO) | mask, dev->hw_io + ENE_IO);
+}
 
-	regvalue = inb(dev->hw_io + ENE_IO) & ~mask;
-	regvalue |= (value & mask);
-	outb(regvalue, dev->hw_io + ENE_IO);
+/* Clear bits in hardware register */
+static void ene_clear_reg_mask(struct ene_device *dev, u16 reg, u8 mask)
+{
+	dbg_regs("reg %04x &= ~%02x ", reg, mask);
+	ene_set_reg_addr(dev, reg);
+	outb(inb(dev->hw_io + ENE_IO) & ~mask, dev->hw_io + ENE_IO);
+}
 
-	ene_dbg_verbose("reg %04x <- %02x (mask=%02x)", reg, value, mask);
+/* A helper to set/clear a bit in register according to boolean variable */
+static void ene_set_clear_reg_mask(struct ene_device *dev, u16 reg, u8 mask,
+								bool set)
+{
+	if (set)
+		ene_set_reg_mask(dev, reg, mask);
+	else
+		ene_clear_reg_mask(dev, reg, mask);
 }
 
 /* detect hardware features */
@@ -83,339 +102,369 @@ static int ene_hw_detect(struct ene_device *dev)
 {
 	u8 chip_major, chip_minor;
 	u8 hw_revision, old_ver;
-	u8 tmp;
-	u8 fw_capabilities;
-	int pll_freq;
-
-	tmp = ene_hw_read_reg(dev, ENE_HW_UNK);
-	ene_hw_write_reg(dev, ENE_HW_UNK, tmp & ~ENE_HW_UNK_CLR);
+	u8 fw_reg2, fw_reg1;
 
-	chip_major = ene_hw_read_reg(dev, ENE_HW_VER_MAJOR);
-	chip_minor = ene_hw_read_reg(dev, ENE_HW_VER_MINOR);
+	ene_clear_reg_mask(dev, ENE_ECSTS, ENE_ECSTS_RSRVD);
+	chip_major = ene_read_reg(dev, ENE_ECVER_MAJOR);
+	chip_minor = ene_read_reg(dev, ENE_ECVER_MINOR);
+	ene_set_reg_mask(dev, ENE_ECSTS, ENE_ECSTS_RSRVD);
 
-	ene_hw_write_reg(dev, ENE_HW_UNK, tmp);
-	hw_revision = ene_hw_read_reg(dev, ENE_HW_VERSION);
-	old_ver = ene_hw_read_reg(dev, ENE_HW_VER_OLD);
+	hw_revision = ene_read_reg(dev, ENE_ECHV);
+	old_ver = ene_read_reg(dev, ENE_HW_VER_OLD);
 
-	pll_freq = (ene_hw_read_reg(dev, ENE_PLLFRH) << 4) +
-		(ene_hw_read_reg(dev, ENE_PLLFRL) >> 4);
-
-	if (pll_freq != 1000)
-		dev->rx_period_adjust = 4;
-	else
-		dev->rx_period_adjust = 2;
+	dev->pll_freq = (ene_read_reg(dev, ENE_PLLFRH) << 4) +
+		(ene_read_reg(dev, ENE_PLLFRL) >> 4);
 
-
-	ene_printk(KERN_NOTICE, "PLL freq = %d\n", pll_freq);
+	if (sample_period != ENE_DEFAULT_SAMPLE_PERIOD)
+		dev->rx_period_adjust =
+			dev->pll_freq == ENE_DEFAULT_PLL_FREQ ? 2 : 4;
 
 	if (hw_revision == 0xFF) {
-
-		ene_printk(KERN_WARNING, "device seems to be disabled\n");
-		ene_printk(KERN_WARNING,
-			"send a mail to lirc-list@lists.sourceforge.net\n");
-		ene_printk(KERN_WARNING, "please attach output of acpidump\n");
+		ene_warn("device seems to be disabled");
+		ene_warn("send a mail to lirc-list@lists.sourceforge.net");
+		ene_warn("please attach output of acpidump and dmidecode");
 		return -ENODEV;
 	}
 
+	ene_notice("chip is 0x%02x%02x - kbver = 0x%02x, rev = 0x%02x",
+		chip_major, chip_minor, old_ver, hw_revision);
+
+	ene_notice("PLL freq = %d", dev->pll_freq);
+
 	if (chip_major == 0x33) {
-		ene_printk(KERN_WARNING, "chips 0x33xx aren't supported\n");
+		ene_warn("chips 0x33xx aren't supported");
 		return -ENODEV;
 	}
 
 	if (chip_major == 0x39 && chip_minor == 0x26 && hw_revision == 0xC0) {
 		dev->hw_revision = ENE_HW_C;
+		ene_notice("KB3926C detected");
 	} else if (old_ver == 0x24 && hw_revision == 0xC0) {
 		dev->hw_revision = ENE_HW_B;
-		ene_printk(KERN_NOTICE, "KB3926B detected\n");
+		ene_notice("KB3926B detected");
 	} else {
 		dev->hw_revision = ENE_HW_D;
-		ene_printk(KERN_WARNING,
-			"unknown ENE chip detected, assuming KB3926D\n");
-		ene_printk(KERN_WARNING,
-			"driver support might be not complete");
-
+		ene_notice("KB3926D or higher detected");
 	}
 
-	ene_printk(KERN_DEBUG,
-		"chip is 0x%02x%02x - kbver = 0x%02x, rev = 0x%02x\n",
-			chip_major, chip_minor, old_ver, hw_revision);
-
 	/* detect features hardware supports */
 	if (dev->hw_revision < ENE_HW_C)
 		return 0;
 
-	fw_capabilities = ene_hw_read_reg(dev, ENE_FW2);
-	ene_dbg("Firmware capabilities: %02x", fw_capabilities);
+	fw_reg1 = ene_read_reg(dev, ENE_FW1);
+	fw_reg2 = ene_read_reg(dev, ENE_FW2);
 
-	dev->hw_gpio40_learning = fw_capabilities & ENE_FW2_GP40_AS_LEARN;
-	dev->hw_learning_and_tx_capable = fw_capabilities & ENE_FW2_LEARNING;
+	ene_notice("Firmware regs: %02x %02x", fw_reg1, fw_reg2);
 
-	dev->hw_fan_as_normal_input = dev->hw_learning_and_tx_capable &&
-	    (fw_capabilities & ENE_FW2_FAN_AS_NRML_IN);
+	dev->hw_use_gpio_0a = fw_reg2 & ENE_FW2_GP0A;
+	dev->hw_learning_and_tx_capable = fw_reg2 & ENE_FW2_LEARNING;
+	dev->hw_extra_buffer = fw_reg1 & ENE_FW1_HAS_EXTRA_BUF;
+	dev->hw_fan_input = dev->hw_learning_and_tx_capable &&
+	    (fw_reg2 & ENE_FW2_FAN_INPUT);
 
-	ene_printk(KERN_NOTICE, "hardware features:\n");
-	ene_printk(KERN_NOTICE,
-		"learning and transmit %s, gpio40_learn %s, fan_in %s\n",
-	       dev->hw_learning_and_tx_capable ? "on" : "off",
-	       dev->hw_gpio40_learning ? "on" : "off",
-	       dev->hw_fan_as_normal_input ? "on" : "off");
+	ene_notice("Hardware features:");
 
 	if (dev->hw_learning_and_tx_capable) {
-		ene_printk(KERN_WARNING,
-		"Device supports transmitting, but that support is\n");
-		ene_printk(KERN_WARNING,
-		"lightly tested. Please test it and mail\n");
-		ene_printk(KERN_WARNING,
-		"lirc-list@lists.sourceforge.net\n");
+		ene_notice("* Supports transmitting & learning mode");
+		ene_notice("   This feature is rare and therefore,");
+		ene_notice("   you are welcome to test it,");
+		ene_notice("   and/or contact the author via:");
+		ene_notice("   lirc-list@lists.sourceforge.net");
+		ene_notice("   or maximlevitsky@gmail.com");
+
+		ene_notice("* Uses GPIO %s for IR raw input",
+			dev->hw_use_gpio_0a ? "40" : "0A");
+
+		if (dev->hw_fan_input)
+			ene_notice("* Uses unused fan feedback input as source"
+					" of demodulated IR data");
 	}
+
+	if (!dev->hw_fan_input)
+		ene_notice("* Uses GPIO %s for IR demodulated input",
+			dev->hw_use_gpio_0a ? "0A" : "40");
+
+	if (dev->hw_extra_buffer)
+		ene_notice("* Uses new style input buffer");
 	return 0;
 }
 
-/* this enables/disables IR input via gpio40*/
-static void ene_enable_gpio40_receive(struct ene_device *dev, int enable)
+/* Sense current received carrier */
+void ene_rx_sense_carrier(struct ene_device *dev)
 {
-	ene_hw_write_reg_mask(dev, ENE_CIR_CONF2, enable ?
-			      0 : ENE_CIR_CONF2_GPIO40DIS,
-			      ENE_CIR_CONF2_GPIO40DIS);
+	int period = ene_read_reg(dev, ENE_CIRCAR_PRD);
+	int hperiod = ene_read_reg(dev, ENE_CIRCAR_HPRD);
+	int carrier, duty_cycle;
+
+
+	if (!(period & ENE_CIRCAR_PRD_VALID))
+		return;
+
+	period &= ~ENE_CIRCAR_PRD_VALID;
+
+	if (!period)
+		return;
+
+	dbg("RX: hardware carrier period = %02x", period);
+	dbg("RX: hardware carrier pulse period = %02x", hperiod);
+
+
+	carrier = 2000000 / period;
+	duty_cycle = (hperiod * 100) / period;
+	dbg("RX: sensed carrier = %d Hz, duty cycle %d%%",
+							carrier, duty_cycle);
+
+	/* TODO: Send carrier & duty cycle to IR layer */
 }
 
-/* this enables/disables IR via standard input */
-static void ene_enable_normal_receive(struct ene_device *dev, int enable)
+/* this enables/disables the CIR RX engine */
+static void ene_enable_cir_engine(struct ene_device *dev, bool enable)
 {
-	ene_hw_write_reg(dev, ENE_CIR_CONF1, enable ? ENE_CIR_CONF1_RX_ON : 0);
+	ene_set_clear_reg_mask(dev, ENE_CIRCFG,
+			ENE_CIRCFG_RX_EN | ENE_CIRCFG_RX_IRQ, enable);
 }
 
-/* this enables/disables IR input via unused fan tachtometer input */
-static void ene_enable_fan_receive(struct ene_device *dev, int enable)
+/* this selects input for CIR engine. Ether GPIO 0A or GPIO40*/
+static void ene_select_rx_input(struct ene_device *dev, bool gpio_0a)
 {
-	if (!enable)
-		ene_hw_write_reg(dev, ENE_FAN_AS_IN1, 0);
-	else {
-		ene_hw_write_reg(dev, ENE_FAN_AS_IN1, ENE_FAN_AS_IN1_EN);
-		ene_hw_write_reg(dev, ENE_FAN_AS_IN2, ENE_FAN_AS_IN2_EN);
-	}
-	dev->rx_fan_input_inuse = enable;
+	ene_set_clear_reg_mask(dev, ENE_CIRCFG2, ENE_CIRCFG2_GPIO0A, gpio_0a);
 }
 
-
-/* Sense current received carrier */
-static int ene_rx_sense_carrier(struct ene_device *dev)
+/*
+ * this enables alternative input via fan tachometer sensor and bypasses
+ * the hw CIR engine
+ */
+static void ene_enable_fan_input(struct ene_device *dev, bool enable)
 {
-	int period = ene_hw_read_reg(dev, ENE_RX_CARRIER);
-	int carrier;
-	ene_dbg("RX: hardware carrier period = %02x", period);
-
-	if (!(period & ENE_RX_CARRIER_VALID))
-		return 0;
-
-	period &= ~ENE_RX_CARRIER_VALID;
-
-	if (!period)
-		return 0;
+	if (!dev->hw_fan_input)
+		return;
 
-	carrier = 2000000 / period;
-	ene_dbg("RX: sensed carrier = %d Hz", carrier);
-	return carrier;
+	if (!enable)
+		ene_write_reg(dev, ENE_FAN_AS_IN1, 0);
+	else {
+		ene_write_reg(dev, ENE_FAN_AS_IN1, ENE_FAN_AS_IN1_EN);
+		ene_write_reg(dev, ENE_FAN_AS_IN2, ENE_FAN_AS_IN2_EN);
+	}
+	dev->rx_fan_input_inuse = enable;
 }
 
-/* determine which input to use*/
-static void ene_rx_set_inputs(struct ene_device *dev)
+/* setup the receiver for RX*/
+static void ene_rx_setup(struct ene_device *dev)
 {
-	int learning_mode = dev->learning_enabled;
+	bool learning_mode = dev->learning_enabled ||
+					dev->carrier_detect_enabled;
+	int sample_period_adjust = 0;
 
-	ene_dbg("RX: setup receiver, learning mode = %d", learning_mode);
 
-	ene_enable_normal_receive(dev, 1);
+	/* set sample period*/
+	if (sample_period == ENE_DEFAULT_SAMPLE_PERIOD)
+		sample_period_adjust =
+			dev->pll_freq == ENE_DEFAULT_PLL_FREQ ? 1 : 2;
 
-	/* old hardware doesn't support learning mode for sure */
-	if (dev->hw_revision <= ENE_HW_B)
-		return;
+	ene_write_reg(dev, ENE_CIRRLC_CFG,
+			(sample_period + sample_period_adjust) |
+						ENE_CIRRLC_CFG_OVERFLOW);
+	/* revB doesn't support inputs */
+	if (dev->hw_revision < ENE_HW_C)
+		goto select_timeout;
 
-	/* receiver not learning capable, still set gpio40 correctly */
-	if (!dev->hw_learning_and_tx_capable) {
-		ene_enable_gpio40_receive(dev, !dev->hw_gpio40_learning);
-		return;
-	}
+	if (learning_mode && dev->hw_learning_and_tx_capable) {
 
-	/* enable learning mode */
-	if (learning_mode) {
-		ene_enable_gpio40_receive(dev, dev->hw_gpio40_learning);
+		/* Enable the opposite of the normal input
+		That means that if GPIO40 is normally used, use GPIO0A
+		and vice versa.
+		This input will carry non demodulated
+		signal, and we will tell the hw to demodulate it itself */
+		ene_select_rx_input(dev, !dev->hw_use_gpio_0a);
+		dev->rx_fan_input_inuse = false;
 
-		/* fan input is not used for learning */
-		if (dev->hw_fan_as_normal_input)
-			ene_enable_fan_receive(dev, 0);
+		/* Enable carrier demodulation */
+		ene_set_reg_mask(dev, ENE_CIRCFG, ENE_CIRCFG_CARR_DEMOD);
 
-	/* disable learning mode */
+		/* Enable carrier detection */
+		ene_set_clear_reg_mask(dev, ENE_CIRCFG2, ENE_CIRCFG2_CARR_DETECT,
+			dev->carrier_detect_enabled || debug);
 	} else {
-		if (dev->hw_fan_as_normal_input) {
-			ene_enable_fan_receive(dev, 1);
-			ene_enable_normal_receive(dev, 0);
-		} else
-			ene_enable_gpio40_receive(dev,
-					!dev->hw_gpio40_learning);
+		if (dev->hw_fan_input)
+			dev->rx_fan_input_inuse = true;
+		else
+			ene_select_rx_input(dev, dev->hw_use_gpio_0a);
+
+		/* Disable carrier detection & demodulation */
+		ene_clear_reg_mask(dev, ENE_CIRCFG, ENE_CIRCFG_CARR_DEMOD);
+		ene_clear_reg_mask(dev, ENE_CIRCFG2, ENE_CIRCFG2_CARR_DETECT);
 	}
 
-	/* set few additional settings for this mode */
-	ene_hw_write_reg_mask(dev, ENE_CIR_CONF1, learning_mode ?
-			      ENE_CIR_CONF1_LEARN1 : 0, ENE_CIR_CONF1_LEARN1);
-
-	ene_hw_write_reg_mask(dev, ENE_CIR_CONF2, learning_mode ?
-			      ENE_CIR_CONF2_LEARN2 : 0, ENE_CIR_CONF2_LEARN2);
-
+select_timeout:
 	if (dev->rx_fan_input_inuse) {
-		dev->props->rx_resolution = ENE_SAMPLE_PERIOD_FAN * 1000;
+		dev->props->rx_resolution = MS_TO_NS(ENE_FW_SAMPLE_PERIOD_FAN);
 
-		dev->props->timeout =
-			ENE_FAN_VALUE_MASK * ENE_SAMPLE_PERIOD_FAN * 1000;
+		/* Fan input doesn't support timeouts, it just ends the
+			input with a maximum sample */
+		dev->props->min_timeout = dev->props->max_timeout =
+			MS_TO_NS(ENE_FW_SMPL_BUF_FAN_MSK *
+				ENE_FW_SAMPLE_PERIOD_FAN);
 	} else {
-		dev->props->rx_resolution = sample_period * 1000;
-		dev->props->timeout = ENE_MAXGAP * 1000;
+		dev->props->rx_resolution = MS_TO_NS(sample_period);
+
+		/* Theoreticly timeout is unlimited, but we cap it
+		 * because it was seen that on one device, it
+		 * would stop sending spaces after around 250 msec.
+		 * Besides, this is close to 2^32 anyway and timeout is u32.
+		 */
+		dev->props->min_timeout = MS_TO_NS(127 * sample_period);
+		dev->props->max_timeout = MS_TO_NS(200000);
 	}
+
+	if (dev->hw_learning_and_tx_capable)
+		dev->props->tx_resolution = MS_TO_NS(sample_period);
+
+	if (dev->props->timeout > dev->props->max_timeout)
+		dev->props->timeout = dev->props->max_timeout;
+	if (dev->props->timeout < dev->props->min_timeout)
+		dev->props->timeout = dev->props->min_timeout;
 }
 
 /* Enable the device for receive */
 static void ene_rx_enable(struct ene_device *dev)
 {
 	u8 reg_value;
+	dbg("RX: setup receiver, learning mode = %d", learning_mode);
 
+	/* Enable system interrupt */
 	if (dev->hw_revision < ENE_HW_C) {
-		ene_hw_write_reg(dev, ENEB_IRQ, dev->irq << 1);
-		ene_hw_write_reg(dev, ENEB_IRQ_UNK1, 0x01);
+		ene_write_reg(dev, ENEB_IRQ, dev->irq << 1);
+		ene_write_reg(dev, ENEB_IRQ_UNK1, 0x01);
 	} else {
-		reg_value = ene_hw_read_reg(dev, ENEC_IRQ) & 0xF0;
-		reg_value |= ENEC_IRQ_UNK_EN;
-		reg_value &= ~ENEC_IRQ_STATUS;
-		reg_value |= (dev->irq & ENEC_IRQ_MASK);
-		ene_hw_write_reg(dev, ENEC_IRQ, reg_value);
-		ene_hw_write_reg(dev, ENE_TX_UNK1, 0x63);
+		reg_value = ene_read_reg(dev, ENE_IRQ) & 0xF0;
+		reg_value |= ENE_IRQ_UNK_EN;
+		reg_value &= ~ENE_IRQ_STATUS;
+		reg_value |= (dev->irq & ENE_IRQ_MASK);
+		ene_write_reg(dev, ENE_IRQ, reg_value);
 	}
 
-	ene_hw_write_reg(dev, ENE_CIR_CONF2, 0x00);
-	ene_rx_set_inputs(dev);
+	if (dev->hw_revision >= ENE_HW_C)
+		ene_write_reg(dev, ENE_CIRCAR_PULS, 0x63);
+
+	/* Enable the inputs */
+	ene_write_reg(dev, ENE_CIRCFG2, 0x00);
 
-	/* set sampling period */
-	ene_hw_write_reg(dev, ENE_CIR_SAMPLE_PERIOD, sample_period);
+	if (dev->rx_fan_input_inuse) {
+		ene_enable_cir_engine(dev, false);
+		ene_enable_fan_input(dev, true);
+	} else {
+		ene_enable_cir_engine(dev, true);
+		ene_enable_fan_input(dev, false);
+	}
 
 	/* ack any pending irqs - just in case */
 	ene_irq_status(dev);
 
 	/* enable firmware bits */
-	ene_hw_write_reg_mask(dev, ENE_FW1,
-			      ENE_FW1_ENABLE | ENE_FW1_IRQ,
-			      ENE_FW1_ENABLE | ENE_FW1_IRQ);
+	ene_set_reg_mask(dev, ENE_FW1, ENE_FW1_ENABLE | ENE_FW1_IRQ);
 
 	/* enter idle mode */
-	ir_raw_event_set_idle(dev->idev, 1);
-	ir_raw_event_reset(dev->idev);
-
+	ir_raw_event_set_idle(dev->idev, true);
+	dev->rx_enabled = true;
 }
 
 /* Disable the device receiver */
 static void ene_rx_disable(struct ene_device *dev)
 {
 	/* disable inputs */
-	ene_enable_normal_receive(dev, 0);
-
-	if (dev->hw_fan_as_normal_input)
-		ene_enable_fan_receive(dev, 0);
+	ene_enable_cir_engine(dev, false);
+	ene_enable_fan_input(dev, false);
 
 	/* disable hardware IRQ and firmware flag */
-	ene_hw_write_reg_mask(dev, ENE_FW1, 0, ENE_FW1_ENABLE | ENE_FW1_IRQ);
+	ene_clear_reg_mask(dev, ENE_FW1, ENE_FW1_ENABLE | ENE_FW1_IRQ);
 
-	ir_raw_event_set_idle(dev->idev, 1);
-	ir_raw_event_reset(dev->idev);
+	ir_raw_event_set_idle(dev->idev, true);
+	dev->rx_enabled = false;
 }
 
-
 /* prepare transmission */
 static void ene_tx_prepare(struct ene_device *dev)
 {
-	u8 conf1;
+	u8 conf1 = ene_read_reg(dev, ENE_CIRCFG);
+	u8 fwreg2 = ene_read_reg(dev, ENE_FW2);
 
-	conf1 = ene_hw_read_reg(dev, ENE_CIR_CONF1);
 	dev->saved_conf1 = conf1;
 
-	if (dev->hw_revision == ENE_HW_C)
-		conf1 &= ~ENE_CIR_CONF1_TX_CLEAR;
+	/* Show information about currently connected transmitter jacks */
+	if (fwreg2 & ENE_FW2_EMMITER1_CONN)
+		dbg("TX: Transmitter #1 is connected");
 
-	/* Enable TX engine */
-	conf1 |= ENE_CIR_CONF1_TX_ON;
+	if (fwreg2 & ENE_FW2_EMMITER2_CONN)
+		dbg("TX: Transmitter #2 is connected");
 
-	/* Set carrier */
-	if (dev->tx_period) {
+	if (!(fwreg2 & (ENE_FW2_EMMITER1_CONN | ENE_FW2_EMMITER2_CONN)))
+		ene_warn("TX: transmitter cable isn't connected!");
+
+	/* Set transmitter mask */
+	ene_set_clear_reg_mask(dev, ENE_GPIOFS8, ENE_GPIOFS8_GPIO41,
+					!!(dev->transmitter_mask & 0x01));
+	ene_set_clear_reg_mask(dev, ENE_GPIOFS1, ENE_GPIOFS1_GPIO0D,
+					!!(dev->transmitter_mask & 0x02));
 
-		/* NOTE: duty cycle handling is just a guess, it might
-			not be aviable. Default values were tested */
-		int tx_period_in500ns = dev->tx_period * 2;
+	/* Set the carrier period && duty cycle */
+	if (dev->tx_period) {
 
-		int tx_pulse_width_in_500ns =
-			tx_period_in500ns / (100 / dev->tx_duty_cycle);
+		int tx_puls_width = dev->tx_period / (100 / dev->tx_duty_cycle);
 
-		if (!tx_pulse_width_in_500ns)
-			tx_pulse_width_in_500ns = 1;
+		if (!tx_puls_width)
+			tx_puls_width = 1;
 
-		ene_dbg("TX: pulse distance = %d * 500 ns", tx_period_in500ns);
-		ene_dbg("TX: pulse width = %d * 500 ns",
-						tx_pulse_width_in_500ns);
+		dbg("TX: pulse distance = %d * 500 ns", dev->tx_period);
+		dbg("TX: pulse width = %d * 500 ns", tx_puls_width);
 
-		ene_hw_write_reg(dev, ENE_TX_PERIOD, ENE_TX_PERIOD_UNKBIT |
-					tx_period_in500ns);
+		ene_write_reg(dev, ENE_CIRMOD_PRD, ENE_CIRMOD_PRD_POL |
+					dev->tx_period);
 
-		ene_hw_write_reg(dev, ENE_TX_PERIOD_PULSE,
-					tx_pulse_width_in_500ns);
+		ene_write_reg(dev, ENE_CIRMOD_HPRD, tx_puls_width);
 
-		conf1 |= ENE_CIR_CONF1_TX_CARR;
+		conf1 |= ENE_CIRCFG_TX_CARR;
 	} else
-		conf1 &= ~ENE_CIR_CONF1_TX_CARR;
+		conf1 &= ~ENE_CIRCFG_TX_CARR;
 
-	ene_hw_write_reg(dev, ENE_CIR_CONF1, conf1);
+	/* disable receive on revc */
+	if (dev->hw_revision == ENE_HW_C)
+		conf1 &= ~ENE_CIRCFG_RX_EN;
 
+	/* Enable TX engine */
+	conf1 |= ENE_CIRCFG_TX_EN | ENE_CIRCFG_TX_IRQ;
+	ene_write_reg(dev, ENE_CIRCFG, conf1);
 }
 
 /* end transmission */
 static void ene_tx_complete(struct ene_device *dev)
 {
-	ene_hw_write_reg(dev, ENE_CIR_CONF1, dev->saved_conf1);
+	ene_write_reg(dev, ENE_CIRCFG, dev->saved_conf1);
 	dev->tx_buffer = NULL;
 }
 
-/* set transmit mask */
-static void ene_tx_hw_set_transmiter_mask(struct ene_device *dev)
-{
-	u8 txport1 = ene_hw_read_reg(dev, ENE_TX_PORT1) & ~ENE_TX_PORT1_EN;
-	u8 txport2 = ene_hw_read_reg(dev, ENE_TX_PORT2) & ~ENE_TX_PORT2_EN;
-
-	if (dev->transmitter_mask & 0x01)
-		txport1 |= ENE_TX_PORT1_EN;
-
-	if (dev->transmitter_mask & 0x02)
-		txport2 |= ENE_TX_PORT2_EN;
-
-	ene_hw_write_reg(dev, ENE_TX_PORT1, txport1);
-	ene_hw_write_reg(dev, ENE_TX_PORT2, txport2);
-}
 
 /* TX one sample - must be called with dev->hw_lock*/
 static void ene_tx_sample(struct ene_device *dev)
 {
 	u8 raw_tx;
 	u32 sample;
+	bool pulse = dev->tx_sample_pulse;
 
 	if (!dev->tx_buffer) {
-		ene_dbg("TX: attempt to transmit NULL buffer");
+		ene_warn("TX: BUG: attempt to transmit NULL buffer");
 		return;
 	}
 
 	/* Grab next TX sample */
 	if (!dev->tx_sample) {
-again:
-		if (dev->tx_pos == dev->tx_len + 1) {
+
+		if (dev->tx_pos == dev->tx_len) {
 			if (!dev->tx_done) {
-				ene_dbg("TX: no more data to send");
-				dev->tx_done = 1;
+				dbg("TX: no more data to send");
+				dev->tx_done = true;
 				goto exit;
 			} else {
-				ene_dbg("TX: last sample sent by hardware");
+				dbg("TX: last sample sent by hardware");
 				ene_tx_complete(dev);
 				complete(&dev->tx_complete);
 				return;
@@ -425,23 +474,23 @@ again:
 		sample = dev->tx_buffer[dev->tx_pos++];
 		dev->tx_sample_pulse = !dev->tx_sample_pulse;
 
-		ene_dbg("TX: sample %8d (%s)", sample, dev->tx_sample_pulse ?
-							"pulse" : "space");
+		dev->tx_sample = DIV_ROUND_CLOSEST(sample, sample_period);
 
-		dev->tx_sample = DIV_ROUND_CLOSEST(sample, ENE_TX_SMPL_PERIOD);
-
-		/* guard against too short samples */
 		if (!dev->tx_sample)
-			goto again;
+			dev->tx_sample = 1;
 	}
 
-	raw_tx = min(dev->tx_sample , (unsigned int)ENE_TX_SMLP_MASK);
+	raw_tx = min(dev->tx_sample , (unsigned int)ENE_CIRRLC_OUT_MASK);
 	dev->tx_sample -= raw_tx;
 
-	if (dev->tx_sample_pulse)
-		raw_tx |= ENE_TX_PULSE_MASK;
+	dbg("TX: sample %8d (%s)", raw_tx * sample_period,
+						pulse ? "pulse" : "space");
+	if (pulse)
+		raw_tx |= ENE_CIRRLC_OUT_PULSE;
+
+	ene_write_reg(dev,
+		dev->tx_reg ? ENE_CIRRLC_OUT1 : ENE_CIRRLC_OUT0, raw_tx);
 
-	ene_hw_write_reg(dev, ENE_TX_INPUT1 + dev->tx_reg, raw_tx);
 	dev->tx_reg = !dev->tx_reg;
 exit:
 	/* simulate TX done interrupt */
@@ -460,82 +509,204 @@ static void ene_tx_irqsim(unsigned long data)
 	spin_unlock_irqrestore(&dev->hw_lock, flags);
 }
 
+/* Read properities of hw sample buffer */
+static void ene_setup_hw_buffer(struct ene_device *dev)
+{
+	u16 tmp;
+
+	ene_read_hw_pointer(dev);
+	dev->r_pointer = dev->w_pointer;
+
+	if (!dev->hw_extra_buffer) {
+		dev->buffer_len = ENE_FW_PACKET_SIZE * 2;
+		return;
+	}
+
+	tmp = ene_read_reg(dev, ENE_FW_SAMPLE_BUFFER);
+	tmp |= ene_read_reg(dev, ENE_FW_SAMPLE_BUFFER+1) << 8;
+	dev->extra_buf1_address = tmp;
+
+	dev->extra_buf1_len = ene_read_reg(dev, ENE_FW_SAMPLE_BUFFER + 2);
+
+	tmp = ene_read_reg(dev, ENE_FW_SAMPLE_BUFFER + 3);
+	tmp |= ene_read_reg(dev, ENE_FW_SAMPLE_BUFFER + 4) << 8;
+	dev->extra_buf2_address = tmp;
+
+	dev->extra_buf2_len = ene_read_reg(dev, ENE_FW_SAMPLE_BUFFER + 5);
+
+	dev->buffer_len = dev->extra_buf1_len + dev->extra_buf2_len + 8;
+
+	ene_notice("Hardware uses 2 extended buffers:");
+	ene_notice("  0x%04x - len : %d", dev->extra_buf1_address,
+						dev->extra_buf1_len);
+	ene_notice("  0x%04x - len : %d", dev->extra_buf2_address,
+						dev->extra_buf2_len);
+
+	ene_notice("Total buffer len = %d", dev->buffer_len);
+
+	if (dev->buffer_len > 64 || dev->buffer_len < 16)
+		goto error;
+
+	if (dev->extra_buf1_address > 0xFBFC ||
+					dev->extra_buf1_address < 0xEC00)
+		goto error;
+
+	if (dev->extra_buf2_address > 0xFBFC ||
+					dev->extra_buf2_address < 0xEC00)
+		goto error;
+
+	if (dev->r_pointer > dev->buffer_len)
+		goto error;
+
+	ene_set_reg_mask(dev, ENE_FW1, ENE_FW1_EXTRA_BUF_HND);
+	return;
+error:
+	ene_warn("Error validating extra buffers, device probably won't work");
+	dev->hw_extra_buffer = false;
+	ene_clear_reg_mask(dev, ENE_FW1, ENE_FW1_EXTRA_BUF_HND);
+}
+
+
+/* Restore the pointers to extra buffers - to make module reload work*/
+static void ene_restore_extra_buffer(struct ene_device *dev)
+{
+	if (!dev->hw_extra_buffer)
+		return;
+
+	ene_write_reg(dev, ENE_FW_SAMPLE_BUFFER + 0,
+				dev->extra_buf1_address & 0xFF);
+	ene_write_reg(dev, ENE_FW_SAMPLE_BUFFER + 1,
+				dev->extra_buf1_address >> 8);
+	ene_write_reg(dev, ENE_FW_SAMPLE_BUFFER + 2, dev->extra_buf1_len);
+
+	ene_write_reg(dev, ENE_FW_SAMPLE_BUFFER + 3,
+				dev->extra_buf2_address & 0xFF);
+	ene_write_reg(dev, ENE_FW_SAMPLE_BUFFER + 4,
+				dev->extra_buf2_address >> 8);
+	ene_write_reg(dev, ENE_FW_SAMPLE_BUFFER + 5,
+				dev->extra_buf2_len);
+	ene_clear_reg_mask(dev, ENE_FW1, ENE_FW1_EXTRA_BUF_HND);
+}
+
 
 /* read irq status and ack it */
 static int ene_irq_status(struct ene_device *dev)
 {
 	u8 irq_status;
 	u8 fw_flags1, fw_flags2;
-	int cur_rx_pointer;
 	int retval = 0;
 
-	fw_flags2 = ene_hw_read_reg(dev, ENE_FW2);
-	cur_rx_pointer = !!(fw_flags2 & ENE_FW2_BUF_HIGH);
+	fw_flags2 = ene_read_reg(dev, ENE_FW2);
 
 	if (dev->hw_revision < ENE_HW_C) {
-		irq_status = ene_hw_read_reg(dev, ENEB_IRQ_STATUS);
+		irq_status = ene_read_reg(dev, ENEB_IRQ_STATUS);
 
 		if (!(irq_status & ENEB_IRQ_STATUS_IR))
 			return 0;
 
-		ene_hw_write_reg(dev, ENEB_IRQ_STATUS,
-				 irq_status & ~ENEB_IRQ_STATUS_IR);
-		dev->rx_pointer = cur_rx_pointer;
+		ene_clear_reg_mask(dev, ENEB_IRQ_STATUS, ENEB_IRQ_STATUS_IR);
 		return ENE_IRQ_RX;
 	}
 
-	irq_status = ene_hw_read_reg(dev, ENEC_IRQ);
-
-	if (!(irq_status & ENEC_IRQ_STATUS))
+	irq_status = ene_read_reg(dev, ENE_IRQ);
+	if (!(irq_status & ENE_IRQ_STATUS))
 		return 0;
 
 	/* original driver does that twice - a workaround ? */
-	ene_hw_write_reg(dev, ENEC_IRQ, irq_status & ~ENEC_IRQ_STATUS);
-	ene_hw_write_reg(dev, ENEC_IRQ, irq_status & ~ENEC_IRQ_STATUS);
+	ene_write_reg(dev, ENE_IRQ, irq_status & ~ENE_IRQ_STATUS);
+	ene_write_reg(dev, ENE_IRQ, irq_status & ~ENE_IRQ_STATUS);
 
-	/* clear unknown flag in F8F9 */
-	if (fw_flags2 & ENE_FW2_IRQ_CLR)
-		ene_hw_write_reg(dev, ENE_FW2, fw_flags2 & ~ENE_FW2_IRQ_CLR);
+	/* check RX interrupt */
+	if (fw_flags2 & ENE_FW2_RXIRQ) {
+		retval |= ENE_IRQ_RX;
+		ene_write_reg(dev, ENE_FW2, fw_flags2 & ~ENE_FW2_RXIRQ);
+	}
 
-	/* check if this is a TX interrupt */
-	fw_flags1 = ene_hw_read_reg(dev, ENE_FW1);
+	/* check TX interrupt */
+	fw_flags1 = ene_read_reg(dev, ENE_FW1);
 	if (fw_flags1 & ENE_FW1_TXIRQ) {
-		ene_hw_write_reg(dev, ENE_FW1, fw_flags1 & ~ENE_FW1_TXIRQ);
+		ene_write_reg(dev, ENE_FW1, fw_flags1 & ~ENE_FW1_TXIRQ);
 		retval |= ENE_IRQ_TX;
 	}
 
-	/* Check if this is RX interrupt */
-	if (dev->rx_pointer != cur_rx_pointer) {
-		retval |= ENE_IRQ_RX;
-		dev->rx_pointer = cur_rx_pointer;
+	return retval;
+}
 
-	} else if (!(retval & ENE_IRQ_TX)) {
-		ene_dbg("RX: interrupt without change in RX pointer(%d)",
-			dev->rx_pointer);
-		retval |= ENE_IRQ_RX;
+/* Read hardware write pointer */
+static void ene_read_hw_pointer(struct ene_device *dev)
+{
+	if (dev->hw_extra_buffer)
+		dev->w_pointer = ene_read_reg(dev, ENE_FW_RX_POINTER);
+	else
+		dev->w_pointer = ene_read_reg(dev, ENE_FW2)
+			& ENE_FW2_BUF_WPTR ? 0 : ENE_FW_PACKET_SIZE;
+
+	dbg_verbose("RB: HW write pointer: %02x, driver read pointer: %02x",
+		dev->w_pointer, dev->r_pointer);
+}
+
+/* Gets address of next sample from HW ring buffer */
+static int ene_get_sample_reg(struct ene_device *dev)
+{
+	int r_pointer;
+
+	if (dev->r_pointer == dev->w_pointer) {
+		dbg_verbose("RB: hit end, try update w_pointer");
+		ene_read_hw_pointer(dev);
 	}
 
-	if ((retval & ENE_IRQ_RX) && (retval & ENE_IRQ_TX))
-		ene_dbg("both RX and TX interrupt at same time");
+	if (dev->r_pointer == dev->w_pointer) {
+		dbg_verbose("RB: end of data at %d", dev->r_pointer);
+		return 0;
+	}
 
-	return retval;
+	dbg_verbose("RB: reading at offset %d", dev->r_pointer);
+	r_pointer = dev->r_pointer;
+
+	dev->r_pointer++;
+	if (dev->r_pointer == dev->buffer_len)
+		dev->r_pointer = 0;
+
+	dbg_verbose("RB: next read will be from offset %d", dev->r_pointer);
+
+	if (r_pointer < 8) {
+		dbg_verbose("RB: read at main buffer at %d", r_pointer);
+		return ENE_FW_SAMPLE_BUFFER + r_pointer;
+	}
+
+	r_pointer -= 8;
+
+	if (r_pointer < dev->extra_buf1_len) {
+		dbg_verbose("RB: read at 1st extra buffer at %d", r_pointer);
+		return dev->extra_buf1_address + r_pointer;
+	}
+
+	r_pointer -= dev->extra_buf1_len;
+
+	if (r_pointer < dev->extra_buf2_len) {
+		dbg_verbose("RB: read at 2nd extra buffer at %d", r_pointer);
+		return dev->extra_buf2_address + r_pointer;
+	}
+
+	dbg("attempt to read beyong ring bufer end");
+	return 0;
 }
 
 /* interrupt handler */
 static irqreturn_t ene_isr(int irq, void *data)
 {
-	u16 hw_value;
-	int i, hw_sample;
-	int pulse;
-	int irq_status;
+	u16 hw_value, reg;
+	int hw_sample, irq_status;
+	bool pulse;
 	unsigned long flags;
-	int carrier = 0;
 	irqreturn_t retval = IRQ_NONE;
 	struct ene_device *dev = (struct ene_device *)data;
 	struct ir_raw_event ev;
 
-
 	spin_lock_irqsave(&dev->hw_lock, flags);
+
+	dbg_verbose("ISR called");
+	ene_read_hw_pointer(dev);
 	irq_status = ene_irq_status(dev);
 
 	if (!irq_status)
@@ -544,9 +715,9 @@ static irqreturn_t ene_isr(int irq, void *data)
 	retval = IRQ_HANDLED;
 
 	if (irq_status & ENE_IRQ_TX) {
-
+		dbg_verbose("TX interrupt");
 		if (!dev->hw_learning_and_tx_capable) {
-			ene_dbg("TX interrupt on unsupported device!");
+			dbg("TX interrupt on unsupported device!");
 			goto unlock;
 		}
 		ene_tx_sample(dev);
@@ -555,48 +726,57 @@ static irqreturn_t ene_isr(int irq, void *data)
 	if (!(irq_status & ENE_IRQ_RX))
 		goto unlock;
 
+	dbg_verbose("RX interrupt");
 
 	if (dev->carrier_detect_enabled || debug)
-		carrier = ene_rx_sense_carrier(dev);
-#if 0
-	/* TODO */
-	if (dev->carrier_detect_enabled && carrier)
-		ir_raw_event_report_frequency(dev->idev, carrier);
-#endif
+		ene_rx_sense_carrier(dev);
+
+	/* On hardware that don't support extra buffer we need to trust
+		the interrupt and not track the read pointer */
+	if (!dev->hw_extra_buffer)
+		dev->r_pointer = dev->w_pointer == 0 ? ENE_FW_PACKET_SIZE : 0;
+
+	while (1) {
 
-	for (i = 0; i < ENE_SAMPLES_SIZE; i++) {
-		hw_value = ene_hw_read_reg(dev,
-				ENE_SAMPLE_BUFFER + dev->rx_pointer * 4 + i);
+		reg = ene_get_sample_reg(dev);
+
+		dbg_verbose("next sample to read at: %04x", reg);
+		if (!reg)
+			break;
+
+		hw_value = ene_read_reg(dev, reg);
 
 		if (dev->rx_fan_input_inuse) {
+
+			int offset = ENE_FW_SMPL_BUF_FAN - ENE_FW_SAMPLE_BUFFER;
+
 			/* read high part of the sample */
-			hw_value |= ene_hw_read_reg(dev,
-			    ENE_SAMPLE_BUFFER_FAN +
-					dev->rx_pointer * 4 + i) << 8;
-			pulse = hw_value & ENE_FAN_SMPL_PULS_MSK;
+			hw_value |= ene_read_reg(dev, reg + offset) << 8;
+			pulse = hw_value & ENE_FW_SMPL_BUF_FAN_PLS;
 
 			/* clear space bit, and other unused bits */
-			hw_value &= ENE_FAN_VALUE_MASK;
-			hw_sample = hw_value * ENE_SAMPLE_PERIOD_FAN;
+			hw_value &= ENE_FW_SMPL_BUF_FAN_MSK;
+			hw_sample = hw_value * ENE_FW_SAMPLE_PERIOD_FAN;
 
 		} else {
-			pulse = !(hw_value & ENE_SAMPLE_SPC_MASK);
-			hw_value &= ENE_SAMPLE_VALUE_MASK;
+			pulse = !(hw_value & ENE_FW_SAMPLE_SPACE);
+			hw_value &= ~ENE_FW_SAMPLE_SPACE;
 			hw_sample = hw_value * sample_period;
 
 			if (dev->rx_period_adjust) {
-				hw_sample *= (100 - dev->rx_period_adjust);
-				hw_sample /= 100;
+				hw_sample *= 100;
+				hw_sample /= (100 + dev->rx_period_adjust);
 			}
 		}
-		/* no more data */
-		if (!(hw_value))
-			break;
 
-		ene_dbg("RX: %d (%s)", hw_sample, pulse ? "pulse" : "space");
+		if (!dev->hw_extra_buffer && !hw_sample) {
+			dev->r_pointer = dev->w_pointer;
+			continue;
+		}
 
+		dbg("RX: %d (%s)", hw_sample, pulse ? "pulse" : "space");
 
-		ev.duration = hw_sample * 1000;
+		ev.duration = MS_TO_NS(hw_sample);
 		ev.pulse = pulse;
 		ir_raw_event_store_with_filter(dev->idev, &ev);
 	}
@@ -611,16 +791,14 @@ unlock:
 static void ene_setup_settings(struct ene_device *dev)
 {
 	dev->tx_period = 32;
-	dev->tx_duty_cycle = 25; /*%*/
-	dev->transmitter_mask = 3;
+	dev->tx_duty_cycle = 50; /*%*/
+	dev->transmitter_mask = 0x03;
 
-	/* Force learning mode if (input == 2), otherwise
-		let user set it with LIRC_SET_REC_CARRIER */
 	dev->learning_enabled =
-		(input == 2 && dev->hw_learning_and_tx_capable);
-
-	dev->rx_pointer = -1;
+		(learning_mode && dev->hw_learning_and_tx_capable);
 
+	/* Set reasonable default timeout */
+	dev->props->timeout = MS_TO_NS(15000);
 }
 
 /* outside interface: called on first open*/
@@ -630,8 +808,6 @@ static int ene_open(void *data)
 	unsigned long flags;
 
 	spin_lock_irqsave(&dev->hw_lock, flags);
-	dev->in_use = 1;
-	ene_setup_settings(dev);
 	ene_rx_enable(dev);
 	spin_unlock_irqrestore(&dev->hw_lock, flags);
 	return 0;
@@ -645,7 +821,6 @@ static void ene_close(void *data)
 	spin_lock_irqsave(&dev->hw_lock, flags);
 
 	ene_rx_disable(dev);
-	dev->in_use = 0;
 	spin_unlock_irqrestore(&dev->hw_lock, flags);
 }
 
@@ -654,11 +829,11 @@ static int ene_set_tx_mask(void *data, u32 tx_mask)
 {
 	struct ene_device *dev = (struct ene_device *)data;
 	unsigned long flags;
-	ene_dbg("TX: attempt to set transmitter mask %02x", tx_mask);
+	dbg("TX: attempt to set transmitter mask %02x", tx_mask);
 
 	/* invalid txmask */
-	if (!tx_mask || tx_mask & ~0x3) {
-		ene_dbg("TX: invalid mask");
+	if (!tx_mask || tx_mask & ~0x03) {
+		dbg("TX: invalid mask");
 		/* return count of transmitters */
 		return 2;
 	}
@@ -674,28 +849,42 @@ static int ene_set_tx_carrier(void *data, u32 carrier)
 {
 	struct ene_device *dev = (struct ene_device *)data;
 	unsigned long flags;
-	u32 period = 1000000 / carrier; /* (1 / freq) (* # usec in 1 sec) */
+	u32 period = 2000000 / carrier;
 
-	ene_dbg("TX: attempt to set tx carrier to %d kHz", carrier);
+	dbg("TX: attempt to set tx carrier to %d kHz", carrier);
 
-	if (period && (period > ENE_TX_PERIOD_MAX ||
-			period < ENE_TX_PERIOD_MIN)) {
+	if (period && (period > ENE_CIRMOD_PRD_MAX ||
+			period < ENE_CIRMOD_PRD_MIN)) {
 
-		ene_dbg("TX: out of range %d-%d carrier, "
-			"falling back to 32 kHz",
-			1000 / ENE_TX_PERIOD_MIN,
-			1000 / ENE_TX_PERIOD_MAX);
+		dbg("TX: out of range %d-%d kHz carrier",
+			2000 / ENE_CIRMOD_PRD_MIN,
+			2000 / ENE_CIRMOD_PRD_MAX);
 
-		period = 32; /* this is just a coincidence!!! */
+		return -1;
 	}
-	ene_dbg("TX: set carrier to %d kHz", carrier);
 
+	dbg("TX: set carrier to %d kHz", carrier);
 	spin_lock_irqsave(&dev->hw_lock, flags);
 	dev->tx_period = period;
 	spin_unlock_irqrestore(&dev->hw_lock, flags);
 	return 0;
 }
 
+/*outside interface : set tx duty cycle */
+static int ene_set_tx_duty_cycle(void *data, u32 duty_cycle)
+{
+	struct ene_device *dev = (struct ene_device *)data;
+	unsigned long flags;
+
+	dbg("TX: setting duty cycle to %d%%", duty_cycle);
+
+	BUG_ON(!duty_cycle || duty_cycle >= 100);
+
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	dev->tx_duty_cycle = duty_cycle;
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
+	return 0;
+}
 
 /* outside interface: enable learning mode */
 static int ene_set_learning_mode(void *data, int enable)
@@ -707,31 +896,25 @@ static int ene_set_learning_mode(void *data, int enable)
 
 	spin_lock_irqsave(&dev->hw_lock, flags);
 	dev->learning_enabled = enable;
-	ene_rx_set_inputs(dev);
+	ene_rx_disable(dev);
+	ene_rx_setup(dev);
+	ene_rx_enable(dev);
 	spin_unlock_irqrestore(&dev->hw_lock, flags);
 	return 0;
 }
 
-/* outside interface: set rec carrier */
-static int ene_set_rec_carrier(void *data, u32 min, u32 max)
-{
-	struct ene_device *dev = (struct ene_device *)data;
-	ene_set_learning_mode(dev,
-		max > ENE_NORMAL_RX_HI || min < ENE_NORMAL_RX_LOW);
-	return 0;
-}
-
 /* outside interface: enable or disable idle mode */
 static void ene_rx_set_idle(void *data, int idle)
 {
 	struct ene_device *dev = (struct ene_device *)data;
-	ene_dbg("%sabling idle mode", idle ? "en" : "dis");
 
-	ene_hw_write_reg_mask(dev, ENE_CIR_SAMPLE_PERIOD,
-		(enable_idle && idle) ? 0 : ENE_CIR_SAMPLE_OVERFLOW,
-			ENE_CIR_SAMPLE_OVERFLOW);
-}
+	if (!idle)
+		return;
 
+	dbg("RX: stopping the receiver");
+	ene_clear_reg_mask(dev, ENE_CIRCFG, ENE_CIRCFG_RX_EN);
+	ene_set_reg_mask(dev, ENE_CIRCFG, ENE_CIRCFG_RX_EN);
+}
 
 /* outside interface: transmit */
 static int ene_transmit(void *data, int *buf, u32 n)
@@ -747,11 +930,10 @@ static int ene_transmit(void *data, int *buf, u32 n)
 	dev->tx_sample = 0;
 	dev->tx_sample_pulse = 0;
 
-	ene_dbg("TX: %d samples", dev->tx_len);
+	dbg("TX: %d samples", dev->tx_len);
 
 	spin_lock_irqsave(&dev->hw_lock, flags);
 
-	ene_tx_hw_set_transmiter_mask(dev);
 	ene_tx_prepare(dev);
 
 	/* Transmit first two samples */
@@ -761,16 +943,15 @@ static int ene_transmit(void *data, int *buf, u32 n)
 	spin_unlock_irqrestore(&dev->hw_lock, flags);
 
 	if (wait_for_completion_timeout(&dev->tx_complete, 2 * HZ) == 0) {
-		ene_dbg("TX: timeout");
+		dbg("TX: timeout");
 		spin_lock_irqsave(&dev->hw_lock, flags);
 		ene_tx_complete(dev);
 		spin_unlock_irqrestore(&dev->hw_lock, flags);
 	} else
-		ene_dbg("TX: done");
+		dbg("TX: done");
 	return n;
 }
 
-
 /* probe entry */
 static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 {
@@ -791,41 +972,47 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 	error = -ENODEV;
 
 	if (!pnp_port_valid(pnp_dev, 0) ||
-	    pnp_port_len(pnp_dev, 0) < ENE_MAX_IO)
+	    pnp_port_len(pnp_dev, 0) < ENE_IO_SIZE)
 		goto error;
 
 	if (!pnp_irq_valid(pnp_dev, 0))
 		goto error;
 
-	dev->hw_io = pnp_port_start(pnp_dev, 0);
-	dev->irq = pnp_irq(pnp_dev, 0);
 	spin_lock_init(&dev->hw_lock);
 
 	/* claim the resources */
 	error = -EBUSY;
-	if (!request_region(dev->hw_io, ENE_MAX_IO, ENE_DRIVER_NAME))
+	dev->hw_io = pnp_port_start(pnp_dev, 0);
+	if (!request_region(dev->hw_io, ENE_IO_SIZE, ENE_DRIVER_NAME)) {
+		dev->hw_io = -1;
+		dev->irq = -1;
 		goto error;
+	}
 
+	dev->irq = pnp_irq(pnp_dev, 0);
 	if (request_irq(dev->irq, ene_isr,
-			IRQF_SHARED, ENE_DRIVER_NAME, (void *)dev))
+			IRQF_SHARED, ENE_DRIVER_NAME, (void *)dev)) {
+		dev->irq = -1;
 		goto error;
+	}
 
 	pnp_set_drvdata(pnp_dev, dev);
 	dev->pnp_dev = pnp_dev;
 
+	/* don't allow too short/long sample periods */
+	if (sample_period < 5 || sample_period > 0x7F)
+		sample_period = ENE_DEFAULT_SAMPLE_PERIOD;
+
 	/* detect hardware version and features */
 	error = ene_hw_detect(dev);
 	if (error)
 		goto error;
 
-	ene_setup_settings(dev);
-
 	if (!dev->hw_learning_and_tx_capable && txsim) {
-		dev->hw_learning_and_tx_capable = 1;
+		dev->hw_learning_and_tx_capable = true;
 		setup_timer(&dev->tx_sim_timer, ene_tx_irqsim,
 						(long unsigned int)dev);
-		ene_printk(KERN_WARNING,
-			"Simulation of TX activated\n");
+		ene_warn("Simulation of TX activated");
 	}
 
 	ir_props->driver_type = RC_DRIVER_IR_RAW;
@@ -833,72 +1020,45 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 	ir_props->priv = dev;
 	ir_props->open = ene_open;
 	ir_props->close = ene_close;
-	ir_props->min_timeout = ENE_MINGAP * 1000;
-	ir_props->max_timeout = ENE_MAXGAP * 1000;
-	ir_props->timeout = ENE_MAXGAP * 1000;
-
-	if (dev->hw_revision == ENE_HW_B)
-		ir_props->s_idle = ene_rx_set_idle;
-
+	ir_props->s_idle = ene_rx_set_idle;
 
 	dev->props = ir_props;
 	dev->idev = input_dev;
 
-	/* don't allow too short/long sample periods */
-	if (sample_period < 5 || sample_period > 0x7F)
-		sample_period = -1;
-
-	/* choose default sample period */
-	if (sample_period == -1) {
-
-		sample_period = 50;
-
-		/* on revB, hardware idle mode eats first sample
-		  if we set too low sample period */
-		if (dev->hw_revision == ENE_HW_B && enable_idle)
-			sample_period = 75;
-	}
-
-	ir_props->rx_resolution = sample_period * 1000;
-
 	if (dev->hw_learning_and_tx_capable) {
-
 		ir_props->s_learning_mode = ene_set_learning_mode;
-
-		if (input == 0)
-			ir_props->s_rx_carrier_range = ene_set_rec_carrier;
-
 		init_completion(&dev->tx_complete);
 		ir_props->tx_ir = ene_transmit;
 		ir_props->s_tx_mask = ene_set_tx_mask;
 		ir_props->s_tx_carrier = ene_set_tx_carrier;
-		ir_props->tx_resolution = ENE_TX_SMPL_PERIOD * 1000;
+		ir_props->s_tx_duty_cycle = ene_set_tx_duty_cycle;
 		/* ir_props->s_carrier_report = ene_set_carrier_report; */
 	}
 
+	ene_setup_hw_buffer(dev);
+	ene_setup_settings(dev);
+	ene_rx_setup(dev);
 
-	device_set_wakeup_capable(&pnp_dev->dev, 1);
-	device_set_wakeup_enable(&pnp_dev->dev, 1);
+	device_set_wakeup_capable(&pnp_dev->dev, true);
+	device_set_wakeup_enable(&pnp_dev->dev, true);
 
 	if (dev->hw_learning_and_tx_capable)
 		input_dev->name = "ENE eHome Infrared Remote Transceiver";
 	else
 		input_dev->name = "ENE eHome Infrared Remote Receiver";
 
-
 	error = -ENODEV;
 	if (ir_input_register(input_dev, RC_MAP_RC6_MCE, ir_props,
 							ENE_DRIVER_NAME))
 		goto error;
 
-
-	ene_printk(KERN_NOTICE, "driver has been succesfully loaded\n");
+	ene_notice("driver has been succesfully loaded");
 	return 0;
 error:
-	if (dev->irq)
+	if (dev && dev->irq >= 0)
 		free_irq(dev->irq, dev);
-	if (dev->hw_io)
-		release_region(dev->hw_io, ENE_MAX_IO);
+	if (dev && dev->hw_io >= 0)
+		release_region(dev->hw_io, ENE_IO_SIZE);
 
 	input_free_device(input_dev);
 	kfree(ir_props);
@@ -914,10 +1074,11 @@ static void ene_remove(struct pnp_dev *pnp_dev)
 
 	spin_lock_irqsave(&dev->hw_lock, flags);
 	ene_rx_disable(dev);
+	ene_restore_extra_buffer(dev);
 	spin_unlock_irqrestore(&dev->hw_lock, flags);
 
 	free_irq(dev->irq, dev);
-	release_region(dev->hw_io, ENE_MAX_IO);
+	release_region(dev->hw_io, ENE_IO_SIZE);
 	ir_input_unregister(dev->idev);
 	kfree(dev->props);
 	kfree(dev);
@@ -927,28 +1088,28 @@ static void ene_remove(struct pnp_dev *pnp_dev)
 static void ene_enable_wake(struct ene_device *dev, int enable)
 {
 	enable = enable && device_may_wakeup(&dev->pnp_dev->dev);
-
-	ene_dbg("wake on IR %s", enable ? "enabled" : "disabled");
-
-	ene_hw_write_reg_mask(dev, ENE_FW1, enable ?
-		ENE_FW1_WAKE : 0, ENE_FW1_WAKE);
+	dbg("wake on IR %s", enable ? "enabled" : "disabled");
+	ene_set_clear_reg_mask(dev, ENE_FW1, ENE_FW1_WAKE, enable);
 }
 
 #ifdef CONFIG_PM
 static int ene_suspend(struct pnp_dev *pnp_dev, pm_message_t state)
 {
 	struct ene_device *dev = pnp_get_drvdata(pnp_dev);
-	ene_enable_wake(dev, 1);
+	ene_enable_wake(dev, true);
+
+	/* TODO: add support for wake pattern */
 	return 0;
 }
 
 static int ene_resume(struct pnp_dev *pnp_dev)
 {
 	struct ene_device *dev = pnp_get_drvdata(pnp_dev);
-	if (dev->in_use)
+	if (dev->rx_enabled) {
+		ene_rx_setup(dev);
 		ene_rx_enable(dev);
-
-	ene_enable_wake(dev, 0);
+	}
+	ene_enable_wake(dev, false);
 	return 0;
 }
 #endif
@@ -956,7 +1117,7 @@ static int ene_resume(struct pnp_dev *pnp_dev)
 static void ene_shutdown(struct pnp_dev *pnp_dev)
 {
 	struct ene_device *dev = pnp_get_drvdata(pnp_dev);
-	ene_enable_wake(dev, 1);
+	ene_enable_wake(dev, true);
 }
 
 static const struct pnp_device_id ene_ids[] = {
@@ -994,18 +1155,11 @@ static void ene_exit(void)
 module_param(sample_period, int, S_IRUGO);
 MODULE_PARM_DESC(sample_period, "Hardware sample period (50 us default)");
 
-module_param(enable_idle, bool, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(enable_idle,
-	"Enables turning off signal sampling after long inactivity time; "
-	"if disabled might help detecting input signal (default: enabled)"
-	" (KB3926B only)");
-
-module_param(input, bool, S_IRUGO);
-MODULE_PARM_DESC(input, "select which input to use "
-	"0 - auto, 1 - standard, 2 - wideband(KB3926C+)");
+module_param(learning_mode, bool, S_IRUGO);
+MODULE_PARM_DESC(learning_mode, "Enable learning mode by default");
 
 module_param(debug, int, S_IRUGO | S_IWUSR);
-MODULE_PARM_DESC(debug, "Enable debug (debug=2 verbose debug output)");
+MODULE_PARM_DESC(debug, "Debug level");
 
 module_param(txsim, bool, S_IRUGO);
 MODULE_PARM_DESC(txsim,
@@ -1013,8 +1167,8 @@ MODULE_PARM_DESC(txsim,
 
 MODULE_DEVICE_TABLE(pnp, ene_ids);
 MODULE_DESCRIPTION
-	("Infrared input driver for KB3926B/KB3926C/KB3926D "
-	"(aka ENE0100/ENE0200/ENE0201) CIR port");
+	("Infrared input driver for KB3926B/C/D/E/F "
+	"(aka ENE0100/ENE0200/ENE0201/ENE0202) CIR port");
 
 MODULE_AUTHOR("Maxim Levitsky");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/IR/ene_ir.h b/drivers/media/IR/ene_ir.h
index 54c76af..39c707b 100644
--- a/drivers/media/IR/ene_ir.h
+++ b/drivers/media/IR/ene_ir.h
@@ -1,5 +1,5 @@
 /*
- * driver for ENE KB3926 B/C/D CIR (also known as ENE0XXX)
+ * driver for ENE KB3926 B/C/D/E/F CIR (also known as ENE0XXX)
  *
  * Copyright (C) 2010 Maxim Levitsky <maximlevitsky@gmail.com>
  *
@@ -26,43 +26,50 @@
 #define ENE_ADDR_HI		1	/* hi byte of register address */
 #define ENE_ADDR_LO		2	/* low byte of register address */
 #define ENE_IO			3	/* read/write window */
-#define ENE_MAX_IO		4
-
-/* 8 bytes of samples, divided in 2 halfs*/
-#define ENE_SAMPLE_BUFFER	0xF8F0	/* regular sample buffer */
-#define ENE_SAMPLE_SPC_MASK	0x80	/* sample is space */
-#define ENE_SAMPLE_VALUE_MASK	0x7F
-#define ENE_SAMPLE_OVERFLOW	0x7F
-#define ENE_SAMPLES_SIZE	4
-
-/* fan input sample buffer */
-#define ENE_SAMPLE_BUFFER_FAN	0xF8FB	/* this buffer holds high byte of */
-					/* each sample of normal buffer */
-#define ENE_FAN_SMPL_PULS_MSK	0x8000	/* this bit of combined sample */
-					/* if set, says that sample is pulse */
-#define ENE_FAN_VALUE_MASK	0x0FFF  /* mask for valid bits of the value */
-
-/* first firmware register */
-#define ENE_FW1			0xF8F8
+#define ENE_IO_SIZE		4
+
+/* 8 bytes of samples, divided in 2 packets*/
+#define ENE_FW_SAMPLE_BUFFER	0xF8F0	/* sample buffer */
+#define ENE_FW_SAMPLE_SPACE	0x80	/* sample is space */
+#define ENE_FW_PACKET_SIZE	4
+
+/* first firmware flag register */
+#define ENE_FW1			0xF8F8  /* flagr */
 #define	ENE_FW1_ENABLE		0x01	/* enable fw processing */
 #define ENE_FW1_TXIRQ		0x02	/* TX interrupt pending */
+#define ENE_FW1_HAS_EXTRA_BUF	0x04	/* fw uses extra buffer*/
+#define ENE_FW1_EXTRA_BUF_HND	0x08	/* extra buffer handshake bit*/
+#define ENE_FW1_LED_ON		0x10	/* turn on a led */
+
+#define ENE_FW1_WPATTERN	0x20	/* enable wake pattern */
 #define ENE_FW1_WAKE		0x40	/* enable wake from S3 */
 #define ENE_FW1_IRQ		0x80	/* enable interrupt */
 
-/* second firmware register */
-#define ENE_FW2			0xF8F9
-#define ENE_FW2_BUF_HIGH	0x01	/* which half of the buffer to read */
-#define ENE_FW2_IRQ_CLR		0x04	/* clear this on IRQ */
-#define ENE_FW2_GP40_AS_LEARN	0x08	/* normal input is used as */
-					/* learning input */
-#define ENE_FW2_FAN_AS_NRML_IN	0x40	/* fan is used as normal input */
+/* second firmware flag register */
+#define ENE_FW2			0xF8F9  /* flagw */
+#define ENE_FW2_BUF_WPTR	0x01	/* which half of the buffer to read */
+#define ENE_FW2_RXIRQ		0x04	/* RX IRQ pending*/
+#define ENE_FW2_GP0A		0x08	/* Use GPIO0A for demodulated input */
+#define ENE_FW2_EMMITER1_CONN	0x10	/* TX emmiter 1 connected */
+#define ENE_FW2_EMMITER2_CONN	0x20	/* TX emmiter 2 connected */
+
+#define ENE_FW2_FAN_INPUT	0x40	/* fan input used for demodulated data*/
 #define ENE_FW2_LEARNING	0x80	/* hardware supports learning and TX */
 
+/* firmware RX pointer for new style buffer */
+#define ENE_FW_RX_POINTER	0xF8FA
+
+/* high parts of samples for fan input (8 samples)*/
+#define ENE_FW_SMPL_BUF_FAN	0xF8FB
+#define ENE_FW_SMPL_BUF_FAN_PLS	0x8000	/* combined sample is pulse */
+#define ENE_FW_SMPL_BUF_FAN_MSK	0x0FFF  /* combined sample maximum value */
+#define ENE_FW_SAMPLE_PERIOD_FAN 61	/* fan input has fixed sample period */
+
 /* transmitter ports */
-#define ENE_TX_PORT2		0xFC01	/* this enables one or both */
-#define ENE_TX_PORT2_EN		0x20	/* TX ports */
-#define ENE_TX_PORT1		0xFC08
-#define ENE_TX_PORT1_EN		0x02
+#define ENE_GPIOFS1		0xFC01
+#define ENE_GPIOFS1_GPIO0D	0x20	/* enable tx output on GPIO0D */
+#define ENE_GPIOFS8		0xFC08
+#define ENE_GPIOFS8_GPIO41	0x02	/* enable tx output on GPIO40 */
 
 /* IRQ registers block (for revision B) */
 #define ENEB_IRQ		0xFD09	/* IRQ number */
@@ -70,97 +77,99 @@
 #define ENEB_IRQ_STATUS		0xFD80	/* irq status */
 #define ENEB_IRQ_STATUS_IR	0x20	/* IR irq */
 
-/* fan as input settings - only if learning capable */
+/* fan as input settings */
 #define ENE_FAN_AS_IN1		0xFE30  /* fan init reg 1 */
 #define ENE_FAN_AS_IN1_EN	0xCD
 #define ENE_FAN_AS_IN2		0xFE31  /* fan init reg 2 */
 #define ENE_FAN_AS_IN2_EN	0x03
-#define ENE_SAMPLE_PERIOD_FAN   61	/* fan input has fixed sample period */
 
 /* IRQ registers block (for revision C,D) */
-#define ENEC_IRQ		0xFE9B	/* new irq settings register */
-#define ENEC_IRQ_MASK		0x0F	/* irq number mask */
-#define ENEC_IRQ_UNK_EN		0x10	/* always enabled */
-#define ENEC_IRQ_STATUS		0x20	/* irq status and ACK */
-
-/* CIR block settings */
-#define ENE_CIR_CONF1		0xFEC0
-#define ENE_CIR_CONF1_TX_CLEAR	0x01	/* clear that on revC */
-					/* while transmitting */
-#define ENE_CIR_CONF1_RX_ON	0x07	/* normal receiver enabled */
-#define ENE_CIR_CONF1_LEARN1	0x08	/* enabled on learning mode */
-#define ENE_CIR_CONF1_TX_ON	0x30	/* enabled on transmit */
-#define ENE_CIR_CONF1_TX_CARR	0x80	/* send TX carrier or not */
-
-#define ENE_CIR_CONF2		0xFEC1	/* unknown setting = 0 */
-#define ENE_CIR_CONF2_LEARN2	0x10	/* set on enable learning */
-#define ENE_CIR_CONF2_GPIO40DIS	0x20	/* disable input via gpio40 */
-
-#define ENE_CIR_SAMPLE_PERIOD	0xFEC8	/* sample period in us */
-#define ENE_CIR_SAMPLE_OVERFLOW	0x80	/* interrupt on overflows if set */
-
-
-/* Two byte tx buffer */
-#define ENE_TX_INPUT1		0xFEC9
-#define ENE_TX_INPUT2		0xFECA
-#define ENE_TX_PULSE_MASK	0x80	/* Transmitted sample is pulse */
-#define ENE_TX_SMLP_MASK	0x7F
-#define ENE_TX_SMPL_PERIOD	50	/* transmit sample period - fixed */
+#define ENE_IRQ			0xFE9B	/* new irq settings register */
+#define ENE_IRQ_MASK		0x0F	/* irq number mask */
+#define ENE_IRQ_UNK_EN		0x10	/* always enabled */
+#define ENE_IRQ_STATUS		0x20	/* irq status and ACK */
+
+/* CIR Config register #1 */
+#define ENE_CIRCFG		0xFEC0
+#define ENE_CIRCFG_RX_EN	0x01	/* RX enable */
+#define ENE_CIRCFG_RX_IRQ	0x02	/* Enable hardware interrupt */
+#define ENE_CIRCFG_REV_POL	0x04	/* Input polarity reversed */
+#define ENE_CIRCFG_CARR_DEMOD	0x08	/* Enable carrier demodulator */
+
+#define ENE_CIRCFG_TX_EN	0x10	/* TX enable */
+#define ENE_CIRCFG_TX_IRQ	0x20	/* Send interrupt on TX done */
+#define ENE_CIRCFG_TX_POL_REV	0x40	/* TX polarity reversed */
+#define ENE_CIRCFG_TX_CARR	0x80	/* send TX carrier or not */
+
+/* CIR config register #2 */
+#define ENE_CIRCFG2		0xFEC1
+#define ENE_CIRCFG2_RLC		0x00
+#define ENE_CIRCFG2_RC5		0x01
+#define ENE_CIRCFG2_RC6		0x02
+#define ENE_CIRCFG2_NEC		0x03
+#define ENE_CIRCFG2_CARR_DETECT	0x10	/* Enable carrier detection */
+#define ENE_CIRCFG2_GPIO0A	0x20	/* Use GPIO0A instead of GPIO40 for input */
+#define ENE_CIRCFG2_FAST_SAMPL1	0x40	/* Fast leading pulse detection for RC6 */
+#define ENE_CIRCFG2_FAST_SAMPL2	0x80	/* Fast data detection for RC6 */
+
+/* Knobs for protocol decoding - will document when/if will use them */
+#define ENE_CIRPF		0xFEC2
+#define ENE_CIRHIGH		0xFEC3
+#define ENE_CIRBIT		0xFEC4
+#define ENE_CIRSTART		0xFEC5
+#define ENE_CIRSTART2		0xFEC6
+
+/* Actual register which contains RLC RX data - read by firmware */
+#define ENE_CIRDAT_IN		0xFEC7
+
+
+/* RLC configuration - sample period (1us resulution) + idle mode */
+#define ENE_CIRRLC_CFG		0xFEC8
+#define ENE_CIRRLC_CFG_OVERFLOW	0x80	/* interrupt on overflows if set */
+#define ENE_DEFAULT_SAMPLE_PERIOD 50
+
+/* Two byte RLC TX buffer */
+#define ENE_CIRRLC_OUT0		0xFEC9
+#define ENE_CIRRLC_OUT1		0xFECA
+#define ENE_CIRRLC_OUT_PULSE	0x80	/* Transmitted sample is pulse */
+#define ENE_CIRRLC_OUT_MASK	0x7F
+
+
+/* Carrier detect setting
+ * Low nibble  - number of carrier pulses to average
+ * High nibble - number of initial carrier pulses to discard
+ */
+#define ENE_CIRCAR_PULS		0xFECB
 
+/* detected RX carrier period (resolution: 500 ns) */
+#define ENE_CIRCAR_PRD		0xFECC
+#define ENE_CIRCAR_PRD_VALID	0x80	/* data valid content valid */
 
-/* Unknown TX setting - TX sample period ??? */
-#define ENE_TX_UNK1		0xFECB	/* set to 0x63 */
+/* detected RX carrier pulse width (resolution: 500 ns) */
+#define ENE_CIRCAR_HPRD		0xFECD
 
-/* Current received carrier period */
-#define ENE_RX_CARRIER		0xFECC	/* RX period (500 ns) */
-#define ENE_RX_CARRIER_VALID	0x80	/* Register content valid */
+/* TX period (resolution: 500 ns, minimum 2)*/
+#define ENE_CIRMOD_PRD		0xFECE
+#define ENE_CIRMOD_PRD_POL	0x80	/* TX carrier polarity*/
 
+#define ENE_CIRMOD_PRD_MAX	0x7F	/* 15.87 kHz */
+#define ENE_CIRMOD_PRD_MIN	0x02	/* 1 Mhz */
 
-/* TX period (1/carrier) */
-#define ENE_TX_PERIOD		0xFECE	/* TX period (500 ns) */
-#define ENE_TX_PERIOD_UNKBIT	0x80	/* This bit set on transmit*/
-#define ENE_TX_PERIOD_PULSE	0xFECF	/* TX pulse period (500 ns)*/
+/* TX pulse width (resolution: 500 ns)*/
+#define ENE_CIRMOD_HPRD		0xFECF
 
 /* Hardware versions */
-#define ENE_HW_VERSION		0xFF00	/* hardware revision */
+#define ENE_ECHV		0xFF00	/* hardware revision */
 #define ENE_PLLFRH		0xFF16
 #define ENE_PLLFRL		0xFF17
+#define ENE_DEFAULT_PLL_FREQ	1000
 
-#define ENE_HW_UNK		0xFF1D
-#define ENE_HW_UNK_CLR		0x04
-#define ENE_HW_VER_MAJOR	0xFF1E	/* chip version */
-#define ENE_HW_VER_MINOR	0xFF1F
-#define ENE_HW_VER_OLD		0xFD00
-
-/* Normal/Learning carrier ranges - only valid if we have learning input*/
-/* TODO: test */
-#define ENE_NORMAL_RX_LOW	34
-#define ENE_NORMAL_RX_HI	38
+#define ENE_ECSTS		0xFF1D
+#define ENE_ECSTS_RSRVD		0x04
 
-/* Tx carrier range */
-/* Hardware might be able to do more, but this range is enough for
-   all purposes */
-#define ENE_TX_PERIOD_MAX	32	/* corresponds to 29.4 kHz */
-#define ENE_TX_PERIOD_MIN	16	/* corrsponds to 62.5 kHz */
-
-
-
-/* Minimal and maximal gaps */
-
-/* Normal case:
-	Minimal gap is 0x7F * sample period
-	Maximum gap depends on hardware.
-	For KB3926B, it is unlimited, for newer models its around
-	250000, after which HW stops sending samples, and that is
-	not possible to change */
-
-/* Fan case:
-	Both minimal and maximal gaps are same, and equal to 0xFFF * 0x61
-	And there is nothing to change this setting
-*/
-
-#define ENE_MAXGAP		250000
-#define ENE_MINGAP		(127 * sample_period)
+#define ENE_ECVER_MAJOR		0xFF1E	/* chip version */
+#define ENE_ECVER_MINOR		0xFF1F
+#define ENE_HW_VER_OLD		0xFD00
 
 /******************************************************************************/
 
@@ -171,46 +180,60 @@
 
 #define  ENE_HW_B		1	/* 3926B */
 #define  ENE_HW_C		2	/* 3926C */
-#define  ENE_HW_D		3	/* 3926D */
+#define  ENE_HW_D		3	/* 3926D or later */
 
 #define ene_printk(level, text, ...) \
-	printk(level ENE_DRIVER_NAME ": " text, ## __VA_ARGS__)
+	printk(level ENE_DRIVER_NAME ": " text "\n", ## __VA_ARGS__)
 
-#define ene_dbg(text, ...) \
-	if (debug) \
-		printk(KERN_DEBUG \
-			ENE_DRIVER_NAME ": " text "\n" , ## __VA_ARGS__)
+#define ene_notice(text, ...) ene_printk(KERN_NOTICE, text, ## __VA_ARGS__)
+#define ene_warn(text, ...) ene_printk(KERN_WARNING, text, ## __VA_ARGS__)
 
-#define ene_dbg_verbose(text, ...) \
-	if (debug > 1) \
-		printk(KERN_DEBUG \
-			ENE_DRIVER_NAME ": " text "\n" , ## __VA_ARGS__)
 
+#define __dbg(level, format, ...) \
+	do { \
+		if (debug >= level) \
+			printk(KERN_DEBUG ENE_DRIVER_NAME \
+				": " format "\n", ## __VA_ARGS__); \
+	} while (0)
+
+
+#define dbg(format, ...)		__dbg(1, format, ## __VA_ARGS__)
+#define dbg_verbose(format, ...)	__dbg(2, format, ## __VA_ARGS__)
+#define dbg_regs(format, ...)		__dbg(3, format, ## __VA_ARGS__)
+
+#define MS_TO_NS(msec) ((msec) * 1000)
 
 struct ene_device {
 	struct pnp_dev *pnp_dev;
 	struct input_dev *idev;
 	struct ir_dev_props *props;
-	int in_use;
 
 	/* hw IO settings */
-	unsigned long hw_io;
+	long hw_io;
 	int irq;
 	spinlock_t hw_lock;
 
 	/* HW features */
 	int hw_revision;			/* hardware revision */
-	bool hw_learning_and_tx_capable;	/* learning capable */
-	bool hw_gpio40_learning;		/* gpio40 is learning */
-	bool hw_fan_as_normal_input;		/* fan input is used as */
-						/* regular input */
+	bool hw_use_gpio_0a;			/* gpio40 is demodulated input*/
+	bool hw_extra_buffer;			/* hardware has 'extra buffer' */
+	bool hw_fan_input;			/* fan input is IR data source */
+	bool hw_learning_and_tx_capable;	/* learning & tx capable */
+	int  pll_freq;
+	int buffer_len;
+
+	/* Extra RX buffer location */
+	int extra_buf1_address;
+	int extra_buf1_len;
+	int extra_buf2_address;
+	int extra_buf2_len;
+
 	/* HW state*/
-	int rx_pointer;				/* hw pointer to rx buffer */
+	int r_pointer;				/* pointer to next sample to read */
+	int w_pointer;				/* pointer to next sample hw will write */
 	bool rx_fan_input_inuse;		/* is fan input in use for rx*/
 	int tx_reg;				/* current reg used for TX */
 	u8  saved_conf1;			/* saved FEC0 reg */
-
-	/* TX sample handling */
 	unsigned int tx_sample;			/* current sample for TX */
 	bool tx_sample_pulse;			/* current sample is pulse */
 
@@ -232,4 +255,8 @@ struct ene_device {
 	bool learning_enabled;			/* learning input enabled */
 	bool carrier_detect_enabled;		/* carrier detect enabled */
 	int rx_period_adjust;
+	bool rx_enabled;
 };
+
+static int ene_irq_status(struct ene_device *dev);
+static void ene_read_hw_pointer(struct ene_device *dev);
-- 
1.7.0.4

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

[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 <maximlevitsky@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)
+			return 0;
+
+		sample = LIRC_TIMEOUT(ev.duration / 1000);
 
-	sample = ev.duration / 1000;
-	if (ev.pulse)
-		sample |= PULSE_BIT;
+	/* Normal sample */
+	} else {
+
+		if (lirc->gap) {
+			int gap_sample;
+
+			lirc->gap_duration += ktime_to_ns(ktime_sub(ktime_get(),
+				lirc->gap_start));
+
+			/* Convert to ms and cap by LIRC_VALUE_MASK */
+			do_div(lirc->gap_duration, 1000);
+			lirc->gap_duration = min(lirc->gap_duration,
+							(u64)LIRC_VALUE_MASK);
+
+			gap_sample = LIRC_SPACE(lirc->gap_duration);
+			lirc_buffer_write(ir_dev->raw->lirc.drv->rbuf,
+						(unsigned char *) &gap_sample);
+			lirc->gap = false;
+		}
+
+		sample = ev.pulse ? LIRC_PULSE(ev.duration / 1000) :
+					LIRC_SPACE(ev.duration / 1000);
+	}
 
 	lirc_buffer_write(ir_dev->raw->lirc.drv->rbuf,
 			  (unsigned char *) &sample);
 	wake_up(&ir_dev->raw->lirc.drv->rbuf->wait_poll);
 
-
 	return 0;
 }
 
@@ -103,6 +140,7 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd,
 	int ret = 0;
 	void *drv_data;
 	unsigned long val = 0;
+	u32 tmp;
 
 	lirc = lirc_get_pdata(filep);
 	if (!lirc)
@@ -201,12 +239,26 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd,
 		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 * 1000;
+		tmp = val * 1000;
+
+		if (tmp < ir_dev->props->min_timeout ||
+			tmp > ir_dev->props->max_timeout)
+				return -EINVAL;
+
+		ir_dev->props->timeout = tmp;
+		break;
+
+	case LIRC_SET_REC_TIMEOUT_REPORTS:
+		lirc->send_timeout_reports = !!val;
 		break;
 
+	case LIRC_SET_MEASURE_CARRIER_MODE:
+
+		if (!ir_dev->props->s_carrier_report)
+			return -ENOSYS;
+		return ir_dev->props->s_carrier_report(
+			ir_dev->props->priv, !!val);
+
 	default:
 		return lirc_dev_fop_ioctl(filep, cmd, arg);
 	}
@@ -277,6 +329,10 @@ static int ir_lirc_register(struct input_dev *input_dev)
 	if (ir_dev->props->s_learning_mode)
 		features |= LIRC_CAN_USE_WIDEBAND_RECEIVER;
 
+	if (ir_dev->props->s_carrier_report)
+		features |= LIRC_CAN_MEASURE_CARRIER;
+
+
 	if (ir_dev->props->max_timeout)
 		features |= LIRC_CAN_SET_REC_TIMEOUT;
 
diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
index d597421..70993f7 100644
--- a/drivers/media/IR/ir-nec-decoder.c
+++ b/drivers/media/IR/ir-nec-decoder.c
@@ -54,8 +54,9 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_NEC))
 		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-raw-event.c b/drivers/media/IR/ir-raw-event.c
index 56797be..d10b8e0 100644
--- a/drivers/media/IR/ir-raw-event.c
+++ b/drivers/media/IR/ir-raw-event.c
@@ -174,7 +174,7 @@ int ir_raw_event_store_with_filter(struct input_dev *input_dev,
 	if (ir->idle && !ev->pulse)
 		return 0;
 	else if (ir->idle)
-		ir_raw_event_set_idle(input_dev, 0);
+		ir_raw_event_set_idle(input_dev, false);
 
 	if (!raw->this_ev.duration) {
 		raw->this_ev = *ev;
@@ -187,48 +187,35 @@ int ir_raw_event_store_with_filter(struct input_dev *input_dev,
 
 	/* Enter idle mode if nessesary */
 	if (!ev->pulse && ir->props->timeout &&
-		raw->this_ev.duration >= ir->props->timeout)
-		ir_raw_event_set_idle(input_dev, 1);
+		raw->this_ev.duration >= ir->props->timeout) {
+		ir_raw_event_set_idle(input_dev, true);
+	}
 	return 0;
 }
 EXPORT_SYMBOL_GPL(ir_raw_event_store_with_filter);
 
-void ir_raw_event_set_idle(struct input_dev *input_dev, int idle)
+/**
+ * ir_raw_event_set_idle() - hint the ir core if device is receiving
+ * IR data or not
+ * @input_dev: the struct input_dev device descriptor
+ * @idle: the hint value
+ */
+void ir_raw_event_set_idle(struct input_dev *input_dev, bool idle)
 {
 	struct ir_input_dev *ir = input_get_drvdata(input_dev);
 	struct ir_raw_event_ctrl *raw = ir->raw;
-	ktime_t now;
-	u64 delta;
 
-	if (!ir->props)
+	if (!ir->props || !ir->raw)
 		return;
 
-	if (!ir->raw)
-		goto out;
+	IR_dprintk(2, "%s idle mode\n", idle ? "enter" : "leave");
 
 	if (idle) {
-		IR_dprintk(2, "enter idle mode\n");
-		raw->last_event = ktime_get();
-	} else {
-		IR_dprintk(2, "exit idle mode\n");
-
-		now = ktime_get();
-		delta = ktime_to_ns(ktime_sub(now, ir->raw->last_event));
-
-		WARN_ON(raw->this_ev.pulse);
-
-		raw->this_ev.duration =
-			min(raw->this_ev.duration + delta,
-						(u64)IR_MAX_DURATION);
-
+		raw->this_ev.timeout = true;
 		ir_raw_event_store(input_dev, &raw->this_ev);
-
-		if (raw->this_ev.duration == IR_MAX_DURATION)
-			ir_raw_event_reset(input_dev);
-
-		raw->this_ev.duration = 0;
+		init_ir_raw_event(&raw->this_ev);
 	}
-out:
+
 	if (ir->props->s_idle)
 		ir->props->s_idle(ir->props->priv, idle);
 	ir->idle = idle;
diff --git a/drivers/media/IR/ir-rc5-decoder.c b/drivers/media/IR/ir-rc5-decoder.c
index df4770d..572ed4c 100644
--- a/drivers/media/IR/ir-rc5-decoder.c
+++ b/drivers/media/IR/ir-rc5-decoder.c
@@ -55,8 +55,9 @@ static int ir_rc5_decode(struct input_dev *input_dev, struct ir_raw_event ev)
         if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC5))
                 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-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)
+			data->state = STATE_INACTIVE;
 		return 0;
 	}
 
diff --git a/drivers/media/IR/ir-sony-decoder.c b/drivers/media/IR/ir-sony-decoder.c
index b9074f0..2d15730 100644
--- a/drivers/media/IR/ir-sony-decoder.c
+++ b/drivers/media/IR/ir-sony-decoder.c
@@ -48,8 +48,9 @@ static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
 	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_SONY))
 		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/mceusb.c b/drivers/media/IR/mceusb.c
index ac6bb2c..d531f9f 100644
--- a/drivers/media/IR/mceusb.c
+++ b/drivers/media/IR/mceusb.c
@@ -656,7 +656,7 @@ static int mceusb_set_tx_carrier(void *priv, u32 carrier)
 
 static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
 {
-	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
+	DEFINE_IR_RAW_EVENT(rawir);
 	int i, start_index = 0;
 	u8 hdr = MCE_CONTROL_HEADER;
 
@@ -993,6 +993,7 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
 	ir->len_in = maxp;
 	ir->flags.microsoft_gen1 = is_microsoft_gen1;
 	ir->flags.tx_mask_inverted = tx_mask_inverted;
+	init_ir_raw_event(&ir->rawir);
 
 	/* Saving usb interface data for use by the transmitter routine */
 	ir->usb_ep_in = ep_in;
diff --git a/drivers/media/IR/streamzap.c b/drivers/media/IR/streamzap.c
index 058e29f..18be3d5 100644
--- a/drivers/media/IR/streamzap.c
+++ b/drivers/media/IR/streamzap.c
@@ -170,7 +170,7 @@ static void streamzap_flush_timeout(unsigned long arg)
 static void streamzap_delay_timeout(unsigned long arg)
 {
 	struct streamzap_ir *sz = (struct streamzap_ir *)arg;
-	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
+	DEFINE_IR_RAW_EVENT(rawir);
 	unsigned long flags;
 	int len, ret;
 	static unsigned long delay;
@@ -215,7 +215,7 @@ static void streamzap_delay_timeout(unsigned long arg)
 
 static void streamzap_flush_delay_buffer(struct streamzap_ir *sz)
 {
-	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
+	DEFINE_IR_RAW_EVENT(rawir);
 	bool wake = false;
 	int ret;
 
@@ -233,7 +233,7 @@ static void streamzap_flush_delay_buffer(struct streamzap_ir *sz)
 
 static void sz_push(struct streamzap_ir *sz)
 {
-	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
+	DEFINE_IR_RAW_EVENT(rawir);
 	unsigned long flags;
 	int ret;
 
@@ -512,6 +512,8 @@ static int __devinit streamzap_probe(struct usb_interface *intf,
 	if (!sz)
 		return -ENOMEM;
 
+	init_ir_raw_event(&sz->rawir);
+
 	sz->usbdev = usbdev;
 	sz->interface = intf;
 
diff --git a/include/media/ir-core.h b/include/media/ir-core.h
index eb7fddf..d88ce2b 100644
--- a/include/media/ir-core.h
+++ b/include/media/ir-core.h
@@ -60,6 +60,7 @@ enum rc_driver_type {
  * @s_idle: optional: enable/disable hardware idle mode, upon which,
 	device doesn't interrupt host until it sees IR pulses
  * @s_learning_mode: enable wide band receiver used for learning
+ * @s_carrier_report: enable carrier reports
  */
 struct ir_dev_props {
 	enum rc_driver_type	driver_type;
@@ -82,8 +83,9 @@ struct ir_dev_props {
 	int			(*s_tx_duty_cycle)(void *priv, u32 duty_cycle);
 	int			(*s_rx_carrier_range)(void *priv, u32 min, u32 max);
 	int			(*tx_ir)(void *priv, int *txbuf, u32 n);
-	void			(*s_idle)(void *priv, int enable);
+	void			(*s_idle)(void *priv, bool enable);
 	int			(*s_learning_mode)(void *priv, int enable);
+	int			(*s_carrier_report) (void *priv, int enable);
 };
 
 struct ir_input_dev {
@@ -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;
 };
 
-#define IR_MAX_DURATION                 0x7FFFFFFF      /* a bit more than 2 seconds */
+#define DEFINE_IR_RAW_EVENT(event) \
+	struct ir_raw_event event = { \
+		{ .duration = 0 } , \
+		.pulse = 0, \
+		.reset = 0, \
+		.timeout = 0, \
+		.carrier_report = 0 }
+
+static inline void init_ir_raw_event(struct ir_raw_event *ev)
+{
+	memset(ev, 0, sizeof(*ev));
+}
+
+#define IR_MAX_DURATION         0xFFFFFFFF      /* a bit more than 4 seconds */
 
 void ir_raw_event_handle(struct input_dev *input_dev);
 int ir_raw_event_store(struct input_dev *input_dev, struct ir_raw_event *ev);
 int ir_raw_event_store_edge(struct input_dev *input_dev, enum raw_event_type type);
 int ir_raw_event_store_with_filter(struct input_dev *input_dev,
 				struct ir_raw_event *ev);
-void ir_raw_event_set_idle(struct input_dev *input_dev, int idle);
+void ir_raw_event_set_idle(struct input_dev *input_dev, bool idle);
 
 static inline void ir_raw_event_reset(struct input_dev *input_dev)
 {
-	struct ir_raw_event ev = { .pulse = false, .duration = 0 };
+	DEFINE_IR_RAW_EVENT(ev);
+	ev.reset = true;
+
 	ir_raw_event_store(input_dev, &ev);
 	ir_raw_event_handle(input_dev);
 }
-- 
1.7.0.4

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

[Jarod Wilson]

On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@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 <maximlevitsky@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
jarod@wilsonet.com

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

[Jarod Wilson]

On Mon, Sep 6, 2010 at 5:26 PM, Maxim Levitsky <maximlevitsky@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 <maximlevitsky@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
jarod@wilsonet.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@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

[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 <maximlevitsky@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 <maximlevitsky@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.

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

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

[Jarod Wilson]

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 <maximlevitsky@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 <maximlevitsky@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.
> 
> 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.

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.

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

-- 
Jarod Wilson
jarod@redhat.com

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

[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 <maximlevitsky@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

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

[David Härdeman]

On Wed, Sep 08, 2010 at 07:02:33PM -0400, Andy Walls wrote:
> 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 <maximlevitsky@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:

Not to mention that the use of a union sends an important message to the 
programmer reading the code - i.e. that only one of the union members 
can be used in the same event.


-- 
David Härdeman

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

[David Härdeman]

On Wed, Sep 08, 2010 at 07:02:33PM -0400, Andy Walls wrote:
> 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 <maximlevitsky@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:

Not to mention that the use of a union sends an important message to the 
programmer reading the code - i.e. that only one of the union members 
can be used in the same event.


-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@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

[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

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

[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 <maximlevitsky@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)
> +			return 0;
> +
> +		sample = LIRC_TIMEOUT(ev.duration / 1000);
>  
> -	sample = ev.duration / 1000;
> -	if (ev.pulse)
> -		sample |= PULSE_BIT;
> +	/* Normal sample */
> +	} else {
> +
> +		if (lirc->gap) {
> +			int gap_sample;
> +
> +			lirc->gap_duration += ktime_to_ns(ktime_sub(ktime_get(),
> +				lirc->gap_start));
> +
> +			/* Convert to ms and cap by LIRC_VALUE_MASK */
> +			do_div(lirc->gap_duration, 1000);
> +			lirc->gap_duration = min(lirc->gap_duration,
> +							(u64)LIRC_VALUE_MASK);
> +
> +			gap_sample = LIRC_SPACE(lirc->gap_duration);
> +			lirc_buffer_write(ir_dev->raw->lirc.drv->rbuf,
> +						(unsigned char *) &gap_sample);
> +			lirc->gap = false;
> +		}
> +
> +		sample = ev.pulse ? LIRC_PULSE(ev.duration / 1000) :
> +					LIRC_SPACE(ev.duration / 1000);
> +	}
>  
>  	lirc_buffer_write(ir_dev->raw->lirc.drv->rbuf,
>  			  (unsigned char *) &sample);
>  	wake_up(&ir_dev->raw->lirc.drv->rbuf->wait_poll);
>  
> -
>  	return 0;
>  }
>  
> @@ -103,6 +140,7 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd,
>  	int ret = 0;
>  	void *drv_data;
>  	unsigned long val = 0;
> +	u32 tmp;
>  
>  	lirc = lirc_get_pdata(filep);
>  	if (!lirc)
> @@ -201,12 +239,26 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd,
>  		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 * 1000;
> +		tmp = val * 1000;
> +
> +		if (tmp < ir_dev->props->min_timeout ||
> +			tmp > ir_dev->props->max_timeout)
> +				return -EINVAL;
> +
> +		ir_dev->props->timeout = tmp;
> +		break;
> +
> +	case LIRC_SET_REC_TIMEOUT_REPORTS:
> +		lirc->send_timeout_reports = !!val;
>  		break;
>  
> +	case LIRC_SET_MEASURE_CARRIER_MODE:
> +
> +		if (!ir_dev->props->s_carrier_report)
> +			return -ENOSYS;
> +		return ir_dev->props->s_carrier_report(
> +			ir_dev->props->priv, !!val);
> +
>  	default:
>  		return lirc_dev_fop_ioctl(filep, cmd, arg);
>  	}
> @@ -277,6 +329,10 @@ static int ir_lirc_register(struct input_dev *input_dev)
>  	if (ir_dev->props->s_learning_mode)
>  		features |= LIRC_CAN_USE_WIDEBAND_RECEIVER;
>  
> +	if (ir_dev->props->s_carrier_report)
> +		features |= LIRC_CAN_MEASURE_CARRIER;
> +
> +
>  	if (ir_dev->props->max_timeout)
>  		features |= LIRC_CAN_SET_REC_TIMEOUT;
>  
> diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
> index d597421..70993f7 100644
> --- a/drivers/media/IR/ir-nec-decoder.c
> +++ b/drivers/media/IR/ir-nec-decoder.c
> @@ -54,8 +54,9 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
>  	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_NEC))
>  		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-raw-event.c b/drivers/media/IR/ir-raw-event.c
> index 56797be..d10b8e0 100644
> --- a/drivers/media/IR/ir-raw-event.c
> +++ b/drivers/media/IR/ir-raw-event.c
> @@ -174,7 +174,7 @@ int ir_raw_event_store_with_filter(struct input_dev *input_dev,
>  	if (ir->idle && !ev->pulse)
>  		return 0;
>  	else if (ir->idle)
> -		ir_raw_event_set_idle(input_dev, 0);
> +		ir_raw_event_set_idle(input_dev, false);
>  
>  	if (!raw->this_ev.duration) {
>  		raw->this_ev = *ev;
> @@ -187,48 +187,35 @@ int ir_raw_event_store_with_filter(struct input_dev *input_dev,
>  
>  	/* Enter idle mode if nessesary */
>  	if (!ev->pulse && ir->props->timeout &&
> -		raw->this_ev.duration >= ir->props->timeout)
> -		ir_raw_event_set_idle(input_dev, 1);
> +		raw->this_ev.duration >= ir->props->timeout) {
> +		ir_raw_event_set_idle(input_dev, true);
> +	}
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(ir_raw_event_store_with_filter);
>  
> -void ir_raw_event_set_idle(struct input_dev *input_dev, int idle)
> +/**
> + * ir_raw_event_set_idle() - hint the ir core if device is receiving
> + * IR data or not
> + * @input_dev: the struct input_dev device descriptor
> + * @idle: the hint value
> + */
> +void ir_raw_event_set_idle(struct input_dev *input_dev, bool idle)
>  {
>  	struct ir_input_dev *ir = input_get_drvdata(input_dev);
>  	struct ir_raw_event_ctrl *raw = ir->raw;
> -	ktime_t now;
> -	u64 delta;
>  
> -	if (!ir->props)
> +	if (!ir->props || !ir->raw)
>  		return;
>  
> -	if (!ir->raw)
> -		goto out;
> +	IR_dprintk(2, "%s idle mode\n", idle ? "enter" : "leave");
>  
>  	if (idle) {
> -		IR_dprintk(2, "enter idle mode\n");
> -		raw->last_event = ktime_get();
> -	} else {
> -		IR_dprintk(2, "exit idle mode\n");
> -
> -		now = ktime_get();
> -		delta = ktime_to_ns(ktime_sub(now, ir->raw->last_event));
> -
> -		WARN_ON(raw->this_ev.pulse);
> -
> -		raw->this_ev.duration =
> -			min(raw->this_ev.duration + delta,
> -						(u64)IR_MAX_DURATION);
> -
> +		raw->this_ev.timeout = true;
>  		ir_raw_event_store(input_dev, &raw->this_ev);
> -
> -		if (raw->this_ev.duration == IR_MAX_DURATION)
> -			ir_raw_event_reset(input_dev);
> -
> -		raw->this_ev.duration = 0;
> +		init_ir_raw_event(&raw->this_ev);
>  	}
> -out:
> +
>  	if (ir->props->s_idle)
>  		ir->props->s_idle(ir->props->priv, idle);
>  	ir->idle = idle;
> diff --git a/drivers/media/IR/ir-rc5-decoder.c b/drivers/media/IR/ir-rc5-decoder.c
> index df4770d..572ed4c 100644
> --- a/drivers/media/IR/ir-rc5-decoder.c
> +++ b/drivers/media/IR/ir-rc5-decoder.c
> @@ -55,8 +55,9 @@ static int ir_rc5_decode(struct input_dev *input_dev, struct ir_raw_event ev)
>          if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC5))
>                  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-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()?

> +			data->state = STATE_INACTIVE;
>  		return 0;
>  	}
>  
> diff --git a/drivers/media/IR/ir-sony-decoder.c b/drivers/media/IR/ir-sony-decoder.c
> index b9074f0..2d15730 100644
> --- a/drivers/media/IR/ir-sony-decoder.c
> +++ b/drivers/media/IR/ir-sony-decoder.c
> @@ -48,8 +48,9 @@ static int ir_sony_decode(struct input_dev *input_dev, struct ir_raw_event ev)
>  	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_SONY))
>  		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/mceusb.c b/drivers/media/IR/mceusb.c
> index ac6bb2c..d531f9f 100644
> --- a/drivers/media/IR/mceusb.c
> +++ b/drivers/media/IR/mceusb.c
> @@ -656,7 +656,7 @@ static int mceusb_set_tx_carrier(void *priv, u32 carrier)
>  
>  static void mceusb_process_ir_data(struct mceusb_dev *ir, int buf_len)
>  {
> -	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
> +	DEFINE_IR_RAW_EVENT(rawir);
>  	int i, start_index = 0;
>  	u8 hdr = MCE_CONTROL_HEADER;
>  
> @@ -993,6 +993,7 @@ static int __devinit mceusb_dev_probe(struct usb_interface *intf,
>  	ir->len_in = maxp;
>  	ir->flags.microsoft_gen1 = is_microsoft_gen1;
>  	ir->flags.tx_mask_inverted = tx_mask_inverted;
> +	init_ir_raw_event(&ir->rawir);
>  
>  	/* Saving usb interface data for use by the transmitter routine */
>  	ir->usb_ep_in = ep_in;
> diff --git a/drivers/media/IR/streamzap.c b/drivers/media/IR/streamzap.c
> index 058e29f..18be3d5 100644
> --- a/drivers/media/IR/streamzap.c
> +++ b/drivers/media/IR/streamzap.c
> @@ -170,7 +170,7 @@ static void streamzap_flush_timeout(unsigned long arg)
>  static void streamzap_delay_timeout(unsigned long arg)
>  {
>  	struct streamzap_ir *sz = (struct streamzap_ir *)arg;
> -	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
> +	DEFINE_IR_RAW_EVENT(rawir);
>  	unsigned long flags;
>  	int len, ret;
>  	static unsigned long delay;
> @@ -215,7 +215,7 @@ static void streamzap_delay_timeout(unsigned long arg)
>  
>  static void streamzap_flush_delay_buffer(struct streamzap_ir *sz)
>  {
> -	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
> +	DEFINE_IR_RAW_EVENT(rawir);
>  	bool wake = false;
>  	int ret;
>  
> @@ -233,7 +233,7 @@ static void streamzap_flush_delay_buffer(struct streamzap_ir *sz)
>  
>  static void sz_push(struct streamzap_ir *sz)
>  {
> -	struct ir_raw_event rawir = { .pulse = false, .duration = 0 };
> +	DEFINE_IR_RAW_EVENT(rawir);
>  	unsigned long flags;
>  	int ret;
>  
> @@ -512,6 +512,8 @@ static int __devinit streamzap_probe(struct usb_interface *intf,
>  	if (!sz)
>  		return -ENOMEM;
>  
> +	init_ir_raw_event(&sz->rawir);
> +
>  	sz->usbdev = usbdev;
>  	sz->interface = intf;
>  
> diff --git a/include/media/ir-core.h b/include/media/ir-core.h
> index eb7fddf..d88ce2b 100644
> --- a/include/media/ir-core.h
> +++ b/include/media/ir-core.h
> @@ -60,6 +60,7 @@ enum rc_driver_type {
>   * @s_idle: optional: enable/disable hardware idle mode, upon which,
>  	device doesn't interrupt host until it sees IR pulses
>   * @s_learning_mode: enable wide band receiver used for learning
> + * @s_carrier_report: enable carrier reports
>   */
>  struct ir_dev_props {
>  	enum rc_driver_type	driver_type;
> @@ -82,8 +83,9 @@ struct ir_dev_props {
>  	int			(*s_tx_duty_cycle)(void *priv, u32 duty_cycle);
>  	int			(*s_rx_carrier_range)(void *priv, u32 min, u32 max);
>  	int			(*tx_ir)(void *priv, int *txbuf, u32 n);
> -	void			(*s_idle)(void *priv, int enable);
> +	void			(*s_idle)(void *priv, bool enable);
>  	int			(*s_learning_mode)(void *priv, int enable);
> +	int			(*s_carrier_report) (void *priv, int enable);
>  };
>  
>  struct ir_input_dev {
> @@ -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;
>  };

Hmm... can an event have, simultaneously, more than one of those bits set? I don't think so.
You could, instead, define the event type as:

#define EV_SPACE	0
#define EV_PULSE	1
#define EV_RESET	2
#define EV_TIMEOUT	3
#define EV_CARRIER	4

unsigned ev_type:3;

And use those macros for the tests:

#define IS_TIMING_EVENT(ev) (ev < EV_RESET)
#define IS_RESET(ev)	    (ev == EV_SPACE)


Also, it is not clear at the data struct that, when carrier_report = 1, it should use the
carrier struct, instead of duration.

My suggestion is to use, instead:

struct ir_raw_event {
	#define EV_SPACE	0
	#define EV_PULSE	1
	#define EV_RESET	2
	#define EV_TIMEOUT	3
	#define EV_CARRIER	4

	unsigned ev_type:3;

	union {
		/* For EV_SPACE and EV_PULSE type of events */
		u32             duration;

		/* For EV_CARRIER type of events */
		struct {
			u32     carrier;
			u8      duty_cycle;
		};
	};

} __attribute__ ((packed));

Btw, as your code is changing the size of the events, it may be a good idea to 
use a larger buffer. For example, defining MAX_IR_EVENT_SIZE as something like

#define MAX_IR_EVENTS_PER_BUF	512

#define MAX_IR_EVENT_SIZE     roundup_pow_of_two((MAX_IR_EVENTS_PER_BUF * sizeof(struct ir_raw_event)))

PS.: I'm not saying that 512 events for the buffer are ok or not. The buffer should be big enough
to at least handle all events generated by one keystroke, as some drivers, like sms, only call the
handler function after receiving all the events for one keystroke, but just leaving 512 without
making it dependent of the size of the struct seems dangerous.

> -#define IR_MAX_DURATION                 0x7FFFFFFF      /* a bit more than 2 seconds */
> +#define DEFINE_IR_RAW_EVENT(event) \
> +	struct ir_raw_event event = { \
> +		{ .duration = 0 } , \
> +		.pulse = 0, \
> +		.reset = 0, \
> +		.timeout = 0, \
> +		.carrier_report = 0 }
> +
> +static inline void init_ir_raw_event(struct ir_raw_event *ev)
> +{
> +	memset(ev, 0, sizeof(*ev));
> +}
> +
> +#define IR_MAX_DURATION         0xFFFFFFFF      /* a bit more than 4 seconds */
>  
>  void ir_raw_event_handle(struct input_dev *input_dev);
>  int ir_raw_event_store(struct input_dev *input_dev, struct ir_raw_event *ev);
>  int ir_raw_event_store_edge(struct input_dev *input_dev, enum raw_event_type type);
>  int ir_raw_event_store_with_filter(struct input_dev *input_dev,
>  				struct ir_raw_event *ev);
> -void ir_raw_event_set_idle(struct input_dev *input_dev, int idle);
> +void ir_raw_event_set_idle(struct input_dev *input_dev, bool idle);
>  
>  static inline void ir_raw_event_reset(struct input_dev *input_dev)
>  {
> -	struct ir_raw_event ev = { .pulse = false, .duration = 0 };
> +	DEFINE_IR_RAW_EVENT(ev);
> +	ev.reset = true;
> +
>  	ir_raw_event_store(input_dev, &ev);
>  	ir_raw_event_handle(input_dev);
>  }

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

[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 <maximlevitsky@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)
> > +			return 0;
> > +
> > +		sample = LIRC_TIMEOUT(ev.duration / 1000);
> >  
> > -	sample = ev.duration / 1000;
> > -	if (ev.pulse)
> > -		sample |= PULSE_BIT;
> > +	/* Normal sample */
> > +	} else {
> > +
> > +		if (lirc->gap) {
> > +			int gap_sample;
> > +
> > +			lirc->gap_duration += ktime_to_ns(ktime_sub(ktime_get(),
> > +				lirc->gap_start));
> > +
> > +			/* Convert to ms and cap by LIRC_VALUE_MASK */
> > +			do_div(lirc->gap_duration, 1000);
> > +			lirc->gap_duration = min(lirc->gap_duration,
> > +							(u64)LIRC_VALUE_MASK);
> > +
> > +			gap_sample = LIRC_SPACE(lirc->gap_duration);
> > +			lirc_buffer_write(ir_dev->raw->lirc.drv->rbuf,
> > +						(unsigned char *) &gap_sample);
> > +			lirc->gap = false;
> > +		}
> > +
> > +		sample = ev.pulse ? LIRC_PULSE(ev.duration / 1000) :
> > +					LIRC_SPACE(ev.duration / 1000);
> > +	}
> >  
> >  	lirc_buffer_write(ir_dev->raw->lirc.drv->rbuf,
> >  			  (unsigned char *) &sample);
> >  	wake_up(&ir_dev->raw->lirc.drv->rbuf->wait_poll);
> >  
> > -
> >  	return 0;
> >  }
> >  
> > @@ -103,6 +140,7 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd,
> >  	int ret = 0;
> >  	void *drv_data;
> >  	unsigned long val = 0;
> > +	u32 tmp;
> >  
> >  	lirc = lirc_get_pdata(filep);
> >  	if (!lirc)
> > @@ -201,12 +239,26 @@ static long ir_lirc_ioctl(struct file *filep, unsigned int cmd,
> >  		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 * 1000;
> > +		tmp = val * 1000;
> > +
> > +		if (tmp < ir_dev->props->min_timeout ||
> > +			tmp > ir_dev->props->max_timeout)
> > +				return -EINVAL;
> > +
> > +		ir_dev->props->timeout = tmp;
> > +		break;
> > +
> > +	case LIRC_SET_REC_TIMEOUT_REPORTS:
> > +		lirc->send_timeout_reports = !!val;
> >  		break;
> >  
> > +	case LIRC_SET_MEASURE_CARRIER_MODE:
> > +
> > +		if (!ir_dev->props->s_carrier_report)
> > +			return -ENOSYS;
> > +		return ir_dev->props->s_carrier_report(
> > +			ir_dev->props->priv, !!val);
> > +
> >  	default:
> >  		return lirc_dev_fop_ioctl(filep, cmd, arg);
> >  	}
> > @@ -277,6 +329,10 @@ static int ir_lirc_register(struct input_dev *input_dev)
> >  	if (ir_dev->props->s_learning_mode)
> >  		features |= LIRC_CAN_USE_WIDEBAND_RECEIVER;
> >  
> > +	if (ir_dev->props->s_carrier_report)
> > +		features |= LIRC_CAN_MEASURE_CARRIER;
> > +
> > +
> >  	if (ir_dev->props->max_timeout)
> >  		features |= LIRC_CAN_SET_REC_TIMEOUT;
> >  
> > diff --git a/drivers/media/IR/ir-nec-decoder.c b/drivers/media/IR/ir-nec-decoder.c
> > index d597421..70993f7 100644
> > --- a/drivers/media/IR/ir-nec-decoder.c
> > +++ b/drivers/media/IR/ir-nec-decoder.c
> > @@ -54,8 +54,9 @@ static int ir_nec_decode(struct input_dev *input_dev, struct ir_raw_event ev)
> >  	if (!(ir_dev->raw->enabled_protocols & IR_TYPE_NEC))
> >  		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-raw-event.c b/drivers/media/IR/ir-raw-event.c
> > index 56797be..d10b8e0 100644
> > --- a/drivers/media/IR/ir-raw-event.c
> > +++ b/drivers/media/IR/ir-raw-event.c
> > @@ -174,7 +174,7 @@ int ir_raw_event_store_with_filter(struct input_dev *input_dev,
> >  	if (ir->idle && !ev->pulse)
> >  		return 0;
> >  	else if (ir->idle)
> > -		ir_raw_event_set_idle(input_dev, 0);
> > +		ir_raw_event_set_idle(input_dev, false);
> >  
> >  	if (!raw->this_ev.duration) {
> >  		raw->this_ev = *ev;
> > @@ -187,48 +187,35 @@ int ir_raw_event_store_with_filter(struct input_dev *input_dev,
> >  
> >  	/* Enter idle mode if nessesary */
> >  	if (!ev->pulse && ir->props->timeout &&
> > -		raw->this_ev.duration >= ir->props->timeout)
> > -		ir_raw_event_set_idle(input_dev, 1);
> > +		raw->this_ev.duration >= ir->props->timeout) {
> > +		ir_raw_event_set_idle(input_dev, true);
> > +	}
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(ir_raw_event_store_with_filter);
> >  
> > -void ir_raw_event_set_idle(struct input_dev *input_dev, int idle)
> > +/**
> > + * ir_raw_event_set_idle() - hint the ir core if device is receiving
> > + * IR data or not
> > + * @input_dev: the struct input_dev device descriptor
> > + * @idle: the hint value
> > + */
> > +void ir_raw_event_set_idle(struct input_dev *input_dev, bool idle)
> >  {
> >  	struct ir_input_dev *ir = input_get_drvdata(input_dev);
> >  	struct ir_raw_event_ctrl *raw = ir->raw;
> > -	ktime_t now;
> > -	u64 delta;
> >  
> > -	if (!ir->props)
> > +	if (!ir->props || !ir->raw)
> >  		return;
> >  
> > -	if (!ir->raw)
> > -		goto out;
> > +	IR_dprintk(2, "%s idle mode\n", idle ? "enter" : "leave");
> >  
> >  	if (idle) {
> > -		IR_dprintk(2, "enter idle mode\n");
> > -		raw->last_event = ktime_get();
> > -	} else {
> > -		IR_dprintk(2, "exit idle mode\n");
> > -
> > -		now = ktime_get();
> > -		delta = ktime_to_ns(ktime_sub(now, ir->raw->last_event));
> > -
> > -		WARN_ON(raw->this_ev.pulse);
> > -
> > -		raw->this_ev.duration =
> > -			min(raw->this_ev.duration + delta,
> > -						(u64)IR_MAX_DURATION);
> > -
> > +		raw->this_ev.timeout = true;
> >  		ir_raw_event_store(input_dev, &raw->this_ev);
> > -
> > -		if (raw->this_ev.duration == IR_MAX_DURATION)
> > -			ir_raw_event_reset(input_dev);
> > -
> > -		raw->this_ev.duration = 0;
> > +		init_ir_raw_event(&raw->this_ev);
> >  	}
> > -out:
> > +
> >  	if (ir->props->s_idle)
> >  		ir->props->s_idle(ir->props->priv, idle);
> >  	ir->idle = idle;
> > diff --git a/drivers/media/IR/ir-rc5-decoder.c b/drivers/media/IR/ir-rc5-decoder.c
> > index df4770d..572ed4c 100644
> > --- a/drivers/media/IR/ir-rc5-decoder.c
> > +++ b/drivers/media/IR/ir-rc5-decoder.c
> > @@ -55,8 +55,9 @@ static int ir_rc5_decode(struct input_dev *input_dev, struct ir_raw_event ev)
> >          if (!(ir_dev->raw->enabled_protocols & IR_TYPE_RC5))
> >                  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-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...
> 
> > +			data->state = STATE_INACTIVE;
> >  		return 0;
> >  	}
> >  

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

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

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

[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 <maximlevitsky@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 <jarod@redhat.com>


-- 
Jarod Wilson
jarod@redhat.com

[PATCH 8/8] IR: ene_ir: add support for carrier reports

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/IR/ene_ir.c |   37 +++++++++++++++++++++++++++++--------
 1 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/media/IR/ene_ir.c b/drivers/media/IR/ene_ir.c
index dc32509..f1ee153 100644
--- a/drivers/media/IR/ene_ir.c
+++ b/drivers/media/IR/ene_ir.c
@@ -192,10 +192,11 @@ static int ene_hw_detect(struct ene_device *dev)
 /* Sense current received carrier */
 void ene_rx_sense_carrier(struct ene_device *dev)
 {
+	DEFINE_IR_RAW_EVENT(ev);
+
+	int carrier, duty_cycle;
 	int period = ene_read_reg(dev, ENE_CIRCAR_PRD);
 	int hperiod = ene_read_reg(dev, ENE_CIRCAR_HPRD);
-	int carrier, duty_cycle;
-
 
 	if (!(period & ENE_CIRCAR_PRD_VALID))
 		return;
@@ -208,13 +209,16 @@ void ene_rx_sense_carrier(struct ene_device *dev)
 	dbg("RX: hardware carrier period = %02x", period);
 	dbg("RX: hardware carrier pulse period = %02x", hperiod);
 
-
 	carrier = 2000000 / period;
 	duty_cycle = (hperiod * 100) / period;
 	dbg("RX: sensed carrier = %d Hz, duty cycle %d%%",
-							carrier, duty_cycle);
-
-	/* TODO: Send carrier & duty cycle to IR layer */
+						carrier, duty_cycle);
+	if (dev->carrier_detect_enabled) {
+		ev.carrier_report = true;
+		ev.carrier = carrier;
+		ev.duty_cycle = duty_cycle;
+		ir_raw_event_store(dev->idev, &ev);
+	}
 }
 
 /* this enables/disables the CIR RX engine */
@@ -728,7 +732,7 @@ static irqreturn_t ene_isr(int irq, void *data)
 
 	dbg_verbose("RX interrupt");
 
-	if (dev->carrier_detect_enabled || debug)
+	if (dev->hw_learning_and_tx_capable)
 		ene_rx_sense_carrier(dev);
 
 	/* On hardware that don't support extra buffer we need to trust
@@ -903,6 +907,23 @@ static int ene_set_learning_mode(void *data, int enable)
 	return 0;
 }
 
+static int ene_set_carrier_report(void *data, int enable)
+{
+	struct ene_device *dev = (struct ene_device *)data;
+	unsigned long flags;
+
+	if (enable == dev->carrier_detect_enabled)
+		return 0;
+
+	spin_lock_irqsave(&dev->hw_lock, flags);
+	dev->carrier_detect_enabled = enable;
+	ene_rx_disable(dev);
+	ene_rx_setup(dev);
+	ene_rx_enable(dev);
+	spin_unlock_irqrestore(&dev->hw_lock, flags);
+	return 0;
+}
+
 /* outside interface: enable or disable idle mode */
 static void ene_rx_set_idle(void *data, bool idle)
 {
@@ -1032,7 +1053,7 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 		ir_props->s_tx_mask = ene_set_tx_mask;
 		ir_props->s_tx_carrier = ene_set_tx_carrier;
 		ir_props->s_tx_duty_cycle = ene_set_tx_duty_cycle;
-		/* ir_props->s_carrier_report = ene_set_carrier_report; */
+		ir_props->s_carrier_report = ene_set_carrier_report;
 	}
 
 	ene_setup_hw_buffer(dev);
-- 
1.7.0.4

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[David Härdeman]

On Tue, Sep 07, 2010 at 12:26:05AM +0300, Maxim Levitsky wrote:
> Hi,
> 
> Here is full overview of my patches:

What changed from V4?


-- 
David Härdeman

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[David Härdeman]

On Tue, Sep 07, 2010 at 12:26:05AM +0300, Maxim Levitsky wrote:
> Hi,
> 
> Here is full overview of my patches:

What changed from V4?


-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Maxim Levitsky]

On Mon, 2010-09-06 at 23:56 +0200, David Härdeman wrote: 
> On Tue, Sep 07, 2010 at 12:26:05AM +0300, Maxim Levitsky wrote:
> > Hi,
> > 
> > Here is full overview of my patches:
> 
> What changed from V4?
Nothing other that 2 build fixes that I forgot to include.
I also cced linux-media because I was asked to.

Best regards,
Maxim Levitsky

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Maxim Levitsky]

On Mon, 2010-09-06 at 23:56 +0200, David Härdeman wrote: 
> On Tue, Sep 07, 2010 at 12:26:05AM +0300, Maxim Levitsky wrote:
> > Hi,
> > 
> > Here is full overview of my patches:
> 
> What changed from V4?
Nothing other that 2 build fixes that I forgot to include.
I also cced linux-media because I was asked to.

Best regards,
Maxim Levitsky

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Mauro Carvalho Chehab]

Em 06-09-2010 18:26, Maxim Levitsky escreveu:
> Hi,
> 
> Here is full overview of my patches:
> 
> Patch #1 fixes races in ir thread.
> It fixes the case when ktherad_stop waits forever for the thread.
> This happens on module unload and therefore it never finishes.
> Sorry for introducing this bug.
> 
> Patch #2, fixes a crash on my module load.
> It happens because ir core initializes the input device a bit early,
> therefore it could be accessed while still not set up.
> 
> Patch #3 fixes a small typo in lirc code that makes it impossible to use tx duty cycle setting.
> 
> Patch #4 fixes a problem seen on my system that results in stuck down forever key.
> 
> Patch #5 adds few keys to MCE keymap that were found on laptop of an user I tested this driver with
> 
> Patch #6, is a combined update ti my driver. It contains lot of refactoring thanks to docs I have now,
> and lot of fixes, and supports latest version of firmware (and I have 4 users asking for that)
> It is quite huge, but it would be a tedios job to break it up. This can't introduce regressions
> because the ene_ir was never released. In addition to that it was tested by me and another two users.
> 
> Patch #7 the really only patch that touches drivers I don't have does touch the ir-core.
> It is quite small, and it adds a proper solution to dilema about what to do with huge space between keypresses.
> Now this space is just truncated by the driver with timeout flag.
> The lirc codec then ensures that right sample is send to the lircd.
> Please review and test it.
> 
> Patch #8 is very simple. It just builds on top of patch #7 and adds carrier reports to ene driver.

For now, I've applied patches 3, 4 and 5, as it is nice to have Jarod's review also.

Cheers,
Mauro

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Jarod Wilson]

On Wed, Sep 08, 2010 at 06:54:02PM -0300, Mauro Carvalho Chehab wrote:
> Em 06-09-2010 18:26, Maxim Levitsky escreveu:
> > Hi,
> > 
> > Here is full overview of my patches:
> > 
> > Patch #1 fixes races in ir thread.
> > It fixes the case when ktherad_stop waits forever for the thread.
> > This happens on module unload and therefore it never finishes.
> > Sorry for introducing this bug.
> > 
> > Patch #2, fixes a crash on my module load.
> > It happens because ir core initializes the input device a bit early,
> > therefore it could be accessed while still not set up.
> > 
> > Patch #3 fixes a small typo in lirc code that makes it impossible to use tx duty cycle setting.
> > 
> > Patch #4 fixes a problem seen on my system that results in stuck down forever key.
> > 
> > Patch #5 adds few keys to MCE keymap that were found on laptop of an user I tested this driver with
> > 
> > Patch #6, is a combined update ti my driver. It contains lot of refactoring thanks to docs I have now,
> > and lot of fixes, and supports latest version of firmware (and I have 4 users asking for that)
> > It is quite huge, but it would be a tedios job to break it up. This can't introduce regressions
> > because the ene_ir was never released. In addition to that it was tested by me and another two users.
> > 
> > Patch #7 the really only patch that touches drivers I don't have does touch the ir-core.
> > It is quite small, and it adds a proper solution to dilema about what to do with huge space between keypresses.
> > Now this space is just truncated by the driver with timeout flag.
> > The lirc codec then ensures that right sample is send to the lircd.
> > Please review and test it.
> > 
> > Patch #8 is very simple. It just builds on top of patch #7 and adds carrier reports to ene driver.
> 
> For now, I've applied patches 3, 4 and 5, as it is nice to have Jarod's review also.

I've finally got them all applied atop current media_tree staging/v2.6.37,
though none of the streamzap bits in patch 7 are applicable any longer.
Will try to get through looking and commenting (and testing) of the rest
of them tonight.


-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Jarod Wilson]

On Wed, Sep 8, 2010 at 6:42 PM, Jarod Wilson <jarod@redhat.com> wrote:
> On Wed, Sep 08, 2010 at 06:54:02PM -0300, Mauro Carvalho Chehab wrote:
>> Em 06-09-2010 18:26, Maxim Levitsky escreveu:
>> > Hi,
>> >
>> > Here is full overview of my patches:
>> >
>> > Patch #1 fixes races in ir thread.
>> > It fixes the case when ktherad_stop waits forever for the thread.
>> > This happens on module unload and therefore it never finishes.
>> > Sorry for introducing this bug.
>> >
>> > Patch #2, fixes a crash on my module load.
>> > It happens because ir core initializes the input device a bit early,
>> > therefore it could be accessed while still not set up.
>> >
>> > Patch #3 fixes a small typo in lirc code that makes it impossible to use tx duty cycle setting.
>> >
>> > Patch #4 fixes a problem seen on my system that results in stuck down forever key.
>> >
>> > Patch #5 adds few keys to MCE keymap that were found on laptop of an user I tested this driver with
>> >
>> > Patch #6, is a combined update ti my driver. It contains lot of refactoring thanks to docs I have now,
>> > and lot of fixes, and supports latest version of firmware (and I have 4 users asking for that)
>> > It is quite huge, but it would be a tedios job to break it up. This can't introduce regressions
>> > because the ene_ir was never released. In addition to that it was tested by me and another two users.
>> >
>> > Patch #7 the really only patch that touches drivers I don't have does touch the ir-core.
>> > It is quite small, and it adds a proper solution to dilema about what to do with huge space between keypresses.
>> > Now this space is just truncated by the driver with timeout flag.
>> > The lirc codec then ensures that right sample is send to the lircd.
>> > Please review and test it.
>> >
>> > Patch #8 is very simple. It just builds on top of patch #7 and adds carrier reports to ene driver.
>>
>> For now, I've applied patches 3, 4 and 5, as it is nice to have Jarod's review also.
>
> I've finally got them all applied atop current media_tree staging/v2.6.37,
> though none of the streamzap bits in patch 7 are applicable any longer.
> Will try to get through looking and commenting (and testing) of the rest
> of them tonight.

Also had to make a minor addition to the rc5-sz decoder (same change
as in the other decoders). Almost have all the requisite test kernels
for David's, Maxim's and Dmitry's patchsets built and installed, wish
my laptop was faster... Probably would have been faster to use a lab
box and copy data over. Oh well. So functional testing to hopefully
commence tomorrow morning.


-- 
Jarod Wilson
jarod@wilsonet.com

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Jarod Wilson]

On Thu, Sep 09, 2010 at 12:34:27AM -0400, Jarod Wilson wrote:
...
> >> For now, I've applied patches 3, 4 and 5, as it is nice to have Jarod's review also.
> >
> > I've finally got them all applied atop current media_tree staging/v2.6.37,
> > though none of the streamzap bits in patch 7 are applicable any longer.
> > Will try to get through looking and commenting (and testing) of the rest
> > of them tonight.
> 
> Also had to make a minor addition to the rc5-sz decoder (same change
> as in the other decoders). Almost have all the requisite test kernels
> for David's, Maxim's and Dmitry's patchsets built and installed, wish
> my laptop was faster... Probably would have been faster to use a lab
> box and copy data over. Oh well. So functional testing to hopefully
> commence tomorrow morning.

Wuff. None of the three builds is at all stable on my laptop, but I can't
actually point the finger at any of the three patchsets, since I'm getting
spontaneous lockups doing nothing at all before even plugging in a
receiver. I did however get occasional periods of a non-panicking (not
starting X seems to help a lot). Initial results:

Dmitry's patchset:
- all good for imon, streamzap and mceusb

David's patchset:
- all good for mceusb, as expected, since David has mce hardware himself,
  did not try the others yet

Maxim's patchset:
- all good for mceusb and imon
- streamzap decoding fails miserably. I have an inkling why, but will need
  to get a stable testing platform before I can really properly dig into
  it.

Still working on that stable testing platform, which is "backport current
ir-core to the latest Fedora 14 kernel", which is 2.6.35.4-based and
rock-solid on this machine. After that, will start applying patchsets.

(I have yet to really look at the lockups, they look like random memory
corruption though).

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[David Härdeman]

On Fri, September 10, 2010 04:01, Jarod Wilson wrote:
> Wuff. None of the three builds is at all stable on my laptop, but I can't
> actually point the finger at any of the three patchsets, since I'm getting
> spontaneous lockups doing nothing at all before even plugging in a
> receiver. I did however get occasional periods of a non-panicking (not
> starting X seems to help a lot). Initial results:

If you get lockups without even plugging in a receiver, does that mean
that the rc-core driver hasn't even been loaded at that point?

-- 
David Härdeman

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[David Härdeman]

On Fri, September 10, 2010 04:01, Jarod Wilson wrote:
> Wuff. None of the three builds is at all stable on my laptop, but I can't
> actually point the finger at any of the three patchsets, since I'm getting
> spontaneous lockups doing nothing at all before even plugging in a
> receiver. I did however get occasional periods of a non-panicking (not
> starting X seems to help a lot). Initial results:

If you get lockups without even plugging in a receiver, does that mean
that the rc-core driver hasn't even been loaded at that point?

-- 
David Härdeman

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Jarod Wilson]

On Fri, Sep 10, 2010 at 10:08:24AM +0200, David Härdeman wrote:
> On Fri, September 10, 2010 04:01, Jarod Wilson wrote:
> > Wuff. None of the three builds is at all stable on my laptop, but I can't
> > actually point the finger at any of the three patchsets, since I'm getting
> > spontaneous lockups doing nothing at all before even plugging in a
> > receiver. I did however get occasional periods of a non-panicking (not
> > starting X seems to help a lot). Initial results:
> 
> If you get lockups without even plugging in a receiver, does that mean
> that the rc-core driver hasn't even been loaded at that point?

Correct, no rc-core bits loaded at all at this point.

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Jarod Wilson]

On Fri, Sep 10, 2010 at 10:08:24AM +0200, David Härdeman wrote:
> On Fri, September 10, 2010 04:01, Jarod Wilson wrote:
> > Wuff. None of the three builds is at all stable on my laptop, but I can't
> > actually point the finger at any of the three patchsets, since I'm getting
> > spontaneous lockups doing nothing at all before even plugging in a
> > receiver. I did however get occasional periods of a non-panicking (not
> > starting X seems to help a lot). Initial results:
> 
> If you get lockups without even plugging in a receiver, does that mean
> that the rc-core driver hasn't even been loaded at that point?

Correct, no rc-core bits loaded at all at this point.

-- 
Jarod Wilson
jarod@redhat.com

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Maxim Levitsky]

[-- Attachment #1: Type: text/plain, Size: 2171 bytes --]

On Thu, 2010-09-09 at 22:01 -0400, Jarod Wilson wrote: 
> On Thu, Sep 09, 2010 at 12:34:27AM -0400, Jarod Wilson wrote:
> ...
> > >> For now, I've applied patches 3, 4 and 5, as it is nice to have Jarod's review also.
> > >
> > > I've finally got them all applied atop current media_tree staging/v2.6.37,
> > > though none of the streamzap bits in patch 7 are applicable any longer.
> > > Will try to get through looking and commenting (and testing) of the rest
> > > of them tonight.
> > 
> > Also had to make a minor addition to the rc5-sz decoder (same change
> > as in the other decoders). Almost have all the requisite test kernels
> > for David's, Maxim's and Dmitry's patchsets built and installed, wish
> > my laptop was faster... Probably would have been faster to use a lab
> > box and copy data over. Oh well. So functional testing to hopefully
> > commence tomorrow morning.
> 
> Wuff. None of the three builds is at all stable on my laptop, but I can't
> actually point the finger at any of the three patchsets, since I'm getting
> spontaneous lockups doing nothing at all before even plugging in a
> receiver. I did however get occasional periods of a non-panicking (not
> starting X seems to help a lot). Initial results:
> 

Btw, my printk blackbox patch could help you a lot.
I can't count how many times it helped me.
I just enable softlockup, hardlockup, and nmi watchdog, and let system
panic on oopses, and reboot. Or if you have hardware reboot button, you
can just use it. The point is that most BIOSES don't clear the ram, and
I take advantage of that.

Recently in an attempt to make it reserve only small portion of memory
(before I would use mem=) I also made this work out of box.

After a reboot, to get crash dmesg, just do
sudo cat /sys/kernel/debug/printk/crash_dmesg | strings

Note that kernel contains now a ramoops module that does similar things.
It doesn't reserve the memory automatically, and (this is the reason I
still don't use it) is only called by kernel to save oopses/panicks.
It should be enough though too, but my patch actually places the printk
buffer itself in the fixed area in the ram.


Best regards,
Maxim Levitsky

[-- Attachment #2: printk_blackbox.diff --]
[-- Type: text/x-patch, Size: 5717 bytes --]

commit 8f1c423046c22dad6aaeca04bfcb0ab301843c36
Author: Maxim Levitsky <maximlevitsky@gmail.com>
Date:   Sat Jul 31 13:43:03 2010 +0300

    printk: Allow to fix the physical address of printk buffer
    
        Allows to put printk buffer at fixed location of ram (default 128M).
    
        If debugfs is enabled, log of last boot is copied into
        system ram, and can be accessed via debugfs, for example
        cat /sys/kernel/debug/printk/crash_dmesg
    
        Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index ef292c7..a6eaf40 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -46,6 +46,7 @@ extern unsigned long saved_video_mode;
 extern void reserve_standard_io_resources(void);
 extern void i386_reserve_resources(void);
 extern void setup_default_timer_irq(void);
+extern void early_reserve_printk_buffer(void);
 
 #ifdef CONFIG_X86_MRST
 extern void x86_mrst_early_setup(void);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index c3a4fbb..6fbf2a0 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -843,6 +843,7 @@ void __init setup_arch(char **cmdline_p)
 
 	/* after early param, so could get panic from serial */
 	reserve_early_setup_data();
+	early_reserve_printk_buffer();
 
 	if (acpi_mps_check()) {
 #ifdef CONFIG_X86_LOCAL_APIC
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 3ba6e06..8854a91 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -106,7 +106,6 @@ static void __iomem *__ioremap_caller(resource_size_t phys_addr,
 
 		if (is_ram && pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))
 			return NULL;
-		WARN_ON_ONCE(is_ram);
 	}
 
 	/*
diff --git a/kernel/printk.c b/kernel/printk.c
index 8fe465a..a6fff63 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -41,6 +41,7 @@
 #include <linux/notifier.h>
 
 #include <asm/uaccess.h>
+#include <linux/debugfs.h>
 
 /*
  * for_each_console() allows you to iterate on each console
@@ -167,6 +168,7 @@ void log_buf_kexec_setup(void)
 }
 #endif
 
+#ifndef CONFIG_HWMEM_PRINTK
 static int __init log_buf_len_setup(char *str)
 {
 	unsigned size = memparse(str, &str);
@@ -207,6 +209,93 @@ out:
 }
 
 __setup("log_buf_len=", log_buf_len_setup);
+#endif
+
+#ifdef CONFIG_HWMEM_PRINTK
+
+char *old_log_buf;
+struct debugfs_blob_wrapper crash_dmesg_wrapper;
+static unsigned int printk_phys_address = CONFIG_HWMEM_PRINTK_DEFAULT_ADDRESS;
+
+
+static int __init printk_address_setup(char *p)
+{
+	char *tmp;
+
+	if (!strncmp(p, "off", 3))
+		printk_phys_address = 0;
+	else
+		printk_phys_address = memparse(p, &tmp);
+	return 0;
+}
+early_param("printk_address", printk_address_setup);
+
+
+void early_reserve_printk_buffer(void)
+{
+	if (printk_phys_address)
+		reserve_early(printk_phys_address,
+			printk_phys_address + __LOG_BUF_LEN, "printk buffer");
+}
+
+static int printk_move_to_fixed_address(void)
+{
+
+	char *mem_address;
+	unsigned long flags;
+	struct dentry *dbgfs_dir;
+
+	if (!printk_phys_address)
+		return 0;
+
+	mem_address = ioremap(printk_phys_address,  __LOG_BUF_LEN);
+
+	if (!mem_address) {
+		printk(KERN_ALERT "Can't map hardware kernel log memory."
+			"printk buffer disabled\n");
+		return  0;
+	}
+
+	printk(KERN_INFO "Logging kernel messages into HW memory at 0x%08x\n",
+			printk_phys_address);
+
+	/* allocate saved log buffer, and save the log memory that we
+	  will otherwise overwrite */
+	old_log_buf = kmalloc(__LOG_BUF_LEN, GFP_KERNEL);
+	if (old_log_buf)
+		memcpy(old_log_buf, mem_address, __LOG_BUF_LEN);
+
+
+	/* copy current log to the new memory */
+	memcpy(mem_address, log_buf, __LOG_BUF_LEN);
+
+	/* save the log memory, and publish it */
+	if (old_log_buf) {
+
+		crash_dmesg_wrapper.data = old_log_buf;
+		crash_dmesg_wrapper.size = __LOG_BUF_LEN;
+
+		dbgfs_dir = debugfs_create_dir("printk", NULL);
+
+		if (dbgfs_dir > 0)
+			debugfs_create_blob("crash_dmesg", S_IRUSR, dbgfs_dir,
+				&crash_dmesg_wrapper);
+	}
+
+
+
+	/* switch to the full log memory now */
+	spin_lock_irqsave(&logbuf_lock, flags);
+	log_buf = mem_address;
+	spin_unlock_irqrestore(&logbuf_lock, flags);
+
+	return 1;
+}
+postcore_initcall(printk_move_to_fixed_address);
+
+#else
+void early_reserve_printk_buffer(void) {}
+#endif
 
 #ifdef CONFIG_BOOT_PRINTK_DELAY
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1b4afd2..1f12584 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -788,6 +788,35 @@ config BOOT_PRINTK_DELAY
 	  BOOT_PRINTK_DELAY also may cause DETECT_SOFTLOCKUP to detect
 	  what it believes to be lockup conditions.
 
+config HWMEM_PRINTK
+	bool "Log printk message buffer into fixed physical address (DANGEROUS)"
+	depends on DEBUG_KERNEL && PRINTK
+	help
+	  This option allows to place kernel log buffer into pre-defined
+	  area, somewhere in memory space.
+
+	  This creates some sort of black box recorder and can be very useful
+	  to debug several problems, especially 'panics' that happen while you
+	  use the X window system.
+
+
+	  If you also select debugfs support, you can easily look at
+	  kernel log of failed boot at:
+	  /sys/kernel/debug/printk/crash_dmesg
+
+	  (Assuming you mounted debugfs on /sys/kernel/debug)
+
+	  Misuse of this option can be DANGEROUS, as it makes kernel write at
+	  arbitary (selected by you) hardware memory range.
+
+	  It is only intended for debbuging, so say 'no' if not sure
+
+config HWMEM_PRINTK_DEFAULT_ADDRESS
+	hex
+	depends on HWMEM_PRINTK
+	prompt "Default address at which store the printk buffer (default 60M)"
+	default "0x3C00000"
+
 config RCU_TORTURE_TEST
 	tristate "torture tests for RCU"
 	depends on DEBUG_KERNEL

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Jarod Wilson]

On Fri, Sep 10, 2010 at 11:35:23AM +0300, Maxim Levitsky wrote:
> On Thu, 2010-09-09 at 22:01 -0400, Jarod Wilson wrote: 
> > On Thu, Sep 09, 2010 at 12:34:27AM -0400, Jarod Wilson wrote:
> > ...
> > > >> For now, I've applied patches 3, 4 and 5, as it is nice to have Jarod's review also.
> > > >
> > > > I've finally got them all applied atop current media_tree staging/v2.6.37,
> > > > though none of the streamzap bits in patch 7 are applicable any longer.
> > > > Will try to get through looking and commenting (and testing) of the rest
> > > > of them tonight.
> > > 
> > > Also had to make a minor addition to the rc5-sz decoder (same change
> > > as in the other decoders). Almost have all the requisite test kernels
> > > for David's, Maxim's and Dmitry's patchsets built and installed, wish
> > > my laptop was faster... Probably would have been faster to use a lab
> > > box and copy data over. Oh well. So functional testing to hopefully
> > > commence tomorrow morning.
> > 
> > Wuff. None of the three builds is at all stable on my laptop, but I can't
> > actually point the finger at any of the three patchsets, since I'm getting
> > spontaneous lockups doing nothing at all before even plugging in a
> > receiver. I did however get occasional periods of a non-panicking (not
> > starting X seems to help a lot). Initial results:
> > 
> 
> Btw, my printk blackbox patch could help you a lot.
> I can't count how many times it helped me.
> I just enable softlockup, hardlockup, and nmi watchdog, and let system
> panic on oopses, and reboot. Or if you have hardware reboot button, you
> can just use it. The point is that most BIOSES don't clear the ram, and
> I take advantage of that.

Interesting. I was thinking perhaps I'd give a go at trying kdump on my
laptop, but I've had pretty mixed results with kdump working correctly on
random kernels (generally works quite well in RHEL, notsomuch in Fedora).

My hope is that this is something already fixed in later Linus' kernels,
so I'll try a current Linus snap before I try looking any deeper. I'll
file this away for consideration though!


-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Maxim Levitsky]

On Tue, 2010-09-07 at 00:26 +0300, Maxim Levitsky wrote:
> Hi,
> 
> Here is full overview of my patches:
> 
> Patch #1 fixes races in ir thread.
> It fixes the case when ktherad_stop waits forever for the thread.
> This happens on module unload and therefore it never finishes.
> Sorry for introducing this bug.
> 
> Patch #2, fixes a crash on my module load.
> It happens because ir core initializes the input device a bit early,
> therefore it could be accessed while still not set up.
> 
> Patch #3 fixes a small typo in lirc code that makes it impossible to use tx duty cycle setting.
> 
> Patch #4 fixes a problem seen on my system that results in stuck down forever key.
> 
> Patch #5 adds few keys to MCE keymap that were found on laptop of an user I tested this driver with
> 
> Patch #6, is a combined update ti my driver. It contains lot of refactoring thanks to docs I have now,
> and lot of fixes, and supports latest version of firmware (and I have 4 users asking for that)
> It is quite huge, but it would be a tedios job to break it up. This can't introduce regressions
> because the ene_ir was never released. In addition to that it was tested by me and another two users.
> 
> Patch #7 the really only patch that touches drivers I don't have does touch the ir-core.
> It is quite small, and it adds a proper solution to dilema about what to do with huge space between keypresses.
> Now this space is just truncated by the driver with timeout flag.
> The lirc codec then ensures that right sample is send to the lircd.
> Please review and test it.
> 
> Patch #8 is very simple. It just builds on top of patch #7 and adds carrier reports to ene driver.
> 

Any update?
Should I resent these (didn't change anything I think).
What about other work that conflicts with this?
What are the plans for (very close) merge window?

Best regards,
	Maxim Levitsky

Re: [PATCH 0/8 V5] Many fixes for in-kernel decoding and for the ENE driver

[Maxim Levitsky]

On Tue, 2010-09-07 at 00:26 +0300, Maxim Levitsky wrote:
> Hi,
> 
> Here is full overview of my patches:
> 
> Patch #1 fixes races in ir thread.
> It fixes the case when ktherad_stop waits forever for the thread.
> This happens on module unload and therefore it never finishes.
> Sorry for introducing this bug.
> 
> Patch #2, fixes a crash on my module load.
> It happens because ir core initializes the input device a bit early,
> therefore it could be accessed while still not set up.
> 
> Patch #3 fixes a small typo in lirc code that makes it impossible to use tx duty cycle setting.
> 
> Patch #4 fixes a problem seen on my system that results in stuck down forever key.
> 
> Patch #5 adds few keys to MCE keymap that were found on laptop of an user I tested this driver with
> 
> Patch #6, is a combined update ti my driver. It contains lot of refactoring thanks to docs I have now,
> and lot of fixes, and supports latest version of firmware (and I have 4 users asking for that)
> It is quite huge, but it would be a tedios job to break it up. This can't introduce regressions
> because the ene_ir was never released. In addition to that it was tested by me and another two users.
> 
> Patch #7 the really only patch that touches drivers I don't have does touch the ir-core.
> It is quite small, and it adds a proper solution to dilema about what to do with huge space between keypresses.
> Now this space is just truncated by the driver with timeout flag.
> The lirc codec then ensures that right sample is send to the lircd.
> Please review and test it.
> 
> Patch #8 is very simple. It just builds on top of patch #7 and adds carrier reports to ene driver.
> 

Any update?
Should I resent these (didn't change anything I think).
What about other work that conflicts with this?
What are the plans for (very close) merge window?

Best regards,
	Maxim Levitsky



------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
Spend less time writing and  rewriting code and more time creating great
experiences on the web. Be a part of the beta today.
http://p.sf.net/sfu/beautyoftheweb