linux-kernel - Re: [PATCH v4] pinctrl: mediatek: Implement wake handler and suspend resume

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 14 Sep 2015 12:16:23 +0100
From:	Sudeep Holla <sudeep.holla@....com>
To:	maoguang meng <maoguang.meng@...iatek.com>
Cc:	Sudeep Holla <sudeep.holla@....com>,
	Daniel Kurtz <djkurtz@...omium.org>,
	Linus Walleij <linus.walleij@...aro.org>,
	Hongzhou Yang <hongzhou.yang@...iatek.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" 
	<linux-mediatek@...ts.infradead.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	Yingjoe Chen (陳英洲) 
	<Yingjoe.Chen@...iatek.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v4] pinctrl: mediatek: Implement wake handler and suspend
 resume



On 12/09/15 10:50, maoguang meng wrote:
> hi Sudeep:
> 
> I test flowlling your blow suggestions,but the system can not be woken.
> 
> beacuse,mtk_eint_suspend will mask it.As we know if eint wakeup system
> it must be unmasked before enter suspend flow.
> 
> e.x
> 
> static int __maybe_unused elan_suspend(struct device *dev)
> {
> 	struct i2c_client *client = to_i2c_client(dev);
> 	struct elan_tp_data *data = i2c_get_clientdata(client);
> 	int ret;
> 
> 	/*
> 	 * We are taking the mutex to make sure sysfs operations are
> 	 * complete before we attempt to bring the device into low[er]
> 	 * power mode.
> 	 */
> 	ret = mutex_lock_interruptible(&data->sysfs_mutex);
> 	if (ret)
> 		return ret;
> 
> 	disable_irq(client->irq);
> 
> 	if (device_may_wakeup(dev)) {
> 		ret = elan_sleep(data);
> 		/* Enable wake from IRQ */
> 		data->irq_wake = (enable_irq_wake(client->irq) == 0);

This is wrong assumption in the driver. enable_irq_wake doesn't
implicitly enable the IRQ. So the disable_irq should be moved to else.
And the resume patch also needs to be fixed accordingly, otherwise you
may get unbalanced irq. But this should not be the reason for fixing the
pinctrl suspend/resume.

Regards,
Sudeep

--->8

diff --git a/drivers/input/mouse/elan_i2c_core.c
b/drivers/input/mouse/elan_i2c_core.c
index fa945304b9a5..7de26b04f45c 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -1117,13 +1117,13 @@ static int __maybe_unused elan_suspend(struct
device *dev)
        if (ret)
                return ret;

-       disable_irq(client->irq);
-
        if (device_may_wakeup(dev)) {
                ret = elan_sleep(data);
                /* Enable wake from IRQ */
                data->irq_wake = (enable_irq_wake(client->irq) == 0);
        } else {
+               disable_irq(client->irq);
+
                ret = elan_disable_power(data);
        }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/