Re: [PATCH 3/3] irqchip: dw-apb-ictl: add PM support

From: Sebastian Hesselbarth
Date: Wed Oct 08 2014 - 14:20:01 EST

On 10/08/2014 01:58 PM, Jisheng Zhang wrote:
Hi Sebastian,

On Wed, 8 Oct 2014 04:50:53 -0700
Jisheng Zhang <jszhang@xxxxxxxxxxx> wrote:

Hi Sebastian,

On Wed, 8 Oct 2014 04:44:49 -0700
Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx> wrote:

On 10/08/2014 01:31 PM, Jisheng Zhang wrote:
Hi Thomas, Sebastian,

On Tue, 30 Sep 2014 14:52:54 -0700
Thomas Gleixner <tglx@xxxxxxxxxxxxx> wrote:

On Tue, 30 Sep 2014, Sebastian Hesselbarth wrote:
On 09/23/2014 08:35 AM, Jisheng Zhang wrote:
This patch adds in support for S2R for dw-apb-ictl irqchip driver.

Signed-off-by: Jisheng Zhang <jszhang@xxxxxxxxxxx>
drivers/irqchip/irq-dw-apb-ictl.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/irqchip/irq-dw-apb-ictl.c
index c136b67..53bb732 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -50,6 +50,21 @@ static void dw_apb_ictl_handler(unsigned int irq,
struct irq_desc *desc)
chained_irq_exit(chip, desc);

+#ifdef CONFIG_PM
+static void dw_apb_ictl_resume(struct irq_data *d)
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct irq_chip_type *ct = irq_data_get_chip_type(d);
+ irq_gc_lock(gc);
+ writel_relaxed(~0, gc->reg_base + ct->regs.enable);
+ writel_relaxed(*ct->mask_cache, gc->reg_base +
+ irq_gc_unlock(gc);

I agree with the overall change, but may this also be suited for a
generic irq_chip helper instead of being a driver specific one?

Maybe Thomas or Jason can comment on this.

If we have enough similar resume callbacks, yes.

Also, now that you are using writel_relaxed, I understand that both
writes above can happen in any order? Are there any implication we
have to consider, i.e. do we require any of the registers above to
be written first?

The registers sits at device type memory, the writes should happen in
the same order as before.

it is not about the location of the register but, as far as I
understand, when using {readl,writel}_relaxed the compiler is
free to reorder the calls. So, if there is a strict order we

The "volatile" in readl/writel relaxed implementations should prevent the
compiler to do reorder. Or I misunderstand something?

My understanding is that the relaxed version imply compiler barriers.
I'm not sure I understand the real/writel relaxed implementations correctly. But
one obvious example which shows the relaxed version won't have the compiler
reorder issue is drivers/irqchip/irq-gic.c, all the configurations must be done
before enable the GIC which is done by "writel_relaxed(1, cpu_base + GIC_CPU_CTRL);"
However, we didn't see any explicit compiler barriers.

Yup, I just checked the discussion here:

You are right, write order is ensured.

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