rc: ene_ir: few fixes

rc: ene_ir: few fixes

[Maxim Levitsky]

Hi,

Could you consider merging few fixes to my driver:

1. Fix accidently introduced error, that is the hardware is a bit unusual
in the way that it needs the interrupt number, and one of the recent patches
moved the irq number read to be too late for that.

2. I just now played with my remote that wakes the system, and noticed that
it wakes the system even if I disable the wake bit.
Just disable the device if wake is disabled, and this fixes the issue.

3. I noticed that debug prints from my driver don't work anymore,
and this is due to conversion to pr_dbg, which is in my opinion too restructive in
enabling it.
If you allow, I want to use pr_info instead.

patch #1 should go to stable as well, as it outright breaks my driver.

PS: I am very short on time, and I will be free in about month, after I pass
another round of exams.

Best regards,
	Maxim Levitsky

[PATCH 1/3] ene_ir: Fix interrupt line passthrough to hardware

[Maxim Levitsky]

While we can delay IRQ intialization, we need the interrupt number
right away because unusually hardware have programable interrupt number,
and thus we give it the number that was allocated by BIOS

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/rc/ene_ir.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
index ee6c984..a9cf3a4 100644
--- a/drivers/media/rc/ene_ir.c
+++ b/drivers/media/rc/ene_ir.c
@@ -1022,6 +1022,8 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 	spin_lock_init(&dev->hw_lock);
 
 	dev->hw_io = pnp_port_start(pnp_dev, 0);
+	dev->irq = pnp_irq(pnp_dev, 0);
+
 
 	pnp_set_drvdata(pnp_dev, dev);
 	dev->pnp_dev = pnp_dev;
@@ -1085,7 +1087,6 @@ static int ene_probe(struct pnp_dev *pnp_dev, const struct pnp_device_id *id)
 		goto exit_unregister_device;
 	}
 
-	dev->irq = pnp_irq(pnp_dev, 0);
 	if (request_irq(dev->irq, ene_isr,
 			IRQF_SHARED, ENE_DRIVER_NAME, (void *)dev)) {
 		goto exit_release_hw_io;
-- 
1.7.9.5

[PATCH 2/3] ene_ir: disable the device if wake is disabled It doesn't hurt and on my notebook despite clearing the wake flag the remote still wakes up the system. This way it doesn't

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/rc/ene_ir.c |   27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/ene_ir.c b/drivers/media/rc/ene_ir.c
index a9cf3a4..61865ba 100644
--- a/drivers/media/rc/ene_ir.c
+++ b/drivers/media/rc/ene_ir.c
@@ -476,7 +476,7 @@ select_timeout:
 }
 
 /* Enable the device for receive */
-static void ene_rx_enable(struct ene_device *dev)
+static void ene_rx_enable_hw(struct ene_device *dev)
 {
 	u8 reg_value;
 
@@ -504,11 +504,17 @@ static void ene_rx_enable(struct ene_device *dev)
 
 	/* enter idle mode */
 	ir_raw_event_set_idle(dev->rdev, true);
+}
+
+/* Enable the device for receive - wrapper to track the state*/
+static void ene_rx_enable(struct ene_device *dev)
+{
+	ene_rx_enable_hw(dev);
 	dev->rx_enabled = true;
 }
 
 /* Disable the device receiver */
-static void ene_rx_disable(struct ene_device *dev)
+static void ene_rx_disable_hw(struct ene_device *dev)
 {
 	/* disable inputs */
 	ene_rx_enable_cir_engine(dev, false);
@@ -516,8 +522,13 @@ static void ene_rx_disable(struct ene_device *dev)
 
 	/* disable hardware IRQ and firmware flag */
 	ene_clear_reg_mask(dev, ENE_FW1, ENE_FW1_ENABLE | ENE_FW1_IRQ);
-
 	ir_raw_event_set_idle(dev->rdev, true);
+}
+
+/* Disable the device receiver - wrapper to track the state */
+static void ene_rx_disable(struct ene_device *dev)
+{
+	ene_rx_disable_hw(dev);
 	dev->rx_enabled = false;
 }
 
@@ -1123,9 +1134,8 @@ static void ene_remove(struct pnp_dev *pnp_dev)
 }
 
 /* enable wake on IR (wakes on specific button on original remote) */
-static void ene_enable_wake(struct ene_device *dev, int enable)
+static void ene_enable_wake(struct ene_device *dev, bool enable)
 {
-	enable = enable && device_may_wakeup(&dev->pnp_dev->dev);
 	dbg("wake on IR %s", enable ? "enabled" : "disabled");
 	ene_set_clear_reg_mask(dev, ENE_FW1, ENE_FW1_WAKE, enable);
 }
@@ -1134,9 +1144,12 @@ static void ene_enable_wake(struct ene_device *dev, int enable)
 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, true);
+	bool wake = device_may_wakeup(&dev->pnp_dev->dev);
+
+	if (!wake && dev->rx_enabled)
+		ene_rx_disable_hw(dev);
 
-	/* TODO: add support for wake pattern */
+	ene_enable_wake(dev, wake);
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 3/3] ene_ir: don't use pr_debug after all

[Maxim Levitsky]

This way to only way to get debug info is to use dynamic debug, but I left debugging
prints to debug hardware issues, and so I want this to be enabled by module param

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/media/rc/ene_ir.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/ene_ir.h b/drivers/media/rc/ene_ir.h
index 6f978e8..a7911e3 100644
--- a/drivers/media/rc/ene_ir.h
+++ b/drivers/media/rc/ene_ir.h
@@ -185,7 +185,7 @@
 #define __dbg(level, format, ...)				\
 do {								\
 	if (debug >= level)					\
-		pr_debug(format "\n", ## __VA_ARGS__);		\
+		pr_info(format "\n", ## __VA_ARGS__);		\
 } while (0)
 
 #define dbg(format, ...)		__dbg(1, format, ## __VA_ARGS__)
-- 
1.7.9.5

Re: rc: ene_ir: few fixes

[Maxim Levitsky]

Any update?

Best regards,
Maxim Levitsky


On Mon, 2013-07-08 at 03:22 +0300, Maxim Levitsky wrote: 
> Hi,
> 
> Could you consider merging few fixes to my driver:
> 
> 1. Fix accidently introduced error, that is the hardware is a bit unusual
> in the way that it needs the interrupt number, and one of the recent patches
> moved the irq number read to be too late for that.
> 
> 2. I just now played with my remote that wakes the system, and noticed that
> it wakes the system even if I disable the wake bit.
> Just disable the device if wake is disabled, and this fixes the issue.
> 
> 3. I noticed that debug prints from my driver don't work anymore,
> and this is due to conversion to pr_dbg, which is in my opinion too restructive in
> enabling it.
> If you allow, I want to use pr_info instead.
> 
> patch #1 should go to stable as well, as it outright breaks my driver.
> 
> PS: I am very short on time, and I will be free in about month, after I pass
> another round of exams.
> 
> Best regards,
> 	Maxim Levitsky
> 

-- 
Best regards,
        Maxim Levitsky

Visit my blog: http://maximlevitsky.wordpress.com
Warning: Above blog contains rants.

Re: rc: ene_ir: few fixes

[Maxim Levitsky]

On Thu, Jul 11, 2013 at 3:29 AM, Maxim Levitsky <maximlevitsky@gmail.com> wrote:
> Any update?
>
> Best regards,
> Maxim Levitsky
>
>
> On Mon, 2013-07-08 at 03:22 +0300, Maxim Levitsky wrote:
>> Hi,
>>
>> Could you consider merging few fixes to my driver:
>>
>> 1. Fix accidently introduced error, that is the hardware is a bit unusual
>> in the way that it needs the interrupt number, and one of the recent patches
>> moved the irq number read to be too late for that.
>>
>> 2. I just now played with my remote that wakes the system, and noticed that
>> it wakes the system even if I disable the wake bit.
>> Just disable the device if wake is disabled, and this fixes the issue.
>>
>> 3. I noticed that debug prints from my driver don't work anymore,
>> and this is due to conversion to pr_dbg, which is in my opinion too restructive in
>> enabling it.
>> If you allow, I want to use pr_info instead.
>>
>> patch #1 should go to stable as well, as it outright breaks my driver.
>>
>> PS: I am very short on time, and I will be free in about month, after I pass
>> another round of exams.
>>
>> Best regards,
>>       Maxim Levitsky
>>
>
> --
> Best regards,
>         Maxim Levitsky
>
> Visit my blog: http://maximlevitsky.wordpress.com
> Warning: Above blog contains rants.
>

Any update?

Best regards,
       Maxim Levitsky