diff mbox series

[1/4] dt-bindings: memory-controllers: ti,gpmc: Add compatible for AM64

Message ID 20211123102607.13002-2-rogerq@kernel.org (mailing list archive)
State New, archived
Headers show
Series memory: omap-gpmc: Add AM64 SoC support | expand

Commit Message

Roger Quadros Nov. 23, 2021, 10:26 a.m. UTC
AM64 SoC contains the GPMC module. Add compatible for it.

Newer SoCs don't necessarily map GPMC data region at the same place
as legacy SoCs. Add reg-names "data", to provide this information to
the device driver.

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
 .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Krzysztof Kozlowski Nov. 23, 2021, 7:47 p.m. UTC | #1
On 23/11/2021 11:26, Roger Quadros wrote:
> AM64 SoC contains the GPMC module. Add compatible for it.
> 
> Newer SoCs don't necessarily map GPMC data region at the same place
> as legacy SoCs. Add reg-names "data", to provide this information to
> the device driver.
> 
> Cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Roger Quadros <rogerq@kernel.org>
> ---
>  .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
> index 25b42d68f9b3..1869cc6f949b 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
> @@ -23,13 +23,20 @@ properties:
>      items:
>        - enum:
>            - ti,am3352-gpmc
> +          - ti,am64-gpmc
>            - ti,omap2420-gpmc
>            - ti,omap2430-gpmc
>            - ti,omap3430-gpmc
>            - ti,omap4430-gpmc
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: cfg
> +      - const: data

I see your driver handles cases with only one reg item, but I have other
question - is it correct to have older (ARMv7) platform with two reg
items? Or can am64-gpmc come with only one reg?
IOW, I am surprised there is no if-else case precising this minItems
requirement for different SocS.

>  
>    interrupts:
>      maxItems: 1
> @@ -44,6 +51,9 @@ properties:
>      items:
>        - const: fck
>  
> +  power-domains:
> +    maxItems: 1

Similar, but looks like a weaker requirement - could an older SoC define
power-domain?

> +
>    dmas:
>      items:
>        - description: DMA channel for GPMC NAND prefetch
> 


Best regards,
Krzysztof
Roger Quadros Nov. 25, 2021, 12:18 p.m. UTC | #2
On 23/11/2021 21:47, Krzysztof Kozlowski wrote:
> On 23/11/2021 11:26, Roger Quadros wrote:
>> AM64 SoC contains the GPMC module. Add compatible for it.
>>
>> Newer SoCs don't necessarily map GPMC data region at the same place
>> as legacy SoCs. Add reg-names "data", to provide this information to
>> the device driver.
>>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>> ---
>>  .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
>> index 25b42d68f9b3..1869cc6f949b 100644
>> --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
>> @@ -23,13 +23,20 @@ properties:
>>      items:
>>        - enum:
>>            - ti,am3352-gpmc
>> +          - ti,am64-gpmc
>>            - ti,omap2420-gpmc
>>            - ti,omap2430-gpmc
>>            - ti,omap3430-gpmc
>>            - ti,omap4430-gpmc
>>  
>>    reg:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  reg-names:
>> +    items:
>> +      - const: cfg
>> +      - const: data
> 
> I see your driver handles cases with only one reg item, but I have other

The support for these two items is added in patch 3 of this series "memory: omap-gpmc: Add support for GPMC on AM64 SoC"

> question - is it correct to have older (ARMv7) platform with two reg
> items? Or can am64-gpmc come with only one reg?

Older platforms currently have only one reg, but they can be updated to come with two without breaking functionality.
am64-gpmc cannot come with one reg as the defaults for data window are not suitable for AM64.

All legacy platforms were using a fixed Data IO window (first 1 GB) but from AM64 this was moved elsewhere, so the need for this change.

> IOW, I am surprised there is no if-else case precising this minItems
> requirement for different SocS.
> 

OK. I will add this.

>>  
>>    interrupts:
>>      maxItems: 1
>> @@ -44,6 +51,9 @@ properties:
>>      items:
>>        - const: fck
>>  
>> +  power-domains:
>> +    maxItems: 1
> 
> Similar, but looks like a weaker requirement - could an older SoC define
> power-domain?

No. Will add SoC specific constraint for this as well.

> 
>> +
>>    dmas:
>>      items:
>>        - description: DMA channel for GPMC NAND prefetch
>>
> 
> 
> Best regards,
> Krzysztof
> 

cheers,
-roger
Rob Herring (Arm) Nov. 30, 2021, 10:02 p.m. UTC | #3
On Tue, Nov 23, 2021 at 08:47:57PM +0100, Krzysztof Kozlowski wrote:
> On 23/11/2021 11:26, Roger Quadros wrote:
> > AM64 SoC contains the GPMC module. Add compatible for it.
> > 
> > Newer SoCs don't necessarily map GPMC data region at the same place
> > as legacy SoCs. Add reg-names "data", to provide this information to
> > the device driver.
> > 
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Signed-off-by: Roger Quadros <rogerq@kernel.org>
> > ---
> >  .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
> > index 25b42d68f9b3..1869cc6f949b 100644
> > --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
> > +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
> > @@ -23,13 +23,20 @@ properties:
> >      items:
> >        - enum:
> >            - ti,am3352-gpmc
> > +          - ti,am64-gpmc
> >            - ti,omap2420-gpmc
> >            - ti,omap2430-gpmc
> >            - ti,omap3430-gpmc
> >            - ti,omap4430-gpmc
> >  
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  reg-names:
> > +    items:
> > +      - const: cfg
> > +      - const: data
> 
> I see your driver handles cases with only one reg item, but I have other
> question - is it correct to have older (ARMv7) platform with two reg
> items? Or can am64-gpmc come with only one reg?
> IOW, I am surprised there is no if-else case precising this minItems
> requirement for different SocS.

I don't think that is needed here. If the assumption is 'reg-names' is 
only present when there are 2 entries, then it is fine. Maybe 
'reg-names' should be required for ti,am64-gpmc though.

Rob
Roger Quadros Dec. 1, 2021, 11:14 a.m. UTC | #4
On 01/12/2021 00:02, Rob Herring wrote:
> On Tue, Nov 23, 2021 at 08:47:57PM +0100, Krzysztof Kozlowski wrote:
>> On 23/11/2021 11:26, Roger Quadros wrote:
>>> AM64 SoC contains the GPMC module. Add compatible for it.
>>>
>>> Newer SoCs don't necessarily map GPMC data region at the same place
>>> as legacy SoCs. Add reg-names "data", to provide this information to
>>> the device driver.
>>>
>>> Cc: Rob Herring <robh+dt@kernel.org>
>>> Signed-off-by: Roger Quadros <rogerq@kernel.org>
>>> ---
>>>  .../bindings/memory-controllers/ti,gpmc.yaml         | 12 +++++++++++-
>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
>>> index 25b42d68f9b3..1869cc6f949b 100644
>>> --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
>>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
>>> @@ -23,13 +23,20 @@ properties:
>>>      items:
>>>        - enum:
>>>            - ti,am3352-gpmc
>>> +          - ti,am64-gpmc
>>>            - ti,omap2420-gpmc
>>>            - ti,omap2430-gpmc
>>>            - ti,omap3430-gpmc
>>>            - ti,omap4430-gpmc
>>>  
>>>    reg:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 2
>>> +
>>> +  reg-names:
>>> +    items:
>>> +      - const: cfg
>>> +      - const: data
>>
>> I see your driver handles cases with only one reg item, but I have other
>> question - is it correct to have older (ARMv7) platform with two reg
>> items? Or can am64-gpmc come with only one reg?
>> IOW, I am surprised there is no if-else case precising this minItems
>> requirement for different SocS.
> 
> I don't think that is needed here. If the assumption is 'reg-names' is 
> only present when there are 2 entries, then it is fine. Maybe 
> 'reg-names' should be required for ti,am64-gpmc though.

Yes, I'll make 'reg-names' property required for ti,am64-gpmc.

cheers,
-roger
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
index 25b42d68f9b3..1869cc6f949b 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml
@@ -23,13 +23,20 @@  properties:
     items:
       - enum:
           - ti,am3352-gpmc
+          - ti,am64-gpmc
           - ti,omap2420-gpmc
           - ti,omap2430-gpmc
           - ti,omap3430-gpmc
           - ti,omap4430-gpmc
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: cfg
+      - const: data
 
   interrupts:
     maxItems: 1
@@ -44,6 +51,9 @@  properties:
     items:
       - const: fck
 
+  power-domains:
+    maxItems: 1
+
   dmas:
     items:
       - description: DMA channel for GPMC NAND prefetch