All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
@ 2019-03-29  7:00 ` Anson Huang
  0 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-03-29  7:00 UTC (permalink / raw)
  To: dmitry.torokhov, shawnguo, s.hauer, kernel, festevam,
	linux-input, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

There is no need to enable release interrupt and disable depress
interrupt in event check, as a timer is setup for checking these
events rather than interrupts.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/input/keyboard/imx_keypad.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 539cb67..7e32c36 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct timer_list *t)
 		reg_val = readw(keypad->mmio_base + KPSR);
 		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
 		writew(reg_val, keypad->mmio_base + KPSR);
-
-		reg_val = readw(keypad->mmio_base + KPSR);
-		reg_val |= KBD_STAT_KRIE;
-		reg_val &= ~KBD_STAT_KDIE;
-		writew(reg_val, keypad->mmio_base + KPSR);
 	}
 }
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
@ 2019-03-29  7:00 ` Anson Huang
  0 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-03-29  7:00 UTC (permalink / raw)
  To: dmitry.torokhov, shawnguo, s.hauer, kernel, festevam,
	linux-input, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

There is no need to enable release interrupt and disable depress
interrupt in event check, as a timer is setup for checking these
events rather than interrupts.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/input/keyboard/imx_keypad.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 539cb67..7e32c36 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct timer_list *t)
 		reg_val = readw(keypad->mmio_base + KPSR);
 		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
 		writew(reg_val, keypad->mmio_base + KPSR);
-
-		reg_val = readw(keypad->mmio_base + KPSR);
-		reg_val |= KBD_STAT_KRIE;
-		reg_val &= ~KBD_STAT_KDIE;
-		writew(reg_val, keypad->mmio_base + KPSR);
 	}
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
@ 2019-03-29  7:00 ` Anson Huang
  0 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-03-29  7:00 UTC (permalink / raw)
  To: dmitry.torokhov, shawnguo, s.hauer, kernel, festevam,
	linux-input, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

There is no need to enable release interrupt and disable depress
interrupt in event check, as a timer is setup for checking these
events rather than interrupts.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/input/keyboard/imx_keypad.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 539cb67..7e32c36 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct timer_list *t)
 		reg_val = readw(keypad->mmio_base + KPSR);
 		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
 		writew(reg_val, keypad->mmio_base + KPSR);
-
-		reg_val = readw(keypad->mmio_base + KPSR);
-		reg_val |= KBD_STAT_KRIE;
-		reg_val &= ~KBD_STAT_KDIE;
-		writew(reg_val, keypad->mmio_base + KPSR);
 	}
 }
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] input: keyboard: imx: make sure keyboard can always wake up system
  2019-03-29  7:00 ` Anson Huang
  (?)
@ 2019-03-29  7:00   ` Anson Huang
  -1 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-03-29  7:00 UTC (permalink / raw)
  To: dmitry.torokhov, shawnguo, s.hauer, kernel, festevam,
	linux-input, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

There are several scenarios that keyboard can NOT wake up system
from suspend, e.g., if a keyboard is depressed between system
device suspend phase and device noirq suspend phase, the keyboard
ISR will be called and both keyboard depress and release interrupts
will be disabled, then keyboard will no longer be able to wake up
system. Another scenario would be, if a keyboard is kept depressed,
and then system goes into suspend, the expected behavior would be
when keyboard is released, system will be waked up, but current
implementation can NOT achieve that, because both depress and release
interrupts are disabled in ISR, and the event check is still in
progress.

To fix these issues, need to make sure keyboard's depress or release
interrupt is enabled after noirq device suspend phase, this patch
moves the suspend/resume callback to noirq suspend/resume phase, and
enable the corresponding interrupt according to current keyboard status.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/input/keyboard/imx_keypad.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 7e32c36..9ae2090 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -521,11 +521,12 @@ static int imx_keypad_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int __maybe_unused imx_kbd_suspend(struct device *dev)
+static int __maybe_unused imx_kbd_noirq_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct imx_keypad *kbd = platform_get_drvdata(pdev);
 	struct input_dev *input_dev = kbd->input_dev;
+	unsigned short reg_val = readw(kbd->mmio_base + KPSR);
 
 	/* imx kbd can wake up system even clock is disabled */
 	mutex_lock(&input_dev->mutex);
@@ -535,13 +536,20 @@ static int __maybe_unused imx_kbd_suspend(struct device *dev)
 
 	mutex_unlock(&input_dev->mutex);
 
-	if (device_may_wakeup(&pdev->dev))
+	if (device_may_wakeup(&pdev->dev)) {
+		if (reg_val & KBD_STAT_KPKD)
+			reg_val |= KBD_STAT_KRIE;
+		if (reg_val & KBD_STAT_KPKR)
+			reg_val |= KBD_STAT_KDIE;
+		writew(reg_val, kbd->mmio_base + KPSR);
+
 		enable_irq_wake(kbd->irq);
+	}
 
 	return 0;
 }
 
-static int __maybe_unused imx_kbd_resume(struct device *dev)
+static int __maybe_unused imx_kbd_noirq_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct imx_keypad *kbd = platform_get_drvdata(pdev);
@@ -565,7 +573,9 @@ static int __maybe_unused imx_kbd_resume(struct device *dev)
 	return ret;
 }
 
-static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend, imx_kbd_resume);
+static const struct dev_pm_ops imx_kbd_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_kbd_noirq_suspend, imx_kbd_noirq_resume)
+};
 
 static struct platform_driver imx_keypad_driver = {
 	.driver		= {
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] input: keyboard: imx: make sure keyboard can always wake up system
@ 2019-03-29  7:00   ` Anson Huang
  0 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-03-29  7:00 UTC (permalink / raw)
  To: dmitry.torokhov, shawnguo, s.hauer, kernel, festevam,
	linux-input, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

There are several scenarios that keyboard can NOT wake up system
from suspend, e.g., if a keyboard is depressed between system
device suspend phase and device noirq suspend phase, the keyboard
ISR will be called and both keyboard depress and release interrupts
will be disabled, then keyboard will no longer be able to wake up
system. Another scenario would be, if a keyboard is kept depressed,
and then system goes into suspend, the expected behavior would be
when keyboard is released, system will be waked up, but current
implementation can NOT achieve that, because both depress and release
interrupts are disabled in ISR, and the event check is still in
progress.

To fix these issues, need to make sure keyboard's depress or release
interrupt is enabled after noirq device suspend phase, this patch
moves the suspend/resume callback to noirq suspend/resume phase, and
enable the corresponding interrupt according to current keyboard status.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/input/keyboard/imx_keypad.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 7e32c36..9ae2090 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -521,11 +521,12 @@ static int imx_keypad_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int __maybe_unused imx_kbd_suspend(struct device *dev)
+static int __maybe_unused imx_kbd_noirq_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct imx_keypad *kbd = platform_get_drvdata(pdev);
 	struct input_dev *input_dev = kbd->input_dev;
+	unsigned short reg_val = readw(kbd->mmio_base + KPSR);
 
 	/* imx kbd can wake up system even clock is disabled */
 	mutex_lock(&input_dev->mutex);
@@ -535,13 +536,20 @@ static int __maybe_unused imx_kbd_suspend(struct device *dev)
 
 	mutex_unlock(&input_dev->mutex);
 
-	if (device_may_wakeup(&pdev->dev))
+	if (device_may_wakeup(&pdev->dev)) {
+		if (reg_val & KBD_STAT_KPKD)
+			reg_val |= KBD_STAT_KRIE;
+		if (reg_val & KBD_STAT_KPKR)
+			reg_val |= KBD_STAT_KDIE;
+		writew(reg_val, kbd->mmio_base + KPSR);
+
 		enable_irq_wake(kbd->irq);
+	}
 
 	return 0;
 }
 
-static int __maybe_unused imx_kbd_resume(struct device *dev)
+static int __maybe_unused imx_kbd_noirq_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct imx_keypad *kbd = platform_get_drvdata(pdev);
@@ -565,7 +573,9 @@ static int __maybe_unused imx_kbd_resume(struct device *dev)
 	return ret;
 }
 
-static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend, imx_kbd_resume);
+static const struct dev_pm_ops imx_kbd_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_kbd_noirq_suspend, imx_kbd_noirq_resume)
+};
 
 static struct platform_driver imx_keypad_driver = {
 	.driver		= {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] input: keyboard: imx: make sure keyboard can always wake up system
@ 2019-03-29  7:00   ` Anson Huang
  0 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-03-29  7:00 UTC (permalink / raw)
  To: dmitry.torokhov, shawnguo, s.hauer, kernel, festevam,
	linux-input, linux-arm-kernel, linux-kernel
  Cc: dl-linux-imx

There are several scenarios that keyboard can NOT wake up system
from suspend, e.g., if a keyboard is depressed between system
device suspend phase and device noirq suspend phase, the keyboard
ISR will be called and both keyboard depress and release interrupts
will be disabled, then keyboard will no longer be able to wake up
system. Another scenario would be, if a keyboard is kept depressed,
and then system goes into suspend, the expected behavior would be
when keyboard is released, system will be waked up, but current
implementation can NOT achieve that, because both depress and release
interrupts are disabled in ISR, and the event check is still in
progress.

To fix these issues, need to make sure keyboard's depress or release
interrupt is enabled after noirq device suspend phase, this patch
moves the suspend/resume callback to noirq suspend/resume phase, and
enable the corresponding interrupt according to current keyboard status.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/input/keyboard/imx_keypad.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 7e32c36..9ae2090 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -521,11 +521,12 @@ static int imx_keypad_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static int __maybe_unused imx_kbd_suspend(struct device *dev)
+static int __maybe_unused imx_kbd_noirq_suspend(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct imx_keypad *kbd = platform_get_drvdata(pdev);
 	struct input_dev *input_dev = kbd->input_dev;
+	unsigned short reg_val = readw(kbd->mmio_base + KPSR);
 
 	/* imx kbd can wake up system even clock is disabled */
 	mutex_lock(&input_dev->mutex);
@@ -535,13 +536,20 @@ static int __maybe_unused imx_kbd_suspend(struct device *dev)
 
 	mutex_unlock(&input_dev->mutex);
 
-	if (device_may_wakeup(&pdev->dev))
+	if (device_may_wakeup(&pdev->dev)) {
+		if (reg_val & KBD_STAT_KPKD)
+			reg_val |= KBD_STAT_KRIE;
+		if (reg_val & KBD_STAT_KPKR)
+			reg_val |= KBD_STAT_KDIE;
+		writew(reg_val, kbd->mmio_base + KPSR);
+
 		enable_irq_wake(kbd->irq);
+	}
 
 	return 0;
 }
 
-static int __maybe_unused imx_kbd_resume(struct device *dev)
+static int __maybe_unused imx_kbd_noirq_resume(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct imx_keypad *kbd = platform_get_drvdata(pdev);
@@ -565,7 +573,9 @@ static int __maybe_unused imx_kbd_resume(struct device *dev)
 	return ret;
 }
 
-static SIMPLE_DEV_PM_OPS(imx_kbd_pm_ops, imx_kbd_suspend, imx_kbd_resume);
+static const struct dev_pm_ops imx_kbd_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(imx_kbd_noirq_suspend, imx_kbd_noirq_resume)
+};
 
 static struct platform_driver imx_keypad_driver = {
 	.driver		= {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
  2019-03-29  7:00 ` Anson Huang
  (?)
@ 2019-04-03 21:48   ` dmitry.torokhov
  -1 siblings, 0 replies; 12+ messages in thread
From: dmitry.torokhov @ 2019-04-03 21:48 UTC (permalink / raw)
  To: Anson Huang
  Cc: shawnguo, s.hauer, kernel, festevam, linux-input,
	linux-arm-kernel, linux-kernel, dl-linux-imx

Hi Anson,

On Fri, Mar 29, 2019 at 07:00:43AM +0000, Anson Huang wrote:
> There is no need to enable release interrupt and disable depress
> interrupt in event check, as a timer is setup for checking these
> events rather than interrupts.

But won't using release interrupt allow signalling key release earlier?

> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/input/keyboard/imx_keypad.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index 539cb67..7e32c36 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct timer_list *t)
>  		reg_val = readw(keypad->mmio_base + KPSR);
>  		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
>  		writew(reg_val, keypad->mmio_base + KPSR);
> -
> -		reg_val = readw(keypad->mmio_base + KPSR);
> -		reg_val |= KBD_STAT_KRIE;
> -		reg_val &= ~KBD_STAT_KDIE;
> -		writew(reg_val, keypad->mmio_base + KPSR);
>  	}
>  }
>  
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
@ 2019-04-03 21:48   ` dmitry.torokhov
  0 siblings, 0 replies; 12+ messages in thread
From: dmitry.torokhov @ 2019-04-03 21:48 UTC (permalink / raw)
  To: Anson Huang
  Cc: shawnguo, s.hauer, kernel, festevam, linux-input,
	linux-arm-kernel, linux-kernel, dl-linux-imx

Hi Anson,

On Fri, Mar 29, 2019 at 07:00:43AM +0000, Anson Huang wrote:
> There is no need to enable release interrupt and disable depress
> interrupt in event check, as a timer is setup for checking these
> events rather than interrupts.

But won't using release interrupt allow signalling key release earlier?

> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/input/keyboard/imx_keypad.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index 539cb67..7e32c36 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct timer_list *t)
>  		reg_val = readw(keypad->mmio_base + KPSR);
>  		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
>  		writew(reg_val, keypad->mmio_base + KPSR);
> -
> -		reg_val = readw(keypad->mmio_base + KPSR);
> -		reg_val |= KBD_STAT_KRIE;
> -		reg_val &= ~KBD_STAT_KDIE;
> -		writew(reg_val, keypad->mmio_base + KPSR);
>  	}
>  }
>  
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
@ 2019-04-03 21:48   ` dmitry.torokhov
  0 siblings, 0 replies; 12+ messages in thread
From: dmitry.torokhov @ 2019-04-03 21:48 UTC (permalink / raw)
  To: Anson Huang
  Cc: shawnguo, s.hauer, linux-kernel, dl-linux-imx, kernel,
	linux-input, festevam, linux-arm-kernel

Hi Anson,

On Fri, Mar 29, 2019 at 07:00:43AM +0000, Anson Huang wrote:
> There is no need to enable release interrupt and disable depress
> interrupt in event check, as a timer is setup for checking these
> events rather than interrupts.

But won't using release interrupt allow signalling key release earlier?

> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/input/keyboard/imx_keypad.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
> index 539cb67..7e32c36 100644
> --- a/drivers/input/keyboard/imx_keypad.c
> +++ b/drivers/input/keyboard/imx_keypad.c
> @@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct timer_list *t)
>  		reg_val = readw(keypad->mmio_base + KPSR);
>  		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
>  		writew(reg_val, keypad->mmio_base + KPSR);
> -
> -		reg_val = readw(keypad->mmio_base + KPSR);
> -		reg_val |= KBD_STAT_KRIE;
> -		reg_val &= ~KBD_STAT_KDIE;
> -		writew(reg_val, keypad->mmio_base + KPSR);
>  	}
>  }
>  
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
  2019-04-03 21:48   ` dmitry.torokhov
  (?)
@ 2019-04-04  1:29     ` Anson Huang
  -1 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-04-04  1:29 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: shawnguo, s.hauer, kernel, festevam, linux-input,
	linux-arm-kernel, linux-kernel, dl-linux-imx

Hi, Dmitry

Best Regards!
Anson Huang

> -----Original Message-----
> From: dmitry.torokhov@gmail.com [mailto:dmitry.torokhov@gmail.com]
> Sent: 2019年4月4日 5:49
> To: Anson Huang <anson.huang@nxp.com>
> Cc: shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-input@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-
> imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/2] input: keyboard: imx: no need to control interrupt
> status in event check
> 
> Hi Anson,
> 
> On Fri, Mar 29, 2019 at 07:00:43AM +0000, Anson Huang wrote:
> > There is no need to enable release interrupt and disable depress
> > interrupt in event check, as a timer is setup for checking these
> > events rather than interrupts.
> 
> But won't using release interrupt allow signalling key release earlier?

It makes sense, patch #1 can be dropped, I will resend the patch #2 for review.

Thanks,
Anson.

> 
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/input/keyboard/imx_keypad.c | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/imx_keypad.c
> > b/drivers/input/keyboard/imx_keypad.c
> > index 539cb67..7e32c36 100644
> > --- a/drivers/input/keyboard/imx_keypad.c
> > +++ b/drivers/input/keyboard/imx_keypad.c
> > @@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct
> timer_list *t)
> >  		reg_val = readw(keypad->mmio_base + KPSR);
> >  		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
> >  		writew(reg_val, keypad->mmio_base + KPSR);
> > -
> > -		reg_val = readw(keypad->mmio_base + KPSR);
> > -		reg_val |= KBD_STAT_KRIE;
> > -		reg_val &= ~KBD_STAT_KDIE;
> > -		writew(reg_val, keypad->mmio_base + KPSR);
> >  	}
> >  }
> >
> > --
> > 2.7.4
> >
> 
> Thanks.
> 
> --
> Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
@ 2019-04-04  1:29     ` Anson Huang
  0 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-04-04  1:29 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: shawnguo, s.hauer, kernel, festevam, linux-input,
	linux-arm-kernel, linux-kernel, dl-linux-imx

Hi, Dmitry

Best Regards!
Anson Huang

> -----Original Message-----
> From: dmitry.torokhov@gmail.com [mailto:dmitry.torokhov@gmail.com]
> Sent: 2019年4月4日 5:49
> To: Anson Huang <anson.huang@nxp.com>
> Cc: shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-input@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-
> imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/2] input: keyboard: imx: no need to control interrupt
> status in event check
> 
> Hi Anson,
> 
> On Fri, Mar 29, 2019 at 07:00:43AM +0000, Anson Huang wrote:
> > There is no need to enable release interrupt and disable depress
> > interrupt in event check, as a timer is setup for checking these
> > events rather than interrupts.
> 
> But won't using release interrupt allow signalling key release earlier?

It makes sense, patch #1 can be dropped, I will resend the patch #2 for review.

Thanks,
Anson.

> 
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/input/keyboard/imx_keypad.c | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/imx_keypad.c
> > b/drivers/input/keyboard/imx_keypad.c
> > index 539cb67..7e32c36 100644
> > --- a/drivers/input/keyboard/imx_keypad.c
> > +++ b/drivers/input/keyboard/imx_keypad.c
> > @@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct
> timer_list *t)
> >  		reg_val = readw(keypad->mmio_base + KPSR);
> >  		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
> >  		writew(reg_val, keypad->mmio_base + KPSR);
> > -
> > -		reg_val = readw(keypad->mmio_base + KPSR);
> > -		reg_val |= KBD_STAT_KRIE;
> > -		reg_val &= ~KBD_STAT_KDIE;
> > -		writew(reg_val, keypad->mmio_base + KPSR);
> >  	}
> >  }
> >
> > --
> > 2.7.4
> >
> 
> Thanks.
> 
> --
> Dmitry

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check
@ 2019-04-04  1:29     ` Anson Huang
  0 siblings, 0 replies; 12+ messages in thread
From: Anson Huang @ 2019-04-04  1:29 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: shawnguo, s.hauer, linux-kernel, dl-linux-imx, kernel,
	linux-input, festevam, linux-arm-kernel

Hi, Dmitry

Best Regards!
Anson Huang

> -----Original Message-----
> From: dmitry.torokhov@gmail.com [mailto:dmitry.torokhov@gmail.com]
> Sent: 2019年4月4日 5:49
> To: Anson Huang <anson.huang@nxp.com>
> Cc: shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-input@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; dl-linux-
> imx <linux-imx@nxp.com>
> Subject: Re: [PATCH 1/2] input: keyboard: imx: no need to control interrupt
> status in event check
> 
> Hi Anson,
> 
> On Fri, Mar 29, 2019 at 07:00:43AM +0000, Anson Huang wrote:
> > There is no need to enable release interrupt and disable depress
> > interrupt in event check, as a timer is setup for checking these
> > events rather than interrupts.
> 
> But won't using release interrupt allow signalling key release earlier?

It makes sense, patch #1 can be dropped, I will resend the patch #2 for review.

Thanks,
Anson.

> 
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/input/keyboard/imx_keypad.c | 5 -----
> >  1 file changed, 5 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/imx_keypad.c
> > b/drivers/input/keyboard/imx_keypad.c
> > index 539cb67..7e32c36 100644
> > --- a/drivers/input/keyboard/imx_keypad.c
> > +++ b/drivers/input/keyboard/imx_keypad.c
> > @@ -276,11 +276,6 @@ static void imx_keypad_check_for_events(struct
> timer_list *t)
> >  		reg_val = readw(keypad->mmio_base + KPSR);
> >  		reg_val |= KBD_STAT_KPKR | KBD_STAT_KRSS;
> >  		writew(reg_val, keypad->mmio_base + KPSR);
> > -
> > -		reg_val = readw(keypad->mmio_base + KPSR);
> > -		reg_val |= KBD_STAT_KRIE;
> > -		reg_val &= ~KBD_STAT_KDIE;
> > -		writew(reg_val, keypad->mmio_base + KPSR);
> >  	}
> >  }
> >
> > --
> > 2.7.4
> >
> 
> Thanks.
> 
> --
> Dmitry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2019-04-04  1:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-29  7:00 [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check Anson Huang
2019-03-29  7:00 ` Anson Huang
2019-03-29  7:00 ` Anson Huang
2019-03-29  7:00 ` [PATCH 2/2] input: keyboard: imx: make sure keyboard can always wake up system Anson Huang
2019-03-29  7:00   ` Anson Huang
2019-03-29  7:00   ` Anson Huang
2019-04-03 21:48 ` [PATCH 1/2] input: keyboard: imx: no need to control interrupt status in event check dmitry.torokhov
2019-04-03 21:48   ` dmitry.torokhov
2019-04-03 21:48   ` dmitry.torokhov
2019-04-04  1:29   ` Anson Huang
2019-04-04  1:29     ` Anson Huang
2019-04-04  1:29     ` Anson Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.