dt-bindings: memory: ti-emif: add edac support under emif

Message ID 1518542129-25813-1-git-send-email-t-kristo@ti.com
State Changes Requested
Headers show
Series
  • dt-bindings: memory: ti-emif: add edac support under emif
Related show

Commit Message

Tero Kristo Feb. 13, 2018, 5:15 p.m.
Certain revisions of the TI EMIF IP contain ECC support in them. Reflect
this in the DT binding. Also, add interrupts property as a required
property for the emif controller, as all revisions of the emif IP contain
interrupt support; this might remain unused by the kernel driver though.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 .../devicetree/bindings/memory-controllers/ti/emif.txt      | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Rob Herring Feb. 19, 2018, 3:10 a.m. | #1
On Tue, Feb 13, 2018 at 07:15:29PM +0200, Tero Kristo wrote:
> Certain revisions of the TI EMIF IP contain ECC support in them. Reflect
> this in the DT binding. Also, add interrupts property as a required
> property for the emif controller, as all revisions of the emif IP contain
> interrupt support; this might remain unused by the kernel driver though.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  .../devicetree/bindings/memory-controllers/ti/emif.txt      | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
> index 621b41c..87022a9 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
> +++ b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
> @@ -3,7 +3,9 @@
>  EMIF - External Memory Interface - is an SDRAM controller used in
>  TI SoCs. EMIF supports, based on the IP revision, one or more of
>  DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance
> -of the EMIF IP and memory parts attached to it.
> +of the EMIF IP and memory parts attached to it. Certain revisions
> +of the EMIF controller also contain optional ECC support, which
> +corrects one bit errors and detects two bit errors.
>  
>  Required properties:
>  - compatible	: Should be of the form "ti,emif-<ip-rev>" where <ip-rev>
> @@ -11,6 +13,8 @@ Required properties:
>    compatible should be one of the following:
>    	     "ti,emif-am3352"
>  	     "ti,emif-am4372"
> +	     "ti,emif-dra7xx"
> +	     "ti,emif-keystone"
>  
>  - phy-type	: <u32> indicating the DDR phy type. Following are the
>    allowed values
> @@ -22,6 +26,7 @@ Required properties:
>  - ti,hwmods	: For TI hwmods processing and omap device creation
>    the value shall be "emif<n>" where <n> is the number of the EMIF
>    instance with base 1.
> +- interrupts	: interrupt used by the controller

Only for the new compatibles?

>  
>  Required only for "ti,emif-am3352" and "ti,emif-am4372":
>  - sram			: Phandles for generic sram driver nodes,
> @@ -71,3 +76,9 @@ emif: emif@4c000000 {
>          sram = <&pm_sram_code
>                  &pm_sram_data>;
>  };
> +
> +emif1: emif@4c000000 {
> +	compatible = "ti,emif-dra7xx";
> +	reg = <0x4c000000 0x200>;
> +	interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
> +};
> -- 
> 1.9.1
> 
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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
Tero Kristo Feb. 19, 2018, 6:21 a.m. | #2
On 19/02/18 05:10, Rob Herring wrote:
> On Tue, Feb 13, 2018 at 07:15:29PM +0200, Tero Kristo wrote:
>> Certain revisions of the TI EMIF IP contain ECC support in them. Reflect
>> this in the DT binding. Also, add interrupts property as a required
>> property for the emif controller, as all revisions of the emif IP contain
>> interrupt support; this might remain unused by the kernel driver though.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>>   .../devicetree/bindings/memory-controllers/ti/emif.txt      | 13 ++++++++++++-
>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>> index 621b41c..87022a9 100644
>> --- a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>> @@ -3,7 +3,9 @@
>>   EMIF - External Memory Interface - is an SDRAM controller used in
>>   TI SoCs. EMIF supports, based on the IP revision, one or more of
>>   DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance
>> -of the EMIF IP and memory parts attached to it.
>> +of the EMIF IP and memory parts attached to it. Certain revisions
>> +of the EMIF controller also contain optional ECC support, which
>> +corrects one bit errors and detects two bit errors.
>>   
>>   Required properties:
>>   - compatible	: Should be of the form "ti,emif-<ip-rev>" where <ip-rev>
>> @@ -11,6 +13,8 @@ Required properties:
>>     compatible should be one of the following:
>>     	     "ti,emif-am3352"
>>   	     "ti,emif-am4372"
>> +	     "ti,emif-dra7xx"
>> +	     "ti,emif-keystone"
>>   
>>   - phy-type	: <u32> indicating the DDR phy type. Following are the
>>     allowed values
>> @@ -22,6 +26,7 @@ Required properties:
>>   - ti,hwmods	: For TI hwmods processing and omap device creation
>>     the value shall be "emif<n>" where <n> is the number of the EMIF
>>     instance with base 1.
>> +- interrupts	: interrupt used by the controller
> 
> Only for the new compatibles?

I added this as required property for all, as all EMIF versions actually 
do have IRQ. Should this still be marked as optional as only certain 
versions of the driver use it? On am3/am4 only it is optional right now, 
and not used by the existing driver. On omap4 and omap5 it is required 
also (list of compatibles for the binding seem to be missing these 
actually; ti,emif-4d and ti,emif-4d5.)

Either way, I can mark this as optional property for am3/am4 if you want 
(I actually asked this already before but did not get clear response), 
what is your final take on this?

-Tero

> 
>>   
>>   Required only for "ti,emif-am3352" and "ti,emif-am4372":
>>   - sram			: Phandles for generic sram driver nodes,
>> @@ -71,3 +76,9 @@ emif: emif@4c000000 {
>>           sram = <&pm_sram_code
>>                   &pm_sram_data>;
>>   };
>> +
>> +emif1: emif@4c000000 {
>> +	compatible = "ti,emif-dra7xx";
>> +	reg = <0x4c000000 0x200>;
>> +	interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
>> +};
>> -- 
>> 1.9.1
>>
>> --

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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 Feb. 20, 2018, 3:52 p.m. | #3
On Mon, Feb 19, 2018 at 12:21 AM, Tero Kristo <t-kristo@ti.com> wrote:
> On 19/02/18 05:10, Rob Herring wrote:
>>
>> On Tue, Feb 13, 2018 at 07:15:29PM +0200, Tero Kristo wrote:
>>>
>>> Certain revisions of the TI EMIF IP contain ECC support in them. Reflect
>>> this in the DT binding. Also, add interrupts property as a required
>>> property for the emif controller, as all revisions of the emif IP contain
>>> interrupt support; this might remain unused by the kernel driver though.
>>>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> ---
>>>   .../devicetree/bindings/memory-controllers/ti/emif.txt      | 13
>>> ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>> b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>> index 621b41c..87022a9 100644
>>> --- a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>> @@ -3,7 +3,9 @@
>>>   EMIF - External Memory Interface - is an SDRAM controller used in
>>>   TI SoCs. EMIF supports, based on the IP revision, one or more of
>>>   DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance
>>> -of the EMIF IP and memory parts attached to it.
>>> +of the EMIF IP and memory parts attached to it. Certain revisions
>>> +of the EMIF controller also contain optional ECC support, which
>>> +corrects one bit errors and detects two bit errors.
>>>     Required properties:
>>>   - compatible  : Should be of the form "ti,emif-<ip-rev>" where <ip-rev>
>>> @@ -11,6 +13,8 @@ Required properties:
>>>     compatible should be one of the following:
>>>              "ti,emif-am3352"
>>>              "ti,emif-am4372"
>>> +            "ti,emif-dra7xx"
>>> +            "ti,emif-keystone"
>>>     - phy-type  : <u32> indicating the DDR phy type. Following are the
>>>     allowed values
>>> @@ -22,6 +26,7 @@ Required properties:
>>>   - ti,hwmods   : For TI hwmods processing and omap device creation
>>>     the value shall be "emif<n>" where <n> is the number of the EMIF
>>>     instance with base 1.
>>> +- interrupts   : interrupt used by the controller
>>
>>
>> Only for the new compatibles?
>
>
> I added this as required property for all, as all EMIF versions actually do
> have IRQ. Should this still be marked as optional as only certain versions
> of the driver use it? On am3/am4 only it is optional right now, and not used
> by the existing driver. On omap4 and omap5 it is required also (list of
> compatibles for the binding seem to be missing these actually; ti,emif-4d
> and ti,emif-4d5.)
>
> Either way, I can mark this as optional property for am3/am4 if you want (I
> actually asked this already before but did not get clear response), what is
> your final take on this?

Okay. I'm fine if it is required. You're going to update all the dts
files, right?

Reviewed-by: Rob Herring <robh@kernel.org>
--
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
Tero Kristo Feb. 21, 2018, 7 p.m. | #4
On 20/02/18 17:52, Rob Herring wrote:
> On Mon, Feb 19, 2018 at 12:21 AM, Tero Kristo <t-kristo@ti.com> wrote:
>> On 19/02/18 05:10, Rob Herring wrote:
>>>
>>> On Tue, Feb 13, 2018 at 07:15:29PM +0200, Tero Kristo wrote:
>>>>
>>>> Certain revisions of the TI EMIF IP contain ECC support in them. Reflect
>>>> this in the DT binding. Also, add interrupts property as a required
>>>> property for the emif controller, as all revisions of the emif IP contain
>>>> interrupt support; this might remain unused by the kernel driver though.
>>>>
>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>> ---
>>>>    .../devicetree/bindings/memory-controllers/ti/emif.txt      | 13
>>>> ++++++++++++-
>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>> b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>> index 621b41c..87022a9 100644
>>>> --- a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>> @@ -3,7 +3,9 @@
>>>>    EMIF - External Memory Interface - is an SDRAM controller used in
>>>>    TI SoCs. EMIF supports, based on the IP revision, one or more of
>>>>    DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance
>>>> -of the EMIF IP and memory parts attached to it.
>>>> +of the EMIF IP and memory parts attached to it. Certain revisions
>>>> +of the EMIF controller also contain optional ECC support, which
>>>> +corrects one bit errors and detects two bit errors.
>>>>      Required properties:
>>>>    - compatible  : Should be of the form "ti,emif-<ip-rev>" where <ip-rev>
>>>> @@ -11,6 +13,8 @@ Required properties:
>>>>      compatible should be one of the following:
>>>>               "ti,emif-am3352"
>>>>               "ti,emif-am4372"
>>>> +            "ti,emif-dra7xx"
>>>> +            "ti,emif-keystone"
>>>>      - phy-type  : <u32> indicating the DDR phy type. Following are the
>>>>      allowed values
>>>> @@ -22,6 +26,7 @@ Required properties:
>>>>    - ti,hwmods   : For TI hwmods processing and omap device creation
>>>>      the value shall be "emif<n>" where <n> is the number of the EMIF
>>>>      instance with base 1.
>>>> +- interrupts   : interrupt used by the controller
>>>
>>>
>>> Only for the new compatibles?
>>
>>
>> I added this as required property for all, as all EMIF versions actually do
>> have IRQ. Should this still be marked as optional as only certain versions
>> of the driver use it? On am3/am4 only it is optional right now, and not used
>> by the existing driver. On omap4 and omap5 it is required also (list of
>> compatibles for the binding seem to be missing these actually; ti,emif-4d
>> and ti,emif-4d5.)
>>
>> Either way, I can mark this as optional property for am3/am4 if you want (I
>> actually asked this already before but did not get clear response), what is
>> your final take on this?
> 
> Okay. I'm fine if it is required. You're going to update all the dts
> files, right?

Yeah, I will post patches for those, thanks for review.

-Tero

> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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
Santosh Shilimkar Feb. 25, 2018, 11:40 p.m. | #5
On 2/21/2018 11:00 AM, Tero Kristo wrote:
> On 20/02/18 17:52, Rob Herring wrote:
>> On Mon, Feb 19, 2018 at 12:21 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>> On 19/02/18 05:10, Rob Herring wrote:
>>>>
>>>> On Tue, Feb 13, 2018 at 07:15:29PM +0200, Tero Kristo wrote:
>>>>>
>>>>> Certain revisions of the TI EMIF IP contain ECC support in them. 
>>>>> Reflect
>>>>> this in the DT binding. Also, add interrupts property as a required
>>>>> property for the emif controller, as all revisions of the emif IP 
>>>>> contain
>>>>> interrupt support; this might remain unused by the kernel driver 
>>>>> though.
>>>>>
>>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>>> ---
>>>>>    .../devicetree/bindings/memory-controllers/ti/emif.txt      | 13
>>>>> ++++++++++++-
>>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>> b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>> index 621b41c..87022a9 100644
>>>>> --- a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>> @@ -3,7 +3,9 @@
>>>>>    EMIF - External Memory Interface - is an SDRAM controller used in
>>>>>    TI SoCs. EMIF supports, based on the IP revision, one or more of
>>>>>    DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance
>>>>> -of the EMIF IP and memory parts attached to it.
>>>>> +of the EMIF IP and memory parts attached to it. Certain revisions
>>>>> +of the EMIF controller also contain optional ECC support, which
>>>>> +corrects one bit errors and detects two bit errors.
>>>>>      Required properties:
>>>>>    - compatible  : Should be of the form "ti,emif-<ip-rev>" where 
>>>>> <ip-rev>
>>>>> @@ -11,6 +13,8 @@ Required properties:
>>>>>      compatible should be one of the following:
>>>>>               "ti,emif-am3352"
>>>>>               "ti,emif-am4372"
>>>>> +            "ti,emif-dra7xx"
>>>>> +            "ti,emif-keystone"
>>>>>      - phy-type  : <u32> indicating the DDR phy type. Following are 
>>>>> the
>>>>>      allowed values
>>>>> @@ -22,6 +26,7 @@ Required properties:
>>>>>    - ti,hwmods   : For TI hwmods processing and omap device creation
>>>>>      the value shall be "emif<n>" where <n> is the number of the EMIF
>>>>>      instance with base 1.
>>>>> +- interrupts   : interrupt used by the controller
>>>>
>>>>
>>>> Only for the new compatibles?
>>>
>>>
>>> I added this as required property for all, as all EMIF versions 
>>> actually do
>>> have IRQ. Should this still be marked as optional as only certain 
>>> versions
>>> of the driver use it? On am3/am4 only it is optional right now, and 
>>> not used
>>> by the existing driver. On omap4 and omap5 it is required also (list of
>>> compatibles for the binding seem to be missing these actually; 
>>> ti,emif-4d
>>> and ti,emif-4d5.)
>>>
>>> Either way, I can mark this as optional property for am3/am4 if you 
>>> want (I
>>> actually asked this already before but did not get clear response), 
>>> what is
>>> your final take on this?
>>
>> Okay. I'm fine if it is required. You're going to update all the dts
>> files, right?
> 
> Yeah, I will post patches for those, thanks for review.
> 
Let me know if you have already posted the follow up series ?
--
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
Tero Kristo Feb. 26, 2018, 7:23 a.m. | #6
On 26/02/18 01:40, Santosh Shilimkar wrote:
> 
> On 2/21/2018 11:00 AM, Tero Kristo wrote:
>> On 20/02/18 17:52, Rob Herring wrote:
>>> On Mon, Feb 19, 2018 at 12:21 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>> On 19/02/18 05:10, Rob Herring wrote:
>>>>>
>>>>> On Tue, Feb 13, 2018 at 07:15:29PM +0200, Tero Kristo wrote:
>>>>>>
>>>>>> Certain revisions of the TI EMIF IP contain ECC support in them. 
>>>>>> Reflect
>>>>>> this in the DT binding. Also, add interrupts property as a required
>>>>>> property for the emif controller, as all revisions of the emif IP 
>>>>>> contain
>>>>>> interrupt support; this might remain unused by the kernel driver 
>>>>>> though.
>>>>>>
>>>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>>>> ---
>>>>>>    .../devicetree/bindings/memory-controllers/ti/emif.txt      | 13
>>>>>> ++++++++++++-
>>>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>>> b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>>> index 621b41c..87022a9 100644
>>>>>> --- 
>>>>>> a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>>> +++ 
>>>>>> b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>>> @@ -3,7 +3,9 @@
>>>>>>    EMIF - External Memory Interface - is an SDRAM controller used in
>>>>>>    TI SoCs. EMIF supports, based on the IP revision, one or more of
>>>>>>    DDR2/DDR3/LPDDR2 protocols. This binding describes a given 
>>>>>> instance
>>>>>> -of the EMIF IP and memory parts attached to it.
>>>>>> +of the EMIF IP and memory parts attached to it. Certain revisions
>>>>>> +of the EMIF controller also contain optional ECC support, which
>>>>>> +corrects one bit errors and detects two bit errors.
>>>>>>      Required properties:
>>>>>>    - compatible  : Should be of the form "ti,emif-<ip-rev>" where 
>>>>>> <ip-rev>
>>>>>> @@ -11,6 +13,8 @@ Required properties:
>>>>>>      compatible should be one of the following:
>>>>>>               "ti,emif-am3352"
>>>>>>               "ti,emif-am4372"
>>>>>> +            "ti,emif-dra7xx"
>>>>>> +            "ti,emif-keystone"
>>>>>>      - phy-type  : <u32> indicating the DDR phy type. Following 
>>>>>> are the
>>>>>>      allowed values
>>>>>> @@ -22,6 +26,7 @@ Required properties:
>>>>>>    - ti,hwmods   : For TI hwmods processing and omap device creation
>>>>>>      the value shall be "emif<n>" where <n> is the number of the EMIF
>>>>>>      instance with base 1.
>>>>>> +- interrupts   : interrupt used by the controller
>>>>>
>>>>>
>>>>> Only for the new compatibles?
>>>>
>>>>
>>>> I added this as required property for all, as all EMIF versions 
>>>> actually do
>>>> have IRQ. Should this still be marked as optional as only certain 
>>>> versions
>>>> of the driver use it? On am3/am4 only it is optional right now, and 
>>>> not used
>>>> by the existing driver. On omap4 and omap5 it is required also (list of
>>>> compatibles for the binding seem to be missing these actually; 
>>>> ti,emif-4d
>>>> and ti,emif-4d5.)
>>>>
>>>> Either way, I can mark this as optional property for am3/am4 if you 
>>>> want (I
>>>> actually asked this already before but did not get clear response), 
>>>> what is
>>>> your final take on this?
>>>
>>> Okay. I'm fine if it is required. You're going to update all the dts
>>> files, right?
>>
>> Yeah, I will post patches for those, thanks for review.
>>
> Let me know if you have already posted the follow up series ?

Haven't posted yet, last week has been a complete mess for me. Will do 
either today or later this week.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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
Tero Kristo March 8, 2018, 9:32 a.m. | #7
On 26/02/18 01:40, Santosh Shilimkar wrote:
> 
> On 2/21/2018 11:00 AM, Tero Kristo wrote:
>> On 20/02/18 17:52, Rob Herring wrote:
>>> On Mon, Feb 19, 2018 at 12:21 AM, Tero Kristo <t-kristo@ti.com> wrote:
>>>> On 19/02/18 05:10, Rob Herring wrote:
>>>>>
>>>>> On Tue, Feb 13, 2018 at 07:15:29PM +0200, Tero Kristo wrote:
>>>>>>
>>>>>> Certain revisions of the TI EMIF IP contain ECC support in them. 
>>>>>> Reflect
>>>>>> this in the DT binding. Also, add interrupts property as a required
>>>>>> property for the emif controller, as all revisions of the emif IP 
>>>>>> contain
>>>>>> interrupt support; this might remain unused by the kernel driver 
>>>>>> though.
>>>>>>
>>>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>>>> ---
>>>>>>    .../devicetree/bindings/memory-controllers/ti/emif.txt      | 13
>>>>>> ++++++++++++-
>>>>>>    1 file changed, 12 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>>> b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>>> index 621b41c..87022a9 100644
>>>>>> --- 
>>>>>> a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>>> +++ 
>>>>>> b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
>>>>>> @@ -3,7 +3,9 @@
>>>>>>    EMIF - External Memory Interface - is an SDRAM controller used in
>>>>>>    TI SoCs. EMIF supports, based on the IP revision, one or more of
>>>>>>    DDR2/DDR3/LPDDR2 protocols. This binding describes a given 
>>>>>> instance
>>>>>> -of the EMIF IP and memory parts attached to it.
>>>>>> +of the EMIF IP and memory parts attached to it. Certain revisions
>>>>>> +of the EMIF controller also contain optional ECC support, which
>>>>>> +corrects one bit errors and detects two bit errors.
>>>>>>      Required properties:
>>>>>>    - compatible  : Should be of the form "ti,emif-<ip-rev>" where 
>>>>>> <ip-rev>
>>>>>> @@ -11,6 +13,8 @@ Required properties:
>>>>>>      compatible should be one of the following:
>>>>>>               "ti,emif-am3352"
>>>>>>               "ti,emif-am4372"
>>>>>> +            "ti,emif-dra7xx"
>>>>>> +            "ti,emif-keystone"
>>>>>>      - phy-type  : <u32> indicating the DDR phy type. Following 
>>>>>> are the
>>>>>>      allowed values
>>>>>> @@ -22,6 +26,7 @@ Required properties:
>>>>>>    - ti,hwmods   : For TI hwmods processing and omap device creation
>>>>>>      the value shall be "emif<n>" where <n> is the number of the EMIF
>>>>>>      instance with base 1.
>>>>>> +- interrupts   : interrupt used by the controller
>>>>>
>>>>>
>>>>> Only for the new compatibles?
>>>>
>>>>
>>>> I added this as required property for all, as all EMIF versions 
>>>> actually do
>>>> have IRQ. Should this still be marked as optional as only certain 
>>>> versions
>>>> of the driver use it? On am3/am4 only it is optional right now, and 
>>>> not used
>>>> by the existing driver. On omap4 and omap5 it is required also (list of
>>>> compatibles for the binding seem to be missing these actually; 
>>>> ti,emif-4d
>>>> and ti,emif-4d5.)
>>>>
>>>> Either way, I can mark this as optional property for am3/am4 if you 
>>>> want (I
>>>> actually asked this already before but did not get clear response), 
>>>> what is
>>>> your final take on this?
>>>
>>> Okay. I'm fine if it is required. You're going to update all the dts
>>> files, right?
>>
>> Yeah, I will post patches for those, thanks for review.
>>
> Let me know if you have already posted the follow up series ?

Anybody planning to pick this patch for merge? Tony/Santosh?

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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
Santosh Shilimkar March 8, 2018, 5:48 p.m. | #8
On 3/8/2018 1:32 AM, Tero Kristo wrote:
> On 26/02/18 01:40, Santosh Shilimkar wrote:
>>

[...]

>>> Yeah, I will post patches for those, thanks for review.
>>>
>> Let me know if you have already posted the follow up series ?
> 
> Anybody planning to pick this patch for merge? Tony/Santosh?
> 
I already did and its part of the pull request [1] I sent
out on 3/5

Regards,
Santosh

[1] https://www.spinics.net/lists/arm-kernel/msg638794.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
Tero Kristo March 9, 2018, 9:15 a.m. | #9
On 08/03/18 19:48, Santosh Shilimkar wrote:
> On 3/8/2018 1:32 AM, Tero Kristo wrote:
>> On 26/02/18 01:40, Santosh Shilimkar wrote:
>>>
> 
> [...]
> 
>>>> Yeah, I will post patches for those, thanks for review.
>>>>
>>> Let me know if you have already posted the follow up series ?
>>
>> Anybody planning to pick this patch for merge? Tony/Santosh?
>>
> I already did and its part of the pull request [1] I sent
> out on 3/5

Oh ok thanks Santosh, just didn't get notified of that.

How about the corresponding DTS patch for keystone?

https://patchwork.kernel.org/patch/10219339/

-Tero

> 
> Regards,
> Santosh
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg638794.html

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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
Santosh Shilimkar March 9, 2018, 7:37 p.m. | #10
On 3/9/2018 1:15 AM, Tero Kristo wrote:
> On 08/03/18 19:48, Santosh Shilimkar wrote:
>> On 3/8/2018 1:32 AM, Tero Kristo wrote:
>>> On 26/02/18 01:40, Santosh Shilimkar wrote:
>>>>
>>
>> [...]
>>
>>>>> Yeah, I will post patches for those, thanks for review.
>>>>>
>>>> Let me know if you have already posted the follow up series ?
>>>
>>> Anybody planning to pick this patch for merge? Tony/Santosh?
>>>
>> I already did and its part of the pull request [1] I sent
>> out on 3/5
> 
> Oh ok thanks Santosh, just didn't get notified of that.
> 
> How about the corresponding DTS patch for keystone?
> 
> https://patchwork.kernel.org/patch/10219339/
> 
Looks like I missed that one. Would have been nice to have both patches
posted in one series. Could you please repost the patch with my ack and
will request Arnd to apply it dierctly to arm-soc dts branch.

Regards,
Santosh
--
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
Tero Kristo March 12, 2018, 7:13 a.m. | #11
On 09/03/18 21:37, Santosh Shilimkar wrote:
> On 3/9/2018 1:15 AM, Tero Kristo wrote:
>> On 08/03/18 19:48, Santosh Shilimkar wrote:
>>> On 3/8/2018 1:32 AM, Tero Kristo wrote:
>>>> On 26/02/18 01:40, Santosh Shilimkar wrote:
>>>>>
>>>
>>> [...]
>>>
>>>>>> Yeah, I will post patches for those, thanks for review.
>>>>>>
>>>>> Let me know if you have already posted the follow up series ?
>>>>
>>>> Anybody planning to pick this patch for merge? Tony/Santosh?
>>>>
>>> I already did and its part of the pull request [1] I sent
>>> out on 3/5
>>
>> Oh ok thanks Santosh, just didn't get notified of that.
>>
>> How about the corresponding DTS patch for keystone?
>>
>> https://patchwork.kernel.org/patch/10219339/
>>
> Looks like I missed that one. Would have been nice to have both patches
> posted in one series. Could you please repost the patch with my ack and
> will request Arnd to apply it dierctly to arm-soc dts branch.

Sent it. The patches were split around this time due to some 
interdependencies between different subsystems; the dt-binding is needed 
for OMAP family parts also. However, the J6+ support for ddr-ecc is 
pushed to next merge window so its fine the binding goes in via your tree.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
index 621b41c..87022a9 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/ti/emif.txt
@@ -3,7 +3,9 @@ 
 EMIF - External Memory Interface - is an SDRAM controller used in
 TI SoCs. EMIF supports, based on the IP revision, one or more of
 DDR2/DDR3/LPDDR2 protocols. This binding describes a given instance
-of the EMIF IP and memory parts attached to it.
+of the EMIF IP and memory parts attached to it. Certain revisions
+of the EMIF controller also contain optional ECC support, which
+corrects one bit errors and detects two bit errors.
 
 Required properties:
 - compatible	: Should be of the form "ti,emif-<ip-rev>" where <ip-rev>
@@ -11,6 +13,8 @@  Required properties:
   compatible should be one of the following:
   	     "ti,emif-am3352"
 	     "ti,emif-am4372"
+	     "ti,emif-dra7xx"
+	     "ti,emif-keystone"
 
 - phy-type	: <u32> indicating the DDR phy type. Following are the
   allowed values
@@ -22,6 +26,7 @@  Required properties:
 - ti,hwmods	: For TI hwmods processing and omap device creation
   the value shall be "emif<n>" where <n> is the number of the EMIF
   instance with base 1.
+- interrupts	: interrupt used by the controller
 
 Required only for "ti,emif-am3352" and "ti,emif-am4372":
 - sram			: Phandles for generic sram driver nodes,
@@ -71,3 +76,9 @@  emif: emif@4c000000 {
         sram = <&pm_sram_code
                 &pm_sram_data>;
 };
+
+emif1: emif@4c000000 {
+	compatible = "ti,emif-dra7xx";
+	reg = <0x4c000000 0x200>;
+	interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+};