Re: [PATCH 2/3] edac: altera: Add Altera L2 Cache and OCRAM EDAC Support

[Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>]

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

Ah, I see. Apologies, I assumed that this was part of the L2C.

[...]

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

It certainly sounds like the ECC registers should be described as a
portion of the system manager somehow.

Thanks,
Mark.
--
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