On 10/01/2014 11:53 AM, Mark Rutland wrote:
On Wed, Oct 01, 2014 at 05:31:31PM +0100, tthayer@xxxxxxxxxxxxxxxxxxxxx wrote:
From: Thor Thayer <tthayer@xxxxxxxxxxxxxxxxxxxxx>
Adding L2 Cache and On-Chip RAM EDAC support for the
Altera SoCs using the EDAC_DEVICE framework. The EDAC
manager abstracts the common probe functionality and
test triggers. The L2 Cache and OCRAM files handle
the specific memory functions.
Signed-off-by: Thor Thayer <tthayer@xxxxxxxxxxxxxxxxxxxxx>
---
.../bindings/arm/altera/socfpga-l2-edac.txt | 15 ++
.../bindings/arm/altera/socfpga-ocram-edac.txt | 16 ++
MAINTAINERS | 1 +
drivers/edac/Kconfig | 14 ++
drivers/edac/Makefile | 4 +
drivers/edac/altera_edac_mgr.c | 261 ++++++++++++++++++++
drivers/edac/altera_edac_mgr.h | 56 +++++
drivers/edac/altera_l2_edac.c | 130 ++++++++++
drivers/edac/altera_ocram_edac.c | 107 ++++++++
9 files changed, 604 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt
create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt
create mode 100644 drivers/edac/altera_edac_mgr.c
create mode 100644 drivers/edac/altera_edac_mgr.h
create mode 100644 drivers/edac/altera_l2_edac.c
create mode 100644 drivers/edac/altera_ocram_edac.c
diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt
new file mode 100644
index 0000000..35b19e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt
@@ -0,0 +1,15 @@
+Altera SoCFPGA L2 cache Error Detection and Correction [EDAC]
+
+Required Properties:
+- compatible : Should be "altr,l2-edac"
That string looks too generic.
Given the EDAC seems to be a portion of the L2, is there not already an
L2 binding?
Just because Linux expects two drivers doesn't mean we should partition
the HW description this way.
Thank you for the quick feedback.
What should the string look like? I was trying to keep it short with the
altr, prefix but I don't mind changing it to something better.
We're using the ARM PL310 L2 cache controller. The ECC is separate from
the PL310 IP and is part of the System Manager. This is true of ECC for
both the L2 and OCRAM.
I followed the Calxeda Highbank L2 EDAC implementation since ours is
similar. The Calxeda Highbank implementation had a separate binding file
and a device node under soc.
+- reg : Address and size for ECC error interrupt clear registers.
+- interrupts : Should be single bit error interrupt, then double bit error
+ interrupt. Note the rising edge type.
+
+Example:
+
+ l2edac@ffd08140 {
+ compatible = "altr,l2-edac";
+ reg = <0xffd08140 0x4>;
+ interrupts = <0 36 1>, <0 37 1>;
+ };
diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt
new file mode 100644
index 0000000..31ab205
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt
@@ -0,0 +1,16 @@
+Altera SoCFPGA On-Chip RAM Error Detection and Correction [EDAC]
+
+OCRAM ECC Required Properties:
+- compatible : Should be "altr,ocram-edac"
+- reg : Address and size for ECC error interrupt clear registers.
+- iram : phandle to On-Chip RAM definition.
Why not just describe this in the OCRAM node? Surely the register is
within the OCRAM controller?
The ECC registers not in the OCRAM controller but they are in the System
Manager. Maybe both the L2 cache and OCRAM ECC bindings should live
there and the device tree node for System Manager would have OCRAM and
L2 cache sub-nodes.
Thanks,
Thor
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
