| Message ID | 1440443055-7291-1-git-send-email-shenwei.wang@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 24 Aug 2015, Shenwei Wang wrote: > > Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com> > Signed-off-by: Anson Huang <b20788@freescale.com> Ok, last question. How is Anson related to this? He did not convey your patch and in the first posted versions his SOB was never there. Thanks, tglx
> -----Original Message----- > From: Thomas Gleixner [mailto:tglx@linutronix.de] > Sent: 2015?8?24? 14:15 > To: Wang Shenwei-B38339 > Cc: shawn.guo@linaro.org; jason@lakedaemon.net; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Huang > Yongcai-B20788 > Subject: Re: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup > sources > > On Mon, 24 Aug 2015, Shenwei Wang wrote: > > > > Signed-off-by: Shenwei Wang <shenwei.wang@freescale.com> > > Signed-off-by: Anson Huang <b20788@freescale.com> > > Ok, last question. How is Anson related to this? He did not convey your patch and > in the first posted versions his SOB was never there. This patch was based on his internal version of GPCv2 driver. I reused some of his implementation and ported it to latest linux kernel. Adding his name here to give the credits to him. Thanks, Shenwei > Thanks, > > tglx
On Mon, 24 Aug 2015, Shenwei Wang wrote: > > Ok, last question. How is Anson related to this? He did not convey your patch and > > in the first posted versions his SOB was never there. > > This patch was based on his internal version of GPCv2 driver. I reused some of his > implementation and ported it to latest linux kernel. Adding his name here to give > the credits to him. So the proper way to express this is: Based-on-a-patch-by: Anson Signed-off-by: You The way you did it suggests that you wrote it and Anson conveyed it, which is not the case. I'll change it that way then. Thanks, tglx
Thank you, Thomas! Shenwei > -----Original Message----- > From: Thomas Gleixner [mailto:tglx@linutronix.de] > Sent: 2015?8?24? 14:30 > To: Wang Shenwei-B38339 > Cc: shawn.guo@linaro.org; jason@lakedaemon.net; > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Huang > Yongcai-B20788 > Subject: RE: [PATCH v9 1/1] irqchip: imx-gpcv2: IMX GPCv2 driver for wakeup > sources > > On Mon, 24 Aug 2015, Shenwei Wang wrote: > > > Ok, last question. How is Anson related to this? He did not convey > > > your patch and in the first posted versions his SOB was never there. > > > > This patch was based on his internal version of GPCv2 driver. I reused > > some of his implementation and ported it to latest linux kernel. > > Adding his name here to give the credits to him. > > So the proper way to express this is: > > Based-on-a-patch-by: Anson > Signed-off-by: You > > The way you did it suggests that you wrote it and Anson conveyed it, which is not > the case. > > I'll change it that way then. > > Thanks, > > tglx
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
> -----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. > > 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? > 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. Thanks, Shenwei > Regards, > Sudeep
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
> -----Original Message----- > From: Sudeep Holla [mailto:sudeep.holla@arm.com] > >>> 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 ? The save/restore is used to handle the two different low power states: one is WFI in cpu idle, and the other case is suspend. This has nothing to do with this IP block itself. > >>> 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. 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. Thanks, Shenwei > Regards, > Sudeep
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
> -----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 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. 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. Regards, Shenwei > Regards, > Sudeep
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
> -----Original Message----- > From: Sudeep Holla [mailto:sudeep.holla@arm.com] > Sent: 2015?8?25? 11:24 > 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 > > > > 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 ? /* * 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. > 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? Regards, Shenwei > Hope this clarifies, sorry if I am still missing to understand something here, but I > don't see anything. Let me know. > > Regards, > Sudeep
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
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
> -----Original Message----- > From: Thomas Gleixner [mailto:tglx@linutronix.de] > Sent: 2015?8?25? 14:30 > To: Wang Shenwei-B38339 > Cc: Sudeep Holla; shawn.guo@linaro.org; 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 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. 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. > > > 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. Same as above. Thanks, Shenwei > Thanks, > > tglx
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
> -----Original Message----- > From: Thomas Gleixner [mailto:tglx@linutronix.de] > Sent: 2015?8?25? 15:16 > To: Wang Shenwei-B38339 > Cc: Sudeep Holla; shawn.guo@linaro.org; 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 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. Sorry. I just took a study on the two flags: MASK_ON_SUSPEND and IRQCHIP_SKIP_SET_WAKE. MASK_ON_SUSPEND flag does simply the implementation. IRQCHIP_SKIP_SET_WAKE flag can't be used here because the wakeup sources are required for power management. I will send out a subsequent patch to simply the implementation by using this idea. Thank you Sudeep for the insightful review! Thanks, Shenwei > Thanks, > > tglx
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
> -----Original Message----- > From: Thomas Gleixner [mailto:tglx@linutronix.de] > Sent: 2015?8?25? 15:50 > To: Wang Shenwei-B38339 > Cc: Sudeep Holla; shawn.guo@linaro.org; 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 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. It is an option too. Thanks, Shenwei > Thanks, > > tglx
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
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); + } +} + +static struct syscore_ops imx_gpcv2_syscore_ops = { + .suspend = gpcv2_wakeup_source_save, + .resume = gpcv2_wakeup_source_restore, +}; + +static int imx_gpcv2_irq_set_wake(struct irq_data *d, unsigned int on) +{ + struct gpcv2_irqchip_data *cd = d->chip_data; + unsigned int idx = d->hwirq / 32; + unsigned long flags; + void __iomem *reg; + u32 mask, val; + + raw_spin_lock_irqsave(&cd->rlock, flags); + reg = cd->gpc_base + cd->cpu2wakeup + idx * 4; + mask = 1 << d->hwirq % 32; + val = cd->wakeup_sources[idx]; + + cd->wakeup_sources[idx] = on ? (val & ~mask) : (val | mask); + raw_spin_unlock_irqrestore(&cd->rlock, flags); + + /* + * Do *not* call into the parent, as the GIC doesn't have any + * wake-up facility... + */ + + return 0; +} + +static void imx_gpcv2_irq_unmask(struct irq_data *d) +{ + struct gpcv2_irqchip_data *cd = d->chip_data; + void __iomem *reg; + u32 val; + + raw_spin_lock(&cd->rlock); + reg = cd->gpc_base + cd->cpu2wakeup + d->hwirq / 32 * 4; + val = readl_relaxed(reg); + val &= ~(1 << d->hwirq % 32); + writel_relaxed(val, reg); + raw_spin_unlock(&cd->rlock); + + irq_chip_unmask_parent(d); +} + +static void imx_gpcv2_irq_mask(struct irq_data *d) +{ + struct gpcv2_irqchip_data *cd = d->chip_data; + void __iomem *reg; + u32 val; + + raw_spin_lock(&cd->rlock); + reg = cd->gpc_base + cd->cpu2wakeup + d->hwirq / 32 * 4; + val = readl_relaxed(reg); + val |= 1 << (d->hwirq % 32); + writel_relaxed(val, reg); + raw_spin_unlock(&cd->rlock); + + irq_chip_mask_parent(d); +} + +static struct irq_chip gpcv2_irqchip_data_chip = { + .name = "GPCv2", + .irq_eoi = irq_chip_eoi_parent, + .irq_mask = imx_gpcv2_irq_mask, + .irq_unmask = imx_gpcv2_irq_unmask, + .irq_set_wake = imx_gpcv2_irq_set_wake, + .irq_retrigger = irq_chip_retrigger_hierarchy, +#ifdef CONFIG_SMP + .irq_set_affinity = irq_chip_set_affinity_parent, +#endif +}; + +static int imx_gpcv2_domain_xlate(struct irq_domain *domain, + struct device_node *controller, + const u32 *intspec, + unsigned int intsize, + unsigned long *out_hwirq, + unsigned int *out_type) +{ + /* Shouldn't happen, really... */ + if (domain->of_node != controller) + return -EINVAL; + + /* Not GIC compliant */ + if (intsize != 3) + return -EINVAL; + + /* No PPI should point to this domain */ + if (intspec[0] != 0) + return -EINVAL; + + *out_hwirq = intspec[1]; + *out_type = intspec[2]; + return 0; +} + +static int imx_gpcv2_domain_alloc(struct irq_domain *domain, + unsigned int irq, unsigned int nr_irqs, + void *data) +{ + struct of_phandle_args *args = data; + struct of_phandle_args parent_args; + irq_hw_number_t hwirq; + int i; + + /* Not GIC compliant */ + if (args->args_count != 3) + return -EINVAL; + + /* No PPI should point to this domain */ + if (args->args[0] != 0) + return -EINVAL; + + /* Can't deal with this */ + hwirq = args->args[1]; + if (hwirq >= GPC_MAX_IRQS) + return -EINVAL; + + for (i = 0; i < nr_irqs; i++) { + irq_domain_set_hwirq_and_chip(domain, irq + i, hwirq + i, + &gpcv2_irqchip_data_chip, domain->host_data); + } + + parent_args = *args; + parent_args.np = domain->parent->of_node; + return irq_domain_alloc_irqs_parent(domain, irq, nr_irqs, &parent_args); +} + +static struct irq_domain_ops gpcv2_irqchip_data_domain_ops = { + .xlate = imx_gpcv2_domain_xlate, + .alloc = imx_gpcv2_domain_alloc, + .free = irq_domain_free_irqs_common, +}; + +static int __init imx_gpcv2_irqchip_init(struct device_node *node, + struct device_node *parent) +{ + struct irq_domain *parent_domain, *domain; + struct gpcv2_irqchip_data *cd; + int i; + + if (!parent) { + pr_err("%s: no parent, giving up\n", node->full_name); + return -ENODEV; + } + + parent_domain = irq_find_host(parent); + if (!parent_domain) { + pr_err("%s: unable to get parent domain\n", node->full_name); + return -ENXIO; + } + + cd = kzalloc(sizeof(struct gpcv2_irqchip_data), GFP_KERNEL); + if (!cd) { + pr_err("kzalloc failed!\n"); + return -ENOMEM; + } + + cd->gpc_base = of_iomap(node, 0); + if (!cd->gpc_base) { + pr_err("fsl-gpcv2: unable to map gpc registers\n"); + kfree(cd); + return -ENOMEM; + } + + domain = irq_domain_add_hierarchy(parent_domain, 0, GPC_MAX_IRQS, + node, &gpcv2_irqchip_data_domain_ops, cd); + if (!domain) { + iounmap(cd->gpc_base); + kfree(cd); + return -ENOMEM; + } + irq_set_default_host(domain); + + /* Initially mask all interrupts */ + for (i = 0; i < IMR_NUM; i++) { + writel_relaxed(~0, cd->gpc_base + GPC_IMR1_CORE0 + i * 4); + writel_relaxed(~0, cd->gpc_base + GPC_IMR1_CORE1 + i * 4); + cd->wakeup_sources[i] = ~0; + } + + /* Let CORE0 as the default CPU to wake up by GPC */ + cd->cpu2wakeup = GPC_IMR1_CORE0; + + /* + * Due to hardware design failure, need to make sure GPR + * interrupt(#32) is unmasked during RUN mode to avoid entering + * DSM by mistake. + */ + writel_relaxed(~0x1, cd->gpc_base + cd->cpu2wakeup); + + imx_gpcv2_instance = cd; + register_syscore_ops(&imx_gpcv2_syscore_ops); + + return 0; +} + +IRQCHIP_DECLARE(imx_gpcv2, "fsl,imx7d-gpc", imx_gpcv2_irqchip_init);