Thread (11 messages) 11 messages, 4 authors, 2019-11-22

Re: [PATCH v3] input: keyboard: snvs_pwrkey: Send key events for i.MX6 S, DL and Q

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2019-11-22 22:48:46
Also in: linux-arm-kernel, lkml

On Thu, Nov 21, 2019 at 01:17:56PM +0000, Robin Gong wrote:
On 2019-11-21 0:33, robin [off-list ref] wrote:
quoted
On 2019-11-20 10:27, Marco Felsch wrote:
quoted
Hi Robin,

On 19-09-16 16:37, Dmitry Torokhov wrote:
quoted
On Mon, Sep 16, 2019 at 07:45:37AM +0000, Robin Gong wrote:
quoted
On 2019/9/13 15:31 robin [off-list ref] wrote:>
quoted
Hi Dmitry,

On 2019-09-12 22:13, Dmitry Torokhov wrote:
quoted
Hi Robin,

On Wed, Sep 04, 2019 at 06:23:29AM +0000, Robin van der Gracht
wrote:
quoted
quoted
quoted
quoted
quoted
quoted
The first generation i.MX6 processors does not send an
interrupt when the power key is pressed. It sends a power down
request interrupt if the key is released before a hard
shutdown (5 second press). This should allow software to bring down
the SoC safely.
quoted
quoted
quoted
quoted
quoted
quoted
For this driver to work as a regular power key with the older
SoCs, we need to send a keypress AND release when we get the
power down request irq.

Signed-off-by: Robin van der Gracht <robin@protonic.nl>
---
@@ -67,13 +83,17 @@ static irqreturn_t
imx_snvs_pwrkey_interrupt(int irq, void *dev_id)  {
 	struct platform_device *pdev = dev_id;
 	struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
+	unsigned long expire = jiffies;
 	u32 lp_status;

 	pm_wakeup_event(pdata->input->dev.parent, 0);

 	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
-	if (lp_status & SNVS_LPSR_SPO)
-		mod_timer(&pdata->check_timer, jiffies +
msecs_to_jiffies(DEBOUNCE_TIME));
+	if (lp_status & SNVS_LPSR_SPO) {
+		if (pdata->minor_rev > 0)
+			expire = jiffies + msecs_to_jiffies(DEBOUNCE_TIME);
+		mod_timer(&pdata->check_timer, expire);
Why do we even need to fire the timer in case of the first
generation hardware? Just send press and release events directly from
the ISR.
quoted
quoted
quoted
That timer looks like a software debounce to prevent unexpected and
meaningless interrupt/event caused by quick press/release.
Right, but in case of the first generation hardware we schedule the
timer immediately (expire == 0) and do not check state of the
hardware in the timer handler, but rather simply emit down/up events,
so we do not really get any benefit from the timer (again, I am
talking about first generation hardware only).
Did you prepared a v4? Just ask to avoid a duplicated work :)
No I haven't. Not sure what the public wants. Use timer, don't use timer..

v3 has had long term testing though ;)
Sorry for that I miss the mail.
Understood a little bit confusion about immediate timer for
the first press, just stand on the view of clean code shape.
But I'm okay if you prefer to remove timer in the first interrupt
generation.
Yes, please prepare v4 without the timer for the first generation of the
hardware.

Thanks.

-- 
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help