Re: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation

From: Sudeep Holla
Date: Wed Aug 26 2015 - 13:11:03 EST

On 26/08/15 17:44, Shenwei Wang wrote:

-----Original Message-----
From: Sudeep Holla [mailto:sudeep.holla@xxxxxxx]
Sent: 2015å8æ26æ 11:13
To: Wang Shenwei-B38339; jason@xxxxxxxxxxxxxx
Cc: shawn.guo@xxxxxxxxxx; tglx@xxxxxxxxxxxxx; Sudeep Holla;
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Huang
Subject: Re: [PATCH 1/1] irqchip: imx-gpcv2: Simplify the implemenation

typo in $subject

Just noticed. Will change it.

On 26/08/15 16:49, Shenwei Wang wrote:
Based on Sudeep Holla's review comments, the implementation can be
simplified by using the two flags: IRQCHIP_SKIP_SET_WAKE and
IRQCHIP_MASK_ON_SUSPEND. This patch enables the flags in the struct
irq_chip and removes the unnecessory syscore_ops callbacks.

Signed-off-by: Shenwei Wang <>
drivers/irqchip/irq-imx-gpcv2.c | 83 +++++++----------------------------------
1 file changed, 13 insertions(+), 70 deletions(-)

diff --git a/drivers/irqchip/irq-imx-gpcv2.c
b/drivers/irqchip/irq-imx-gpcv2.c index 4a97afa..e25df78 100644
--- a/drivers/irqchip/irq-imx-gpcv2.c
+++ b/drivers/irqchip/irq-imx-gpcv2.c
@@ -22,7 +22,6 @@ 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;

@@ -30,79 +29,25 @@ static struct gpcv2_irqchip_data

u32 imx_gpcv2_get_wakeup_source(u32 **sources)

I assume this patch is against -next and I don't see any users of
imx_gpcv2_get_wakeup_source in -next.

If possible I would avoid exposing this function by implementing suspend_ops just
as before(just saving raw h/w reg values and restoring then back on resume w/o
tagging them as wakeup mask though they might be indeed wakeup mask).

In that way, this driver is self-contained and whatever imx code calls this function
will not have dependency on this driver, no ? Do you need access to
imx_gpcv2_get_wakeup_source too early in resume much before suspend_ops
resume ? I would like to see the user of that function to comment on that any

It is linux-next. The user is in the following patch which is under review.

I got lost trying to follow through that 1000 odd line of code with lots
of register accesses. I couldn't understand much, so I gave up.

Such details are abstracted well and hidden in firmware with PSCI(good
that it's enforced in ARM64, hopefully ARM32 also sees more adoption).

So, for the parts adding those 2 flags and removing the unnecessary
code, you can add:
Reviewed-by: Sudeep Holla <sudeep.holla@xxxxxxx>

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at