LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: "tthayer@opensource.altera.com" <tthayer@opensource.altera.com>
Cc: "bp@alien8.de" <bp@alien8.de>,
	"dougthompson@xmission.com" <dougthompson@xmission.com>,
	"m.chehab@samsung.com" <m.chehab@samsung.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	Pawel Moll <Pawel.Moll@arm.com>,
	"ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>,
	"galak@codeaurora.org" <galak@codeaurora.org>,
	"linux@arm.linux.org.uk" <linux@arm.linux.org.uk>,
	"dinguyen@opensource.altera.com" <dinguyen@opensource.altera.com>,
	"grant.likely@linaro.org" <grant.likely@linaro.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"tthayer.linux@gmail.com" <tthayer.linux@gmail.com>
Subject: Re: [PATCHv6 5/5] arm: dts: Add Altera L2 Cache and OCRAM EDAC entries
Date: Fri, 6 Feb 2015 19:24:44 +0000	[thread overview]
Message-ID: <20150206192444.GF10324@leverpostej> (raw)
In-Reply-To: <1420772036-3112-6-git-send-email-tthayer@opensource.altera.com>

On Fri, Jan 09, 2015 at 02:53:56AM +0000, tthayer@opensource.altera.com wrote:
> From: Thor Thayer <tthayer@opensource.altera.com>
> 
> Adding the device tree entries and bindings needed to support
> the Altera L2 cache and On-Chip RAM EDAC. This patch relies upon
> an earlier patch to declare and setup On-chip RAM properly.
> http://www.spinics.net/lists/devicetree/msg51117.html
> 
> Signed-off-by: Thor Thayer <tthayer@opensource.altera.com>
> ---
> v2: Remove OCRAM declaration and reference prior patch.
> 
> v3-5: No Change
> 
> v6: Change to nested EDAC device nodes based on community
>     feedback. Remove L2 syscon. Use consolidated binding.
> ---
>  .../bindings/arm/altera/socfpga-edac.txt           |   46 ++++++++++++++++++++
>  arch/arm/boot/dts/socfpga.dtsi                     |   20 +++++++++
>  2 files changed, 66 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-edac.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-edac.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-edac.txt
> new file mode 100644
> index 0000000..4bf32e1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-edac.txt
> @@ -0,0 +1,46 @@
> +Altera SoCFPGA Error Detection and Correction [EDAC]
> +
> +Required Properties:
> +- compatible : Should be "altr,edac"

Is there a single large EDAC block that happens to perform EDAC
functions for various components, or various small EDAC blocks that
happen to be close to each other in the MMIO space?

> +- #address-cells: must be 1
> +- #size-cells: must be 1
> +- ranges : standard definition, should translate from local addresses

Surely these just need to be suitable for mapping the child nodes, and
you shouldn't care about their precise values?

> +
> +Subcomponents:
> +
> +L2 Cache ECC
> +Required Properties:
> +- compatible : Should be "altr,l2-edac"
> +- 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.
> +
> +On Chip RAM 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.
> +- interrupts : Should be single bit error interrupt, then double bit error
> +	interrupt. Note the rising edge type.

Do these actually differ in programming interface, or just w.r.t. the
component they are used to monitor?

It would be good to have an explicit link to what they monitor rather
than relying on there being single instances with particular compatible
strings. That will make this easier to generalise for multiple instances
in future SoCs that may reuse the EDAC block.

This doesn't look too bad, but I'd like to hear some response to this
and my other queries before I can say whether this is a good way of
describing the hardware; it all looks a bit vague.

As an aside, please place the documentation at the start of the series,
before the driver rework. The dts patches can come after the docs and
code patches.

Thanks,
Mark.

> +
> +Example:
> +
> +	soc_ecc {
> +		compatible = "altr,edac";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		l2edac@ffd08140 {
> +			compatible = "altr,l2-edac";
> +			reg = <0xffd08140 0x4>;
> +			interrupts = <0 36 1>, <0 37 1>;
> +		};
> +
> +		ocramedac@ffd08144 {
> +			compatible = "altr,ocram-edac";
> +			reg = <0xffd08144 0x4>;
> +			iram = <&ocram>;
> +			interrupts = <0 178 1>, <0 179 1>;
> +		};
> +	};
> diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
> index 252c3d1..e546e47 100644
> --- a/arch/arm/boot/dts/socfpga.dtsi
> +++ b/arch/arm/boot/dts/socfpga.dtsi
> @@ -618,6 +618,26 @@
>  			interrupts = <0 39 4>;
>  		};
>  
> +		soc_ecc {
> +			compatible = "altr,edac";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			l2edac@ffd08140 {
> +				compatible = "altr,l2-edac";
> +				reg = <0xffd08140 0x4>;
> +				interrupts = <0 36 1>, <0 37 1>;
> +			};
> +
> +			ocramedac@ffd08144 {
> +				compatible = "altr,ocram-edac";
> +				reg = <0xffd08144 0x4>;
> +				iram = <&ocram>;
> +				interrupts = <0 178 1>, <0 179 1>;
> +			};
> +		};
> +
>  		L2: l2-cache@fffef000 {
>  			compatible = "arm,pl310-cache";
>  			reg = <0xfffef000 0x1000>;
> -- 
> 1.7.9.5
> 
> --
> 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
> 

  parent reply	other threads:[~2015-02-06 19:25 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-09  2:53 [PATCHv6 0/5] Add Altera peripheral memories to EDAC framework tthayer
2015-01-09  2:53 ` [PATCHv6 1/5] arm: socfpga: Enable L2 Cache ECC on startup tthayer
2015-02-06 18:52   ` Mark Rutland
2015-01-09  2:53 ` [PATCHv6 2/5] arm: socfpga: Enable OCRAM " tthayer
2015-02-06 18:45   ` Mark Rutland
2015-02-06 22:05     ` Thor Thayer
2015-01-09  2:53 ` [PATCHv6 3/5] edac: altera: Remove SDRAM module compile tthayer
2015-01-09  2:53 ` [PATCHv6 4/5] edac: altera: Add Altera L2 Cache and OCRAM EDAC Support tthayer
2015-02-06 19:17   ` Mark Rutland
2015-02-06 22:09     ` Thor Thayer
2015-02-07 10:02   ` Russell King - ARM Linux
2015-01-09  2:53 ` [PATCHv6 5/5] arm: dts: Add Altera L2 Cache and OCRAM EDAC entries tthayer
2015-02-06 17:03   ` [RESEND PATCHv6 " Thor Thayer
2015-02-06 19:24   ` Mark Rutland [this message]
2015-02-06 22:04     ` [PATCHv6 " Thor Thayer
2015-01-29 20:53 ` [PATCHv6 0/5] Add Altera peripheral memories to EDAC framework Thor Thayer
2015-02-06 19:29   ` Mark Rutland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150206192444.GF10324@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=Pawel.Moll@arm.com \
    --cc=bp@alien8.de \
    --cc=devicetree@vger.kernel.org \
    --cc=dinguyen@opensource.altera.com \
    --cc=dougthompson@xmission.com \
    --cc=galak@codeaurora.org \
    --cc=grant.likely@linaro.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=m.chehab@samsung.com \
    --cc=robh+dt@kernel.org \
    --cc=tthayer.linux@gmail.com \
    --cc=tthayer@opensource.altera.com \
    --subject='Re: [PATCHv6 5/5] arm: dts: Add Altera L2 Cache and OCRAM EDAC entries' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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