diff mbox series

[V1,1/2] dt-bindings: PCI: tegra234: Add ECAM support

Message ID 20221027051214.22003-2-vidyas@nvidia.com
State New
Headers
Series Add ECAM aperture for Tegra234 |

Commit Message

Vidya Sagar Oct. 27, 2022, 5:12 a.m. UTC 
  Add support for ECAM aperture for Tegra234.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
 .../devicetree/bindings/pci/nvidia,tegra194-pcie.yaml  | 10 ++++++++--
 .../devicetree/bindings/pci/snps,dw-pcie.yaml          |  2 +-
 2 files changed, 9 insertions(+), 3 deletions(-)

Comments

Krzysztof Kozlowski Oct. 27, 2022, 9:18 p.m. UTC | #1 
On 27/10/2022 01:12, Vidya Sagar wrote:
> Add support for ECAM aperture for Tegra234.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
>  .../devicetree/bindings/pci/nvidia,tegra194-pcie.yaml  | 10 ++++++++--
>  .../devicetree/bindings/pci/snps,dw-pcie.yaml          |  2 +-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> index 75da3e8eecb9..15cc2d2055bb 100644
> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> @@ -27,6 +27,7 @@ properties:
>        - nvidia,tegra234-pcie
>  
>    reg:
> +    minItems: 4
>      items:
>        - description: controller's application logic registers
>        - description: configuration registers
> @@ -35,13 +36,17 @@ properties:
>            available for software access.
>        - description: aperture where the Root Port's own configuration
>            registers are available.
> +      - description: aperture to access the configuration space through ECAM.
> +          This is applicable only for Tegra234.

Then restrict it per compatible in allOf

>  

Best regards,
Krzysztof
Vidya Sagar Oct. 28, 2022, 12:09 p.m. UTC | #2 
On 10/28/2022 2:48 AM, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 27/10/2022 01:12, Vidya Sagar wrote:
>> Add support for ECAM aperture for Tegra234.
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>>   .../devicetree/bindings/pci/nvidia,tegra194-pcie.yaml  | 10 ++++++++--
>>   .../devicetree/bindings/pci/snps,dw-pcie.yaml          |  2 +-
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> index 75da3e8eecb9..15cc2d2055bb 100644
>> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> @@ -27,6 +27,7 @@ properties:
>>         - nvidia,tegra234-pcie
>>
>>     reg:
>> +    minItems: 4
>>       items:
>>         - description: controller's application logic registers
>>         - description: configuration registers
>> @@ -35,13 +36,17 @@ properties:
>>             available for software access.
>>         - description: aperture where the Root Port's own configuration
>>             registers are available.
>> +      - description: aperture to access the configuration space through ECAM.
>> +          This is applicable only for Tegra234.
> 
> Then restrict it per compatible in allOf
> 

Thanks Krzysztof for your review.
For a similar change here 
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20220707081301.29961-3-vidyas@nvidia.com/ 
Rob had said that may be it is not worth it.
Let me know if you really want this change and I'll push a new patch.

Thanks,
Vidya Sagar

>>
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Oct. 28, 2022, 12:47 p.m. UTC | #3 
On 28/10/2022 08:09, Vidya Sagar wrote:
> 
> 
> On 10/28/2022 2:48 AM, Krzysztof Kozlowski wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 27/10/2022 01:12, Vidya Sagar wrote:
>>> Add support for ECAM aperture for Tegra234.
>>>
>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>> ---
>>>   .../devicetree/bindings/pci/nvidia,tegra194-pcie.yaml  | 10 ++++++++--
>>>   .../devicetree/bindings/pci/snps,dw-pcie.yaml          |  2 +-
>>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>>> index 75da3e8eecb9..15cc2d2055bb 100644
>>> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>>> @@ -27,6 +27,7 @@ properties:
>>>         - nvidia,tegra234-pcie
>>>
>>>     reg:
>>> +    minItems: 4
>>>       items:
>>>         - description: controller's application logic registers
>>>         - description: configuration registers
>>> @@ -35,13 +36,17 @@ properties:
>>>             available for software access.
>>>         - description: aperture where the Root Port's own configuration
>>>             registers are available.
>>> +      - description: aperture to access the configuration space through ECAM.
>>> +          This is applicable only for Tegra234.
>>
>> Then restrict it per compatible in allOf
>>
> 
> Thanks Krzysztof for your review.
> For a similar change here 
> https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20220707081301.29961-3-vidyas@nvidia.com/ 
> Rob had said that may be it is not worth it.
> Let me know if you really want this change and I'll push a new patch.
> 

That's a bit different. Restricting it per variant works and should be
trivial.

Best regards,
Krzysztof
Rob Herring Oct. 31, 2022, 4:33 p.m. UTC | #4 
On Thu, Oct 27, 2022 at 10:42:13AM +0530, Vidya Sagar wrote:
> Add support for ECAM aperture for Tegra234.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
>  .../devicetree/bindings/pci/nvidia,tegra194-pcie.yaml  | 10 ++++++++--
>  .../devicetree/bindings/pci/snps,dw-pcie.yaml          |  2 +-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> index 75da3e8eecb9..15cc2d2055bb 100644
> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
> @@ -27,6 +27,7 @@ properties:
>        - nvidia,tegra234-pcie
>  
>    reg:
> +    minItems: 4
>      items:
>        - description: controller's application logic registers
>        - description: configuration registers
> @@ -35,13 +36,17 @@ properties:
>            available for software access.
>        - description: aperture where the Root Port's own configuration
>            registers are available.
> +      - description: aperture to access the configuration space through ECAM.
> +          This is applicable only for Tegra234.

Is it really only Tegra234 or that's all you've tested or care 
about? For the former, I agree with Krzysztof.

Rob
Vidya Sagar Oct. 31, 2022, 4:42 p.m. UTC | #5 
On 10/31/2022 10:03 PM, Rob Herring wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Oct 27, 2022 at 10:42:13AM +0530, Vidya Sagar wrote:
>> Add support for ECAM aperture for Tegra234.
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>>   .../devicetree/bindings/pci/nvidia,tegra194-pcie.yaml  | 10 ++++++++--
>>   .../devicetree/bindings/pci/snps,dw-pcie.yaml          |  2 +-
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> index 75da3e8eecb9..15cc2d2055bb 100644
>> --- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
>> @@ -27,6 +27,7 @@ properties:
>>         - nvidia,tegra234-pcie
>>
>>     reg:
>> +    minItems: 4
>>       items:
>>         - description: controller's application logic registers
>>         - description: configuration registers
>> @@ -35,13 +36,17 @@ properties:
>>             available for software access.
>>         - description: aperture where the Root Port's own configuration
>>             registers are available.
>> +      - description: aperture to access the configuration space through ECAM.
>> +          This is applicable only for Tegra234.
> 
> Is it really only Tegra234 or that's all you've tested or care
> about? For the former, I agree with Krzysztof.
It is applicable only for Tegra234 and not applicable for Tegra194 (I 
meant the support for ECAM itself is not there in Tegra194).
I'll address the review comments in the next version.

- Vidya Sagar

> 
> Rob
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
index 75da3e8eecb9..15cc2d2055bb 100644
--- a/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/nvidia,tegra194-pcie.yaml
@@ -27,6 +27,7 @@  properties:
       - nvidia,tegra234-pcie
 
   reg:
+    minItems: 4
     items:
       - description: controller's application logic registers
       - description: configuration registers
@@ -35,13 +36,17 @@  properties:
           available for software access.
       - description: aperture where the Root Port's own configuration
           registers are available.
+      - description: aperture to access the configuration space through ECAM.
+          This is applicable only for Tegra234.
 
   reg-names:
+    minItems: 4
     items:
       - const: appl
       - const: config
       - const: atu_dma
       - const: dbi
+      - const: ecam
 
   interrupts:
     items:
@@ -305,8 +310,9 @@  examples:
             reg = <0x00 0x14160000 0x0 0x00020000>, /* appl registers (128K)      */
                   <0x00 0x36000000 0x0 0x00040000>, /* configuration space (256K) */
                   <0x00 0x36040000 0x0 0x00040000>, /* iATU_DMA reg space (256K)  */
-                  <0x00 0x36080000 0x0 0x00040000>; /* DBI reg space (256K)       */
-            reg-names = "appl", "config", "atu_dma", "dbi";
+                  <0x00 0x36080000 0x0 0x00040000>, /* DBI reg space (256K)       */
+                  <0x24 0x30000000 0x0 0x10000000>; /* ECAM (256MB)               */
+            reg-names = "appl", "config", "atu_dma", "dbi", "ecam";
 
             #address-cells = <3>;
             #size-cells = <2>;
diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 7287d395e1b6..7e0b015f1414 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -35,7 +35,7 @@  properties:
     maxItems: 5
     items:
       enum: [ dbi, dbi2, config, atu, atu_dma, app, appl, elbi, mgmt, ctrl,
-              parf, cfg, link, ulreg, smu, mpu, apb, phy ]
+              parf, cfg, link, ulreg, smu, mpu, apb, phy, ecam ]
 
   num-lanes:
     description: |