Linux Input Archive on lore.kernel.org
 help / color / Atom feed
From: Robin Gong <yibin.gong@nxp.com>
To: robin <robin@protonic.nl>, Marco Felsch <m.felsch@pengutronix.de>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	"linux-input @ vger . kernel . org" <linux-input@vger.kernel.org>,
	"linux-kernel @ vger . kernel . org"
	<linux-kernel@vger.kernel.org>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>, Adam Ford <aford173@gmail.com>,
	"linux-arm-kernel @ lists . infradead . org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: RE: [PATCH v3] input: keyboard: snvs_pwrkey: Send key events for i.MX6 S, DL and Q
Date: Thu, 21 Nov 2019 13:17:56 +0000
Message-ID: <VE1PR04MB6638D79F8C929100F6D96479894E0@VE1PR04MB6638.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <fb98fa1fde9a367d7ab33d04a5804684@protonic.nl>


On 2019-11-21 0:33, robin <robin@protonic.nl> wrote:
> On 2019-11-20 10:27, Marco Felsch wrote:
> > Hi Robin,
> >
> > On 19-09-16 16:37, Dmitry Torokhov wrote:
> >> On Mon, Sep 16, 2019 at 07:45:37AM +0000, Robin Gong wrote:
> >> > On 2019/9/13 15:31 robin <robin@protonic.nl> wrote:>
> >> > > Hi Dmitry,
> >> > >
> >> > > On 2019-09-12 22:13, Dmitry Torokhov wrote:
> >> > > > Hi Robin,
> >> > > >
> >> > > > On Wed, Sep 04, 2019 at 06:23:29AM +0000, Robin van der Gracht
> wrote:
> >> > > >> 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.
> >> > > >>
> >> > > >> 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.
> >> > 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.
> 
> Regards,
> Robin van der Gracht

  reply index

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-04  6:23 Robin van der Gracht
2019-09-04  6:52 ` Marco Felsch
2019-09-13  7:39   ` robin
2019-09-12 20:13 ` Dmitry Torokhov
2019-09-13  7:30   ` robin
2019-09-16  7:45     ` Robin Gong
2019-09-16 23:37       ` Dmitry Torokhov
2019-11-20  9:27         ` Marco Felsch
2019-11-20 16:32           ` robin
2019-11-21 13:17             ` Robin Gong [this message]
2019-11-22 22:48               ` Dmitry Torokhov

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VE1PR04MB6638D79F8C929100F6D96479894E0@VE1PR04MB6638.eurprd04.prod.outlook.com \
    --to=yibin.gong@nxp.com \
    --cc=aford173@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.felsch@pengutronix.de \
    --cc=robin@protonic.nl \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux Input Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-input/0 linux-input/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-input linux-input/ https://lore.kernel.org/linux-input \
		linux-input@vger.kernel.org
	public-inbox-index linux-input

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-input


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git