Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1212904 > unrolled thread

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

Started bySudeep Holla <sudeep.holla@arm.com>
First post2015-08-25 11:30 +0200
Last post2015-08-25 23:00 +0200
Articles 16 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Sudeep Holla <sudeep.holla@arm.com> - 2015-08-25 11:30 +0200
    RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-25 15:40 +0200
      Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Sudeep Holla <sudeep.holla@arm.com> - 2015-08-25 16:00 +0200
        RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-25 16:40 +0200
          Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Sudeep Holla <sudeep.holla@arm.com> - 2015-08-25 16:50 +0200
            RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-25 17:00 +0200
              Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Sudeep Holla <sudeep.holla@arm.com> - 2015-08-25 18:30 +0200
                Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Thomas Gleixner <tglx@linutronix.de> - 2015-08-25 21:30 +0200
                  Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Sudeep Holla <sudeep.holla@arm.com> - 2015-08-26 11:00 +0200
                RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-25 21:30 +0200
                  RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Thomas Gleixner <tglx@linutronix.de> - 2015-08-25 21:40 +0200
                    RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-25 22:00 +0200
                      RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Thomas Gleixner <tglx@linutronix.de> - 2015-08-25 22:20 +0200
                        RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-25 22:50 +0200
                          RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Shenwei Wang <Shenwei.Wang@freescale.com> - 2015-08-25 23:00 +0200
                          RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup  sources Thomas Gleixner <tglx@linutronix.de> - 2015-08-25 23:00 +0200

#1212904 — Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup sources

FromSudeep Holla <sudeep.holla@arm.com>
Date2015-08-25 11:30 +0200
SubjectRe: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup sources
Message-ID<q1iIk-2lN-47@gated-at.bofh.it>

On 24/08/15 20:04, Shenwei Wang wrote:
> IMX7D contains a new version of GPC IP block (GPCv2). It has two
> major functions: power management and wakeup source management.
> This patch adds a new irqchip driver to manage the interrupt wakeup
> sources on IMX7D.

Interesting, you mention that this IP block has mainly power management
and it itself requires save/restore. Is it not in always on domain ?

> When the system is in WFI (wait for interrupt) mode, this GPC block
> will be the first block on the platform to be activated and signaled.
> Under normal wait mode during cpu idle, the system can be woke up
> by any enabled interrupts. Under standby or suspend mode, the system
> can only be woke up by the pre-defined wakeup sources.
>
> Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com>
> Signed-off-by: Anson Huang <b20788@freescale.com>
> ---
> Change log:
> 	Renamed the enabled_irqs to saved_irq_mask in struct gpcv2_irqchip_data
> 	Removed "BUG_ON()" in imx_gpcv2_irqchip_init to unify the error handling codes.
>
>   drivers/irqchip/Kconfig         |   7 +
>   drivers/irqchip/Makefile        |   1 +
>   drivers/irqchip/irq-imx-gpcv2.c | 275 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 283 insertions(+)
>   create mode 100644 drivers/irqchip/irq-imx-gpcv2.c
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 120d815..3fc0fac 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -177,3 +177,10 @@ config RENESAS_H8300H_INTC
>   config RENESAS_H8S_INTC
>           bool
>   	select IRQ_DOMAIN
> +
> +config IMX_GPCV2
> +	bool
> +	select IRQ_DOMAIN
> +	help
> +	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
> +
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index b8d4e96..8eb5f60 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -52,3 +52,4 @@ obj-$(CONFIG_RENESAS_H8300H_INTC)	+= irq-renesas-h8300h.o
>   obj-$(CONFIG_RENESAS_H8S_INTC)		+= irq-renesas-h8s.o
>   obj-$(CONFIG_ARCH_SA1100)		+= irq-sa11x0.o
>   obj-$(CONFIG_INGENIC_IRQ)		+= irq-ingenic.o
> +obj-$(CONFIG_IMX_GPCV2)			+= irq-imx-gpcv2.o
> diff --git a/drivers/irqchip/irq-imx-gpcv2.c b/drivers/irqchip/irq-imx-gpcv2.c
> new file mode 100644
> index 0000000..4a97afa
> --- /dev/null
> +++ b/drivers/irqchip/irq-imx-gpcv2.c
> @@ -0,0 +1,275 @@
> +/*
> + * Copyright (C) 2015 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/slab.h>
> +#include <linux/irqchip.h>
> +#include <linux/syscore_ops.h>
> +
> +#define IMR_NUM			4
> +#define GPC_MAX_IRQS            (IMR_NUM * 32)
> +
> +#define GPC_IMR1_CORE0		0x30
> +#define GPC_IMR1_CORE1		0x40
> +
> +struct gpcv2_irqchip_data {
> +	struct raw_spinlock    rlock;
> +	void __iomem       *gpc_base;
> +	u32 wakeup_sources[IMR_NUM];
> +	u32 saved_irq_mask[IMR_NUM];
> +	u32 cpu2wakeup;
> +};
> +
> +static struct gpcv2_irqchip_data *imx_gpcv2_instance;
> +
> +u32 imx_gpcv2_get_wakeup_source(u32 **sources)
> +{
> +	if (!imx_gpcv2_instance)
> +		return 0;
> +
> +	if (sources)
> +		*sources = imx_gpcv2_instance->wakeup_sources;
> +
> +	return IMR_NUM;
> +}
> +
> +static int gpcv2_wakeup_source_save(void)
> +{
> +	struct gpcv2_irqchip_data *cd;
> +	void __iomem *reg;
> +	int i;
> +
> +	cd = imx_gpcv2_instance;
> +	if (!cd)
> +		return 0;
> +
> +	for (i = 0; i < IMR_NUM; i++) {
> +		reg = cd->gpc_base + cd->cpu2wakeup + i * 4;
> +		cd->saved_irq_mask[i] = readl_relaxed(reg);
> +		writel_relaxed(cd->wakeup_sources[i], reg);
> +	}
> +
> +	return 0;
> +}
> +
> +static void gpcv2_wakeup_source_restore(void)
> +{
> +	struct gpcv2_irqchip_data *cd;
> +	void __iomem *reg;
> +	int i;
> +
> +	cd = imx_gpcv2_instance;
> +	if (!cd)
> +		return;
> +
> +	for (i = 0; i < IMR_NUM; i++) {
> +		reg = cd->gpc_base + cd->cpu2wakeup + i * 4;
> +		writel_relaxed(cd->saved_irq_mask[i], reg);

Instead of saving all the non-wakeup sources, can't you use raw
save/restore of these registers and mask all the non-wakeup sources
by setting MASK_ON_SUSPEND ?

Also your interrupt controller seems like has no special way to
configure wakeups, you are just leaving them enabled. i.e. I see
cpu2wakeup used for both {un,}masking and wakeup enable. So you can just
use IRQCHIP_SKIP_SET_WAKE. Correct me if my understanding is wrong.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1213041

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-25 15:40 +0200
Message-ID<q1mCe-7Ts-11@gated-at.bofh.it>
In reply to#1212904
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogU3VkZWVwIEhvbGxhIFtt
YWlsdG86c3VkZWVwLmhvbGxhQGFybS5jb21dDQo+IFNlbnQ6IDIwMTXE6jjUwjI1yNUgNDoyNQ0K
PiBUbzogV2FuZyBTaGVud2VpLUIzODMzOQ0KPiBDYzogc2hhd24uZ3VvQGxpbmFyby5vcmc7IHRn
bHhAbGludXRyb25peC5kZTsgamFzb25AbGFrZWRhZW1vbi5uZXQ7IFN1ZGVlcA0KPiBIb2xsYTsg
SHVhbmcgWW9uZ2NhaS1CMjA3ODg7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+IGxp
bnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BBVENI
IHY5IDEvMV0gaXJxY2hpcDogaW14LWdwY3YyOiBJTVggR1BDdjIgZHJpdmVyIGZvciB3YWtldXAN
Cj4gc291cmNlcw0KPiANCj4gDQo+IA0KPiBPbiAyNC8wOC8xNSAyMDowNCwgU2hlbndlaSBXYW5n
IHdyb3RlOg0KPiA+IElNWDdEIGNvbnRhaW5zIGEgbmV3IHZlcnNpb24gb2YgR1BDIElQIGJsb2Nr
IChHUEN2MikuIEl0IGhhcyB0d28gbWFqb3INCj4gPiBmdW5jdGlvbnM6IHBvd2VyIG1hbmFnZW1l
bnQgYW5kIHdha2V1cCBzb3VyY2UgbWFuYWdlbWVudC4NCj4gPiBUaGlzIHBhdGNoIGFkZHMgYSBu
ZXcgaXJxY2hpcCBkcml2ZXIgdG8gbWFuYWdlIHRoZSBpbnRlcnJ1cHQgd2FrZXVwDQo+ID4gc291
cmNlcyBvbiBJTVg3RC4NCj4gDQo+IEludGVyZXN0aW5nLCB5b3UgbWVudGlvbiB0aGF0IHRoaXMg
SVAgYmxvY2sgaGFzIG1haW5seSBwb3dlciBtYW5hZ2VtZW50IGFuZCBpdA0KPiBpdHNlbGYgcmVx
dWlyZXMgc2F2ZS9yZXN0b3JlLiBJcyBpdCBub3QgaW4gYWx3YXlzIG9uIGRvbWFpbiA/DQoNClll
cywgaXQgaXMgaW4gYWx3YXlzIG9uIGRvbWFpbi4NCg0KPiA+IFdoZW4gdGhlIHN5c3RlbSBpcyBp
biBXRkkgKHdhaXQgZm9yIGludGVycnVwdCkgbW9kZSwgdGhpcyBHUEMgYmxvY2sNCj4gPiB3aWxs
IGJlIHRoZSBmaXJzdCBibG9jayBvbiB0aGUgcGxhdGZvcm0gdG8gYmUgYWN0aXZhdGVkIGFuZCBz
aWduYWxlZC4NCj4gPiBVbmRlciBub3JtYWwgd2FpdCBtb2RlIGR1cmluZyBjcHUgaWRsZSwgdGhl
IHN5c3RlbSBjYW4gYmUgd29rZSB1cCBieQ0KPiA+ICtzdGF0aWMgdm9pZCBncGN2Ml93YWtldXBf
c291cmNlX3Jlc3RvcmUodm9pZCkgew0KPiA+ICsJc3RydWN0IGdwY3YyX2lycWNoaXBfZGF0YSAq
Y2Q7DQo+ID4gKwl2b2lkIF9faW9tZW0gKnJlZzsNCj4gPiArCWludCBpOw0KPiA+ICsNCj4gPiAr
CWNkID0gaW14X2dwY3YyX2luc3RhbmNlOw0KPiA+ICsJaWYgKCFjZCkNCj4gPiArCQlyZXR1cm47
DQo+ID4gKw0KPiA+ICsJZm9yIChpID0gMDsgaSA8IElNUl9OVU07IGkrKykgew0KPiA+ICsJCXJl
ZyA9IGNkLT5ncGNfYmFzZSArIGNkLT5jcHUyd2FrZXVwICsgaSAqIDQ7DQo+ID4gKwkJd3JpdGVs
X3JlbGF4ZWQoY2QtPnNhdmVkX2lycV9tYXNrW2ldLCByZWcpOw0KPiANCj4gSW5zdGVhZCBvZiBz
YXZpbmcgYWxsIHRoZSBub24td2FrZXVwIHNvdXJjZXMsIGNhbid0IHlvdSB1c2UgcmF3IHNhdmUv
cmVzdG9yZSBvZg0KPiB0aGVzZSByZWdpc3RlcnMgYW5kIG1hc2sgYWxsIHRoZSBub24td2FrZXVw
IHNvdXJjZXMgYnkgc2V0dGluZw0KPiBNQVNLX09OX1NVU1BFTkQgPw0KDQpJIGNhbid0IGNhdGNo
IHdoYXQgeW91IG1lYW4uIENhbiB5b3Ugc2hvdyBtZSBhbiBleGFtcGxlPw0KDQo+IEFsc28geW91
ciBpbnRlcnJ1cHQgY29udHJvbGxlciBzZWVtcyBsaWtlIGhhcyBubyBzcGVjaWFsIHdheSB0byBj
b25maWd1cmUgd2FrZXVwcywNCj4geW91IGFyZSBqdXN0IGxlYXZpbmcgdGhlbSBlbmFibGVkLiBp
LmUuIEkgc2VlIGNwdTJ3YWtldXAgdXNlZCBmb3IgYm90aA0KPiB7dW4sfW1hc2tpbmcgYW5kIHdh
a2V1cCBlbmFibGUuIFNvIHlvdSBjYW4ganVzdCB1c2UgSVJRQ0hJUF9TS0lQX1NFVF9XQUtFLg0K
PiBDb3JyZWN0IG1lIGlmIG15IHVuZGVyc3RhbmRpbmcgaXMgd3JvbmcuDQoNCkluIHRoaXMgZHJp
dmVyLCB0aGUgQ29yZTAgaXMgY29uZmlndXJlZCB0byBiZSB0aGUgZGVmYXVsdCBjb3JlIHRvIGJl
IHdva2UgdXAsIG5vdCBib3RoLiBZb3UgY2FuDQpjb25maWd1cmUgaXQgdG8gQ29yZTEgYnkgY2hh
bmdpbmcgdGhlIGNwdTJ3YWtldXAgdmFsdWUuIEkgZG9uJ3Qgc2VlIGFueSByZWFzb24gdG8NCnVz
ZSBJUlFDSElQX1NLSVBfU0VUX1dBS0UgZmxhZy4NCg0KVGhhbmtzLA0KU2hlbndlaQ0KIA0KDQoN
Cj4gUmVnYXJkcywNCj4gU3VkZWVwDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213051

FromSudeep Holla <sudeep.holla@arm.com>
Date2015-08-25 16:00 +0200
Message-ID<q1mVB-8gb-27@gated-at.bofh.it>
In reply to#1213041

On 25/08/15 14:38, Shenwei Wang wrote:
>
>
>> -----Original Message-----
>> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
>> Sent: 2015年8月25日 4:25
>> To: Wang Shenwei-B38339
>> Cc: shawn.guo@linaro.org; tglx@linutronix.de; jason@lakedaemon.net; Sudeep
>> Holla; Huang Yongcai-B20788; linux-kernel@vger.kernel.org;
>> linux-arm-kernel@lists.infradead.org
>> Subject: Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup
>> sources
>>
>>
>>
>> On 24/08/15 20:04, Shenwei Wang wrote:
>>> IMX7D contains a new version of GPC IP block (GPCv2). It has two major
>>> functions: power management and wakeup source management.
>>> This patch adds a new irqchip driver to manage the interrupt wakeup
>>> sources on IMX7D.
>>
>> Interesting, you mention that this IP block has mainly power management and it
>> itself requires save/restore. Is it not in always on domain ?
>
> Yes, it is in always on domain.
>

Hmm, then why do you need to save and restore the mask ?

>>> When the system is in WFI (wait for interrupt) mode, this GPC block
>>> will be the first block on the platform to be activated and signaled.
>>> Under normal wait mode during cpu idle, the system can be woke up by
>>> +static void gpcv2_wakeup_source_restore(void) {
>>> +	struct gpcv2_irqchip_data *cd;
>>> +	void __iomem *reg;
>>> +	int i;
>>> +
>>> +	cd = imx_gpcv2_instance;
>>> +	if (!cd)
>>> +		return;
>>> +
>>> +	for (i = 0; i < IMR_NUM; i++) {
>>> +		reg = cd->gpc_base + cd->cpu2wakeup + i * 4;
>>> +		writel_relaxed(cd->saved_irq_mask[i], reg);
>>
>> Instead of saving all the non-wakeup sources, can't you use raw save/restore of
>> these registers and mask all the non-wakeup sources by setting
>> MASK_ON_SUSPEND ?
>
> I can't catch what you mean. Can you show me an example?
>

What I meant is instead of you tracking all the enabled irqs(both wakeup
and non-wakeup) in saved_irq_mask, just set MASK_ON_SUSPEND flag
where all the non-wakeup interrupts are masked on suspend and unmasked
on resume by the irq core. You can then just save and restore the wakeup
irq mask, but as I asked above do you have to do that as you are saying
it's in always on domain ?

>> Also your interrupt controller seems like has no special way to configure wakeups,
>> you are just leaving them enabled. i.e. I see cpu2wakeup used for both
>> {un,}masking and wakeup enable. So you can just use IRQCHIP_SKIP_SET_WAKE.
>> Correct me if my understanding is wrong.
>
> In this driver, the Core0 is configured to be the default core to be woke up, not both. You can
> configure it to Core1 by changing the cpu2wakeup value. I don't see any reason to
> use IRQCHIP_SKIP_SET_WAKE flag.
>

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 could infer.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213089

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-25 16:40 +0200
Message-ID<q1nyi-Od-19@gated-at.bofh.it>
In reply to#1213051
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogU3VkZWVwIEhvbGxhIFtt
YWlsdG86c3VkZWVwLmhvbGxhQGFybS5jb21dDQo+ID4+PiBJTVg3RCBjb250YWlucyBhIG5ldyB2
ZXJzaW9uIG9mIEdQQyBJUCBibG9jayAoR1BDdjIpLiBJdCBoYXMgdHdvDQo+ID4+PiBtYWpvcg0K
PiA+Pj4gZnVuY3Rpb25zOiBwb3dlciBtYW5hZ2VtZW50IGFuZCB3YWtldXAgc291cmNlIG1hbmFn
ZW1lbnQuDQo+ID4+PiBUaGlzIHBhdGNoIGFkZHMgYSBuZXcgaXJxY2hpcCBkcml2ZXIgdG8gbWFu
YWdlIHRoZSBpbnRlcnJ1cHQgd2FrZXVwDQo+ID4+PiBzb3VyY2VzIG9uIElNWDdELg0KPiA+Pg0K
PiA+PiBJbnRlcmVzdGluZywgeW91IG1lbnRpb24gdGhhdCB0aGlzIElQIGJsb2NrIGhhcyBtYWlu
bHkgcG93ZXINCj4gPj4gbWFuYWdlbWVudCBhbmQgaXQgaXRzZWxmIHJlcXVpcmVzIHNhdmUvcmVz
dG9yZS4gSXMgaXQgbm90IGluIGFsd2F5cyBvbg0KPiBkb21haW4gPw0KPiA+DQo+ID4gWWVzLCBp
dCBpcyBpbiBhbHdheXMgb24gZG9tYWluLg0KPiA+DQo+IA0KPiBIbW0sIHRoZW4gd2h5IGRvIHlv
dSBuZWVkIHRvIHNhdmUgYW5kIHJlc3RvcmUgdGhlIG1hc2sgPw0KDQpUaGUgc2F2ZS9yZXN0b3Jl
IGlzIHVzZWQgdG8gaGFuZGxlIHRoZSB0d28gZGlmZmVyZW50IGxvdyBwb3dlciBzdGF0ZXM6IG9u
ZSBpcw0KV0ZJIGluIGNwdSBpZGxlLCBhbmQgdGhlIG90aGVyIGNhc2UgaXMgc3VzcGVuZC4gVGhp
cyBoYXMgbm90aGluZyB0byBkbyB3aXRoIHRoaXMNCklQIGJsb2NrIGl0c2VsZi4NCg0KPiA+Pj4g
V2hlbiB0aGUgc3lzdGVtIGlzIGluIFdGSSAod2FpdCBmb3IgaW50ZXJydXB0KSBtb2RlLCB0aGlz
IEdQQyBibG9jaw0KPiA+Pj4gd2lsbCBiZSB0aGUgZmlyc3QgYmxvY2sgb24gdGhlIHBsYXRmb3Jt
IHRvIGJlIGFjdGl2YXRlZCBhbmQgc2lnbmFsZWQuDQo+ID4+PiBVbmRlciBub3JtYWwgd2FpdCBt
b2RlIGR1cmluZyBjcHUgaWRsZSwgdGhlIHN5c3RlbSBjYW4gYmUgd29rZSB1cCBieQ0KPiA+Pj4g
K3N0YXRpYyB2b2lkIGdwY3YyX3dha2V1cF9zb3VyY2VfcmVzdG9yZSh2b2lkKSB7DQo+ID4+PiAr
CXN0cnVjdCBncGN2Ml9pcnFjaGlwX2RhdGEgKmNkOw0KPiA+Pj4gKwl2b2lkIF9faW9tZW0gKnJl
ZzsNCj4gPj4+ICsJaW50IGk7DQo+ID4+PiArDQo+ID4+PiArCWNkID0gaW14X2dwY3YyX2luc3Rh
bmNlOw0KPiA+Pj4gKwlpZiAoIWNkKQ0KPiA+Pj4gKwkJcmV0dXJuOw0KPiA+Pj4gKw0KPiA+Pj4g
Kwlmb3IgKGkgPSAwOyBpIDwgSU1SX05VTTsgaSsrKSB7DQo+ID4+PiArCQlyZWcgPSBjZC0+Z3Bj
X2Jhc2UgKyBjZC0+Y3B1Mndha2V1cCArIGkgKiA0Ow0KPiA+Pj4gKwkJd3JpdGVsX3JlbGF4ZWQo
Y2QtPnNhdmVkX2lycV9tYXNrW2ldLCByZWcpOw0KPiA+Pg0KPiA+PiBJbnN0ZWFkIG9mIHNhdmlu
ZyBhbGwgdGhlIG5vbi13YWtldXAgc291cmNlcywgY2FuJ3QgeW91IHVzZSByYXcNCj4gPj4gc2F2
ZS9yZXN0b3JlIG9mIHRoZXNlIHJlZ2lzdGVycyBhbmQgbWFzayBhbGwgdGhlIG5vbi13YWtldXAg
c291cmNlcw0KPiA+PiBieSBzZXR0aW5nIE1BU0tfT05fU1VTUEVORCA/DQo+ID4NCj4gPiBJIGNh
bid0IGNhdGNoIHdoYXQgeW91IG1lYW4uIENhbiB5b3Ugc2hvdyBtZSBhbiBleGFtcGxlPw0KPiA+
DQo+IA0KPiBXaGF0IEkgbWVhbnQgaXMgaW5zdGVhZCBvZiB5b3UgdHJhY2tpbmcgYWxsIHRoZSBl
bmFibGVkIGlycXMoYm90aCB3YWtldXAgYW5kDQo+IG5vbi13YWtldXApIGluIHNhdmVkX2lycV9t
YXNrLCBqdXN0IHNldCBNQVNLX09OX1NVU1BFTkQgZmxhZyB3aGVyZSBhbGwgdGhlDQo+IG5vbi13
YWtldXAgaW50ZXJydXB0cyBhcmUgbWFza2VkIG9uIHN1c3BlbmQgYW5kIHVubWFza2VkIG9uIHJl
c3VtZSBieSB0aGUNCj4gaXJxIGNvcmUuIFlvdSBjYW4gdGhlbiBqdXN0IHNhdmUgYW5kIHJlc3Rv
cmUgdGhlIHdha2V1cCBpcnEgbWFzaywgYnV0IGFzIEkgYXNrZWQNCj4gYWJvdmUgZG8geW91IGhh
dmUgdG8gZG8gdGhhdCBhcyB5b3UgYXJlIHNheWluZyBpdCdzIGluIGFsd2F5cyBvbiBkb21haW4g
Pw0KPiANCj4gPj4gQWxzbyB5b3VyIGludGVycnVwdCBjb250cm9sbGVyIHNlZW1zIGxpa2UgaGFz
IG5vIHNwZWNpYWwgd2F5IHRvDQo+ID4+IGNvbmZpZ3VyZSB3YWtldXBzLCB5b3UgYXJlIGp1c3Qg
bGVhdmluZyB0aGVtIGVuYWJsZWQuIGkuZS4gSSBzZWUNCj4gPj4gY3B1Mndha2V1cCB1c2VkIGZv
ciBib3RoIHt1bix9bWFza2luZyBhbmQgd2FrZXVwIGVuYWJsZS4gU28geW91IGNhbiBqdXN0DQo+
IHVzZSBJUlFDSElQX1NLSVBfU0VUX1dBS0UuDQo+ID4+IENvcnJlY3QgbWUgaWYgbXkgdW5kZXJz
dGFuZGluZyBpcyB3cm9uZy4NCj4gPg0KPiA+IEluIHRoaXMgZHJpdmVyLCB0aGUgQ29yZTAgaXMg
Y29uZmlndXJlZCB0byBiZSB0aGUgZGVmYXVsdCBjb3JlIHRvIGJlDQo+ID4gd29rZSB1cCwgbm90
IGJvdGguIFlvdSBjYW4gY29uZmlndXJlIGl0IHRvIENvcmUxIGJ5IGNoYW5naW5nIHRoZQ0KPiA+
IGNwdTJ3YWtldXAgdmFsdWUuIEkgZG9uJ3Qgc2VlIGFueSByZWFzb24gdG8gdXNlIElSUUNISVBf
U0tJUF9TRVRfV0FLRSBmbGFnLg0KPiA+DQo+IA0KPiBJIGRvbid0IHNlZSB0aGlzIGRyaXZlciBk
b2luZyBhbnl0aGluZyBleHRyYSBhcGFydCBmcm9tIGtlZXBpbmcgdGhlIHdha2V1cCBpcnFzDQo+
IGVuYWJsZWQuIGkuZS4gWW91IHVzZSB0aGUgc2FtZSBjcHUqd2FrZSByZWdpc3RlciB0byBtYXNr
L3VubWFzayB0aGUgaW50ZXJydXB0DQo+IGFzIHdlbGwgYXMgc2V0IHRoZSB3YWtldXAgc291cmNl
LiBTaW5jZSB0aGUgd2FrZXVwIGludGVycnVwdCB3aWxsIGJlIGVuYWJsZWQgYnkNCj4gdGhlIGRy
aXZlciwgeW91IGp1c3QgbmVlZCB0byBtYXJrIGl0IGFzIHdha2UtdXAgc291cmNlIGFuZCBub3Ro
aW5nIGV4dHJhIGluIHRoZQ0KPiBjb250cm9sbGVyIHJpZ2h0ID8NCj4gSWYgc28sIHlvdSBuZWVk
IHRvIHNldCBJUlFDSElQX1NLSVBfU0VUX1dBS0UgYXMgeW91IGFyZSBqdXN0IGxlYXZpbmcgdGhh
dCBpcnENCj4gZW5hYmxlZCBhbmQgbm90IGRvaW5nIGFueSBleHRyYSBjb25maWd1cmF0aW9uIHRv
IGVuYWJsZSBpdCBhcyB3YWtldXAgc291cmNlLg0KPiBQbGVhc2UgY29ycmVjdCBpZiB0aGF0IHdy
b25nLCBidXQgZnJvbSB0aGUgY29kZSB0aGF0J3Mgd2hhdCBJIGNvdWxkIGluZmVyLg0KDQpUaGVy
ZSBpcyBubyBzcGVjaWFsIGZvciB0aGlzIGRyaXZlci4gV2UganVzdCB1c2UgdGhlIElSUUNISVAg
ZHJpdmVyIGZyYW1ld29yayB0bw0KbWFuYWdlIHRoZSB3YWtldXAgc291cmNlcy4gV2h5IGRpZCB5
b3UgcHJvcG9zZSB0byBzZXQgSVJRQ0hJUF9TS0lQX1NFVF9XQUtFDQpmbGFnIGhlcmU/IElmIHlv
dSBkb24ndCBuZWVkIHRoZSB3YWtldXAgZmVhdHVyZSwgeW91IHNob3VsZCBqdXN0IG5vdCBlbmFi
bGUgdGhpcw0KZHJpdmVyIGluIHRoZSBjb25maWd1cmF0aW9uLg0KDQpUaGFua3MsDQpTaGVud2Vp
DQoNCj4gUmVnYXJkcywNCj4gU3VkZWVwDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213090

FromSudeep Holla <sudeep.holla@arm.com>
Date2015-08-25 16:50 +0200
Message-ID<q1nHY-ZC-5@gated-at.bofh.it>
In reply to#1213089

On 25/08/15 15:14, Shenwei Wang wrote:
>
>
>> -----Original Message-----
>> From: Sudeep Holla [mailto:sudeep.holla@arm.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 could infer.
>
> 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.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213091

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-25 17:00 +0200
Message-ID<q1nRE-1aY-1@gated-at.bofh.it>
In reply to#1213090
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogU3VkZWVwIEhvbGxhIFtt
YWlsdG86c3VkZWVwLmhvbGxhQGFybS5jb21dDQo+IFNlbnQ6IDIwMTXlubQ45pyIMjXml6UgOTo0
Ng0KPiBUbzogV2FuZyBTaGVud2VpLUIzODMzOQ0KPiBDYzogU3VkZWVwIEhvbGxhOyBzaGF3bi5n
dW9AbGluYXJvLm9yZzsgdGdseEBsaW51dHJvbml4LmRlOw0KPiBqYXNvbkBsYWtlZGFlbW9uLm5l
dDsgSHVhbmcgWW9uZ2NhaS1CMjA3ODg7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+
IGxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZw0KPiBTdWJqZWN0OiBSZTogW1BB
VENIIHY5IDEvMV0gaXJxY2hpcDogaW14LWdwY3YyOiBJTVggR1BDdjIgZHJpdmVyIGZvciB3YWtl
dXANCj4gc291cmNlcw0KPiANCj4gDQo+IA0KPiBPbiAyNS8wOC8xNSAxNToxNCwgU2hlbndlaSBX
YW5nIHdyb3RlOg0KPiA+DQo+ID4NCj4gPj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4g
Pj4gRnJvbTogU3VkZWVwIEhvbGxhIFttYWlsdG86c3VkZWVwLmhvbGxhQGFybS5jb21dDQo+IA0K
PiBbLi4uXQ0KPiANCj4gPj4gSSBkb24ndCBzZWUgdGhpcyBkcml2ZXIgZG9pbmcgYW55dGhpbmcg
ZXh0cmEgYXBhcnQgZnJvbSBrZWVwaW5nIHRoZQ0KPiA+PiB3YWtldXAgaXJxcyBlbmFibGVkLiBp
LmUuIFlvdSB1c2UgdGhlIHNhbWUgY3B1Kndha2UgcmVnaXN0ZXIgdG8NCj4gPj4gbWFzay91bm1h
c2sgdGhlIGludGVycnVwdCBhcyB3ZWxsIGFzIHNldCB0aGUgd2FrZXVwIHNvdXJjZS4gU2luY2Ug
dGhlDQo+ID4+IHdha2V1cCBpbnRlcnJ1cHQgd2lsbCBiZSBlbmFibGVkIGJ5IHRoZSBkcml2ZXIs
IHlvdSBqdXN0IG5lZWQgdG8gbWFyaw0KPiA+PiBpdCBhcyB3YWtlLXVwIHNvdXJjZSBhbmQgbm90
aGluZyBleHRyYSBpbiB0aGUgY29udHJvbGxlciByaWdodCA/DQo+ID4+IElmIHNvLCB5b3UgbmVl
ZCB0byBzZXQgSVJRQ0hJUF9TS0lQX1NFVF9XQUtFIGFzIHlvdSBhcmUganVzdCBsZWF2aW5nDQo+
ID4+IHRoYXQgaXJxIGVuYWJsZWQgYW5kIG5vdCBkb2luZyBhbnkgZXh0cmEgY29uZmlndXJhdGlv
biB0byBlbmFibGUgaXQgYXMgd2FrZXVwDQo+IHNvdXJjZS4NCj4gPj4gUGxlYXNlIGNvcnJlY3Qg
aWYgdGhhdCB3cm9uZywgYnV0IGZyb20gdGhlIGNvZGUgdGhhdCdzIHdoYXQgSSBjb3VsZCBpbmZl
ci4NCj4gPg0KPiA+IFRoZXJlIGlzIG5vIHNwZWNpYWwgZm9yIHRoaXMgZHJpdmVyLiBXZSBqdXN0
IHVzZSB0aGUgSVJRQ0hJUCBkcml2ZXINCj4gPiBmcmFtZXdvcmsgdG8gbWFuYWdlIHRoZSB3YWtl
dXAgc291cmNlcy4gV2h5IGRpZCB5b3UgcHJvcG9zZSB0byBzZXQNCj4gPiBJUlFDSElQX1NLSVBf
U0VUX1dBS0UgZmxhZyBoZXJlPyBJZiB5b3UgZG9uJ3QgbmVlZCB0aGUgd2FrZXVwIGZlYXR1cmUs
DQo+ID4geW91IHNob3VsZCBqdXN0IG5vdCBlbmFibGUgdGhpcyBkcml2ZXIgaW4gdGhlIGNvbmZp
Z3VyYXRpb24uDQo+ID4NCj4gDQo+IE5vLCBpZiB0aGUgZHJpdmVyIGRvZXNuJ3Qgbm90aGluZyBl
eHRyYSB0byBjb25maWd1cmUgdGhlIHdha2UgdXAgc291cmNlIG90aGVyIHRoYW4NCj4ga2VlcGlu
ZyBpdCBlbmFibGVkLCB0aGVuIGl0IGZpdHMgdGhlIGNhc2Ugb2YgU0tJUF9TRVRfV0FLRS4NCj4g
VGhlIGRyaXZlciB1c2luZyB0aGlzIHdha2Ugd291bGQgaGF2ZSByZXF1ZXN0ZWQgYW5kIGVuYWJs
ZWQgdGhlIGlycS4NCj4gV2hlbiBpdCBjYWxscyBlbmFibGVfaXJxX3dha2UsIHlvdSBoYXZlIG5v
dGhpbmcgZXh0cmEgdG8gc2V0KGF0bGVhc3QgZnJvbSB0aGUNCj4gbG9va3Mgb2YgdGhlIGRyaXZl
ciksIHNvIHNldHRpbmcgU0tJUF9TRVRfV0FLRSB3aWxsIHNraXAgdGhlIGNhbGwgYW5kIHVwZGF0
ZWQgdGhlDQo+IHdha2UgZmxhZ3MgaW4gaXJxIGNvcmUuDQo+IA0KPiBJIGRvbid0IHNlZSB0aGUg
cmVhbCBuZWVkIG9mIDIgc2VwYXJhdGUgc2V0cyBvZiBpcnEgbWFzayBiZWluZyBzYXZlZCBpbiBl
aXRoZXIgY2FzZS4NCg0KWW91IGRvbid0IHJlYWxseSB1bmRlcnN0YW5kIHdoYXQgaGFwcGVucyBh
ZnRlciBhIGRyaXZlciBjYWxscyBlbmFibGVfaXJxX3dha2UuIEluIHN1c3BlbmQgc3RhdGUsIGV2
ZW4gdGhlIGludGVycnVwdA0KY29udHJvbGxlciBpdHNlbGYgaXMgcG93ZXJlZCBvZmYuIEhvdyBj
YW4geW91IGdldCB0aGUgc3lzdGVtIHVwIGFnYWluIGJ5IGp1c3QgdXNpbmcgYSBTS0lQX1NFVF9X
QUtFLg0KDQpSZWdhcmRzLA0KU2hlbndlaQ0KDQo+IFJlZ2FyZHMsDQo+IFN1ZGVlcA0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213160

FromSudeep Holla <sudeep.holla@arm.com>
Date2015-08-25 18:30 +0200
Message-ID<q1pgK-3rY-21@gated-at.bofh.it>
In reply to#1213091

On 25/08/15 15:54, Shenwei Wang wrote:
>
>
>> -----Original Message-----
>> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
>> Sent: 2015年8月25日 9:46
>> To: Wang Shenwei-B38339
>> Cc: Sudeep Holla; shawn.guo@linaro.org; tglx@linutronix.de;
>> jason@lakedaemon.net; Huang Yongcai-B20788; linux-kernel@vger.kernel.org;
>> linux-arm-kernel@lists.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@arm.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213269

FromThomas Gleixner <tglx@linutronix.de>
Date2015-08-25 21:30 +0200
Message-ID<q1s4V-7ul-15@gated-at.bofh.it>
In reply to#1213160
On Tue, 25 Aug 2015, Sudeep Holla wrote:
> On 25/08/15 15:54, Shenwei Wang wrote:
> > 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 ?

I missed that when I reviewed the patch. You are right, it can be
simplified.
 
> Now that it's already done for you, you need not do anything extra and
> hence just set SKIP_SET_WAKE to do nothing.

He still needs the set_wake function to capture the wake enabled
interrupts as they are handed over to the low level asm code via
imx_gpcv2_get_wakeup_source(). Though they could be read back from the
hw registers as well.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213675

FromSudeep Holla <sudeep.holla@arm.com>
Date2015-08-26 11:00 +0200
Message-ID<q1EIO-OI-7@gated-at.bofh.it>
In reply to#1213269
Hi Thomas,

On 25/08/15 20:26, Thomas Gleixner wrote:
> On Tue, 25 Aug 2015, Sudeep Holla wrote:
>> On 25/08/15 15:54, Shenwei Wang wrote:
>>> 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 ?
>
> I missed that when I reviewed the patch. You are right, it can be
> simplified.
>
>> Now that it's already done for you, you need not do anything extra and
>> hence just set SKIP_SET_WAKE to do nothing.

Thanks for confirming this.

>
> He still needs the set_wake function to capture the wake enabled
> interrupts as they are handed over to the low level asm code via
> imx_gpcv2_get_wakeup_source(). Though they could be read back from the
> hw registers as well.
>

Correct, I have no objection on how it's saved/restored but was against
having 2 different masks and with the approach taken in this patch the
non-wakeup interrupts are enabled until syscore_suspend which is wrong.

There's whole lot of drivers blindly copy pasting this as theme which I
am going through and trying to clean up. I wanted to ensure no more such
additions happen meanwhile, so I am watching such patches closely.

Regards,
Sudeep
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213271

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-25 21:30 +0200
Message-ID<q1s4W-7ul-33@gated-at.bofh.it>
In reply to#1213160
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogU3VkZWVwIEhvbGxhIFtt
YWlsdG86c3VkZWVwLmhvbGxhQGFybS5jb21dDQo+IFNlbnQ6IDIwMTXlubQ45pyIMjXml6UgMTE6
MjQNCj4gVG86IFdhbmcgU2hlbndlaS1CMzgzMzkNCj4gQ2M6IFN1ZGVlcCBIb2xsYTsgc2hhd24u
Z3VvQGxpbmFyby5vcmc7IHRnbHhAbGludXRyb25peC5kZTsNCj4gamFzb25AbGFrZWRhZW1vbi5u
ZXQ7IEh1YW5nIFlvbmdjYWktQjIwNzg4OyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnOw0K
PiBsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcNCj4gU3ViamVjdDogUmU6IFtQ
QVRDSCB2OSAxLzFdIGlycWNoaXA6IGlteC1ncGN2MjogSU1YIEdQQ3YyIGRyaXZlciBmb3Igd2Fr
ZXVwDQo+ID4NCj4gPiBZb3UgZG9uJ3QgcmVhbGx5IHVuZGVyc3RhbmQgd2hhdCBoYXBwZW5zIGFm
dGVyIGEgZHJpdmVyIGNhbGxzDQo+ID4gZW5hYmxlX2lycV93YWtlLiBJbiBzdXNwZW5kIHN0YXRl
LCBldmVuIHRoZSBpbnRlcnJ1cHQgY29udHJvbGxlcg0KPiA+IGl0c2VsZiBpcyBwb3dlcmVkIG9m
Zi4gSG93IGNhbiB5b3UgZ2V0IHRoZSBzeXN0ZW0gdXAgYWdhaW4gIGJ5IGp1c3QNCj4gPiB1c2lu
ZyBhIFNLSVBfU0VUX1dBS0UuDQo+ID4NCj4gDQo+IFNvcnJ5IGZvciB0aGF0LCBsZXQgbWUgdHJ5
IHRvIHVuZGVyc3RhbmQgYWxvdWQuIFNvIHlvdSBoYXZlIGlycV97dW4sfW1hc2sgZnVuY3Rpb24N
Cj4gdGhhdCBhcmUgY2FsbGVkIHdoZW4gaW50ZXJydXB0cyBhcmUgZW5hYmxlZCBhbmQgZGlzYWJs
ZWQuIFNvIHN1cHBvc2UgeW91IGhhdmUgMw0KPiBpcnFzIHRoYXQgYXJlIGVuYWJsZWQgYW5kIG9u
bHkgb25lIG9mIHRoZW4gaXMgc2V0IGFzIHdha2V1cCBzb3VyY2UuDQo+IA0KPiBOb3cgeW91IGNh
bGwgZW5hYmxlX2lycV93YWtlLCB5b3Ugc2F2ZSB0aGF0IGluIHdha2V1cF9zb3VyY2VzLCBmaW5l
Lg0KPiBMYXRlciB3aGVuIHlvdSBlbnRlciBzdXNwZW5kLCB5b3Ugc2F2ZSBhbGwgdGhlIDMgYWN0
aXZlIGlycXMgaW4gc2F2ZWRfaXJxX21hc2sNCj4gYW5kIG92ZXItd3JpdGUgY3B1Mndha2V1cCB3
aXRoIHdha2V1cF9zb3VyY2VzLCByaWdodD8NCj4gDQo+IEFsbCBmaW5lLCB3aGF0IEkgYW0gc2F5
aW5nIGlzIGxldCBpcnEtY29yZSBrbm93IHRoYXQgeW91IHdhbnQgdG8gbWFzayB0aGUgMg0KPiBu
b24td2FrZXVwIGlycXMgeW91IGhhdmUgdXNpbmcgTUFTS19PTl9TVVNQRU5ELiBTbyB3aGVuDQo+
IHN1c3BlbmRfZGV2aWNlX2lycXMgaXMgY2FsbGVkIGluIHN1c3BlbmQgcGF0aCwgdGhhdCdzIGRv
bmUgZm9yIHlvdSBhdXRvbWF0aWNhbGx5DQo+IGFuZCB0aGUgY3B1Mndha2V1cCB3aWxsIGhhdmUg
anVzdCAxIHdha2V1cCBlbmFibGVkIHdoaWNoIGlzIHdoYXQgeW91IGFyZSBkb2luZw0KPiBpbiBz
dXNwZW5kIGNhbGxiYWNrLCByaWdodCA/DQoNCgkvKg0KCSAqIEhhcmR3YXJlIHdoaWNoIGhhcyBu
byB3YWtldXAgc291cmNlIGNvbmZpZ3VyYXRpb24gZmFjaWxpdHkNCgkgKiByZXF1aXJlcyB0aGF0
IHRoZSBub24gd2FrZXVwIGludGVycnVwdHMgYXJlIG1hc2tlZCBhdCB0aGUNCgkgKiBjaGlwIGxl
dmVsLiBUaGUgY2hpcCBpbXBsZW1lbnRhdGlvbiBpbmRpY2F0ZXMgdGhhdCB3aXRoDQoJICogSVJR
Q0hJUF9NQVNLX09OX1NVU1BFTkQuDQoJICovDQoJaWYgKGlycV9kZXNjX2dldF9jaGlwKGRlc2Mp
LT5mbGFncyAmIElSUUNISVBfTUFTS19PTl9TVVNQRU5EKQ0KCQltYXNrX2lycShkZXNjKTsNCg0K
SVJRQ0hJUF9NQVNLX09OX1NVU1BFTkQgZmxhZyBpcyBmb3IgdGhlIGhhcmR3YXJlIHRoYXQgaGFz
IG5vIHdha2V1cCBzb3VyY2UgY2FwYWJpbGl0eS4NClRoaXMgR1BDdjIgYmxvY2sgaXMgZGVzaWdu
ZWQgdG8gbWFuYWdlIHRoZSB3YWtldXAgc291cmNlLCBzbyB0aGUgZmxhZyBkb2VzIG5vdCBtYWtl
IGFueSBzZW5zZS4NCg0KPiBOb3cgdGhhdCBpdCdzIGFscmVhZHkgZG9uZSBmb3IgeW91LCB5b3Ug
bmVlZCBub3QgZG8gYW55dGhpbmcgZXh0cmEgYW5kIGhlbmNlIGp1c3QNCj4gc2V0IFNLSVBfU0VU
X1dBS0UgdG8gZG8gbm90aGluZy4NCg0Kc3RhdGljIGludCBzZXRfaXJxX3dha2VfcmVhbCh1bnNp
Z25lZCBpbnQgaXJxLCB1bnNpZ25lZCBpbnQgb24pDQp7DQoJc3RydWN0IGlycV9kZXNjICpkZXNj
ID0gaXJxX3RvX2Rlc2MoaXJxKTsNCglpbnQgcmV0ID0gLUVOWElPOw0KDQoJaWYgKGlycV9kZXNj
X2dldF9jaGlwKGRlc2MpLT5mbGFncyAmICBJUlFDSElQX1NLSVBfU0VUX1dBS0UpDQoJCXJldHVy
biAwOw0KDQoJaWYgKGRlc2MtPmlycV9kYXRhLmNoaXAtPmlycV9zZXRfd2FrZSkNCgkJcmV0ID0g
ZGVzYy0+aXJxX2RhdGEuY2hpcC0+aXJxX3NldF93YWtlKCZkZXNjLT5pcnFfZGF0YSwgb24pOw0K
DQoJcmV0dXJuIHJldDsNCn0NCkZyb20gdGhlIGNvZGVzIGFib3ZlLCBpZiBhIGlycWNoaXAgY2Fu
IG5vdCBoYW5kbGUgdGhlIHdha2V1cCBzb3VyY2VzLCBpdCBjYW4gc2V0IFNLSVAgZmxhZy4NClRo
aXMgZHJpdmVyIGlzIGludGVuZGVkIHRvIG1hbmFnZSB0aGUgd2FrZXVwIHNvdXJjZXMsIHdoYXQn
cyB0aGUgcmVhc29uIHRvIHNraXAgaGVyZT8NCg0KUmVnYXJkcywNClNoZW53ZWkNCg0KDQo+IEhv
cGUgdGhpcyBjbGFyaWZpZXMsIHNvcnJ5IGlmIEkgYW0gc3RpbGwgbWlzc2luZyB0byB1bmRlcnN0
YW5kIHNvbWV0aGluZyBoZXJlLCBidXQgSQ0KPiBkb24ndCBzZWUgYW55dGhpbmcuIExldCBtZSBr
bm93Lg0KPiANCj4gUmVnYXJkcywNCj4gU3VkZWVwDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213273

FromThomas Gleixner <tglx@linutronix.de>
Date2015-08-25 21:40 +0200
Message-ID<q1seC-7G4-19@gated-at.bofh.it>
In reply to#1213271
On Tue, 25 Aug 2015, Shenwei Wang wrote:
> > From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> > 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 ?
> 
> 	/*
> 	 * Hardware which has no wakeup source configuration facility
> 	 * requires that the non wakeup interrupts are masked at the
> 	 * chip level. The chip implementation indicates that with
> 	 * IRQCHIP_MASK_ON_SUSPEND.
> 	 */
> 	if (irq_desc_get_chip(desc)->flags & IRQCHIP_MASK_ON_SUSPEND)
> 		mask_irq(desc);
> 
> IRQCHIP_MASK_ON_SUSPEND flag is for the hardware that has no wakeup
> source capability.  This GPCv2 block is designed to manage the
> wakeup source, so the flag does not make any sense.

You have no seperate wakeup source mechanism. All you do is to mask all
non wakeup sources and keep the wakeup sources unmask.

That's what happens in gpcv2_wakeup_source_save()

       writel_relaxed(cd->wakeup_sources[i], reg);

So it's the same as letting the core mask all non wakeup sources and
leave the wakeup sources unmask.

> > Now that it's already done for you, you need not do anything extra and hence just
> > set SKIP_SET_WAKE to do nothing.
> 
> static int set_irq_wake_real(unsigned int irq, unsigned int on)
> {
> 	struct irq_desc *desc = irq_to_desc(irq);
> 	int ret = -ENXIO;
> 
> 	if (irq_desc_get_chip(desc)->flags &  IRQCHIP_SKIP_SET_WAKE)
> 		return 0;
> 
> 	if (desc->irq_data.chip->irq_set_wake)
> 		ret = desc->irq_data.chip->irq_set_wake(&desc->irq_data, on);
> 
> 	return ret;
> }
> From the codes above, if a irqchip can not handle the wakeup
> sources, it can set SKIP flag.  This driver is intended to manage
> the wakeup sources, what's the reason to skip here?

To reduce code. Everything the core can do for you is something you
don't have to implement.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213285

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-25 22:00 +0200
Message-ID<q1sxY-870-7@gated-at.bofh.it>
In reply to#1213273
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogVGhvbWFzIEdsZWl4bmVy
IFttYWlsdG86dGdseEBsaW51dHJvbml4LmRlXQ0KPiBTZW50OiAyMDE1xOo41MIyNcjVIDE0OjMw
DQo+IFRvOiBXYW5nIFNoZW53ZWktQjM4MzM5DQo+IENjOiBTdWRlZXAgSG9sbGE7IHNoYXduLmd1
b0BsaW5hcm8ub3JnOyBqYXNvbkBsYWtlZGFlbW9uLm5ldDsgSHVhbmcNCj4gWW9uZ2NhaS1CMjA3
ODg7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+IGxpbnV4LWFybS1rZXJuZWxAbGlz
dHMuaW5mcmFkZWFkLm9yZw0KPiBTdWJqZWN0OiBSRTogW1BBVENIIHY5IDEvMV0gaXJxY2hpcDog
aW14LWdwY3YyOiBJTVggR1BDdjIgZHJpdmVyIGZvciB3YWtldXANCj4gc291cmNlcw0KPiANCj4g
T24gVHVlLCAyNSBBdWcgMjAxNSwgU2hlbndlaSBXYW5nIHdyb3RlOg0KPiA+ID4gRnJvbTogU3Vk
ZWVwIEhvbGxhIFttYWlsdG86c3VkZWVwLmhvbGxhQGFybS5jb21dIEFsbCBmaW5lLCB3aGF0IEkg
YW0NCj4gPiA+IHNheWluZyBpcyBsZXQgaXJxLWNvcmUga25vdyB0aGF0IHlvdSB3YW50IHRvIG1h
c2sgdGhlIDIgbm9uLXdha2V1cA0KPiA+ID4gaXJxcyB5b3UgaGF2ZSB1c2luZyBNQVNLX09OX1NV
U1BFTkQuIFNvIHdoZW4gc3VzcGVuZF9kZXZpY2VfaXJxcyBpcw0KPiA+ID4gY2FsbGVkIGluIHN1
c3BlbmQgcGF0aCwgdGhhdCdzIGRvbmUgZm9yIHlvdSBhdXRvbWF0aWNhbGx5IGFuZCB0aGUNCj4g
PiA+IGNwdTJ3YWtldXAgd2lsbCBoYXZlIGp1c3QgMSB3YWtldXAgZW5hYmxlZCB3aGljaCBpcyB3
aGF0IHlvdSBhcmUNCj4gPiA+IGRvaW5nIGluIHN1c3BlbmQgY2FsbGJhY2ssIHJpZ2h0ID8NCj4g
Pg0KPiA+IAkvKg0KPiA+IAkgKiBIYXJkd2FyZSB3aGljaCBoYXMgbm8gd2FrZXVwIHNvdXJjZSBj
b25maWd1cmF0aW9uIGZhY2lsaXR5DQo+ID4gCSAqIHJlcXVpcmVzIHRoYXQgdGhlIG5vbiB3YWtl
dXAgaW50ZXJydXB0cyBhcmUgbWFza2VkIGF0IHRoZQ0KPiA+IAkgKiBjaGlwIGxldmVsLiBUaGUg
Y2hpcCBpbXBsZW1lbnRhdGlvbiBpbmRpY2F0ZXMgdGhhdCB3aXRoDQo+ID4gCSAqIElSUUNISVBf
TUFTS19PTl9TVVNQRU5ELg0KPiA+IAkgKi8NCj4gPiAJaWYgKGlycV9kZXNjX2dldF9jaGlwKGRl
c2MpLT5mbGFncyAmIElSUUNISVBfTUFTS19PTl9TVVNQRU5EKQ0KPiA+IAkJbWFza19pcnEoZGVz
Yyk7DQo+ID4NCj4gPiBJUlFDSElQX01BU0tfT05fU1VTUEVORCBmbGFnIGlzIGZvciB0aGUgaGFy
ZHdhcmUgdGhhdCBoYXMgbm8gd2FrZXVwDQo+ID4gc291cmNlIGNhcGFiaWxpdHkuICBUaGlzIEdQ
Q3YyIGJsb2NrIGlzIGRlc2lnbmVkIHRvIG1hbmFnZSB0aGUgd2FrZXVwDQo+ID4gc291cmNlLCBz
byB0aGUgZmxhZyBkb2VzIG5vdCBtYWtlIGFueSBzZW5zZS4NCj4gDQo+IFlvdSBoYXZlIG5vIHNl
cGVyYXRlIHdha2V1cCBzb3VyY2UgbWVjaGFuaXNtLiBBbGwgeW91IGRvIGlzIHRvIG1hc2sgYWxs
IG5vbg0KPiB3YWtldXAgc291cmNlcyBhbmQga2VlcCB0aGUgd2FrZXVwIHNvdXJjZXMgdW5tYXNr
Lg0KPiANCj4gVGhhdCdzIHdoYXQgaGFwcGVucyBpbiBncGN2Ml93YWtldXBfc291cmNlX3NhdmUo
KQ0KPiANCj4gICAgICAgIHdyaXRlbF9yZWxheGVkKGNkLT53YWtldXBfc291cmNlc1tpXSwgcmVn
KTsNCj4gDQo+IFNvIGl0J3MgdGhlIHNhbWUgYXMgbGV0dGluZyB0aGUgY29yZSBtYXNrIGFsbCBu
b24gd2FrZXVwIHNvdXJjZXMgYW5kIGxlYXZlIHRoZQ0KPiB3YWtldXAgc291cmNlcyB1bm1hc2su
DQoNCkRvZXMgaXQgbWVhbiBhbiB1bmV4cGVjdGVkIGludGVycnVwdCBtYXkgYWN0aXZhdGUgdGhl
IHN5c3RlbSwgYW5kIHRoZSBjb3JlIHdpbGwgbGV0DQp0aGUgc3lzdGVtIGdvIGludG8gc3VzcGVu
ZCBhZ2FpbiBpZiB0aGUgY29yZSBkZXRlcm1pbmVzIGl0IG5vdCBhIHdha2V1cCBzb3VyY2U/IFRo
ZSBjdXJyZW50IGRlc2lnbg0KaXMgdG8gaWdub3JlIGFsbCB0aGUgdW5leHBlY3RlZCBpbnRlcnJ1
cHRzIGluIHRoZSBoYXJkd2FyZSBsZXZlbC4gT25seSB0aGUgcHJlc2V0dGluZyANCndha2V1cCBz
b3VyY2VzIGNhbiBhY3RpdmF0ZSB0aGUgcGxhdGZvcm0uIEhlcmUgcG93ZXIgY29uc3VtcHRpb24g
aXMgbW9yZSANCmltcG9ydGFudC4NCg0KPiA+ID4gTm93IHRoYXQgaXQncyBhbHJlYWR5IGRvbmUg
Zm9yIHlvdSwgeW91IG5lZWQgbm90IGRvIGFueXRoaW5nIGV4dHJhDQo+ID4gPiBhbmQgaGVuY2Ug
anVzdCBzZXQgU0tJUF9TRVRfV0FLRSB0byBkbyBub3RoaW5nLg0KPiA+DQo+ID4gc3RhdGljIGlu
dCBzZXRfaXJxX3dha2VfcmVhbCh1bnNpZ25lZCBpbnQgaXJxLCB1bnNpZ25lZCBpbnQgb24pIHsN
Cj4gPiAJc3RydWN0IGlycV9kZXNjICpkZXNjID0gaXJxX3RvX2Rlc2MoaXJxKTsNCj4gPiAJaW50
IHJldCA9IC1FTlhJTzsNCj4gPg0KPiA+IAlpZiAoaXJxX2Rlc2NfZ2V0X2NoaXAoZGVzYyktPmZs
YWdzICYgIElSUUNISVBfU0tJUF9TRVRfV0FLRSkNCj4gPiAJCXJldHVybiAwOw0KPiA+DQo+ID4g
CWlmIChkZXNjLT5pcnFfZGF0YS5jaGlwLT5pcnFfc2V0X3dha2UpDQo+ID4gCQlyZXQgPSBkZXNj
LT5pcnFfZGF0YS5jaGlwLT5pcnFfc2V0X3dha2UoJmRlc2MtPmlycV9kYXRhLCBvbik7DQo+ID4N
Cj4gPiAJcmV0dXJuIHJldDsNCj4gPiB9DQo+ID4gRnJvbSB0aGUgY29kZXMgYWJvdmUsIGlmIGEg
aXJxY2hpcCBjYW4gbm90IGhhbmRsZSB0aGUgd2FrZXVwIHNvdXJjZXMsDQo+ID4gaXQgY2FuIHNl
dCBTS0lQIGZsYWcuICBUaGlzIGRyaXZlciBpcyBpbnRlbmRlZCB0byBtYW5hZ2UgdGhlIHdha2V1
cA0KPiA+IHNvdXJjZXMsIHdoYXQncyB0aGUgcmVhc29uIHRvIHNraXAgaGVyZT8NCj4gDQo+IFRv
IHJlZHVjZSBjb2RlLiBFdmVyeXRoaW5nIHRoZSBjb3JlIGNhbiBkbyBmb3IgeW91IGlzIHNvbWV0
aGluZyB5b3UgZG9uJ3QgaGF2ZSB0bw0KPiBpbXBsZW1lbnQuDQoNClNhbWUgYXMgYWJvdmUuDQoN
ClRoYW5rcywNClNoZW53ZWkNCg0KPiBUaGFua3MsDQo+IA0KPiAJdGdseA0K
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213297

FromThomas Gleixner <tglx@linutronix.de>
Date2015-08-25 22:20 +0200
Message-ID<q1sRk-ky-11@gated-at.bofh.it>
In reply to#1213285
On Tue, 25 Aug 2015, Shenwei Wang wrote:
> > From: Thomas Gleixner [mailto:tglx@linutronix.de]
> > > IRQCHIP_MASK_ON_SUSPEND flag is for the hardware that has no wakeup
> > > source capability.  This GPCv2 block is designed to manage the wakeup
> > > source, so the flag does not make any sense.
> > 
> > You have no seperate wakeup source mechanism. All you do is to mask all non
> > wakeup sources and keep the wakeup sources unmask.
> > 
> > That's what happens in gpcv2_wakeup_source_save()
> > 
> >        writel_relaxed(cd->wakeup_sources[i], reg);
> > 
> > So it's the same as letting the core mask all non wakeup sources and leave the
> > wakeup sources unmask.
> 
> Does it mean an unexpected interrupt may activate the system, and
> the core will let the system go into suspend again if the core
> determines it not a wakeup source? The current design is to ignore
> all the unexpected interrupts in the hardware level. Only the
> presetting wakeup sources can activate the platform. Here power
> consumption is more important.

Did you actually read, what I wrote?

The core does in case of MASK_ON_SUSPEND

    for_each_irq() {
	if (!irq->wakeupsource)
	   mask(irq)
    }

That's identical to what you are doing. You just do it differently by
saving the active wakeup sources in your own data structure and then
write that info to the mask register, which leaves only the wakeup
sources unmasked.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213337

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-25 22:50 +0200
Message-ID<q1tkm-SD-13@gated-at.bofh.it>
In reply to#1213297
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogVGhvbWFzIEdsZWl4bmVy
IFttYWlsdG86dGdseEBsaW51dHJvbml4LmRlXQ0KPiBTZW50OiAyMDE1xOo41MIyNcjVIDE1OjE2
DQo+IFRvOiBXYW5nIFNoZW53ZWktQjM4MzM5DQo+IENjOiBTdWRlZXAgSG9sbGE7IHNoYXduLmd1
b0BsaW5hcm8ub3JnOyBqYXNvbkBsYWtlZGFlbW9uLm5ldDsgSHVhbmcNCj4gWW9uZ2NhaS1CMjA3
ODg7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+IGxpbnV4LWFybS1rZXJuZWxAbGlz
dHMuaW5mcmFkZWFkLm9yZw0KPiBTdWJqZWN0OiBSRTogW1BBVENIIHY5IDEvMV0gaXJxY2hpcDog
aW14LWdwY3YyOiBJTVggR1BDdjIgZHJpdmVyIGZvciB3YWtldXANCj4gc291cmNlcw0KPiANCj4g
T24gVHVlLCAyNSBBdWcgMjAxNSwgU2hlbndlaSBXYW5nIHdyb3RlOg0KPiA+ID4gRnJvbTogVGhv
bWFzIEdsZWl4bmVyIFttYWlsdG86dGdseEBsaW51dHJvbml4LmRlXQ0KPiA+ID4gPiBJUlFDSElQ
X01BU0tfT05fU1VTUEVORCBmbGFnIGlzIGZvciB0aGUgaGFyZHdhcmUgdGhhdCBoYXMgbm8NCj4g
PiA+ID4gd2FrZXVwIHNvdXJjZSBjYXBhYmlsaXR5LiAgVGhpcyBHUEN2MiBibG9jayBpcyBkZXNp
Z25lZCB0byBtYW5hZ2UNCj4gPiA+ID4gdGhlIHdha2V1cCBzb3VyY2UsIHNvIHRoZSBmbGFnIGRv
ZXMgbm90IG1ha2UgYW55IHNlbnNlLg0KPiA+ID4NCj4gPiA+IFlvdSBoYXZlIG5vIHNlcGVyYXRl
IHdha2V1cCBzb3VyY2UgbWVjaGFuaXNtLiBBbGwgeW91IGRvIGlzIHRvIG1hc2sNCj4gPiA+IGFs
bCBub24gd2FrZXVwIHNvdXJjZXMgYW5kIGtlZXAgdGhlIHdha2V1cCBzb3VyY2VzIHVubWFzay4N
Cj4gPiA+DQo+ID4gPiBUaGF0J3Mgd2hhdCBoYXBwZW5zIGluIGdwY3YyX3dha2V1cF9zb3VyY2Vf
c2F2ZSgpDQo+ID4gPg0KPiA+ID4gICAgICAgIHdyaXRlbF9yZWxheGVkKGNkLT53YWtldXBfc291
cmNlc1tpXSwgcmVnKTsNCj4gPiA+DQo+ID4gPiBTbyBpdCdzIHRoZSBzYW1lIGFzIGxldHRpbmcg
dGhlIGNvcmUgbWFzayBhbGwgbm9uIHdha2V1cCBzb3VyY2VzIGFuZA0KPiA+ID4gbGVhdmUgdGhl
IHdha2V1cCBzb3VyY2VzIHVubWFzay4NCj4gPg0KPiA+IERvZXMgaXQgbWVhbiBhbiB1bmV4cGVj
dGVkIGludGVycnVwdCBtYXkgYWN0aXZhdGUgdGhlIHN5c3RlbSwgYW5kIHRoZQ0KPiA+IGNvcmUg
d2lsbCBsZXQgdGhlIHN5c3RlbSBnbyBpbnRvIHN1c3BlbmQgYWdhaW4gaWYgdGhlIGNvcmUgZGV0
ZXJtaW5lcw0KPiA+IGl0IG5vdCBhIHdha2V1cCBzb3VyY2U/IFRoZSBjdXJyZW50IGRlc2lnbiBp
cyB0byBpZ25vcmUgYWxsIHRoZQ0KPiA+IHVuZXhwZWN0ZWQgaW50ZXJydXB0cyBpbiB0aGUgaGFy
ZHdhcmUgbGV2ZWwuIE9ubHkgdGhlIHByZXNldHRpbmcNCj4gPiB3YWtldXAgc291cmNlcyBjYW4g
YWN0aXZhdGUgdGhlIHBsYXRmb3JtLiBIZXJlIHBvd2VyIGNvbnN1bXB0aW9uIGlzDQo+ID4gbW9y
ZSBpbXBvcnRhbnQuDQo+IA0KPiBEaWQgeW91IGFjdHVhbGx5IHJlYWQsIHdoYXQgSSB3cm90ZT8N
Cj4gDQo+IFRoZSBjb3JlIGRvZXMgaW4gY2FzZSBvZiBNQVNLX09OX1NVU1BFTkQNCj4gDQo+ICAg
ICBmb3JfZWFjaF9pcnEoKSB7DQo+IAlpZiAoIWlycS0+d2FrZXVwc291cmNlKQ0KPiAJICAgbWFz
ayhpcnEpDQo+ICAgICB9DQo+IA0KPiBUaGF0J3MgaWRlbnRpY2FsIHRvIHdoYXQgeW91IGFyZSBk
b2luZy4gWW91IGp1c3QgZG8gaXQgZGlmZmVyZW50bHkgYnkgc2F2aW5nIHRoZQ0KPiBhY3RpdmUg
d2FrZXVwIHNvdXJjZXMgaW4geW91ciBvd24gZGF0YSBzdHJ1Y3R1cmUgYW5kIHRoZW4gd3JpdGUg
dGhhdCBpbmZvIHRvIHRoZQ0KPiBtYXNrIHJlZ2lzdGVyLCB3aGljaCBsZWF2ZXMgb25seSB0aGUg
d2FrZXVwIHNvdXJjZXMgdW5tYXNrZWQuDQoNClNvcnJ5LiBJIGp1c3QgdG9vayBhIHN0dWR5IG9u
IHRoZSB0d28gZmxhZ3M6IE1BU0tfT05fU1VTUEVORCBhbmQgSVJRQ0hJUF9TS0lQX1NFVF9XQUtF
Lg0KTUFTS19PTl9TVVNQRU5EIGZsYWcgZG9lcyBzaW1wbHkgdGhlIGltcGxlbWVudGF0aW9uLiAN
CklSUUNISVBfU0tJUF9TRVRfV0FLRSBmbGFnIGNhbid0IGJlIHVzZWQgaGVyZSBiZWNhdXNlIHRo
ZSB3YWtldXAgc291cmNlcyBhcmUgcmVxdWlyZWQgZm9yIHBvd2VyIG1hbmFnZW1lbnQuDQpJIHdp
bGwgc2VuZCBvdXQgYSBzdWJzZXF1ZW50IHBhdGNoIHRvIHNpbXBseSB0aGUgaW1wbGVtZW50YXRp
b24gYnkgdXNpbmcgdGhpcyBpZGVhLg0KDQpUaGFuayB5b3UgU3VkZWVwIGZvciB0aGUgaW5zaWdo
dGZ1bCByZXZpZXchDQoNClRoYW5rcywNClNoZW53ZWkNCg0KDQoNCj4gVGhhbmtzLA0KPiANCj4g
CXRnbHgNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213341

FromShenwei Wang <Shenwei.Wang@freescale.com>
Date2015-08-25 23:00 +0200
Message-ID<q1tu3-142-31@gated-at.bofh.it>
In reply to#1213337
DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogVGhvbWFzIEdsZWl4bmVy
IFttYWlsdG86dGdseEBsaW51dHJvbml4LmRlXQ0KPiBTZW50OiAyMDE1xOo41MIyNcjVIDE1OjUw
DQo+IFRvOiBXYW5nIFNoZW53ZWktQjM4MzM5DQo+IENjOiBTdWRlZXAgSG9sbGE7IHNoYXduLmd1
b0BsaW5hcm8ub3JnOyBqYXNvbkBsYWtlZGFlbW9uLm5ldDsgSHVhbmcNCj4gWW9uZ2NhaS1CMjA3
ODg7IGxpbnV4LWtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7DQo+IGxpbnV4LWFybS1rZXJuZWxAbGlz
dHMuaW5mcmFkZWFkLm9yZw0KPiBTdWJqZWN0OiBSRTogW1BBVENIIHY5IDEvMV0gaXJxY2hpcDog
aW14LWdwY3YyOiBJTVggR1BDdjIgZHJpdmVyIGZvciB3YWtldXANCj4gc291cmNlcw0KPiANCj4g
T24gVHVlLCAyNSBBdWcgMjAxNSwgU2hlbndlaSBXYW5nIHdyb3RlOg0KPiA+DQo+ID4gSVJRQ0hJ
UF9TS0lQX1NFVF9XQUtFIGZsYWcgY2FuJ3QgYmUgdXNlZCBoZXJlIGJlY2F1c2UgdGhlIHdha2V1
cA0KPiA+IHNvdXJjZXMgYXJlIHJlcXVpcmVkIGZvciBwb3dlciBtYW5hZ2VtZW50Lg0KPiANCj4g
WW91IGNvdWxkIHVzZSBpdC4gSW5zdGVhZCBvZiBjb3B5aW5nIHRoZSBzYXZlZCB2YWx1ZXMsIHlv
dSBjYW4gcmVhZCB0aGUgdmFsdWVzDQo+IGJhY2sgZnJvbSB0aGUgcmVnaXN0ZXIuDQoNCkl0IGlz
IGFuIG9wdGlvbiB0b28uIA0KDQpUaGFua3MsDQpTaGVud2VpDQoNCj4gVGhhbmtzLA0KPiANCj4g
CXRnbHgNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1213342

FromThomas Gleixner <tglx@linutronix.de>
Date2015-08-25 23:00 +0200
Message-ID<q1tu3-142-33@gated-at.bofh.it>
In reply to#1213337
On Tue, 25 Aug 2015, Shenwei Wang wrote:
> 
> IRQCHIP_SKIP_SET_WAKE flag can't be used here because the wakeup
> sources are required for power management.

You could use it. Instead of copying the saved values, you can read
the values back from the register.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web