linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] arm64: ARM: Fix the Generic Timers interrupt active level description
@ 2014-11-27 16:21 Liviu Dudau
  2014-11-27 18:39 ` Marc Zyngier
  0 siblings, 1 reply; 6+ messages in thread
From: Liviu Dudau @ 2014-11-27 16:21 UTC (permalink / raw)
  To: Arnd Bergmann, Marc Zyngier, Olof Johansson
  Cc: Mark Rutland, Rob Herring, Ian Campbell, Will Deacon, devicetree,
	linux-arm-kernel, linux-kernel

The Cortex-A5x TRM states in paragraph "9.2 Generic Timer functional
description" that generic timers provide a level not edge interrupt
output. Fix the device trees to correctly describe this.

While doing this update the CPU mask to match the number of described
CPUs as well as the DT bindings documentation for Generic Timers.

Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
---

Arnd, Olof: This is on top of linux-next/master as it patches Juno's
as well as all the other ARM DTs.

--

 Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++----
 arch/arm64/boot/dts/arm/foundation-v8.dts            | 8 ++++----
 arch/arm64/boot/dts/arm/juno.dts                     | 8 ++++----
 arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts           | 8 ++++----
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index 37b2caf..6d2aa87 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -27,10 +27,10 @@ Example:
 	timer {
 		compatible = "arm,cortex-a15-timer",
 			     "arm,armv7-timer";
-		interrupts = <1 13 0xf08>,
-			     <1 14 0xf08>,
-			     <1 11 0xf08>,
-			     <1 10 0xf08>;
+		interrupts = <1 13 0xf04>,
+			     <1 14 0xf04>,
+			     <1 11 0xf04>,
+			     <1 10 0xf04>;
 		clock-frequency = <100000000>;
 	};
 
diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dts b/arch/arm64/boot/dts/arm/foundation-v8.dts
index 4a06090..8c0214e 100644
--- a/arch/arm64/boot/dts/arm/foundation-v8.dts
+++ b/arch/arm64/boot/dts/arm/foundation-v8.dts
@@ -78,10 +78,10 @@
 
 	timer {
 		compatible = "arm,armv8-timer";
-		interrupts = <1 13 0xff01>,
-			     <1 14 0xff01>,
-			     <1 11 0xff01>,
-			     <1 10 0xff01>;
+		interrupts = <1 13 0xf04>,
+			     <1 14 0xf04>,
+			     <1 11 0xf04>,
+			     <1 10 0xf04>;
 		clock-frequency = <100000000>;
 	};
 
diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
index 097ecc4..551a688 100644
--- a/arch/arm64/boot/dts/arm/juno.dts
+++ b/arch/arm64/boot/dts/arm/juno.dts
@@ -98,10 +98,10 @@
 
 	timer {
 		compatible = "arm,armv8-timer";
-		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_EDGE_RISING)>,
-			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_EDGE_RISING)>,
-			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_EDGE_RISING)>,
-			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_EDGE_RISING)>;
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
 	};
 
 	pmu {
diff --git a/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts b/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
index 572005e..49d1b19 100644
--- a/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
+++ b/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
@@ -81,10 +81,10 @@
 
 	timer {
 		compatible = "arm,armv8-timer";
-		interrupts = <1 13 0xff01>,
-			     <1 14 0xff01>,
-			     <1 11 0xff01>,
-			     <1 10 0xff01>;
+		interrupts = <1 13 0xf04>,
+			     <1 14 0xf04>,
+			     <1 11 0xf04>,
+			     <1 10 0xf04>;
 		clock-frequency = <100000000>;
 	};
 
-- 
2.1.3


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

* Re: [PATCH v2] arm64: ARM: Fix the Generic Timers interrupt active level description
  2014-11-27 16:21 [PATCH v2] arm64: ARM: Fix the Generic Timers interrupt active level description Liviu Dudau
@ 2014-11-27 18:39 ` Marc Zyngier
  2014-11-28  3:12   ` Jisheng Zhang
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2014-11-27 18:39 UTC (permalink / raw)
  To: Liviu Dudau, Arnd Bergmann, Olof Johansson
  Cc: Mark Rutland, Rob Herring, Ian Campbell, Will Deacon, devicetree,
	linux-arm-kernel, linux-kernel

On 27/11/14 16:21, Liviu Dudau wrote:
> The Cortex-A5x TRM states in paragraph "9.2 Generic Timer functional
> description" that generic timers provide a level not edge interrupt
> output. Fix the device trees to correctly describe this.
> 
> While doing this update the CPU mask to match the number of described
> CPUs as well as the DT bindings documentation for Generic Timers.
> 
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
> 
> Arnd, Olof: This is on top of linux-next/master as it patches Juno's
> as well as all the other ARM DTs.
> 
> --
> 
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++----
>  arch/arm64/boot/dts/arm/foundation-v8.dts            | 8 ++++----
>  arch/arm64/boot/dts/arm/juno.dts                     | 8 ++++----
>  arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts           | 8 ++++----
>  4 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 37b2caf..6d2aa87 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -27,10 +27,10 @@ Example:
>  	timer {
>  		compatible = "arm,cortex-a15-timer",
>  			     "arm,armv7-timer";
> -		interrupts = <1 13 0xf08>,
> -			     <1 14 0xf08>,
> -			     <1 11 0xf08>,
> -			     <1 10 0xf08>;
> +		interrupts = <1 13 0xf04>,
> +			     <1 14 0xf04>,
> +			     <1 11 0xf04>,
> +			     <1 10 0xf04>;
>  		clock-frequency = <100000000>;
>  	};
>  
> diff --git a/arch/arm64/boot/dts/arm/foundation-v8.dts b/arch/arm64/boot/dts/arm/foundation-v8.dts
> index 4a06090..8c0214e 100644
> --- a/arch/arm64/boot/dts/arm/foundation-v8.dts
> +++ b/arch/arm64/boot/dts/arm/foundation-v8.dts
> @@ -78,10 +78,10 @@
>  
>  	timer {
>  		compatible = "arm,armv8-timer";
> -		interrupts = <1 13 0xff01>,
> -			     <1 14 0xff01>,
> -			     <1 11 0xff01>,
> -			     <1 10 0xff01>;
> +		interrupts = <1 13 0xf04>,
> +			     <1 14 0xf04>,
> +			     <1 11 0xf04>,
> +			     <1 10 0xf04>;
>  		clock-frequency = <100000000>;
>  	};
>  
> diff --git a/arch/arm64/boot/dts/arm/juno.dts b/arch/arm64/boot/dts/arm/juno.dts
> index 097ecc4..551a688 100644
> --- a/arch/arm64/boot/dts/arm/juno.dts
> +++ b/arch/arm64/boot/dts/arm/juno.dts
> @@ -98,10 +98,10 @@
>  
>  	timer {
>  		compatible = "arm,armv8-timer";
> -		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_EDGE_RISING)>,
> -			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_EDGE_RISING)>,
> -			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_EDGE_RISING)>,
> -			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_EDGE_RISING)>;
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
>  	};
>  
>  	pmu {
> diff --git a/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts b/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
> index 572005e..49d1b19 100644
> --- a/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
> +++ b/arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts
> @@ -81,10 +81,10 @@
>  
>  	timer {
>  		compatible = "arm,armv8-timer";
> -		interrupts = <1 13 0xff01>,
> -			     <1 14 0xff01>,
> -			     <1 11 0xff01>,
> -			     <1 10 0xff01>;
> +		interrupts = <1 13 0xf04>,
> +			     <1 14 0xf04>,
> +			     <1 11 0xf04>,
> +			     <1 10 0xf04>;
>  		clock-frequency = <100000000>;
>  	};
>  
> 


-- 
Jazz is not dead. It just smells funny...

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

* Re: [PATCH v2] arm64: ARM: Fix the Generic Timers interrupt active level description
  2014-11-27 18:39 ` Marc Zyngier
@ 2014-11-28  3:12   ` Jisheng Zhang
  2014-11-28 10:38     ` Liviu Dudau
  0 siblings, 1 reply; 6+ messages in thread
From: Jisheng Zhang @ 2014-11-28  3:12 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Liviu Dudau, Arnd Bergmann, Olof Johansson, Mark Rutland,
	devicetree, Ian Campbell, Will Deacon, linux-kernel,
	linux-arm-kernel

Dear Marc and Liviu,

On Thu, 27 Nov 2014 10:39:28 -0800
Marc Zyngier <marc.zyngier@arm.com> wrote:

> On 27/11/14 16:21, Liviu Dudau wrote:
> > The Cortex-A5x TRM states in paragraph "9.2 Generic Timer functional
> > description" that generic timers provide a level not edge interrupt
> > output. Fix the device trees to correctly describe this.
> > 
> > While doing this update the CPU mask to match the number of described
> > CPUs as well as the DT bindings documentation for Generic Timers.
> > 
> > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> 
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> 
> 	M.
> 
> > ---
> > 
> > Arnd, Olof: This is on top of linux-next/master as it patches Juno's
> > as well as all the other ARM DTs.
> > 
> > --
> > 
> >  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++----
> >  arch/arm64/boot/dts/arm/foundation-v8.dts            | 8 ++++----
> >  arch/arm64/boot/dts/arm/juno.dts                     | 8 ++++----
> >  arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts           | 8 ++++----
> >  4 files changed, 16 insertions(+), 16 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt
> > b/Documentation/devicetree/bindings/arm/arch_timer.txt index
> > 37b2caf..6d2aa87 100644 ---
> > a/Documentation/devicetree/bindings/arm/arch_timer.txt +++
> > b/Documentation/devicetree/bindings/arm/arch_timer.txt @@ -27,10 +27,10
> > @@ Example: timer {
> >  		compatible = "arm,cortex-a15-timer",
> >  			     "arm,armv7-timer";
> > -		interrupts = <1 13 0xf08>,
> > -			     <1 14 0xf08>,
> > -			     <1 11 0xf08>,
> > -			     <1 10 0xf08>;
> > +		interrupts = <1 13 0xf04>,
> > +			     <1 14 0xf04>,
> > +			     <1 11 0xf04>,
> > +			     <1 10 0xf04>;
> >  		clock-frequency = <100000000>;
> >  	};
> >  

Does it mean we also need to fix the interrupt level description under
arch/arm/boot/dts? I found they are also wrong or I misunderstand something?

Thanks,
Jisheng

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

* Re: [PATCH v2] arm64: ARM: Fix the Generic Timers interrupt active level description
  2014-11-28  3:12   ` Jisheng Zhang
@ 2014-11-28 10:38     ` Liviu Dudau
  2014-11-28 10:51       ` Jisheng Zhang
  2014-11-28 10:56       ` Marc Zyngier
  0 siblings, 2 replies; 6+ messages in thread
From: Liviu Dudau @ 2014-11-28 10:38 UTC (permalink / raw)
  To: Jisheng Zhang
  Cc: Marc Zyngier, Arnd Bergmann, Olof Johansson, Mark Rutland,
	devicetree, Ian Campbell, Will Deacon, linux-kernel,
	linux-arm-kernel

On Fri, Nov 28, 2014 at 03:12:52AM +0000, Jisheng Zhang wrote:
> Dear Marc and Liviu,
> 
> On Thu, 27 Nov 2014 10:39:28 -0800
> Marc Zyngier <marc.zyngier@arm.com> wrote:
> 
> > On 27/11/14 16:21, Liviu Dudau wrote:
> > > The Cortex-A5x TRM states in paragraph "9.2 Generic Timer functional
> > > description" that generic timers provide a level not edge interrupt
> > > output. Fix the device trees to correctly describe this.
> > > 
> > > While doing this update the CPU mask to match the number of described
> > > CPUs as well as the DT bindings documentation for Generic Timers.
> > > 
> > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > 
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > 
> > 	M.
> > 
> > > ---
> > > 
> > > Arnd, Olof: This is on top of linux-next/master as it patches Juno's
> > > as well as all the other ARM DTs.
> > > 
> > > --
> > > 
> > >  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++----
> > >  arch/arm64/boot/dts/arm/foundation-v8.dts            | 8 ++++----
> > >  arch/arm64/boot/dts/arm/juno.dts                     | 8 ++++----
> > >  arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts           | 8 ++++----
> > >  4 files changed, 16 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt
> > > b/Documentation/devicetree/bindings/arm/arch_timer.txt index
> > > 37b2caf..6d2aa87 100644 ---
> > > a/Documentation/devicetree/bindings/arm/arch_timer.txt +++
> > > b/Documentation/devicetree/bindings/arm/arch_timer.txt @@ -27,10 +27,10
> > > @@ Example: timer {
> > >  		compatible = "arm,cortex-a15-timer",
> > >  			     "arm,armv7-timer";
> > > -		interrupts = <1 13 0xf08>,
> > > -			     <1 14 0xf08>,
> > > -			     <1 11 0xf08>,
> > > -			     <1 10 0xf08>;
> > > +		interrupts = <1 13 0xf04>,
> > > +			     <1 14 0xf04>,
> > > +			     <1 11 0xf04>,
> > > +			     <1 10 0xf04>;
> > >  		clock-frequency = <100000000>;
> > >  	};
> > >  
> 
> Does it mean we also need to fix the interrupt level description under
> arch/arm/boot/dts? I found they are also wrong or I misunderstand something?

Hi Jisheng,

It looks like Marc and I managed to confuse ourselves. The TRM for GIC-500 and
GIC-400 (basically covering most of GICv1 and > GICv2) clearly says that for *PPIs*
the level triggered interrupts are active-LOW.

So, this patch is invalid and my v1 version is correct, but Marc tells that in
that case the GIC driver needs patching (which I'm going to look into).

Best regards,
Liviu

> 
> Thanks,
> Jisheng
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯


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

* Re: [PATCH v2] arm64: ARM: Fix the Generic Timers interrupt active level description
  2014-11-28 10:38     ` Liviu Dudau
@ 2014-11-28 10:51       ` Jisheng Zhang
  2014-11-28 10:56       ` Marc Zyngier
  1 sibling, 0 replies; 6+ messages in thread
From: Jisheng Zhang @ 2014-11-28 10:51 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: Marc Zyngier, Arnd Bergmann, Olof Johansson, Mark Rutland,
	devicetree, Ian Campbell, Will Deacon, linux-kernel,
	linux-arm-kernel

On Fri, 28 Nov 2014 02:38:43 -0800
Liviu Dudau <Liviu.Dudau@arm.com> wrote:

> On Fri, Nov 28, 2014 at 03:12:52AM +0000, Jisheng Zhang wrote:
> > Dear Marc and Liviu,
> > 
> > On Thu, 27 Nov 2014 10:39:28 -0800
> > Marc Zyngier <marc.zyngier@arm.com> wrote:
> > 
> > > On 27/11/14 16:21, Liviu Dudau wrote:
> > > > The Cortex-A5x TRM states in paragraph "9.2 Generic Timer functional
> > > > description" that generic timers provide a level not edge interrupt
> > > > output. Fix the device trees to correctly describe this.
> > > > 
> > > > While doing this update the CPU mask to match the number of described
> > > > CPUs as well as the DT bindings documentation for Generic Timers.
> > > > 
> > > > Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> > > 
> > > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> > > 
> > > 	M.
> > > 
> > > > ---
> > > > 
> > > > Arnd, Olof: This is on top of linux-next/master as it patches Juno's
> > > > as well as all the other ARM DTs.
> > > > 
> > > > --
> > > > 
> > > >  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++----
> > > >  arch/arm64/boot/dts/arm/foundation-v8.dts            | 8 ++++----
> > > >  arch/arm64/boot/dts/arm/juno.dts                     | 8 ++++----
> > > >  arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts           | 8 ++++----
> > > >  4 files changed, 16 insertions(+), 16 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt
> > > > b/Documentation/devicetree/bindings/arm/arch_timer.txt index
> > > > 37b2caf..6d2aa87 100644 ---
> > > > a/Documentation/devicetree/bindings/arm/arch_timer.txt +++
> > > > b/Documentation/devicetree/bindings/arm/arch_timer.txt @@ -27,10
> > > > +27,10 @@ Example: timer {
> > > >  		compatible = "arm,cortex-a15-timer",
> > > >  			     "arm,armv7-timer";
> > > > -		interrupts = <1 13 0xf08>,
> > > > -			     <1 14 0xf08>,
> > > > -			     <1 11 0xf08>,
> > > > -			     <1 10 0xf08>;
> > > > +		interrupts = <1 13 0xf04>,
> > > > +			     <1 14 0xf04>,
> > > > +			     <1 11 0xf04>,
> > > > +			     <1 10 0xf04>;
> > > >  		clock-frequency = <100000000>;
> > > >  	};
> > > >  
> > 
> > Does it mean we also need to fix the interrupt level description under
> > arch/arm/boot/dts? I found they are also wrong or I misunderstand
> > something?
> 
> Hi Jisheng,
> 
> It looks like Marc and I managed to confuse ourselves. The TRM for GIC-500
> and GIC-400 (basically covering most of GICv1 and > GICv2) clearly says
> that for *PPIs* the level triggered interrupts are active-LOW.
> 
> So, this patch is invalid and my v1 version is correct, but Marc tells that
> in that case the GIC driver needs patching (which I'm going to look into).
> 

Got it. Thanks for clarification.

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

* Re: [PATCH v2] arm64: ARM: Fix the Generic Timers interrupt active level description
  2014-11-28 10:38     ` Liviu Dudau
  2014-11-28 10:51       ` Jisheng Zhang
@ 2014-11-28 10:56       ` Marc Zyngier
  1 sibling, 0 replies; 6+ messages in thread
From: Marc Zyngier @ 2014-11-28 10:56 UTC (permalink / raw)
  To: Liviu Dudau, Jisheng Zhang
  Cc: Arnd Bergmann, Olof Johansson, Mark Rutland, devicetree,
	Ian Campbell, Will Deacon, linux-kernel, linux-arm-kernel

On 28/11/14 10:38, Liviu Dudau wrote:
> On Fri, Nov 28, 2014 at 03:12:52AM +0000, Jisheng Zhang wrote:
>> Dear Marc and Liviu,
>>
>> On Thu, 27 Nov 2014 10:39:28 -0800
>> Marc Zyngier <marc.zyngier@arm.com> wrote:
>>
>>> On 27/11/14 16:21, Liviu Dudau wrote:
>>>> The Cortex-A5x TRM states in paragraph "9.2 Generic Timer functional
>>>> description" that generic timers provide a level not edge interrupt
>>>> output. Fix the device trees to correctly describe this.
>>>>
>>>> While doing this update the CPU mask to match the number of described
>>>> CPUs as well as the DT bindings documentation for Generic Timers.
>>>>
>>>> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
>>>
>>> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>>>
>>> 	M.
>>>
>>>> ---
>>>>
>>>> Arnd, Olof: This is on top of linux-next/master as it patches Juno's
>>>> as well as all the other ARM DTs.
>>>>
>>>> --
>>>>
>>>>  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++----
>>>>  arch/arm64/boot/dts/arm/foundation-v8.dts            | 8 ++++----
>>>>  arch/arm64/boot/dts/arm/juno.dts                     | 8 ++++----
>>>>  arch/arm64/boot/dts/arm/rtsm_ve-aemv8a.dts           | 8 ++++----
>>>>  4 files changed, 16 insertions(+), 16 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt
>>>> b/Documentation/devicetree/bindings/arm/arch_timer.txt index
>>>> 37b2caf..6d2aa87 100644 ---
>>>> a/Documentation/devicetree/bindings/arm/arch_timer.txt +++
>>>> b/Documentation/devicetree/bindings/arm/arch_timer.txt @@ -27,10 +27,10
>>>> @@ Example: timer {
>>>>  		compatible = "arm,cortex-a15-timer",
>>>>  			     "arm,armv7-timer";
>>>> -		interrupts = <1 13 0xf08>,
>>>> -			     <1 14 0xf08>,
>>>> -			     <1 11 0xf08>,
>>>> -			     <1 10 0xf08>;
>>>> +		interrupts = <1 13 0xf04>,
>>>> +			     <1 14 0xf04>,
>>>> +			     <1 11 0xf04>,
>>>> +			     <1 10 0xf04>;
>>>>  		clock-frequency = <100000000>;
>>>>  	};
>>>>  
>>
>> Does it mean we also need to fix the interrupt level description under
>> arch/arm/boot/dts? I found they are also wrong or I misunderstand something?
> 
> Hi Jisheng,
> 
> It looks like Marc and I managed to confuse ourselves. The TRM for GIC-500 and
> GIC-400 (basically covering most of GICv1 and > GICv2) clearly says that for *PPIs*
> the level triggered interrupts are active-LOW.
> 
> So, this patch is invalid and my v1 version is correct, but Marc tells that in
> that case the GIC driver needs patching (which I'm going to look into).

Yeah, we have a pathological case of confusion of config information
between PPIs and SPIs:
- SPIs are always rising-edge or active-high
- PPIs are implementation defined, and even their configurability is
implementation defined

The fact that on all ARM implementations, the PPI are not configurable
is the saving grace. No matter how wrong the DTs are, we'll do the right
thing, and all the existing code actually relies on the defaults values
anyway.

Well, until someone tries and configure a PPI (at least Qualcomm's
implementations are actually programmable), and asks for an active-low
value, which will get rejected.

Basically, all can we do at the GIC level is to accept all combinations
of edge/level for PPIs, reduce that to the single bit of config we have,
and check that it sticks. The result may not be what the driver asked
for, but we have no way to know.

GIC bindings will have to be updated too.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

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

end of thread, other threads:[~2014-11-28 10:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-27 16:21 [PATCH v2] arm64: ARM: Fix the Generic Timers interrupt active level description Liviu Dudau
2014-11-27 18:39 ` Marc Zyngier
2014-11-28  3:12   ` Jisheng Zhang
2014-11-28 10:38     ` Liviu Dudau
2014-11-28 10:51       ` Jisheng Zhang
2014-11-28 10:56       ` Marc Zyngier

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).