linux-kernel - Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup sources

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 25 Aug 2015 17:24:26 +0100
From:	Sudeep Holla <sudeep.holla@....com>
To:	Shenwei Wang <Shenwei.Wang@...escale.com>
CC:	Sudeep Holla <sudeep.holla@....com>,
	"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"jason@...edaemon.net" <jason@...edaemon.net>,
	Huang Anson <Anson.Huang@...escale.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup
 sources



On 25/08/15 15:54, Shenwei Wang wrote:
>
>
>> -----Original Message-----
>> From: Sudeep Holla [mailto:sudeep.holla@....com]
>> Sent: 2015年8月25日 9:46
>> To: Wang Shenwei-B38339
>> Cc: Sudeep Holla; shawn.guo@...aro.org; tglx@...utronix.de;
>> jason@...edaemon.net; Huang Yongcai-B20788; linux-kernel@...r.kernel.org;
>> linux-arm-kernel@...ts.infradead.org
>> Subject: Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup
>> sources
>>
>>
>>
>> On 25/08/15 15:14, Shenwei Wang wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Sudeep Holla [mailto:sudeep.holla@....com]
>>
>> [...]
>>
>>>> I don't see this driver doing anything extra apart from keeping
>>>> the wakeup irqs enabled. i.e. You use the same cpu*wake
>>>> register to mask/unmask the interrupt as well as set the wakeup
>>>> source. Since the wakeup interrupt will be enabled by the
>>>> driver, you just need to mark it as wake-up source and nothing
>>>> extra in the controller right ? If so, you need to set
>>>> IRQCHIP_SKIP_SET_WAKE as you are just leaving that irq enabled
>>>> and not doing any extra configuration to enable it  as wakeup source.
>>>> Please correct if that wrong, but from the code that's what I
>>>> couldinfer.
>>>
>>> There is no special for this driver. We just use the IRQCHIP
>>> driver framework to manage the wakeup sources. Why did you
>>> propose to set IRQCHIP_SKIP_SET_WAKE flag here? If you don't need
>>> the wakeup feature, you should just not enable this driver in the
>>> configuration.
>>>
>>
>> No, if the driver doesn't nothing extra to configure the wake up
>> source other than  keeping it enabled, then it fits the case of
>> SKIP_SET_WAKE. The driver using this wake would have requested
>> and enabled the irq. When it calls enable_irq_wake, you have
>> nothing extra to set(atleast  from the looks of the driver), so
>> setting SKIP_SET_WAKE will skip the call and  updated the wake
>> flags in irq core.
>>
>> I don't see the real need of 2 separate sets of irq mask being
>> saved  in either case.
>
> You don't really understand what happens after a driver calls
> enable_irq_wake. In suspend state, even the interrupt
> controller itself is powered off. How can you get the system up
> again  by just using a SKIP_SET_WAKE.
>

Sorry for that, let me try to understand aloud. So you have
irq_{un,}mask function that are called when interrupts are enabled and
disabled. So suppose you have 3 irqs that are enabled and only one of
then is set as wakeup source.

Now you call enable_irq_wake, you save that in wakeup_sources, fine.
Later when you enter suspend, you save all the 3 active irqs in
saved_irq_mask and over-write cpu2wakeup with wakeup_sources, right?

All fine, what I am saying is let irq-core know that you want to mask
the 2 non-wakeup irqs you have using MASK_ON_SUSPEND. So when
suspend_device_irqs is called in suspend path, that's done for you
automatically and the cpu2wakeup will have just 1 wakeup enabled which
is what you are doing in suspend callback, right ?

Now that it's already done for you, you need not do anything extra and
hence just set SKIP_SET_WAKE to do nothing.

Hope this clarifies, sorry if I am still missing to understand something
here, but I don't see anything. Let me know.

Regards,
Sudeep
--
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/