devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Krait L1/L2 EDAC driver
@ 2013-10-29  0:31 Stephen Boyd
  2013-10-29  0:31 ` [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding Stephen Boyd
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2013-10-29  0:31 UTC (permalink / raw)
  To: linux-edac
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree,
	Doug Thompson, Thomas Gleixner, Russell King, Stepan Moskovchenko,
	David Brown

This patchset adds support for the Krait L1/L2 cache error detection
hardware. The first patch fixes a generic framework bug. The next
two patches lay the groundwork for this driver to be added by 
exporting percpu irq functions as well as adding the Krait l2 indirection
register code. The next two patches add the driver and the binding and 
the final patch hooks it all up by adding the device tree node.

I'm not sure which tree this is supposed to go through. Ideally we could
send the first 3 plus the 5th one through an edac tree. The final dts changes
could go through arm-soc via davidb's tree and the Documentation patch could
go through the devicetree tree.

Stephen Boyd (6):
  edac: Don't try to cancel workqueue when it's never setup
  genirq: export percpu irq functions for module usage
  ARM: Add Krait L2 accessor functions
  edac: Document Krait L1/L2 EDAC driver binding
  edac: Add support for Krait CPU cache error detection
  ARM: dts: msm: Add Krait edac node

 .../bindings/arm/qcom,krait-cache-erp.txt          |  16 +
 arch/arm/boot/dts/qcom-msm8974.dtsi                |   5 +
 arch/arm/common/Kconfig                            |   3 +
 arch/arm/common/Makefile                           |   1 +
 arch/arm/common/krait-l2-accessors.c               |  52 ++++
 arch/arm/include/asm/krait-l2-accessors.h          |  20 ++
 drivers/edac/Kconfig                               |   8 +
 drivers/edac/Makefile                              |   2 +
 drivers/edac/edac_device.c                         |   3 +
 drivers/edac/krait_edac.c                          | 335 +++++++++++++++++++++
 kernel/irq/manage.c                                |   2 +
 11 files changed, 447 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
 create mode 100644 arch/arm/common/krait-l2-accessors.c
 create mode 100644 arch/arm/include/asm/krait-l2-accessors.h
 create mode 100644 drivers/edac/krait_edac.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-29  0:31 [PATCH 0/6] Krait L1/L2 EDAC driver Stephen Boyd
@ 2013-10-29  0:31 ` Stephen Boyd
  2013-10-29  1:34   ` Mark Rutland
       [not found]   ` <1383006690-6754-5-git-send-email-sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  0 siblings, 2 replies; 11+ messages in thread
From: Stephen Boyd @ 2013-10-29  0:31 UTC (permalink / raw)
  To: linux-edac; +Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree

The Krait L1/L2 error reporting device is made up of two
interrupts, one per-CPU interrupt for the L1 caches and one
interrupt for the L2 cache.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 .../devicetree/bindings/arm/qcom,krait-cache-erp.txt     | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt

diff --git a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
new file mode 100644
index 0000000..01fe8a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
@@ -0,0 +1,16 @@
+* Qualcomm Krait L1 / L2 cache error reporting
+
+Required properties:
+- compatible: Should be "qcom,krait-cache-erp"
+- interrupts: Should contain the L1/CPU error interrupt number and
+  then the L2 cache error interrupt number
+
+Optional properties:
+- interrupt-names: Should contain the interrupt names "l1_irq" and
+  "l2_irq"
+
+Example:
+	edac {
+		compatible = "qcom,krait-cache-erp";
+		interrupts = <1 9 0xf04>, <0 2 0x4>;
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-29  0:31 ` [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding Stephen Boyd
@ 2013-10-29  1:34   ` Mark Rutland
  2013-10-29  5:06     ` Stephen Boyd
       [not found]   ` <1383006690-6754-5-git-send-email-sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2013-10-29  1:34 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org

On Tue, Oct 29, 2013 at 12:31:28AM +0000, Stephen Boyd wrote:
> The Krait L1/L2 error reporting device is made up of two
> interrupts, one per-CPU interrupt for the L1 caches and one
> interrupt for the L2 cache.
> 
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  .../devicetree/bindings/arm/qcom,krait-cache-erp.txt     | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> new file mode 100644
> index 0000000..01fe8a8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> @@ -0,0 +1,16 @@
> +* Qualcomm Krait L1 / L2 cache error reporting
> +
> +Required properties:
> +- compatible: Should be "qcom,krait-cache-erp"
> +- interrupts: Should contain the L1/CPU error interrupt number and
> +  then the L2 cache error interrupt number
> +
> +Optional properties:
> +- interrupt-names: Should contain the interrupt names "l1_irq" and
> +  "l2_irq"

As with my comment on the parsing code, I'd prefer that if interrupt-names was
present it defined the order of interrupts. Otherwise it's redundant and of no
value.

Otherwise, the binding looks fine to me:

Acked-by: Mark Rutland <mark.rutland@arm.com>

> +
> +Example:
> +	edac {
> +		compatible = "qcom,krait-cache-erp";
> +		interrupts = <1 9 0xf04>, <0 2 0x4>;
> +	};
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-29  1:34   ` Mark Rutland
@ 2013-10-29  5:06     ` Stephen Boyd
  2013-10-30  0:34       ` Mark Rutland
  0 siblings, 1 reply; 11+ messages in thread
From: Stephen Boyd @ 2013-10-29  5:06 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org

On 10/28, Mark Rutland wrote:
> On Tue, Oct 29, 2013 at 12:31:28AM +0000, Stephen Boyd wrote:
> > +
> > +Optional properties:
> > +- interrupt-names: Should contain the interrupt names "l1_irq" and
> > +  "l2_irq"
> 
> As with my comment on the parsing code, I'd prefer that if interrupt-names was
> present it defined the order of interrupts. Otherwise it's redundant and of no
> value.
> 
> Otherwise, the binding looks fine to me:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

How about I just drop the interrupt-names property? It isn't
adding much and is a holdover from the vendor kernel.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
       [not found]   ` <1383006690-6754-5-git-send-email-sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
@ 2013-10-29  8:21     ` Kumar Gala
  2013-10-29 18:00       ` Stephen Boyd
  0 siblings, 1 reply; 11+ messages in thread
From: Kumar Gala @ 2013-10-29  8:21 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-edac-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA


On Oct 28, 2013, at 7:31 PM, Stephen Boyd wrote:

> The Krait L1/L2 error reporting device is made up of two
> interrupts, one per-CPU interrupt for the L1 caches and one
> interrupt for the L2 cache.
> 
> Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Signed-off-by: Stephen Boyd <sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> .../devicetree/bindings/arm/qcom,krait-cache-erp.txt     | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> new file mode 100644
> index 0000000..01fe8a8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> @@ -0,0 +1,16 @@
> +* Qualcomm Krait L1 / L2 cache error reporting
> +
> +Required properties:
> +- compatible: Should be "qcom,krait-cache-erp"
> +- interrupts: Should contain the L1/CPU error interrupt number and
> +  then the L2 cache error interrupt number
> +
> +Optional properties:
> +- interrupt-names: Should contain the interrupt names "l1_irq" and
> +  "l2_irq"
> +
> +Example:
> +	edac {
> +		compatible = "qcom,krait-cache-erp";
> +		interrupts = <1 9 0xf04>, <0 2 0x4>;
> +	};

Why wouldn't we have these as part of cache nodes in the dts?  (which begs the question why we don't have cache nodes?)

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-29  8:21     ` Kumar Gala
@ 2013-10-29 18:00       ` Stephen Boyd
  2013-10-29 20:22         ` Olof Johansson
                           ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Stephen Boyd @ 2013-10-29 18:00 UTC (permalink / raw)
  To: Kumar Gala
  Cc: linux-edac, linux-kernel, linux-arm-msm, linux-arm-kernel,
	devicetree, Mark Rutland

On 10/29/13 01:21, Kumar Gala wrote:
> On Oct 28, 2013, at 7:31 PM, Stephen Boyd wrote:
>
>> The Krait L1/L2 error reporting device is made up of two
>> interrupts, one per-CPU interrupt for the L1 caches and one
>> interrupt for the L2 cache.
>>
>> Cc: <devicetree@vger.kernel.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>> .../devicetree/bindings/arm/qcom,krait-cache-erp.txt     | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>> new file mode 100644
>> index 0000000..01fe8a8
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>> @@ -0,0 +1,16 @@
>> +* Qualcomm Krait L1 / L2 cache error reporting
>> +
>> +Required properties:
>> +- compatible: Should be "qcom,krait-cache-erp"
>> +- interrupts: Should contain the L1/CPU error interrupt number and
>> +  then the L2 cache error interrupt number
>> +
>> +Optional properties:
>> +- interrupt-names: Should contain the interrupt names "l1_irq" and
>> +  "l2_irq"
>> +
>> +Example:
>> +	edac {
>> +		compatible = "qcom,krait-cache-erp";
>> +		interrupts = <1 9 0xf04>, <0 2 0x4>;
>> +	};
> Why wouldn't we have these as part of cache nodes in the dts?  (which begs the question why we don't have cache nodes?)
>

I can certainly add cache nodes and cpu nodes and then put the
interrupts in those nodes. I was thinking along those same lines when I
ported this driver but figured it would be good to get something out
there. The only question I have is how am I supposed to hook that up
into the linux device model? Will the edac driver bind to the device
created for the cpus node and the cache node? I guess it will have to be
a driver that binds to two devices.

One could argue that we should put the cp15 based architected timers in
the cpus node also but so far nobody has done that and I think there was
some reasoning behind that, Mark?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-29 18:00       ` Stephen Boyd
@ 2013-10-29 20:22         ` Olof Johansson
  2013-10-30  0:07         ` Stephen Boyd
  2013-10-30  0:38         ` Mark Rutland
  2 siblings, 0 replies; 11+ messages in thread
From: Olof Johansson @ 2013-10-29 20:22 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Kumar Gala, linux-edac, linux-kernel@vger.kernel.org,
	linux-arm-msm, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, Mark Rutland

On Tue, Oct 29, 2013 at 11:00 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 10/29/13 01:21, Kumar Gala wrote:
>> On Oct 28, 2013, at 7:31 PM, Stephen Boyd wrote:
>>
>>> The Krait L1/L2 error reporting device is made up of two
>>> interrupts, one per-CPU interrupt for the L1 caches and one
>>> interrupt for the L2 cache.
>>>
>>> Cc: <devicetree@vger.kernel.org>
>>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>>> ---
>>> .../devicetree/bindings/arm/qcom,krait-cache-erp.txt     | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>>> new file mode 100644
>>> index 0000000..01fe8a8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>>> @@ -0,0 +1,16 @@
>>> +* Qualcomm Krait L1 / L2 cache error reporting
>>> +
>>> +Required properties:
>>> +- compatible: Should be "qcom,krait-cache-erp"
>>> +- interrupts: Should contain the L1/CPU error interrupt number and
>>> +  then the L2 cache error interrupt number
>>> +
>>> +Optional properties:
>>> +- interrupt-names: Should contain the interrupt names "l1_irq" and
>>> +  "l2_irq"
>>> +
>>> +Example:
>>> +    edac {
>>> +            compatible = "qcom,krait-cache-erp";
>>> +            interrupts = <1 9 0xf04>, <0 2 0x4>;
>>> +    };
>> Why wouldn't we have these as part of cache nodes in the dts?  (which begs the question why we don't have cache nodes?)

In particular, naming the node edac seems like a suboptimal choice,
since that's a very linux-specific name for the error reporting
framework.

> I can certainly add cache nodes and cpu nodes and then put the
> interrupts in those nodes. I was thinking along those same lines when I
> ported this driver but figured it would be good to get something out
> there. The only question I have is how am I supposed to hook that up
> into the linux device model? Will the edac driver bind to the device
> created for the cpus node and the cache node? I guess it will have to be
> a driver that binds to two devices.

Or it could bind to one device and look up the info for the other from
the devicetree without relying on the driver core to instantiate
devices.


-Olof

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-29 18:00       ` Stephen Boyd
  2013-10-29 20:22         ` Olof Johansson
@ 2013-10-30  0:07         ` Stephen Boyd
  2013-10-30  0:38         ` Mark Rutland
  2 siblings, 0 replies; 11+ messages in thread
From: Stephen Boyd @ 2013-10-30  0:07 UTC (permalink / raw)
  To: Kumar Gala
  Cc: linux-edac, linux-kernel, linux-arm-msm, linux-arm-kernel,
	devicetree, Mark Rutland

On 10/29, Stephen Boyd wrote:
> On 10/29/13 01:21, Kumar Gala wrote:
> > On Oct 28, 2013, at 7:31 PM, Stephen Boyd wrote:
> >
> >> The Krait L1/L2 error reporting device is made up of two
> >> interrupts, one per-CPU interrupt for the L1 caches and one
> >> interrupt for the L2 cache.
> >>
> >> Cc: <devicetree@vger.kernel.org>
> >> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> >> ---
> >> .../devicetree/bindings/arm/qcom,krait-cache-erp.txt     | 16 ++++++++++++++++
> >> 1 file changed, 16 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> >> new file mode 100644
> >> index 0000000..01fe8a8
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> >> @@ -0,0 +1,16 @@
> >> +* Qualcomm Krait L1 / L2 cache error reporting
> >> +
> >> +Required properties:
> >> +- compatible: Should be "qcom,krait-cache-erp"
> >> +- interrupts: Should contain the L1/CPU error interrupt number and
> >> +  then the L2 cache error interrupt number
> >> +
> >> +Optional properties:
> >> +- interrupt-names: Should contain the interrupt names "l1_irq" and
> >> +  "l2_irq"
> >> +
> >> +Example:
> >> +	edac {
> >> +		compatible = "qcom,krait-cache-erp";
> >> +		interrupts = <1 9 0xf04>, <0 2 0x4>;
> >> +	};
> > Why wouldn't we have these as part of cache nodes in the dts?  (which begs the question why we don't have cache nodes?)
> >
> 
> I can certainly add cache nodes and cpu nodes and then put the
> interrupts in those nodes. I was thinking along those same lines when I
> ported this driver but figured it would be good to get something out
> there. The only question I have is how am I supposed to hook that up
> into the linux device model? Will the edac driver bind to the device
> created for the cpus node and the cache node? I guess it will have to be
> a driver that binds to two devices.
> 
> One could argue that we should put the cp15 based architected timers in
> the cpus node also but so far nobody has done that and I think there was
> some reasoning behind that, Mark?
> 

Ok I've come up with this:

	cpus {
		#address-cells = <1>;
		#size-cells = <0>;
		compatible = "qcom,krait";
		interrupts = <1 9 0xf04>;

		cpu@0 {
			reg = <0>;
			next-level-cache = <&L2>;
		};

		cpu@1 {
			reg = <1>;
			next-level-cache = <&L2>;
		};

		cpu@2 {
			reg = <2>;
			next-level-cache = <&L2>;
		};

		cpu@3 {
			reg = <3>;
			next-level-cache = <&L2>;
		};

		L2: l2-cache {
			compatible = "qcom,krait-l2", "cache";
			cache-level = <2>;
			interrupts = <0 2 0x4>;
		};
	};

But now I don't know where to document this. Should I make a
krait-edac.txt file and document it there? The problem is now
we're documenting more than the error interrupts.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-29  5:06     ` Stephen Boyd
@ 2013-10-30  0:34       ` Mark Rutland
  0 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2013-10-30  0:34 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org

On Tue, Oct 29, 2013 at 05:06:45AM +0000, Stephen Boyd wrote:
> On 10/28, Mark Rutland wrote:
> > On Tue, Oct 29, 2013 at 12:31:28AM +0000, Stephen Boyd wrote:
> > > +
> > > +Optional properties:
> > > +- interrupt-names: Should contain the interrupt names "l1_irq" and
> > > +  "l2_irq"
> > 
> > As with my comment on the parsing code, I'd prefer that if interrupt-names was
> > present it defined the order of interrupts. Otherwise it's redundant and of no
> > value.
> > 
> > Otherwise, the binding looks fine to me:
> > 
> > Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> How about I just drop the interrupt-names property? It isn't
> adding much and is a holdover from the vendor kernel.

That's also fine given that this is a very specific binding.

Mark.

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-29 18:00       ` Stephen Boyd
  2013-10-29 20:22         ` Olof Johansson
  2013-10-30  0:07         ` Stephen Boyd
@ 2013-10-30  0:38         ` Mark Rutland
  2013-10-30  7:19           ` Kumar Gala
  2 siblings, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2013-10-30  0:38 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Kumar Gala, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org

On Tue, Oct 29, 2013 at 06:00:59PM +0000, Stephen Boyd wrote:
> On 10/29/13 01:21, Kumar Gala wrote:
> > On Oct 28, 2013, at 7:31 PM, Stephen Boyd wrote:
> >
> >> The Krait L1/L2 error reporting device is made up of two
> >> interrupts, one per-CPU interrupt for the L1 caches and one
> >> interrupt for the L2 cache.
> >>
> >> Cc: <devicetree@vger.kernel.org>
> >> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> >> ---
> >> .../devicetree/bindings/arm/qcom,krait-cache-erp.txt     | 16 ++++++++++++++++
> >> 1 file changed, 16 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> >> new file mode 100644
> >> index 0000000..01fe8a8
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
> >> @@ -0,0 +1,16 @@
> >> +* Qualcomm Krait L1 / L2 cache error reporting
> >> +
> >> +Required properties:
> >> +- compatible: Should be "qcom,krait-cache-erp"
> >> +- interrupts: Should contain the L1/CPU error interrupt number and
> >> +  then the L2 cache error interrupt number
> >> +
> >> +Optional properties:
> >> +- interrupt-names: Should contain the interrupt names "l1_irq" and
> >> +  "l2_irq"
> >> +
> >> +Example:
> >> +	edac {
> >> +		compatible = "qcom,krait-cache-erp";
> >> +		interrupts = <1 9 0xf04>, <0 2 0x4>;
> >> +	};
> > Why wouldn't we have these as part of cache nodes in the dts?  (which begs the question why we don't have cache nodes?)
> >
> 
> I can certainly add cache nodes and cpu nodes and then put the
> interrupts in those nodes. I was thinking along those same lines when I
> ported this driver but figured it would be good to get something out
> there. The only question I have is how am I supposed to hook that up
> into the linux device model? Will the edac driver bind to the device
> created for the cpus node and the cache node? I guess it will have to be
> a driver that binds to two devices.
> 
> One could argue that we should put the cp15 based architected timers in
> the cpus node also but so far nobody has done that and I think there was
> some reasoning behind that, Mark?

The architected timer binding was created at a time I wasn't involved in kernel
development, and I'm not aware of any particular reasoning. I've heard that
there was a decision to not duplicate banked resources, which would explain not
having the timer under /cpus/cpu@N, but doesn't imply that having it under
/cpus is bad.

Do we have precedent for putting any devices other than CPUs in /cpus?

Thanks,
Mark.

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

* Re: [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding
  2013-10-30  0:38         ` Mark Rutland
@ 2013-10-30  7:19           ` Kumar Gala
  0 siblings, 0 replies; 11+ messages in thread
From: Kumar Gala @ 2013-10-30  7:19 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Stephen Boyd, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org


On Oct 29, 2013, at 7:38 PM, Mark Rutland wrote:

> On Tue, Oct 29, 2013 at 06:00:59PM +0000, Stephen Boyd wrote:
>> On 10/29/13 01:21, Kumar Gala wrote:
>>> On Oct 28, 2013, at 7:31 PM, Stephen Boyd wrote:
>>> 
>>>> The Krait L1/L2 error reporting device is made up of two
>>>> interrupts, one per-CPU interrupt for the L1 caches and one
>>>> interrupt for the L2 cache.
>>>> 
>>>> Cc: <devicetree@vger.kernel.org>
>>>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>>>> ---
>>>> .../devicetree/bindings/arm/qcom,krait-cache-erp.txt     | 16 ++++++++++++++++
>>>> 1 file changed, 16 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>>>> 
>>>> diff --git a/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>>>> new file mode 100644
>>>> index 0000000..01fe8a8
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/arm/qcom,krait-cache-erp.txt
>>>> @@ -0,0 +1,16 @@
>>>> +* Qualcomm Krait L1 / L2 cache error reporting
>>>> +
>>>> +Required properties:
>>>> +- compatible: Should be "qcom,krait-cache-erp"
>>>> +- interrupts: Should contain the L1/CPU error interrupt number and
>>>> +  then the L2 cache error interrupt number
>>>> +
>>>> +Optional properties:
>>>> +- interrupt-names: Should contain the interrupt names "l1_irq" and
>>>> +  "l2_irq"
>>>> +
>>>> +Example:
>>>> +	edac {
>>>> +		compatible = "qcom,krait-cache-erp";
>>>> +		interrupts = <1 9 0xf04>, <0 2 0x4>;
>>>> +	};
>>> Why wouldn't we have these as part of cache nodes in the dts?  (which begs the question why we don't have cache nodes?)
>>> 
>> 
>> I can certainly add cache nodes and cpu nodes and then put the
>> interrupts in those nodes. I was thinking along those same lines when I
>> ported this driver but figured it would be good to get something out
>> there. The only question I have is how am I supposed to hook that up
>> into the linux device model? Will the edac driver bind to the device
>> created for the cpus node and the cache node? I guess it will have to be
>> a driver that binds to two devices.
>> 
>> One could argue that we should put the cp15 based architected timers in
>> the cpus node also but so far nobody has done that and I think there was
>> some reasoning behind that, Mark?
> 
> The architected timer binding was created at a time I wasn't involved in kernel
> development, and I'm not aware of any particular reasoning. I've heard that
> there was a decision to not duplicate banked resources, which would explain not
> having the timer under /cpus/cpu@N, but doesn't imply that having it under
> /cpus is bad.
> 
> Do we have precedent for putting any devices other than CPUs in /cpus?

On PPC we had core cache's (depending on topology) that would be there, and thus why I raised the suggestion.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

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

end of thread, other threads:[~2013-10-30  7:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-29  0:31 [PATCH 0/6] Krait L1/L2 EDAC driver Stephen Boyd
2013-10-29  0:31 ` [PATCH 4/6] edac: Document Krait L1/L2 EDAC driver binding Stephen Boyd
2013-10-29  1:34   ` Mark Rutland
2013-10-29  5:06     ` Stephen Boyd
2013-10-30  0:34       ` Mark Rutland
     [not found]   ` <1383006690-6754-5-git-send-email-sboyd-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2013-10-29  8:21     ` Kumar Gala
2013-10-29 18:00       ` Stephen Boyd
2013-10-29 20:22         ` Olof Johansson
2013-10-30  0:07         ` Stephen Boyd
2013-10-30  0:38         ` Mark Rutland
2013-10-30  7:19           ` Kumar Gala

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