diff mbox series

[v3,2/7] dt-bindings: PCI: ti,am65: Extend for use with PVU

Message ID 752fb193661bb5e60e5aae6f87704784cbad145d.1724868080.git.jan.kiszka@siemens.com (mailing list archive)
State Superseded
Headers show
Series soc: ti: Add and use PVU on K3-AM65 for DMA isolation | expand

Commit Message

Jan Kiszka Aug. 28, 2024, 6:01 p.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

The PVU on the AM65 SoC is capable of restricting DMA from PCIe devices
to specific regions of host memory. Add the optional property
"memory-regions" to point to such regions of memory when PVU is used.

Since the PVU deals with system physical addresses, utilizing the PVU
with PCIe devices also requires setting up the VMAP registers to map the
Requester ID of the PCIe device to the CBA Virtual ID, which in turn is
mapped to the system physical address. Hence, describe the VMAP
registers which are optionally unless the PVU shall used for PCIe.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
CC: Lorenzo Pieralisi <lpieralisi@kernel.org>
CC: "Krzysztof Wilczyński" <kw@linux.com>
CC: Bjorn Helgaas <bhelgaas@google.com>
CC: linux-pci@vger.kernel.org
---
 .../bindings/pci/ti,am65-pci-host.yaml        | 52 ++++++++++++++-----
 1 file changed, 40 insertions(+), 12 deletions(-)

Comments

Krzysztof Kozlowski Aug. 29, 2024, 6:14 a.m. UTC | #1
On Wed, Aug 28, 2024 at 08:01:15PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> The PVU on the AM65 SoC is capable of restricting DMA from PCIe devices
> to specific regions of host memory. Add the optional property
> "memory-regions" to point to such regions of memory when PVU is used.
> 
> Since the PVU deals with system physical addresses, utilizing the PVU
> with PCIe devices also requires setting up the VMAP registers to map the
> Requester ID of the PCIe device to the CBA Virtual ID, which in turn is
> mapped to the system physical address. Hence, describe the VMAP
> registers which are optionally unless the PVU shall used for PCIe.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> CC: Lorenzo Pieralisi <lpieralisi@kernel.org>
> CC: "Krzysztof Wilczyński" <kw@linux.com>
> CC: Bjorn Helgaas <bhelgaas@google.com>
> CC: linux-pci@vger.kernel.org
> ---
>  .../bindings/pci/ti,am65-pci-host.yaml        | 52 ++++++++++++++-----
>  1 file changed, 40 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
> index 0a9d10532cc8..d8182bad92de 100644
> --- a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
> +++ b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
> @@ -19,16 +19,6 @@ properties:
>        - ti,am654-pcie-rc
>        - ti,keystone-pcie
>  
> -  reg:
> -    maxItems: 4
> -
> -  reg-names:
> -    items:
> -      - const: app
> -      - const: dbics
> -      - const: config
> -      - const: atu
> -

Properties must be defined in top-level.

https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml

>    interrupts:
>      maxItems: 1
>  
> @@ -84,12 +74,48 @@ if:
>        enum:
>          - ti,am654-pcie-rc
>  then:
> +  properties:
> +    reg:
> +      minItems: 4
> +      maxItems: 6
> +
> +    reg-names:
> +      minItems: 4
> +      items:
> +        - const: app
> +        - const: dbics
> +        - const: config
> +        - const: atu
> +        - const: vmap_lp
> +        - const: vmap_hp

This as well goes to the top.

> +
> +    memory-region:
> +      minItems: 1

Missing maxItems and this must be defined in top-level.

Best regards,
Krzysztof
Jan Kiszka Aug. 29, 2024, 7:38 a.m. UTC | #2
On 29.08.24 08:14, Krzysztof Kozlowski wrote:
> On Wed, Aug 28, 2024 at 08:01:15PM +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> The PVU on the AM65 SoC is capable of restricting DMA from PCIe devices
>> to specific regions of host memory. Add the optional property
>> "memory-regions" to point to such regions of memory when PVU is used.
>>
>> Since the PVU deals with system physical addresses, utilizing the PVU
>> with PCIe devices also requires setting up the VMAP registers to map the
>> Requester ID of the PCIe device to the CBA Virtual ID, which in turn is
>> mapped to the system physical address. Hence, describe the VMAP
>> registers which are optionally unless the PVU shall used for PCIe.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> CC: Lorenzo Pieralisi <lpieralisi@kernel.org>
>> CC: "Krzysztof Wilczyński" <kw@linux.com>
>> CC: Bjorn Helgaas <bhelgaas@google.com>
>> CC: linux-pci@vger.kernel.org
>> ---
>>  .../bindings/pci/ti,am65-pci-host.yaml        | 52 ++++++++++++++-----
>>  1 file changed, 40 insertions(+), 12 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>> index 0a9d10532cc8..d8182bad92de 100644
>> --- a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>> +++ b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>> @@ -19,16 +19,6 @@ properties:
>>        - ti,am654-pcie-rc
>>        - ti,keystone-pcie
>>  
>> -  reg:
>> -    maxItems: 4
>> -
>> -  reg-names:
>> -    items:
>> -      - const: app
>> -      - const: dbics
>> -      - const: config
>> -      - const: atu
>> -
> 
> Properties must be defined in top-level.
> 
> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
> 

Tried that already, moving the else: part to top-level, but dtschema
(2024.5) checks fail then. Could you explain why?

>>    interrupts:
>>      maxItems: 1
>>  
>> @@ -84,12 +74,48 @@ if:
>>        enum:
>>          - ti,am654-pcie-rc
>>  then:
>> +  properties:
>> +    reg:
>> +      minItems: 4
>> +      maxItems: 6
>> +
>> +    reg-names:
>> +      minItems: 4
>> +      items:
>> +        - const: app
>> +        - const: dbics
>> +        - const: config
>> +        - const: atu
>> +        - const: vmap_lp
>> +        - const: vmap_hp
> 
> This as well goes to the top.
> 
>> +
>> +    memory-region:
>> +      minItems: 1
> 
> Missing maxItems and this must be defined in top-level.
> 

As explained (and documented), there is no maximum. And this does not
apply to ti,keystone-pcie because only the AM65 supports the PVU.

Jan
Krzysztof Kozlowski Sept. 5, 2024, 6:54 a.m. UTC | #3
On 29/08/2024 09:38, Jan Kiszka wrote:
> On 29.08.24 08:14, Krzysztof Kozlowski wrote:
>> On Wed, Aug 28, 2024 at 08:01:15PM +0200, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> The PVU on the AM65 SoC is capable of restricting DMA from PCIe devices
>>> to specific regions of host memory. Add the optional property
>>> "memory-regions" to point to such regions of memory when PVU is used.
>>>
>>> Since the PVU deals with system physical addresses, utilizing the PVU
>>> with PCIe devices also requires setting up the VMAP registers to map the
>>> Requester ID of the PCIe device to the CBA Virtual ID, which in turn is
>>> mapped to the system physical address. Hence, describe the VMAP
>>> registers which are optionally unless the PVU shall used for PCIe.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>> CC: Lorenzo Pieralisi <lpieralisi@kernel.org>
>>> CC: "Krzysztof Wilczyński" <kw@linux.com>
>>> CC: Bjorn Helgaas <bhelgaas@google.com>
>>> CC: linux-pci@vger.kernel.org
>>> ---
>>>  .../bindings/pci/ti,am65-pci-host.yaml        | 52 ++++++++++++++-----
>>>  1 file changed, 40 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>>> index 0a9d10532cc8..d8182bad92de 100644
>>> --- a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>>> @@ -19,16 +19,6 @@ properties:
>>>        - ti,am654-pcie-rc
>>>        - ti,keystone-pcie
>>>  
>>> -  reg:
>>> -    maxItems: 4
>>> -
>>> -  reg-names:
>>> -    items:
>>> -      - const: app
>>> -      - const: dbics
>>> -      - const: config
>>> -      - const: atu
>>> -
>>
>> Properties must be defined in top-level.
>>
>> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml
>>
> 
> Tried that already, moving the else: part to top-level, but dtschema
> (2024.5) checks fail then. Could you explain why?

The example does not "move the else to top-level", so why would you do
it? I gave you the code to copy&paste. I don't know how to write it simpler.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
index 0a9d10532cc8..d8182bad92de 100644
--- a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
+++ b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
@@ -19,16 +19,6 @@  properties:
       - ti,am654-pcie-rc
       - ti,keystone-pcie
 
-  reg:
-    maxItems: 4
-
-  reg-names:
-    items:
-      - const: app
-      - const: dbics
-      - const: config
-      - const: atu
-
   interrupts:
     maxItems: 1
 
@@ -84,12 +74,48 @@  if:
       enum:
         - ti,am654-pcie-rc
 then:
+  properties:
+    reg:
+      minItems: 4
+      maxItems: 6
+
+    reg-names:
+      minItems: 4
+      items:
+        - const: app
+        - const: dbics
+        - const: config
+        - const: atu
+        - const: vmap_lp
+        - const: vmap_hp
+
+    memory-region:
+      minItems: 1
+      description: |
+        phandle to one or more restricted DMA pools to be used for all devices
+        behind this controller. The regions should be defined according to
+        reserved-memory/shared-dma-pool.yaml.
+      items:
+        maxItems: 1
+
   required:
     - dma-coherent
     - power-domains
     - msi-map
     - num-viewport
 
+else:
+  properties:
+    reg:
+      maxItems: 4
+
+    reg-names:
+      items:
+        - const: app
+        - const: dbics
+        - const: config
+        - const: atu
+
 unevaluatedProperties: false
 
 examples:
@@ -104,8 +130,10 @@  examples:
         reg =  <0x5500000 0x1000>,
                <0x5501000 0x1000>,
                <0x10000000 0x2000>,
-               <0x5506000 0x1000>;
-        reg-names = "app", "dbics", "config", "atu";
+               <0x5506000 0x1000>,
+               <0x2900000 0x1000>,
+               <0x2908000 0x1000>;
+        reg-names = "app", "dbics", "config", "atu", "vmap_lp", "vmap_hp";
         power-domains = <&k3_pds 120 TI_SCI_PD_EXCLUSIVE>;
         #address-cells = <3>;
         #size-cells = <2>;