| Message ID | 1415751263-1830-6-git-send-email-tthayer@opensource.altera.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi all, On 11/11/2014 06:14 PM, 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 Any comments on these devicetree additions? Thanks, Thor > Signed-off-by: Thor Thayer<tthayer@opensource.altera.com> > --- > v2: Remove OCRAM declaration and reference prior patch. > > v3-5: No Change > --- > .../bindings/arm/altera/socfpga-l2-edac.txt | 15 +++++++++++++++ > .../bindings/arm/altera/socfpga-ocram-edac.txt | 16 ++++++++++++++++ > arch/arm/boot/dts/socfpga.dtsi | 15 ++++++++++++++- > 3 files changed, 45 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt > create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt > > 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" > +- 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. > +- interrupts : Should be single bit error interrupt, then double bit error > + interrupt. Note the rising edge type. > + > +Example: > + 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 6af96ed..32c63a3 100644 > --- a/arch/arm/boot/dts/socfpga.dtsi > +++ b/arch/arm/boot/dts/socfpga.dtsi > @@ -618,8 +618,21 @@ > interrupts = <0 39 4>; > }; > > + 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"; > + compatible = "arm,pl310-cache", "syscon"; > reg = <0xfffef000 0x1000>; > interrupts = <0 38 0x04>; > cache-unified;
Hi Boris, On 11/18/2014 02:56 PM, Thor Thayer wrote: > Hi all, > > On 11/11/2014 06:14 PM, 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 > > Any comments on these devicetree additions? > > Thanks, > Thor >> Signed-off-by: Thor Thayer<tthayer@opensource.altera.com> >> --- >> v2: Remove OCRAM declaration and reference prior patch. >> >> v3-5: No Change >> --- I originally submitted this series on November 11, 2014. I haven't received any comments or ACKs on the device tree patch portion (patch 5 of 5). According to Documentation/devicetree/bindings/submitting-patches.txt, if the patch hasn't been Acked after a couple of weeks, the maintainer can pull the changes if they are comfortable with those changes. Dinh checked for merge conflicts in the DTS files in his November 7 email (queued patches for 3.19). Dinh acked patches 1 & 2 on November 12. Thanks, Thor
On Wed, Nov 12, 2014 at 12:14:23AM +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 > --- > .../bindings/arm/altera/socfpga-l2-edac.txt | 15 +++++++++++++++ > .../bindings/arm/altera/socfpga-ocram-edac.txt | 16 ++++++++++++++++ > arch/arm/boot/dts/socfpga.dtsi | 15 ++++++++++++++- > 3 files changed, 45 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-l2-edac.txt > create mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-ocram-edac.txt > > 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" > +- 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>; > + }; Judging by the size of the reg entry, this is part of a larger block (the same one the OCRAM EDAC lives in). Why isn't that larger block described? EDAC is a Linux subsystem name, but typically not the HW block name. What HW block does this live in? > 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. > +- interrupts : Should be single bit error interrupt, then double bit error > + interrupt. Note the rising edge type. > + > +Example: > + 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 6af96ed..32c63a3 100644 > --- a/arch/arm/boot/dts/socfpga.dtsi > +++ b/arch/arm/boot/dts/socfpga.dtsi > @@ -618,8 +618,21 @@ > interrupts = <0 39 4>; > }; > > + 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"; > + compatible = "arm,pl310-cache", "syscon"; NAK. Why are you marking the PL310 as a syscon device? It is most definitely _NOT_ a shared set of registers lumped together. Thanks, Mark.
Hi Thor, On Mon, Dec 01, 2014 at 08:47:41PM +0000, Thor Thayer wrote: > Hi Boris, > > On 11/18/2014 02:56 PM, Thor Thayer wrote: > > Hi all, > > > > On 11/11/2014 06:14 PM, 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 > > > > Any comments on these devicetree additions? > > > > Thanks, > > Thor > >> Signed-off-by: Thor Thayer<tthayer@opensource.altera.com> > >> --- > >> v2: Remove OCRAM declaration and reference prior patch. > >> > >> v3-5: No Change > >> --- > > I originally submitted this series on November 11, 2014. > > I haven't received any comments or ACKs on the device tree patch portion > (patch 5 of 5). According to > Documentation/devicetree/bindings/submitting-patches.txt, if the patch > hasn't been Acked after a couple of weeks, the maintainer can pull the > changes if they are comfortable with those changes. Apologies it has taken so long for this to be looked at. Unfortunately my inbox is overflowing (as are all of ours) and things fall by the wayside. I am sorry that this has lead to such a delayed response, especially given that it was negative. I'll try to keep up with this a bit more actively. Thanks, Mark.
On 12/02/2014 09:01 AM, Mark Rutland wrote: > Hi Thor, > > On Mon, Dec 01, 2014 at 08:47:41PM +0000, Thor Thayer wrote: >> Hi Boris, >> >> On 11/18/2014 02:56 PM, Thor Thayer wrote: >>> Hi all, >>> >>> On 11/11/2014 06:14 PM, 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 >>> >>> Any comments on these devicetree additions? >>> >>> Thanks, >>> Thor >>>> Signed-off-by: Thor Thayer<tthayer@opensource.altera.com> >>>> --- >>>> v2: Remove OCRAM declaration and reference prior patch. >>>> >>>> v3-5: No Change >>>> --- >> >> I originally submitted this series on November 11, 2014. >> >> I haven't received any comments or ACKs on the device tree patch portion >> (patch 5 of 5). According to >> Documentation/devicetree/bindings/submitting-patches.txt, if the patch >> hasn't been Acked after a couple of weeks, the maintainer can pull the >> changes if they are comfortable with those changes. > > Apologies it has taken so long for this to be looked at. Unfortunately > my inbox is overflowing (as are all of ours) and things fall by the > wayside. I am sorry that this has lead to such a delayed response, > especially given that it was negative. > > I'll try to keep up with this a bit more actively. > > Thanks, > Mark. > Hi Mark, Thank you for reviewing this. I have seen how busy the device tree ML is so I completely understand. You have reviewed previous patch revisions in this series - thanks. I thought that I'd addressed most of the issues (in some cases by explaining my implementation) and since I hadn't heard anything, I assumed there weren't concerns. Thanks again, Thor
On 12/02/2014 08:57 AM, Mark Rutland wrote: > On Wed, Nov 12, 2014 at 12:14:23AM +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> <snip> >> +++ 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" >> +- 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>; >> + }; > > Judging by the size of the reg entry, this is part of a larger block > (the same one the OCRAM EDAC lives in). Why isn't that larger block > described? > > EDAC is a Linux subsystem name, but typically not the HW block name. > What HW block does this live in? > Yes, this register is part of the ECC block of registers. In order to get each probe function to run, the L2 EDAC and OCRAM EDAC had to be at the top level of the device tree. If they are children of a parent node, the probe functions aren't executed. Is there a better way or example I should follow? >> 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 <snip> >> + >> L2: l2-cache@fffef000 { >> - compatible = "arm,pl310-cache"; >> + compatible = "arm,pl310-cache", "syscon"; > > NAK. > > Why are you marking the PL310 as a syscon device? It is most definitely > _NOT_ a shared set of registers lumped together. > Unfortunately, the register is locked for exclusive access by the L2 cache driver. I read your comment on patch 4 and will find a better way. > Thanks, > Mark. >
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" +- 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. +- interrupts : Should be single bit error interrupt, then double bit error + interrupt. Note the rising edge type. + +Example: + 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 6af96ed..32c63a3 100644 --- a/arch/arm/boot/dts/socfpga.dtsi +++ b/arch/arm/boot/dts/socfpga.dtsi @@ -618,8 +618,21 @@ interrupts = <0 39 4>; }; + 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"; + compatible = "arm,pl310-cache", "syscon"; reg = <0xfffef000 0x1000>; interrupts = <0 38 0x04>; cache-unified;