diff mbox series

[1/3] Documentation: dt: socfpga: Add Stratix10 ECC Manager binding

Message ID 1524594959-5259-2-git-send-email-thor.thayer@linux.intel.com
State Superseded, archived
Headers show
Series Add SDRAM ECC support for Stratix10 | expand

Commit Message

Thor Thayer April 24, 2018, 6:35 p.m. UTC 
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(+)

Comments

Dinh Nguyen April 26, 2018, 2:16 a.m. UTC | #1 
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
--
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
Thor Thayer April 26, 2018, 2:43 p.m. UTC | #2 
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
> 

--
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
Thor Thayer April 26, 2018, 2:58 p.m. UTC | #3 
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
> 

--
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
Dinh Nguyen April 26, 2018, 3:01 p.m. UTC | #4 
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
--
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
Rob Herring April 26, 2018, 3:06 p.m. UTC | #5 
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
--
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
diff mbox series

Patch

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>;
+		};
+	};
+