| Message ID | 1524594959-5259-2-git-send-email-thor.thayer@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/24/2018 01:35 PM, thor.thayer@linux.intel.com wrote: > From: Thor Thayer <thor.thayer@linux.intel.com> > > Add the device tree bindings needed to support the Stratix10 > ECC Manager and SDRAM ECC to the existing bindings. > > Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com> > --- > .../bindings/arm/altera/socfpga-eccmgr.txt | 47 ++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > index 4a1714f96bab..fe48ad293a24 100644 > --- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > @@ -231,3 +231,50 @@ Example: > <48 IRQ_TYPE_LEVEL_HIGH>; > }; > }; > + > +Stratix10 SoCFPGA ECC Manager > +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral > +in a shared register similar to the Arria10. However, ECC requires > +access to registers that can only be read in EL3 with SMC calls. > +Therefore the device tree is slightly different. > + > +Required Properties: > +- compatible : Should be "altr,socfpga-s10-ecc-manager" Altera technically doesn't exist anymore, should this be "intel,stratix10-ecc-manager"? Dinh
On 04/25/2018 09:16 PM, Dinh Nguyen wrote: > > > On 04/24/2018 01:35 PM, thor.thayer@linux.intel.com wrote: >> From: Thor Thayer <thor.thayer@linux.intel.com> >> >> Add the device tree bindings needed to support the Stratix10 >> ECC Manager and SDRAM ECC to the existing bindings. >> >> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com> >> --- >> .../bindings/arm/altera/socfpga-eccmgr.txt | 47 ++++++++++++++++++++++ >> 1 file changed, 47 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >> index 4a1714f96bab..fe48ad293a24 100644 >> --- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >> +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >> @@ -231,3 +231,50 @@ Example: >> <48 IRQ_TYPE_LEVEL_HIGH>; >> }; >> }; >> + >> +Stratix10 SoCFPGA ECC Manager >> +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral >> +in a shared register similar to the Arria10. However, ECC requires >> +access to registers that can only be read in EL3 with SMC calls. >> +Therefore the device tree is slightly different. >> + >> +Required Properties: >> +- compatible : Should be "altr,socfpga-s10-ecc-manager" > > Altera technically doesn't exist anymore, should this be > "intel,stratix10-ecc-manager"? > I was trying to be consistent with the older names but I agree with your argument. I will change this. Thanks > Dinh > -- > To unsubscribe from this list: send the line "unsubscribe linux-edac" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 04/26/2018 09:43 AM, Thor Thayer wrote: > On 04/25/2018 09:16 PM, Dinh Nguyen wrote: >> >> >> On 04/24/2018 01:35 PM, thor.thayer@linux.intel.com wrote: >>> From: Thor Thayer <thor.thayer@linux.intel.com> >>> >>> Add the device tree bindings needed to support the Stratix10 >>> ECC Manager and SDRAM ECC to the existing bindings. >>> >>> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com> >>> --- >>> .../bindings/arm/altera/socfpga-eccmgr.txt | 47 >>> ++++++++++++++++++++++ >>> 1 file changed, 47 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>> b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>> index 4a1714f96bab..fe48ad293a24 100644 >>> --- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>> +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>> @@ -231,3 +231,50 @@ Example: >>> <48 IRQ_TYPE_LEVEL_HIGH>; >>> }; >>> }; >>> + >>> +Stratix10 SoCFPGA ECC Manager >>> +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral >>> +in a shared register similar to the Arria10. However, ECC requires >>> +access to registers that can only be read in EL3 with SMC calls. >>> +Therefore the device tree is slightly different. >>> + >>> +Required Properties: >>> +- compatible : Should be "altr,socfpga-s10-ecc-manager" >> >> Altera technically doesn't exist anymore, should this be >> "intel,stratix10-ecc-manager"? >> > I was trying to be consistent with the older names but I agree with your > argument. I will change this. Thanks > After looking at the Stratix10 device tree, there are only "altr," in there. For instance the top node is: compatible = "altr,socfpga-stratix10"; and even the directory that the device tree is in is named "altera" so I'll stick with the existing "altr," designation to be consistent. Thanks, Thor >> Dinh >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-edac" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-edac" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 04/26/2018 09:58 AM, Thor Thayer wrote: > On 04/26/2018 09:43 AM, Thor Thayer wrote: >> On 04/25/2018 09:16 PM, Dinh Nguyen wrote: >>> >>> >>> On 04/24/2018 01:35 PM, thor.thayer@linux.intel.com wrote: >>>> From: Thor Thayer <thor.thayer@linux.intel.com> >>>> >>>> Add the device tree bindings needed to support the Stratix10 >>>> ECC Manager and SDRAM ECC to the existing bindings. >>>> >>>> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com> >>>> --- >>>> .../bindings/arm/altera/socfpga-eccmgr.txt | 47 >>>> ++++++++++++++++++++++ >>>> 1 file changed, 47 insertions(+) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>>> b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>>> index 4a1714f96bab..fe48ad293a24 100644 >>>> --- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>>> +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>>> @@ -231,3 +231,50 @@ Example: >>>> <48 IRQ_TYPE_LEVEL_HIGH>; >>>> }; >>>> }; >>>> + >>>> +Stratix10 SoCFPGA ECC Manager >>>> +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral >>>> +in a shared register similar to the Arria10. However, ECC requires >>>> +access to registers that can only be read in EL3 with SMC calls. >>>> +Therefore the device tree is slightly different. >>>> + >>>> +Required Properties: >>>> +- compatible : Should be "altr,socfpga-s10-ecc-manager" >>> >>> Altera technically doesn't exist anymore, should this be >>> "intel,stratix10-ecc-manager"? >>> >> I was trying to be consistent with the older names but I agree with >> your argument. I will change this. Thanks >> > After looking at the Stratix10 device tree, there are only "altr," in > there. For instance the top node is: > > compatible = "altr,socfpga-stratix10"; > > and even the directory that the device tree is in is named "altera" > > so I'll stick with the existing "altr," designation to be consistent. > That's fine and I don't have any strong inclination for 1 way or another. But it's not quite true that there are only "altr" in the dts file. I did recently switch the clock manager to an intel designation. I upstreamed the original stratix10 DTS file back in 2015, before Altera was purchased by Intel. My only argument is that if you're adding new support for devices, it should be an intel binding. Dinh
On Thu, Apr 26, 2018 at 10:01 AM, Dinh Nguyen <dinguyen@kernel.org> wrote: > > > On 04/26/2018 09:58 AM, Thor Thayer wrote: >> On 04/26/2018 09:43 AM, Thor Thayer wrote: >>> On 04/25/2018 09:16 PM, Dinh Nguyen wrote: >>>> >>>> >>>> On 04/24/2018 01:35 PM, thor.thayer@linux.intel.com wrote: >>>>> From: Thor Thayer <thor.thayer@linux.intel.com> >>>>> >>>>> Add the device tree bindings needed to support the Stratix10 >>>>> ECC Manager and SDRAM ECC to the existing bindings. >>>>> >>>>> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com> >>>>> --- >>>>> .../bindings/arm/altera/socfpga-eccmgr.txt | 47 >>>>> ++++++++++++++++++++++ >>>>> 1 file changed, 47 insertions(+) >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>>>> b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>>>> index 4a1714f96bab..fe48ad293a24 100644 >>>>> --- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>>>> +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt >>>>> @@ -231,3 +231,50 @@ Example: >>>>> <48 IRQ_TYPE_LEVEL_HIGH>; >>>>> }; >>>>> }; >>>>> + >>>>> +Stratix10 SoCFPGA ECC Manager >>>>> +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral >>>>> +in a shared register similar to the Arria10. However, ECC requires >>>>> +access to registers that can only be read in EL3 with SMC calls. >>>>> +Therefore the device tree is slightly different. >>>>> + >>>>> +Required Properties: >>>>> +- compatible : Should be "altr,socfpga-s10-ecc-manager" >>>> >>>> Altera technically doesn't exist anymore, should this be >>>> "intel,stratix10-ecc-manager"? >>>> >>> I was trying to be consistent with the older names but I agree with >>> your argument. I will change this. Thanks >>> >> After looking at the Stratix10 device tree, there are only "altr," in >> there. For instance the top node is: >> >> compatible = "altr,socfpga-stratix10"; >> >> and even the directory that the device tree is in is named "altera" >> >> so I'll stick with the existing "altr," designation to be consistent. >> > > That's fine and I don't have any strong inclination for 1 way or > another. But it's not quite true that there are only "altr" in the dts file. > > I did recently switch the clock manager to an intel designation. I > upstreamed the original stratix10 DTS file back in 2015, before Altera > was purchased by Intel. > > My only argument is that if you're adding new support for devices, it > should be an intel binding. I think I would make the switch when you change it at the SoC level (for a new SoC). IOW, if the top level compatible is using 'intel', then use that for the device bindings otherwise stick with 'altr' for that SoC. Rob
diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt index 4a1714f96bab..fe48ad293a24 100644 --- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt @@ -231,3 +231,50 @@ Example: <48 IRQ_TYPE_LEVEL_HIGH>; }; }; + +Stratix10 SoCFPGA ECC Manager +The Stratix10 SoC ECC Manager handles the IRQs for each peripheral +in a shared register similar to the Arria10. However, ECC requires +access to registers that can only be read in EL3 with SMC calls. +Therefore the device tree is slightly different. + +Required Properties: +- compatible : Should be "altr,socfpga-s10-ecc-manager" +- altr,sysgr-syscon : phandle to Stratix10 System Manager Block + containing the ECC manager registers. +- #address-cells: must be 1 +- #size-cells: must be 1 +- interrupts : Should be single bit error interrupt, then double bit error + interrupt. +- interrupt-controller : boolean indicator that ECC Manager is an interrupt controller +- #interrupt-cells : must be set to 2. +- ranges : standard definition, should translate from local addresses + +Subcomponents: + +SDRAM ECC +Required Properties: +- compatible : Should be "altr,sdram-edac-s10" +- reg : Address and size for ECC error interrupt clear registers. +- interrupts : Should be single bit error interrupt, then double bit error + interrupt, in this order. + +Example: + + eccmgr: eccmgr@ffd12000 { + compatible = "altr,socfpga-s10-ecc-manager"; + altr,sysmgr-syscon = <&sysmgr>; + #address-cells = <1>; + #size-cells = <1>; + interrupts = <0 15 4>, <0 95 4>; + interrupt-controller; + #interrupt-cells = <2>; + ranges; + + sdramedac@f8011100 { + compatible = "altr,sdram-edac-s10"; + reg = <0xf8011100 0xC0>; + interrupts = <16 4>, <48 4>; + }; + }; +