linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] PCI: cadence: Deprecate inbound/outbound specific bindings
@ 2020-03-27 10:47 Kishon Vijay Abraham I
  2020-03-27 10:47 ` [PATCH 1/3] dt-bindings: " Kishon Vijay Abraham I
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-03-27 10:47 UTC (permalink / raw)
  To: Tom Joseph, Rob Herring, Lorenzo Pieralisi, Andrew Murray
  Cc: Bjorn Helgaas, Mark Rutland, Kishon Vijay Abraham I, linux-pci,
	devicetree, linux-kernel

This series is a result of comments given by Rob Herring @ [1].
Patch series changes the DT bindings and makes the corresponding driver
changes.

[1] -> http://lore.kernel.org/r/20200219202700.GA21908@bogus

Kishon Vijay Abraham I (3):
  dt-bindings: PCI: cadence: Deprecate inbound/outbound specific
    bindings
  PCI: cadence: Use "dma-ranges" instead of "cdns,no-bar-match-nbits"
    property
  PCI: Cadence: Remove using "cdns,max-outbound-regions" DT property

 .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
 .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
 .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
 .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
 .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
 .../controller/cadence/pcie-cadence-host.c    | 17 +++++++------
 drivers/pci/controller/cadence/pcie-cadence.h |  2 --
 7 files changed, 47 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml

-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] dt-bindings: PCI: cadence: Deprecate inbound/outbound specific bindings
  2020-03-27 10:47 [PATCH 0/3] PCI: cadence: Deprecate inbound/outbound specific bindings Kishon Vijay Abraham I
@ 2020-03-27 10:47 ` Kishon Vijay Abraham I
  2020-03-30 16:01   ` Rob Herring
  2020-04-10 16:38   ` Rob Herring
  2020-03-27 10:47 ` [PATCH 2/3] PCI: cadence: Use "dma-ranges" instead of "cdns,no-bar-match-nbits" property Kishon Vijay Abraham I
  2020-03-27 10:47 ` [PATCH 3/3] PCI: Cadence: Remove using "cdns,max-outbound-regions" DT property Kishon Vijay Abraham I
  2 siblings, 2 replies; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-03-27 10:47 UTC (permalink / raw)
  To: Tom Joseph, Rob Herring, Lorenzo Pieralisi, Andrew Murray
  Cc: Bjorn Helgaas, Mark Rutland, Kishon Vijay Abraham I, linux-pci,
	devicetree, linux-kernel

Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for
host mode as both these could be derived from "ranges" and "dma-ranges"
property. "cdns,max-outbound-regions" property would still be required
for EP mode.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
 .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
 .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
 .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
 .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
 5 files changed, 37 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml

diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
index 2996f8d4777c..50ce5d79d2c7 100644
--- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
+++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Tom Joseph <tjoseph@cadence.com>
 
 allOf:
-  - $ref: "cdns-pcie.yaml#"
+  - $ref: "cdns-pcie-ep.yaml#"
   - $ref: "pci-ep.yaml#"
 
 properties:
diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
index cabbe46ff578..84a8f095d031 100644
--- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
+++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
@@ -45,8 +45,6 @@ examples:
             #size-cells = <2>;
             bus-range = <0x0 0xff>;
             linux,pci-domain = <0>;
-            cdns,max-outbound-regions = <16>;
-            cdns,no-bar-match-nbits = <32>;
             vendor-id = <0x17cd>;
             device-id = <0x0200>;
 
@@ -57,6 +55,7 @@ examples:
 
             ranges = <0x02000000 0x0 0x42000000  0x0 0x42000000  0x0 0x1000000>,
                      <0x01000000 0x0 0x43000000  0x0 0x43000000  0x0 0x0010000>;
+            dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>;
 
             #interrupt-cells = <0x1>;
 
diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
new file mode 100644
index 000000000000..6150a7a7bdbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Cadence PCIe Device
+
+maintainers:
+  - Tom Joseph <tjoseph@cadence.com>
+
+allOf:
+  - $ref: "cdns-pcie.yaml#"
+
+properties:
+  cdns,max-outbound-regions:
+    description: maximum number of outbound regions
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 32
+    default: 32
+
+required:
+  - cdns,max-outbound-regions
diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml
index ab6e43b636ec..3d64f85aeb39 100644
--- a/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml
+++ b/Documentation/devicetree/bindings/pci/cdns-pcie-host.yaml
@@ -14,6 +14,15 @@ allOf:
   - $ref: "cdns-pcie.yaml#"
 
 properties:
+  cdns,max-outbound-regions:
+    description: maximum number of outbound regions
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 32
+    default: 32
+    deprecated: true
+
   cdns,no-bar-match-nbits:
     description:
       Set into the no BAR match register to configure the number of least
@@ -23,5 +32,6 @@ properties:
     minimum: 0
     maximum: 64
     default: 32
+    deprecated: true
 
   msi-parent: true
diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie.yaml
index 6887ccc339cc..02553d5e6c51 100644
--- a/Documentation/devicetree/bindings/pci/cdns-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/cdns-pcie.yaml
@@ -10,14 +10,6 @@ maintainers:
   - Tom Joseph <tjoseph@cadence.com>
 
 properties:
-  cdns,max-outbound-regions:
-    description: maximum number of outbound regions
-    allOf:
-      - $ref: /schemas/types.yaml#/definitions/uint32
-    minimum: 1
-    maximum: 32
-    default: 32
-
   phys:
     description:
       One per lane if more than one in the list. If only one PHY listed it must
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/3] PCI: cadence: Use "dma-ranges" instead of "cdns,no-bar-match-nbits" property
  2020-03-27 10:47 [PATCH 0/3] PCI: cadence: Deprecate inbound/outbound specific bindings Kishon Vijay Abraham I
  2020-03-27 10:47 ` [PATCH 1/3] dt-bindings: " Kishon Vijay Abraham I
@ 2020-03-27 10:47 ` Kishon Vijay Abraham I
  2020-03-27 14:52   ` Bjorn Helgaas
  2020-03-27 10:47 ` [PATCH 3/3] PCI: Cadence: Remove using "cdns,max-outbound-regions" DT property Kishon Vijay Abraham I
  2 siblings, 1 reply; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-03-27 10:47 UTC (permalink / raw)
  To: Tom Joseph, Rob Herring, Lorenzo Pieralisi, Andrew Murray
  Cc: Bjorn Helgaas, Mark Rutland, Kishon Vijay Abraham I, linux-pci,
	devicetree, linux-kernel

Cadence PCIe core dirver (host mode) uses "cdns,no-bar-match-nbits"
property to configure the number of bits passed through from PCIe
address to internal address in Inbound Address Translation register.

However standard PCI dt-binding already defines "dma-ranges" to
describe the address range accessible by PCIe controller. Parse
"dma-ranges" property to configure the number of bits passed
through from PCIe address to internal address in Inbound Address
Translation register.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/pci/controller/cadence/pcie-cadence-host.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 9b1c3966414b..60f912a657b9 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -206,8 +206,10 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 	struct device *dev = rc->pcie.dev;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct device_node *np = dev->of_node;
+	struct of_pci_range_parser parser;
 	struct pci_host_bridge *bridge;
 	struct list_head resources;
+	struct of_pci_range range;
 	struct cdns_pcie *pcie;
 	struct resource *res;
 	int ret;
@@ -222,8 +224,15 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 	rc->max_regions = 32;
 	of_property_read_u32(np, "cdns,max-outbound-regions", &rc->max_regions);
 
-	rc->no_bar_nbits = 32;
-	of_property_read_u32(np, "cdns,no-bar-match-nbits", &rc->no_bar_nbits);
+	if (!of_pci_dma_range_parser_init(&parser, np))
+		if (of_pci_range_parser_one(&parser, &range))
+			rc->no_bar_nbits = ilog2(range.size);
+
+	if (!rc->no_bar_nbits) {
+		rc->no_bar_nbits = 32;
+		of_property_read_u32(np, "cdns,no-bar-match-nbits",
+				     &rc->no_bar_nbits);
+	}
 
 	rc->vendor_id = 0xffff;
 	of_property_read_u16(np, "vendor-id", &rc->vendor_id);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/3] PCI: Cadence: Remove using "cdns,max-outbound-regions" DT property
  2020-03-27 10:47 [PATCH 0/3] PCI: cadence: Deprecate inbound/outbound specific bindings Kishon Vijay Abraham I
  2020-03-27 10:47 ` [PATCH 1/3] dt-bindings: " Kishon Vijay Abraham I
  2020-03-27 10:47 ` [PATCH 2/3] PCI: cadence: Use "dma-ranges" instead of "cdns,no-bar-match-nbits" property Kishon Vijay Abraham I
@ 2020-03-27 10:47 ` Kishon Vijay Abraham I
  2020-03-27 14:54   ` Bjorn Helgaas
  2 siblings, 1 reply; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-03-27 10:47 UTC (permalink / raw)
  To: Tom Joseph, Rob Herring, Lorenzo Pieralisi, Andrew Murray
  Cc: Bjorn Helgaas, Mark Rutland, Kishon Vijay Abraham I, linux-pci,
	devicetree, linux-kernel

"cdns,max-outbound-regions" device tree property provides the
maximum number of outbound regions supported by the Host PCIe
controller. However the outbound regions are configured based
on what is populated in the "ranges" DT property.
Avoid using two properties for configuring outbound regions and
use only "ranges" property instead.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/pci/controller/cadence/pcie-cadence-host.c | 6 ------
 drivers/pci/controller/cadence/pcie-cadence.h      | 2 --
 2 files changed, 8 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 60f912a657b9..8f72967f298f 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -140,9 +140,6 @@ static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
 	for_each_of_pci_range(&parser, &range) {
 		bool is_io;
 
-		if (r >= rc->max_regions)
-			break;
-
 		if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
 			is_io = false;
 		else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
@@ -221,9 +218,6 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 	pcie = &rc->pcie;
 	pcie->is_rc = true;
 
-	rc->max_regions = 32;
-	of_property_read_u32(np, "cdns,max-outbound-regions", &rc->max_regions);
-
 	if (!of_pci_dma_range_parser_init(&parser, np))
 		if (of_pci_range_parser_one(&parser, &range))
 			rc->no_bar_nbits = ilog2(range.size);
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index a2b28b912ca4..6bd89a21bb1c 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -251,7 +251,6 @@ struct cdns_pcie {
  * @bus_range: first/last buses behind the PCIe host controller
  * @cfg_base: IO mapped window to access the PCI configuration space of a
  *            single function at a time
- * @max_regions: maximum number of regions supported by the hardware
  * @no_bar_nbits: Number of bits to keep for inbound (PCIe -> CPU) address
  *                translation (nbits sets into the "no BAR match" register)
  * @vendor_id: PCI vendor ID
@@ -262,7 +261,6 @@ struct cdns_pcie_rc {
 	struct resource		*cfg_res;
 	struct resource		*bus_range;
 	void __iomem		*cfg_base;
-	u32			max_regions;
 	u32			no_bar_nbits;
 	u16			vendor_id;
 	u16			device_id;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] PCI: cadence: Use "dma-ranges" instead of "cdns,no-bar-match-nbits" property
  2020-03-27 10:47 ` [PATCH 2/3] PCI: cadence: Use "dma-ranges" instead of "cdns,no-bar-match-nbits" property Kishon Vijay Abraham I
@ 2020-03-27 14:52   ` Bjorn Helgaas
  0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2020-03-27 14:52 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Tom Joseph, Rob Herring, Lorenzo Pieralisi, Andrew Murray,
	Mark Rutland, linux-pci, devicetree, linux-kernel

On Fri, Mar 27, 2020 at 04:17:26PM +0530, Kishon Vijay Abraham I wrote:
> Cadence PCIe core dirver (host mode) uses "cdns,no-bar-match-nbits"
> property to configure the number of bits passed through from PCIe
> address to internal address in Inbound Address Translation register.

s/dirver/driver/

I love this series, thanks a lot for doing this!

> However standard PCI dt-binding already defines "dma-ranges" to
> describe the address range accessible by PCIe controller. Parse
> "dma-ranges" property to configure the number of bits passed
> through from PCIe address to internal address in Inbound Address
> Translation register.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/controller/cadence/pcie-cadence-host.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index 9b1c3966414b..60f912a657b9 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -206,8 +206,10 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>  	struct device *dev = rc->pcie.dev;
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct device_node *np = dev->of_node;
> +	struct of_pci_range_parser parser;
>  	struct pci_host_bridge *bridge;
>  	struct list_head resources;
> +	struct of_pci_range range;
>  	struct cdns_pcie *pcie;
>  	struct resource *res;
>  	int ret;
> @@ -222,8 +224,15 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>  	rc->max_regions = 32;
>  	of_property_read_u32(np, "cdns,max-outbound-regions", &rc->max_regions);
>  
> -	rc->no_bar_nbits = 32;
> -	of_property_read_u32(np, "cdns,no-bar-match-nbits", &rc->no_bar_nbits);
> +	if (!of_pci_dma_range_parser_init(&parser, np))
> +		if (of_pci_range_parser_one(&parser, &range))
> +			rc->no_bar_nbits = ilog2(range.size);
> +
> +	if (!rc->no_bar_nbits) {
> +		rc->no_bar_nbits = 32;
> +		of_property_read_u32(np, "cdns,no-bar-match-nbits",
> +				     &rc->no_bar_nbits);
> +	}
>  
>  	rc->vendor_id = 0xffff;
>  	of_property_read_u16(np, "vendor-id", &rc->vendor_id);
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] PCI: Cadence: Remove using "cdns,max-outbound-regions" DT property
  2020-03-27 10:47 ` [PATCH 3/3] PCI: Cadence: Remove using "cdns,max-outbound-regions" DT property Kishon Vijay Abraham I
@ 2020-03-27 14:54   ` Bjorn Helgaas
  2020-03-27 15:27     ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 14+ messages in thread
From: Bjorn Helgaas @ 2020-03-27 14:54 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Tom Joseph, Rob Herring, Lorenzo Pieralisi, Andrew Murray,
	Mark Rutland, linux-pci, devicetree, linux-kernel

Update subject to match capitalization of others:

  PCI: cadence: Remove "cdns,max-outbound-regions" DT property

On Fri, Mar 27, 2020 at 04:17:27PM +0530, Kishon Vijay Abraham I wrote:
> "cdns,max-outbound-regions" device tree property provides the
> maximum number of outbound regions supported by the Host PCIe
> controller. However the outbound regions are configured based
> on what is populated in the "ranges" DT property.

Looks like this is missing a blank line here?  Or it should be
rewrapped as part of the above paragraph?  I think the below makes
more sense as a separate paragraph, though.

Again, thanks for doing this; this is a great cleanup.

> Avoid using two properties for configuring outbound regions and
> use only "ranges" property instead.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  drivers/pci/controller/cadence/pcie-cadence-host.c | 6 ------
>  drivers/pci/controller/cadence/pcie-cadence.h      | 2 --
>  2 files changed, 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index 60f912a657b9..8f72967f298f 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -140,9 +140,6 @@ static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
>  	for_each_of_pci_range(&parser, &range) {
>  		bool is_io;
>  
> -		if (r >= rc->max_regions)
> -			break;
> -
>  		if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
>  			is_io = false;
>  		else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
> @@ -221,9 +218,6 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>  	pcie = &rc->pcie;
>  	pcie->is_rc = true;
>  
> -	rc->max_regions = 32;
> -	of_property_read_u32(np, "cdns,max-outbound-regions", &rc->max_regions);
> -
>  	if (!of_pci_dma_range_parser_init(&parser, np))
>  		if (of_pci_range_parser_one(&parser, &range))
>  			rc->no_bar_nbits = ilog2(range.size);
> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
> index a2b28b912ca4..6bd89a21bb1c 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence.h
> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
> @@ -251,7 +251,6 @@ struct cdns_pcie {
>   * @bus_range: first/last buses behind the PCIe host controller
>   * @cfg_base: IO mapped window to access the PCI configuration space of a
>   *            single function at a time
> - * @max_regions: maximum number of regions supported by the hardware
>   * @no_bar_nbits: Number of bits to keep for inbound (PCIe -> CPU) address
>   *                translation (nbits sets into the "no BAR match" register)
>   * @vendor_id: PCI vendor ID
> @@ -262,7 +261,6 @@ struct cdns_pcie_rc {
>  	struct resource		*cfg_res;
>  	struct resource		*bus_range;
>  	void __iomem		*cfg_base;
> -	u32			max_regions;
>  	u32			no_bar_nbits;
>  	u16			vendor_id;
>  	u16			device_id;
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] PCI: Cadence: Remove using "cdns,max-outbound-regions" DT property
  2020-03-27 14:54   ` Bjorn Helgaas
@ 2020-03-27 15:27     ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-03-27 15:27 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Tom Joseph, Rob Herring, Lorenzo Pieralisi, Andrew Murray,
	Mark Rutland, linux-pci, devicetree, linux-kernel

Hi Bjorn,

On 3/27/2020 8:24 PM, Bjorn Helgaas wrote:
> Update subject to match capitalization of others:
> 
>   PCI: cadence: Remove "cdns,max-outbound-regions" DT property
> 
> On Fri, Mar 27, 2020 at 04:17:27PM +0530, Kishon Vijay Abraham I wrote:
>> "cdns,max-outbound-regions" device tree property provides the
>> maximum number of outbound regions supported by the Host PCIe
>> controller. However the outbound regions are configured based
>> on what is populated in the "ranges" DT property.
> 
> Looks like this is missing a blank line here?  Or it should be
> rewrapped as part of the above paragraph?  I think the below makes
> more sense as a separate paragraph, though.

I'll add a blank line for the next paragraph and re-post once I get Ack from
Rob on the DT binding patch.

Thanks for reviewing!

Regards
Kishon
> 
> Again, thanks for doing this; this is a great cleanup.
> 
>> Avoid using two properties for configuring outbound regions and
>> use only "ranges" property instead.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  drivers/pci/controller/cadence/pcie-cadence-host.c | 6 ------
>>  drivers/pci/controller/cadence/pcie-cadence.h      | 2 --
>>  2 files changed, 8 deletions(-)
>>
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
>> index 60f912a657b9..8f72967f298f 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
>> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
>> @@ -140,9 +140,6 @@ static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
>>  	for_each_of_pci_range(&parser, &range) {
>>  		bool is_io;
>>  
>> -		if (r >= rc->max_regions)
>> -			break;
>> -
>>  		if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_MEM)
>>  			is_io = false;
>>  		else if ((range.flags & IORESOURCE_TYPE_BITS) == IORESOURCE_IO)
>> @@ -221,9 +218,6 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
>>  	pcie = &rc->pcie;
>>  	pcie->is_rc = true;
>>  
>> -	rc->max_regions = 32;
>> -	of_property_read_u32(np, "cdns,max-outbound-regions", &rc->max_regions);
>> -
>>  	if (!of_pci_dma_range_parser_init(&parser, np))
>>  		if (of_pci_range_parser_one(&parser, &range))
>>  			rc->no_bar_nbits = ilog2(range.size);
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
>> index a2b28b912ca4..6bd89a21bb1c 100644
>> --- a/drivers/pci/controller/cadence/pcie-cadence.h
>> +++ b/drivers/pci/controller/cadence/pcie-cadence.h
>> @@ -251,7 +251,6 @@ struct cdns_pcie {
>>   * @bus_range: first/last buses behind the PCIe host controller
>>   * @cfg_base: IO mapped window to access the PCI configuration space of a
>>   *            single function at a time
>> - * @max_regions: maximum number of regions supported by the hardware
>>   * @no_bar_nbits: Number of bits to keep for inbound (PCIe -> CPU) address
>>   *                translation (nbits sets into the "no BAR match" register)
>>   * @vendor_id: PCI vendor ID
>> @@ -262,7 +261,6 @@ struct cdns_pcie_rc {
>>  	struct resource		*cfg_res;
>>  	struct resource		*bus_range;
>>  	void __iomem		*cfg_base;
>> -	u32			max_regions;
>>  	u32			no_bar_nbits;
>>  	u16			vendor_id;
>>  	u16			device_id;
>> -- 
>> 2.17.1
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] dt-bindings: PCI: cadence: Deprecate inbound/outbound specific bindings
  2020-03-27 10:47 ` [PATCH 1/3] dt-bindings: " Kishon Vijay Abraham I
@ 2020-03-30 16:01   ` Rob Herring
  2020-03-31  3:38     ` Kishon Vijay Abraham I
  2020-04-10 16:38   ` Rob Herring
  1 sibling, 1 reply; 14+ messages in thread
From: Rob Herring @ 2020-03-30 16:01 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Tom Joseph, Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
	Mark Rutland, linux-pci, devicetree, linux-kernel

On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote:
> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for
> host mode as both these could be derived from "ranges" and "dma-ranges"
> property. "cdns,max-outbound-regions" property would still be required
> for EP mode.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
>  .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
>  .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
>  .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
>  .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
>  5 files changed, 37 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
> index 2996f8d4777c..50ce5d79d2c7 100644
> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
> @@ -10,7 +10,7 @@ maintainers:
>    - Tom Joseph <tjoseph@cadence.com>
>  
>  allOf:
> -  - $ref: "cdns-pcie.yaml#"
> +  - $ref: "cdns-pcie-ep.yaml#"
>    - $ref: "pci-ep.yaml#"
>  
>  properties:
> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
> index cabbe46ff578..84a8f095d031 100644
> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
> @@ -45,8 +45,6 @@ examples:
>              #size-cells = <2>;
>              bus-range = <0x0 0xff>;
>              linux,pci-domain = <0>;
> -            cdns,max-outbound-regions = <16>;
> -            cdns,no-bar-match-nbits = <32>;
>              vendor-id = <0x17cd>;
>              device-id = <0x0200>;
>  
> @@ -57,6 +55,7 @@ examples:
>  
>              ranges = <0x02000000 0x0 0x42000000  0x0 0x42000000  0x0 0x1000000>,
>                       <0x01000000 0x0 0x43000000  0x0 0x43000000  0x0 0x0010000>;
> +            dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>;
>  
>              #interrupt-cells = <0x1>;
>  
> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
> new file mode 100644
> index 000000000000..6150a7a7bdbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
> @@ -0,0 +1,25 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Cadence PCIe Device
> +
> +maintainers:
> +  - Tom Joseph <tjoseph@cadence.com>
> +
> +allOf:
> +  - $ref: "cdns-pcie.yaml#"
> +
> +properties:
> +  cdns,max-outbound-regions:
> +    description: maximum number of outbound regions
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 32
> +    default: 32

I have a feeling that as the PCI endpoint binding evolves this won't be 
necessary. I can see a common need to define the number of BARs for an 
endpoint and then this will again just be error checking.

What's the result if you write to a non-existent region in register 
CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and 
doesn't abort, you could detect this instead.

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] dt-bindings: PCI: cadence: Deprecate inbound/outbound specific bindings
  2020-03-30 16:01   ` Rob Herring
@ 2020-03-31  3:38     ` Kishon Vijay Abraham I
  2020-03-31 16:45       ` Rob Herring
  0 siblings, 1 reply; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-03-31  3:38 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tom Joseph, Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
	Mark Rutland, linux-pci, devicetree, linux-kernel

Hi Rob,

On 3/30/2020 9:31 PM, Rob Herring wrote:
> On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote:
>> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for
>> host mode as both these could be derived from "ranges" and "dma-ranges"
>> property. "cdns,max-outbound-regions" property would still be required
>> for EP mode.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
>>  .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
>>  .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
>>  .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
>>  .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
>>  5 files changed, 37 insertions(+), 11 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>> index 2996f8d4777c..50ce5d79d2c7 100644
>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>> @@ -10,7 +10,7 @@ maintainers:
>>    - Tom Joseph <tjoseph@cadence.com>
>>  
>>  allOf:
>> -  - $ref: "cdns-pcie.yaml#"
>> +  - $ref: "cdns-pcie-ep.yaml#"
>>    - $ref: "pci-ep.yaml#"
>>  
>>  properties:
>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>> index cabbe46ff578..84a8f095d031 100644
>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>> @@ -45,8 +45,6 @@ examples:
>>              #size-cells = <2>;
>>              bus-range = <0x0 0xff>;
>>              linux,pci-domain = <0>;
>> -            cdns,max-outbound-regions = <16>;
>> -            cdns,no-bar-match-nbits = <32>;
>>              vendor-id = <0x17cd>;
>>              device-id = <0x0200>;
>>  
>> @@ -57,6 +55,7 @@ examples:
>>  
>>              ranges = <0x02000000 0x0 0x42000000  0x0 0x42000000  0x0 0x1000000>,
>>                       <0x01000000 0x0 0x43000000  0x0 0x43000000  0x0 0x0010000>;
>> +            dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>;
>>  
>>              #interrupt-cells = <0x1>;
>>  
>> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>> new file mode 100644
>> index 000000000000..6150a7a7bdbf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>> @@ -0,0 +1,25 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Cadence PCIe Device
>> +
>> +maintainers:
>> +  - Tom Joseph <tjoseph@cadence.com>
>> +
>> +allOf:
>> +  - $ref: "cdns-pcie.yaml#"
>> +
>> +properties:
>> +  cdns,max-outbound-regions:
>> +    description: maximum number of outbound regions
>> +    allOf:
>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 1
>> +    maximum: 32
>> +    default: 32
> 
> I have a feeling that as the PCI endpoint binding evolves this won't be 
> necessary. I can see a common need to define the number of BARs for an 
> endpoint and then this will again just be error checking.

For every buffer given by the host, we have to create a new outbound
translation. If there are no outbound regions, we have to report the error to
the endpoint function driver. At-least for reporting the error, we'd need to
have this binding no?
> 
> What's the result if you write to a non-existent region in register 
> CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and 
> doesn't abort, you could detect this instead.

I'm not sure if we should ever try to write to a non-existent register though
the behavior could be different in different platforms. IMHO maximum number of
outbound regions is a HW property and is best described in device tree.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] dt-bindings: PCI: cadence: Deprecate inbound/outbound specific bindings
  2020-03-31  3:38     ` Kishon Vijay Abraham I
@ 2020-03-31 16:45       ` Rob Herring
  2020-04-01  3:08         ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2020-03-31 16:45 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Tom Joseph, Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
	Mark Rutland, linux-pci, devicetree, linux-kernel

On Tue, Mar 31, 2020 at 09:08:12AM +0530, Kishon Vijay Abraham I wrote:
> Hi Rob,
> 
> On 3/30/2020 9:31 PM, Rob Herring wrote:
> > On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote:
> >> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for
> >> host mode as both these could be derived from "ranges" and "dma-ranges"
> >> property. "cdns,max-outbound-regions" property would still be required
> >> for EP mode.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> >> ---
> >>  .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
> >>  .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
> >>  .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
> >>  .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
> >>  .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
> >>  5 files changed, 37 insertions(+), 11 deletions(-)
> >>  create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
> >> index 2996f8d4777c..50ce5d79d2c7 100644
> >> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
> >> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
> >> @@ -10,7 +10,7 @@ maintainers:
> >>    - Tom Joseph <tjoseph@cadence.com>
> >>  
> >>  allOf:
> >> -  - $ref: "cdns-pcie.yaml#"
> >> +  - $ref: "cdns-pcie-ep.yaml#"
> >>    - $ref: "pci-ep.yaml#"
> >>  
> >>  properties:
> >> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
> >> index cabbe46ff578..84a8f095d031 100644
> >> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
> >> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
> >> @@ -45,8 +45,6 @@ examples:
> >>              #size-cells = <2>;
> >>              bus-range = <0x0 0xff>;
> >>              linux,pci-domain = <0>;
> >> -            cdns,max-outbound-regions = <16>;
> >> -            cdns,no-bar-match-nbits = <32>;
> >>              vendor-id = <0x17cd>;
> >>              device-id = <0x0200>;
> >>  
> >> @@ -57,6 +55,7 @@ examples:
> >>  
> >>              ranges = <0x02000000 0x0 0x42000000  0x0 0x42000000  0x0 0x1000000>,
> >>                       <0x01000000 0x0 0x43000000  0x0 0x43000000  0x0 0x0010000>;
> >> +            dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>;
> >>  
> >>              #interrupt-cells = <0x1>;
> >>  
> >> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
> >> new file mode 100644
> >> index 000000000000..6150a7a7bdbf
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
> >> @@ -0,0 +1,25 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#"
> >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> >> +
> >> +title: Cadence PCIe Device
> >> +
> >> +maintainers:
> >> +  - Tom Joseph <tjoseph@cadence.com>
> >> +
> >> +allOf:
> >> +  - $ref: "cdns-pcie.yaml#"
> >> +
> >> +properties:
> >> +  cdns,max-outbound-regions:
> >> +    description: maximum number of outbound regions
> >> +    allOf:
> >> +      - $ref: /schemas/types.yaml#/definitions/uint32
> >> +    minimum: 1
> >> +    maximum: 32
> >> +    default: 32
> > 
> > I have a feeling that as the PCI endpoint binding evolves this won't be 
> > necessary. I can see a common need to define the number of BARs for an 
> > endpoint and then this will again just be error checking.
> 
> For every buffer given by the host, we have to create a new outbound
> translation. If there are no outbound regions, we have to report the error to
> the endpoint function driver. At-least for reporting the error, we'd need to
> have this binding no?

But isn't the endpoint defined to have some number of BARs? The PCI host 
doesn't decide that.

> > 
> > What's the result if you write to a non-existent region in register 
> > CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and 
> > doesn't abort, you could detect this instead.
> 
> I'm not sure if we should ever try to write to a non-existent register though
> the behavior could be different in different platforms. IMHO maximum number of
> outbound regions is a HW property and is best described in device tree.

AIUI, PCI defines non-existent (config space) registers to return all 
1s. Not sure if this register is in PCI config space or the host SoC bus 
(e.g. AXI). It seems PCI bridges get done both ways from what I've seen.

Rob

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] dt-bindings: PCI: cadence: Deprecate inbound/outbound specific bindings
  2020-03-31 16:45       ` Rob Herring
@ 2020-04-01  3:08         ` Kishon Vijay Abraham I
  2020-04-03  5:42           ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-04-01  3:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tom Joseph, Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
	Mark Rutland, linux-pci, devicetree, linux-kernel

Hi Rob,

On 3/31/2020 10:15 PM, Rob Herring wrote:
> On Tue, Mar 31, 2020 at 09:08:12AM +0530, Kishon Vijay Abraham I wrote:
>> Hi Rob,
>>
>> On 3/30/2020 9:31 PM, Rob Herring wrote:
>>> On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote:
>>>> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for
>>>> host mode as both these could be derived from "ranges" and "dma-ranges"
>>>> property. "cdns,max-outbound-regions" property would still be required
>>>> for EP mode.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>> ---
>>>>  .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
>>>>  .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
>>>>  .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
>>>>  .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
>>>>  .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
>>>>  5 files changed, 37 insertions(+), 11 deletions(-)
>>>>  create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>>>> index 2996f8d4777c..50ce5d79d2c7 100644
>>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>>>> @@ -10,7 +10,7 @@ maintainers:
>>>>    - Tom Joseph <tjoseph@cadence.com>
>>>>  
>>>>  allOf:
>>>> -  - $ref: "cdns-pcie.yaml#"
>>>> +  - $ref: "cdns-pcie-ep.yaml#"
>>>>    - $ref: "pci-ep.yaml#"
>>>>  
>>>>  properties:
>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>> index cabbe46ff578..84a8f095d031 100644
>>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>> @@ -45,8 +45,6 @@ examples:
>>>>              #size-cells = <2>;
>>>>              bus-range = <0x0 0xff>;
>>>>              linux,pci-domain = <0>;
>>>> -            cdns,max-outbound-regions = <16>;
>>>> -            cdns,no-bar-match-nbits = <32>;
>>>>              vendor-id = <0x17cd>;
>>>>              device-id = <0x0200>;
>>>>  
>>>> @@ -57,6 +55,7 @@ examples:
>>>>  
>>>>              ranges = <0x02000000 0x0 0x42000000  0x0 0x42000000  0x0 0x1000000>,
>>>>                       <0x01000000 0x0 0x43000000  0x0 0x43000000  0x0 0x0010000>;
>>>> +            dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>;
>>>>  
>>>>              #interrupt-cells = <0x1>;
>>>>  
>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>>>> new file mode 100644
>>>> index 000000000000..6150a7a7bdbf
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>>>> @@ -0,0 +1,25 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#"
>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>>>> +
>>>> +title: Cadence PCIe Device
>>>> +
>>>> +maintainers:
>>>> +  - Tom Joseph <tjoseph@cadence.com>
>>>> +
>>>> +allOf:
>>>> +  - $ref: "cdns-pcie.yaml#"
>>>> +
>>>> +properties:
>>>> +  cdns,max-outbound-regions:
>>>> +    description: maximum number of outbound regions
>>>> +    allOf:
>>>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    minimum: 1
>>>> +    maximum: 32
>>>> +    default: 32
>>>
>>> I have a feeling that as the PCI endpoint binding evolves this won't be 
>>> necessary. I can see a common need to define the number of BARs for an 
>>> endpoint and then this will again just be error checking.
>>
>> For every buffer given by the host, we have to create a new outbound
>> translation. If there are no outbound regions, we have to report the error to
>> the endpoint function driver. At-least for reporting the error, we'd need to
>> have this binding no?
> 
> But isn't the endpoint defined to have some number of BARs? The PCI host 
> doesn't decide that.

cdns,max-outbound-regions defined here doesn't configure the BARs. BARs provide
an interface for the host to access the endpoints memory. IOW for BARs we
configure the inbound address translation unit.

cdns,max-outbound-regions is used while configuring the outbound address
translation unit. Outbound regions are used while the endpoint access host
memory and in that path endpoint BARs doesn't come.
> 
>>>
>>> What's the result if you write to a non-existent region in register 
>>> CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and 
>>> doesn't abort, you could detect this instead.
>>
>> I'm not sure if we should ever try to write to a non-existent register though
>> the behavior could be different in different platforms. IMHO maximum number of
>> outbound regions is a HW property and is best described in device tree.
> 
> AIUI, PCI defines non-existent (config space) registers to return all 
> 1s. Not sure if this register is in PCI config space or the host SoC bus 
> (e.g. AXI). It seems PCI bridges get done both ways from what I've seen.

All of that is correct for the Host or RC. However here
cdns,max-outbound-regions is an endpoint specific property (defined only in
cdns-pcie-ep.yaml) and is useful while configuring OB address translation unit
for the endpoint to access host memory.

Thanks
Kishon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] dt-bindings: PCI: cadence: Deprecate inbound/outbound specific bindings
  2020-04-01  3:08         ` Kishon Vijay Abraham I
@ 2020-04-03  5:42           ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-04-03  5:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tom Joseph, Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
	Mark Rutland, linux-pci, devicetree, linux-kernel

Hi Rob,

On 4/1/2020 8:38 AM, Kishon Vijay Abraham I wrote:
> Hi Rob,
> 
> On 3/31/2020 10:15 PM, Rob Herring wrote:
>> On Tue, Mar 31, 2020 at 09:08:12AM +0530, Kishon Vijay Abraham I wrote:
>>> Hi Rob,
>>>
>>> On 3/30/2020 9:31 PM, Rob Herring wrote:
>>>> On Fri, Mar 27, 2020 at 04:17:25PM +0530, Kishon Vijay Abraham I wrote:
>>>>> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for
>>>>> host mode as both these could be derived from "ranges" and "dma-ranges"
>>>>> property. "cdns,max-outbound-regions" property would still be required
>>>>> for EP mode.
>>>>>
>>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>>>>> ---
>>>>>  .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
>>>>>  .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
>>>>>  .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
>>>>>  .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
>>>>>  .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
>>>>>  5 files changed, 37 insertions(+), 11 deletions(-)
>>>>>  create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>>>>> index 2996f8d4777c..50ce5d79d2c7 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>>>>> @@ -10,7 +10,7 @@ maintainers:
>>>>>    - Tom Joseph <tjoseph@cadence.com>
>>>>>  
>>>>>  allOf:
>>>>> -  - $ref: "cdns-pcie.yaml#"
>>>>> +  - $ref: "cdns-pcie-ep.yaml#"
>>>>>    - $ref: "pci-ep.yaml#"
>>>>>  
>>>>>  properties:
>>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>>> index cabbe46ff578..84a8f095d031 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>>> @@ -45,8 +45,6 @@ examples:
>>>>>              #size-cells = <2>;
>>>>>              bus-range = <0x0 0xff>;
>>>>>              linux,pci-domain = <0>;
>>>>> -            cdns,max-outbound-regions = <16>;
>>>>> -            cdns,no-bar-match-nbits = <32>;
>>>>>              vendor-id = <0x17cd>;
>>>>>              device-id = <0x0200>;
>>>>>  
>>>>> @@ -57,6 +55,7 @@ examples:
>>>>>  
>>>>>              ranges = <0x02000000 0x0 0x42000000  0x0 0x42000000  0x0 0x1000000>,
>>>>>                       <0x01000000 0x0 0x43000000  0x0 0x43000000  0x0 0x0010000>;
>>>>> +            dma-ranges = <0x02000000 0x0 0x0 0x0 0x0 0x1 0x00000000>;
>>>>>  
>>>>>              #interrupt-cells = <0x1>;
>>>>>  
>>>>> diff --git a/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..6150a7a7bdbf
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>>>>> @@ -0,0 +1,25 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: "http://devicetree.org/schemas/pci/cdns-pcie-ep.yaml#"
>>>>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>>>>> +
>>>>> +title: Cadence PCIe Device
>>>>> +
>>>>> +maintainers:
>>>>> +  - Tom Joseph <tjoseph@cadence.com>
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: "cdns-pcie.yaml#"
>>>>> +
>>>>> +properties:
>>>>> +  cdns,max-outbound-regions:
>>>>> +    description: maximum number of outbound regions
>>>>> +    allOf:
>>>>> +      - $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    minimum: 1
>>>>> +    maximum: 32
>>>>> +    default: 32
>>>>
>>>> I have a feeling that as the PCI endpoint binding evolves this won't be 
>>>> necessary. I can see a common need to define the number of BARs for an 
>>>> endpoint and then this will again just be error checking.
>>>
>>> For every buffer given by the host, we have to create a new outbound
>>> translation. If there are no outbound regions, we have to report the error to
>>> the endpoint function driver. At-least for reporting the error, we'd need to
>>> have this binding no?
>>
>> But isn't the endpoint defined to have some number of BARs? The PCI host 
>> doesn't decide that.
> 
> cdns,max-outbound-regions defined here doesn't configure the BARs. BARs provide
> an interface for the host to access the endpoints memory. IOW for BARs we
> configure the inbound address translation unit.
> 
> cdns,max-outbound-regions is used while configuring the outbound address
> translation unit. Outbound regions are used while the endpoint access host
> memory and in that path endpoint BARs doesn't come.
>>
>>>>
>>>> What's the result if you write to a non-existent region in register 
>>>> CDNS_PCIE_AT_OB_REGION_PCI_ADDR0/1? If the register is non-existent and 
>>>> doesn't abort, you could detect this instead.
>>>
>>> I'm not sure if we should ever try to write to a non-existent register though
>>> the behavior could be different in different platforms. IMHO maximum number of
>>> outbound regions is a HW property and is best described in device tree.
>>
>> AIUI, PCI defines non-existent (config space) registers to return all 
>> 1s. Not sure if this register is in PCI config space or the host SoC bus 
>> (e.g. AXI). It seems PCI bridges get done both ways from what I've seen.
> 
> All of that is correct for the Host or RC. However here
> cdns,max-outbound-regions is an endpoint specific property (defined only in
> cdns-pcie-ep.yaml) and is useful while configuring OB address translation unit
> for the endpoint to access host memory.

Do you still have concerns regarding this? If you don't have any further
comments on this, can you give your Acked-by please?

Thanks
Kishon

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] dt-bindings: PCI: cadence: Deprecate inbound/outbound specific bindings
  2020-03-27 10:47 ` [PATCH 1/3] dt-bindings: " Kishon Vijay Abraham I
  2020-03-30 16:01   ` Rob Herring
@ 2020-04-10 16:38   ` Rob Herring
  2020-04-11  2:21     ` Kishon Vijay Abraham I
  1 sibling, 1 reply; 14+ messages in thread
From: Rob Herring @ 2020-04-10 16:38 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Tom Joseph, Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
	Mark Rutland, Kishon Vijay Abraham I, linux-pci, devicetree,
	linux-kernel

On Fri, 27 Mar 2020 16:17:25 +0530, Kishon Vijay Abraham I wrote:
> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for
> host mode as both these could be derived from "ranges" and "dma-ranges"
> property. "cdns,max-outbound-regions" property would still be required
> for EP mode.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
>  .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
>  .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
>  .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
>  .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
>  .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
>  5 files changed, 37 insertions(+), 11 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
> 

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] dt-bindings: PCI: cadence: Deprecate inbound/outbound specific bindings
  2020-04-10 16:38   ` Rob Herring
@ 2020-04-11  2:21     ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2020-04-11  2:21 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tom Joseph, Lorenzo Pieralisi, Andrew Murray, Bjorn Helgaas,
	Mark Rutland, linux-pci, devicetree, linux-kernel



On 4/10/2020 10:08 PM, Rob Herring wrote:
> On Fri, 27 Mar 2020 16:17:25 +0530, Kishon Vijay Abraham I wrote:
>> Deprecate cdns,max-outbound-regions and cdns,no-bar-match-nbits for
>> host mode as both these could be derived from "ranges" and "dma-ranges"
>> property. "cdns,max-outbound-regions" property would still be required
>> for EP mode.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> ---
>>  .../bindings/pci/cdns,cdns-pcie-ep.yaml       |  2 +-
>>  .../bindings/pci/cdns,cdns-pcie-host.yaml     |  3 +--
>>  .../devicetree/bindings/pci/cdns-pcie-ep.yaml | 25 +++++++++++++++++++
>>  .../bindings/pci/cdns-pcie-host.yaml          | 10 ++++++++
>>  .../devicetree/bindings/pci/cdns-pcie.yaml    |  8 ------
>>  5 files changed, 37 insertions(+), 11 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/pci/cdns-pcie-ep.yaml
>>
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Thank you Rob!

Regards
Kishon

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-04-11  2:21 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-27 10:47 [PATCH 0/3] PCI: cadence: Deprecate inbound/outbound specific bindings Kishon Vijay Abraham I
2020-03-27 10:47 ` [PATCH 1/3] dt-bindings: " Kishon Vijay Abraham I
2020-03-30 16:01   ` Rob Herring
2020-03-31  3:38     ` Kishon Vijay Abraham I
2020-03-31 16:45       ` Rob Herring
2020-04-01  3:08         ` Kishon Vijay Abraham I
2020-04-03  5:42           ` Kishon Vijay Abraham I
2020-04-10 16:38   ` Rob Herring
2020-04-11  2:21     ` Kishon Vijay Abraham I
2020-03-27 10:47 ` [PATCH 2/3] PCI: cadence: Use "dma-ranges" instead of "cdns,no-bar-match-nbits" property Kishon Vijay Abraham I
2020-03-27 14:52   ` Bjorn Helgaas
2020-03-27 10:47 ` [PATCH 3/3] PCI: Cadence: Remove using "cdns,max-outbound-regions" DT property Kishon Vijay Abraham I
2020-03-27 14:54   ` Bjorn Helgaas
2020-03-27 15:27     ` Kishon Vijay Abraham I

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).