linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V3] irqchip:gic: change access of gicc_ctrl register to read modify write.
@ 2014-02-27 21:36 Feng Kan
  2014-09-08  8:52 ` Arun Chandran
  0 siblings, 1 reply; 5+ messages in thread
From: Feng Kan @ 2014-02-27 21:36 UTC (permalink / raw)
  To: tglx, patches, linux-arm-kernel, linux-kernel, marc.zyngier
  Cc: Feng Kan, Vinayak Kale

This change is made to preserve the GIC v2 releated bits in the
GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
The original code only set the enable/disable group bit in this register.
This code will preserve the bypass bits configured by the bootload except
the enable/disable bit. The main reason for this change is to allow the
bypass bits specified in the v2 spec to remain untouched by the current
GIC code. In the X-Gene platform, the bypass functionality is not used
and bypass must be disabled at all time.

Signed-off-by: Vinayak Kale <vkale@apm.com>
Acked-by: Anup Patel <apatel@apm.com>
Signed-off-by: Feng Kan <fkan@apm.com>
---
V3 Changes:
	- Sorry, forgot to change the mask for cpu_init path
          assumed bootloader setup bits correctly.
V2 Changes:
	- only mask off v2 bypass bits
 drivers/irqchip/irq-gic.c |   32 +++++++++++++++++++++++++++++---
 1 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 341c601..3ca7995 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 	void __iomem *dist_base = gic_data_dist_base(gic);
 	void __iomem *base = gic_data_cpu_base(gic);
 	unsigned int cpu_mask, cpu = smp_processor_id();
+	unsigned int ctrl_mask;
 	int i;
 
 	/*
@@ -449,13 +450,29 @@ static void gic_cpu_init(struct gic_chip_data *gic)
 		writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
 
 	writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
-	writel_relaxed(1, base + GIC_CPU_CTRL);
+
+	ctrl_mask = readl(base + GIC_CPU_CTRL);
+
+	/* Mask out the gic v2 bypass bits */
+	ctrl_mask &= 0x1e0;
+
+	/* Enable group 0 */
+	ctrl_mask |= 0x1;
+	writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL);
 }
 
 void gic_cpu_if_down(void)
 {
+	unsigned int ctrl_mask;
 	void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
-	writel_relaxed(0, cpu_base + GIC_CPU_CTRL);
+
+	ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
+	/*
+	 * Disable grp enable bit, leave the bypass bits alone as changing
+	 * them could leave the system unstable
+	 */
+	ctrl_mask &= 0x1e0;
+	writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
 }
 
 #ifdef CONFIG_CPU_PM
@@ -566,6 +583,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
 {
 	int i;
 	u32 *ptr;
+	unsigned int ctrl_mask;
 	void __iomem *dist_base;
 	void __iomem *cpu_base;
 
@@ -590,7 +608,15 @@ static void gic_cpu_restore(unsigned int gic_nr)
 		writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
 
 	writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
-	writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
+
+	ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
+
+	/* Mask out the gic v2 bypass bits */
+	ctrl_mask &= 0x1e0;
+
+	/* Enable group 0 */
+	ctrl_mask |= 0x1;
+	writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
 }
 
 static int gic_notifier(struct notifier_block *self, unsigned long cmd,	void *v)
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] irqchip:gic: change access of gicc_ctrl register to read modify write.
  2014-02-27 21:36 [PATCH V3] irqchip:gic: change access of gicc_ctrl register to read modify write Feng Kan
@ 2014-09-08  8:52 ` Arun Chandran
  2014-09-08 16:57   ` Feng Kan
  0 siblings, 1 reply; 5+ messages in thread
From: Arun Chandran @ 2014-09-08  8:52 UTC (permalink / raw)
  To: Feng Kan
  Cc: tglx, patches, linux-arm-kernel, linux-kernel, Marc Zyngier,
	Vinayak Kale

Hi Feng,

Is this patch still needed to fix perf calltrace?
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265814.html

On Fri, Feb 28, 2014 at 3:06 AM, Feng Kan <fkan@apm.com> wrote:
> This change is made to preserve the GIC v2 releated bits in the
> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
> The original code only set the enable/disable group bit in this register.
> This code will preserve the bypass bits configured by the bootload except
> the enable/disable bit. The main reason for this change is to allow the
> bypass bits specified in the v2 spec to remain untouched by the current
> GIC code. In the X-Gene platform, the bypass functionality is not used
> and bypass must be disabled at all time.
>
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> Acked-by: Anup Patel <apatel@apm.com>
> Signed-off-by: Feng Kan <fkan@apm.com>
> ---
> V3 Changes:
>         - Sorry, forgot to change the mask for cpu_init path
>           assumed bootloader setup bits correctly.
> V2 Changes:
>         - only mask off v2 bypass bits
>  drivers/irqchip/irq-gic.c |   32 +++++++++++++++++++++++++++++---
>  1 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 341c601..3ca7995 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>         void __iomem *dist_base = gic_data_dist_base(gic);
>         void __iomem *base = gic_data_cpu_base(gic);
>         unsigned int cpu_mask, cpu = smp_processor_id();
> +       unsigned int ctrl_mask;
>         int i;
>
>         /*
> @@ -449,13 +450,29 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
>
>         writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
> -       writel_relaxed(1, base + GIC_CPU_CTRL);
> +
> +       ctrl_mask = readl(base + GIC_CPU_CTRL);

readl_relaxed() here?

> +
> +       /* Mask out the gic v2 bypass bits */
> +       ctrl_mask &= 0x1e0;
> +
> +       /* Enable group 0 */
> +       ctrl_mask |= 0x1;
> +       writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL);
>  }
>
>  void gic_cpu_if_down(void)
>  {
> +       unsigned int ctrl_mask;
>         void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
> -       writel_relaxed(0, cpu_base + GIC_CPU_CTRL);
> +
> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
readl_relaxed() here?

> +       /*
> +        * Disable grp enable bit, leave the bypass bits alone as changing
> +        * them could leave the system unstable
> +        */
> +       ctrl_mask &= 0x1e0;
> +       writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
>  }
>
>  #ifdef CONFIG_CPU_PM
> @@ -566,6 +583,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>  {
>         int i;
>         u32 *ptr;
> +       unsigned int ctrl_mask;
>         void __iomem *dist_base;
>         void __iomem *cpu_base;
>
> @@ -590,7 +608,15 @@ static void gic_cpu_restore(unsigned int gic_nr)
>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>
>         writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
> -       writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
> +
> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);

readl_relaxed() here?

> +
> +       /* Mask out the gic v2 bypass bits */
> +       ctrl_mask &= 0x1e0;
> +
> +       /* Enable group 0 */
> +       ctrl_mask |= 0x1;
> +       writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
>  }
>

--Arun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] irqchip:gic: change access of gicc_ctrl register to read modify write.
  2014-09-08  8:52 ` Arun Chandran
@ 2014-09-08 16:57   ` Feng Kan
  2014-09-09  6:01     ` Arun Chandran
  0 siblings, 1 reply; 5+ messages in thread
From: Feng Kan @ 2014-09-08 16:57 UTC (permalink / raw)
  To: Arun Chandran
  Cc: tglx, patches, linux-arm-kernel, linux-kernel, Marc Zyngier,
	Vinayak Kale

On Mon, Sep 8, 2014 at 1:52 AM, Arun Chandran <achandran@mvista.com> wrote:
> Hi Feng,
>
> Is this patch still needed to fix perf calltrace?
Yes, if the someone decide to run the secure mode of the GIC, which is
what is present in
the kernel tree now.

> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265814.html
>
> On Fri, Feb 28, 2014 at 3:06 AM, Feng Kan <fkan@apm.com> wrote:
>> This change is made to preserve the GIC v2 releated bits in the
>> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
>> The original code only set the enable/disable group bit in this register.
>> This code will preserve the bypass bits configured by the bootload except
>> the enable/disable bit. The main reason for this change is to allow the
>> bypass bits specified in the v2 spec to remain untouched by the current
>> GIC code. In the X-Gene platform, the bypass functionality is not used
>> and bypass must be disabled at all time.
>>
>> Signed-off-by: Vinayak Kale <vkale@apm.com>
>> Acked-by: Anup Patel <apatel@apm.com>
>> Signed-off-by: Feng Kan <fkan@apm.com>
>> ---
>> V3 Changes:
>>         - Sorry, forgot to change the mask for cpu_init path
>>           assumed bootloader setup bits correctly.
>> V2 Changes:
>>         - only mask off v2 bypass bits
>>  drivers/irqchip/irq-gic.c |   32 +++++++++++++++++++++++++++++---
>>  1 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index 341c601..3ca7995 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>         void __iomem *dist_base = gic_data_dist_base(gic);
>>         void __iomem *base = gic_data_cpu_base(gic);
>>         unsigned int cpu_mask, cpu = smp_processor_id();
>> +       unsigned int ctrl_mask;
>>         int i;
>>
>>         /*
>> @@ -449,13 +450,29 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
>>
>>         writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>> -       writel_relaxed(1, base + GIC_CPU_CTRL);
>> +
>> +       ctrl_mask = readl(base + GIC_CPU_CTRL);
>
> readl_relaxed() here?
>
>> +
>> +       /* Mask out the gic v2 bypass bits */
>> +       ctrl_mask &= 0x1e0;
>> +
>> +       /* Enable group 0 */
>> +       ctrl_mask |= 0x1;
>> +       writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL);
>>  }
>>
>>  void gic_cpu_if_down(void)
>>  {
>> +       unsigned int ctrl_mask;
>>         void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
>> -       writel_relaxed(0, cpu_base + GIC_CPU_CTRL);
>> +
>> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
> readl_relaxed() here?
>
>> +       /*
>> +        * Disable grp enable bit, leave the bypass bits alone as changing
>> +        * them could leave the system unstable
>> +        */
>> +       ctrl_mask &= 0x1e0;
>> +       writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
>>  }
>>
>>  #ifdef CONFIG_CPU_PM
>> @@ -566,6 +583,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>  {
>>         int i;
>>         u32 *ptr;
>> +       unsigned int ctrl_mask;
>>         void __iomem *dist_base;
>>         void __iomem *cpu_base;
>>
>> @@ -590,7 +608,15 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>
>>         writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>> -       writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>> +
>> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
>
> readl_relaxed() here?
Trying to make sure read is completed before write back out to gic.
>
>> +
>> +       /* Mask out the gic v2 bypass bits */
>> +       ctrl_mask &= 0x1e0;
>> +
>> +       /* Enable group 0 */
>> +       ctrl_mask |= 0x1;
>> +       writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
>>  }
>>
>
> --Arun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] irqchip:gic: change access of gicc_ctrl register to read modify write.
  2014-09-08 16:57   ` Feng Kan
@ 2014-09-09  6:01     ` Arun Chandran
  2014-09-09 10:57       ` Arun KS
  0 siblings, 1 reply; 5+ messages in thread
From: Arun Chandran @ 2014-09-09  6:01 UTC (permalink / raw)
  To: Feng Kan
  Cc: tglx, patches, linux-arm-kernel, linux-kernel, Marc Zyngier,
	Vinayak Kale

On Mon, Sep 8, 2014 at 10:27 PM, Feng Kan <fkan@apm.com> wrote:
> On Mon, Sep 8, 2014 at 1:52 AM, Arun Chandran <achandran@mvista.com> wrote:
>> Hi Feng,
>>
>> Is this patch still needed to fix perf calltrace?
> Yes, if the someone decide to run the secure mode of the GIC, which is
> what is present in
> the kernel tree now.
>
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265814.html
>>
>> On Fri, Feb 28, 2014 at 3:06 AM, Feng Kan <fkan@apm.com> wrote:
>>> This change is made to preserve the GIC v2 releated bits in the
>>> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
>>> The original code only set the enable/disable group bit in this register.
>>> This code will preserve the bypass bits configured by the bootload except
>>> the enable/disable bit. The main reason for this change is to allow the
>>> bypass bits specified in the v2 spec to remain untouched by the current
>>> GIC code. In the X-Gene platform, the bypass functionality is not used
>>> and bypass must be disabled at all time.
>>>
>>> Signed-off-by: Vinayak Kale <vkale@apm.com>
>>> Acked-by: Anup Patel <apatel@apm.com>
>>> Signed-off-by: Feng Kan <fkan@apm.com>
>>> ---
>>> V3 Changes:
>>>         - Sorry, forgot to change the mask for cpu_init path
>>>           assumed bootloader setup bits correctly.
>>> V2 Changes:
>>>         - only mask off v2 bypass bits
>>>  drivers/irqchip/irq-gic.c |   32 +++++++++++++++++++++++++++++---
>>>  1 files changed, 29 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>> index 341c601..3ca7995 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>>         void __iomem *dist_base = gic_data_dist_base(gic);
>>>         void __iomem *base = gic_data_cpu_base(gic);
>>>         unsigned int cpu_mask, cpu = smp_processor_id();
>>> +       unsigned int ctrl_mask;
>>>         int i;
>>>
>>>         /*
>>> @@ -449,13 +450,29 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
>>>
>>>         writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>>> -       writel_relaxed(1, base + GIC_CPU_CTRL);
>>> +
>>> +       ctrl_mask = readl(base + GIC_CPU_CTRL);
>>
>> readl_relaxed() here?
>>
>>> +
>>> +       /* Mask out the gic v2 bypass bits */
>>> +       ctrl_mask &= 0x1e0;
>>> +
>>> +       /* Enable group 0 */
>>> +       ctrl_mask |= 0x1;
>>> +       writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL);
>>>  }
>>>
>>>  void gic_cpu_if_down(void)
>>>  {
>>> +       unsigned int ctrl_mask;
>>>         void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
>>> -       writel_relaxed(0, cpu_base + GIC_CPU_CTRL);
>>> +
>>> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
>> readl_relaxed() here?
>>
>>> +       /*
>>> +        * Disable grp enable bit, leave the bypass bits alone as changing
>>> +        * them could leave the system unstable
>>> +        */
>>> +       ctrl_mask &= 0x1e0;
>>> +       writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
>>>  }
>>>
>>>  #ifdef CONFIG_CPU_PM
>>> @@ -566,6 +583,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>>  {
>>>         int i;
>>>         u32 *ptr;
>>> +       unsigned int ctrl_mask;
>>>         void __iomem *dist_base;
>>>         void __iomem *cpu_base;
>>>
>>> @@ -590,7 +608,15 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>>
>>>         writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>>> -       writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>>> +
>>> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
>>
>> readl_relaxed() here?
> Trying to make sure read is completed before write back out to gic.
>>

Will the cpu reorder  readl_relaxed() and a writel_relaxed() calls?

val = readl_relaxed(reg1);
val += 1;
wrietl_relaxed(val, reg1);

In the above code reg1 getting 'val+1' is not guaranteed?

--Arun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V3] irqchip:gic: change access of gicc_ctrl register to read modify write.
  2014-09-09  6:01     ` Arun Chandran
@ 2014-09-09 10:57       ` Arun KS
  0 siblings, 0 replies; 5+ messages in thread
From: Arun KS @ 2014-09-09 10:57 UTC (permalink / raw)
  To: Arun Chandran
  Cc: Feng Kan, Marc Zyngier, patches, linux-kernel, tglx,
	Vinayak Kale, linux-arm-kernel

Hi Arun,

On Tue, Sep 9, 2014 at 11:31 AM, Arun Chandran <achandran@mvista.com> wrote:
> On Mon, Sep 8, 2014 at 10:27 PM, Feng Kan <fkan@apm.com> wrote:
>> On Mon, Sep 8, 2014 at 1:52 AM, Arun Chandran <achandran@mvista.com> wrote:
>>> Hi Feng,
>>>
>>> Is this patch still needed to fix perf calltrace?
>> Yes, if the someone decide to run the secure mode of the GIC, which is
>> what is present in
>> the kernel tree now.
>>
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265814.html
>>>
>>> On Fri, Feb 28, 2014 at 3:06 AM, Feng Kan <fkan@apm.com> wrote:
>>>> This change is made to preserve the GIC v2 releated bits in the
>>>> GIC_CPU_CTRL register (also known as the GICC_CTLR register in spec).
>>>> The original code only set the enable/disable group bit in this register.
>>>> This code will preserve the bypass bits configured by the bootload except
>>>> the enable/disable bit. The main reason for this change is to allow the
>>>> bypass bits specified in the v2 spec to remain untouched by the current
>>>> GIC code. In the X-Gene platform, the bypass functionality is not used
>>>> and bypass must be disabled at all time.
>>>>
>>>> Signed-off-by: Vinayak Kale <vkale@apm.com>
>>>> Acked-by: Anup Patel <apatel@apm.com>
>>>> Signed-off-by: Feng Kan <fkan@apm.com>
>>>> ---
>>>> V3 Changes:
>>>>         - Sorry, forgot to change the mask for cpu_init path
>>>>           assumed bootloader setup bits correctly.
>>>> V2 Changes:
>>>>         - only mask off v2 bypass bits
>>>>  drivers/irqchip/irq-gic.c |   32 +++++++++++++++++++++++++++++---
>>>>  1 files changed, 29 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>>> index 341c601..3ca7995 100644
>>>> --- a/drivers/irqchip/irq-gic.c
>>>> +++ b/drivers/irqchip/irq-gic.c
>>>> @@ -418,6 +418,7 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>>>         void __iomem *dist_base = gic_data_dist_base(gic);
>>>>         void __iomem *base = gic_data_cpu_base(gic);
>>>>         unsigned int cpu_mask, cpu = smp_processor_id();
>>>> +       unsigned int ctrl_mask;
>>>>         int i;
>>>>
>>>>         /*
>>>> @@ -449,13 +450,29 @@ static void gic_cpu_init(struct gic_chip_data *gic)
>>>>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4 / 4);
>>>>
>>>>         writel_relaxed(0xf0, base + GIC_CPU_PRIMASK);
>>>> -       writel_relaxed(1, base + GIC_CPU_CTRL);
>>>> +
>>>> +       ctrl_mask = readl(base + GIC_CPU_CTRL);
>>>
>>> readl_relaxed() here?
>>>
>>>> +
>>>> +       /* Mask out the gic v2 bypass bits */
>>>> +       ctrl_mask &= 0x1e0;
>>>> +
>>>> +       /* Enable group 0 */
>>>> +       ctrl_mask |= 0x1;
>>>> +       writel_relaxed(ctrl_mask, base + GIC_CPU_CTRL);
>>>>  }
>>>>
>>>>  void gic_cpu_if_down(void)
>>>>  {
>>>> +       unsigned int ctrl_mask;
>>>>         void __iomem *cpu_base = gic_data_cpu_base(&gic_data[0]);
>>>> -       writel_relaxed(0, cpu_base + GIC_CPU_CTRL);
>>>> +
>>>> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
>>> readl_relaxed() here?
>>>
>>>> +       /*
>>>> +        * Disable grp enable bit, leave the bypass bits alone as changing
>>>> +        * them could leave the system unstable
>>>> +        */
>>>> +       ctrl_mask &= 0x1e0;
>>>> +       writel_relaxed(ctrl_mask, cpu_base + GIC_CPU_CTRL);
>>>>  }
>>>>
>>>>  #ifdef CONFIG_CPU_PM
>>>> @@ -566,6 +583,7 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>>>  {
>>>>         int i;
>>>>         u32 *ptr;
>>>> +       unsigned int ctrl_mask;
>>>>         void __iomem *dist_base;
>>>>         void __iomem *cpu_base;
>>>>
>>>> @@ -590,7 +608,15 @@ static void gic_cpu_restore(unsigned int gic_nr)
>>>>                 writel_relaxed(0xa0a0a0a0, dist_base + GIC_DIST_PRI + i * 4);
>>>>
>>>>         writel_relaxed(0xf0, cpu_base + GIC_CPU_PRIMASK);
>>>> -       writel_relaxed(1, cpu_base + GIC_CPU_CTRL);
>>>> +
>>>> +       ctrl_mask = readl(cpu_base + GIC_CPU_CTRL);
>>>
>>> readl_relaxed() here?
>> Trying to make sure read is completed before write back out to gic.
>>>
>
> Will the cpu reorder  readl_relaxed() and a writel_relaxed() calls?
>
> val = readl_relaxed(reg1);
> val += 1;
> wrietl_relaxed(val, reg1);
>
> In the above code reg1 getting 'val+1' is not guaranteed?

reg1 getting 'val+1' is guaranteed because of the address dependency.
If we use writel instead of writel_relaxed(), there will be a dsb()
instruction inserted before actual write.
That is, all write operations before dsb() on all cores will be
committed. The processors stall during this period.
dsb also waits for any TLB or cache maintenance operations to finish
across cores/clusters.

In this scenario it is okey to use _relaxed(). I might be wrong, in
that case some experts will correct me here.

The rule is like writel() handle write buffer flushing by default. And
for performance related areas use _relaxed variants.

Thanks,
Arun KS

>
> --Arun
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-09-09 10:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-27 21:36 [PATCH V3] irqchip:gic: change access of gicc_ctrl register to read modify write Feng Kan
2014-09-08  8:52 ` Arun Chandran
2014-09-08 16:57   ` Feng Kan
2014-09-09  6:01     ` Arun Chandran
2014-09-09 10:57       ` Arun KS

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).