All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Fix ecam size value to discover 256 buses during
@ 2023-08-07 11:07 ` Thippeswamy Havalige
  0 siblings, 0 replies; 14+ messages in thread
From: Thippeswamy Havalige @ 2023-08-07 11:07 UTC (permalink / raw)
  To: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree
  Cc: lpieralisi, bharat.kumar.gogada, michal.simek, linux-arm-kernel,
	Thippeswamy Havalige

Current driver is supports up to 16 buses. The following code fixes 
to support up to 256 buses.

update "NWL_ECAM_VALUE_DEFAULT " to 16  can access up to 256MB ECAM
region to detect 256 buses.

Remove redundant code which updates primary,secondary and sub-ordinate
register offset 0x18 in type1 header.

Update ecam size to 256MB in device tree binding example.

Thippeswamy Havalige (2):
  PCI: xilinx-nwl: Update ECAM default value and remove unnecessary    
    code.
  dt-bindings: PCI: xilinx-nwl: Modify ECAM size in example.

 .../devicetree/bindings/pci/xlnx,nwl-pcie.yaml         |  2 +-
 drivers/pci/controller/pcie-xilinx-nwl.c               | 18 +++---------------
 2 files changed, 4 insertions(+), 16 deletions(-)

-- 
1.8.3.1


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

* [PATCH v1 0/2] Fix ecam size value to discover 256 buses during
@ 2023-08-07 11:07 ` Thippeswamy Havalige
  0 siblings, 0 replies; 14+ messages in thread
From: Thippeswamy Havalige @ 2023-08-07 11:07 UTC (permalink / raw)
  To: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree
  Cc: lpieralisi, bharat.kumar.gogada, michal.simek, linux-arm-kernel,
	Thippeswamy Havalige

Current driver is supports up to 16 buses. The following code fixes 
to support up to 256 buses.

update "NWL_ECAM_VALUE_DEFAULT " to 16  can access up to 256MB ECAM
region to detect 256 buses.

Remove redundant code which updates primary,secondary and sub-ordinate
register offset 0x18 in type1 header.

Update ecam size to 256MB in device tree binding example.

Thippeswamy Havalige (2):
  PCI: xilinx-nwl: Update ECAM default value and remove unnecessary    
    code.
  dt-bindings: PCI: xilinx-nwl: Modify ECAM size in example.

 .../devicetree/bindings/pci/xlnx,nwl-pcie.yaml         |  2 +-
 drivers/pci/controller/pcie-xilinx-nwl.c               | 18 +++---------------
 2 files changed, 4 insertions(+), 16 deletions(-)

-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
  2023-08-07 11:07 ` Thippeswamy Havalige
@ 2023-08-07 11:07   ` Thippeswamy Havalige
  -1 siblings, 0 replies; 14+ messages in thread
From: Thippeswamy Havalige @ 2023-08-07 11:07 UTC (permalink / raw)
  To: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree
  Cc: lpieralisi, bharat.kumar.gogada, michal.simek, linux-arm-kernel,
	Thippeswamy Havalige

Our controller is expecting ECAM size to be programmed by software.
By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access up to
16MB ECAM region which is used to detect 16 buses, so by updating
"NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to 256MB
ECAM region to detect 256 buses.

E_ECAM_CONTROL register from bit 16 to 20 uses this value as input
to calculate ECAM Size.

The primary,secondary and sub-ordinate bus number registers are updated
by Linux PCI core, so removing code which is updating primary,secondary
and sub-ordinate bus numbers of type 1 header 18th offset of ECAM.

Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
---
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: 
| https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
| com/
---
 drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 176686b..b515019 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -126,7 +126,7 @@
 #define E_ECAM_CR_ENABLE		BIT(0)
 #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
 #define E_ECAM_SIZE_SHIFT		16
-#define NWL_ECAM_VALUE_DEFAULT		12
+#define NWL_ECAM_VALUE_DEFAULT		16
 
 #define CFG_DMA_REG_BAR			GENMASK(2, 0)
 #define CFG_PCIE_CACHE			GENMASK(7, 0)
@@ -165,8 +165,6 @@ struct nwl_pcie {
 	u32 ecam_size;
 	int irq_intx;
 	int irq_misc;
-	u32 ecam_value;
-	u8 last_busno;
 	struct nwl_msi msi;
 	struct irq_domain *legacy_irq_domain;
 	struct clk *clk;
@@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
 	struct platform_device *pdev = to_platform_device(dev);
-	u32 breg_val, ecam_val, first_busno = 0;
+	u32 breg_val, ecam_val;
 	int err;
 
 	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) & BREG_PRESENT;
@@ -675,7 +673,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
 
 	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
-			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
+			  (NWL_ECAM_VALUE_DEFAULT << E_ECAM_SIZE_SHIFT),
 			  E_ECAM_CONTROL);
 
 	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
@@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
 			  E_ECAM_BASE_HI);
 
-	/* Get bus range */
-	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
-	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >> E_ECAM_SIZE_SHIFT;
-	/* Write primary, secondary and subordinate bus numbers */
-	ecam_val = first_busno;
-	ecam_val |= (first_busno + 1) << 8;
-	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
-	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
-
 	if (nwl_pcie_link_up(pcie))
 		dev_info(dev, "Link is UP\n");
 	else
@@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 	pcie = pci_host_bridge_priv(bridge);
 
 	pcie->dev = dev;
-	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
 
 	err = nwl_pcie_parse_dt(pcie, pdev);
 	if (err) {
-- 
1.8.3.1


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

* [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
@ 2023-08-07 11:07   ` Thippeswamy Havalige
  0 siblings, 0 replies; 14+ messages in thread
From: Thippeswamy Havalige @ 2023-08-07 11:07 UTC (permalink / raw)
  To: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree
  Cc: lpieralisi, bharat.kumar.gogada, michal.simek, linux-arm-kernel,
	Thippeswamy Havalige

Our controller is expecting ECAM size to be programmed by software.
By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access up to
16MB ECAM region which is used to detect 16 buses, so by updating
"NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to 256MB
ECAM region to detect 256 buses.

E_ECAM_CONTROL register from bit 16 to 20 uses this value as input
to calculate ECAM Size.

The primary,secondary and sub-ordinate bus number registers are updated
by Linux PCI core, so removing code which is updating primary,secondary
and sub-ordinate bus numbers of type 1 header 18th offset of ECAM.

Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
---
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: 
| https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
| com/
---
 drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 176686b..b515019 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -126,7 +126,7 @@
 #define E_ECAM_CR_ENABLE		BIT(0)
 #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
 #define E_ECAM_SIZE_SHIFT		16
-#define NWL_ECAM_VALUE_DEFAULT		12
+#define NWL_ECAM_VALUE_DEFAULT		16
 
 #define CFG_DMA_REG_BAR			GENMASK(2, 0)
 #define CFG_PCIE_CACHE			GENMASK(7, 0)
@@ -165,8 +165,6 @@ struct nwl_pcie {
 	u32 ecam_size;
 	int irq_intx;
 	int irq_misc;
-	u32 ecam_value;
-	u8 last_busno;
 	struct nwl_msi msi;
 	struct irq_domain *legacy_irq_domain;
 	struct clk *clk;
@@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
 	struct platform_device *pdev = to_platform_device(dev);
-	u32 breg_val, ecam_val, first_busno = 0;
+	u32 breg_val, ecam_val;
 	int err;
 
 	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) & BREG_PRESENT;
@@ -675,7 +673,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
 
 	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
-			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
+			  (NWL_ECAM_VALUE_DEFAULT << E_ECAM_SIZE_SHIFT),
 			  E_ECAM_CONTROL);
 
 	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
@@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
 			  E_ECAM_BASE_HI);
 
-	/* Get bus range */
-	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
-	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >> E_ECAM_SIZE_SHIFT;
-	/* Write primary, secondary and subordinate bus numbers */
-	ecam_val = first_busno;
-	ecam_val |= (first_busno + 1) << 8;
-	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
-	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
-
 	if (nwl_pcie_link_up(pcie))
 		dev_info(dev, "Link is UP\n");
 	else
@@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 	pcie = pci_host_bridge_priv(bridge);
 
 	pcie->dev = dev;
-	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
 
 	err = nwl_pcie_parse_dt(pcie, pdev);
 	if (err) {
-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v1 2/2] dt-bindings: PCI: xilinx-nwl: Modify ECAM size in example.
  2023-08-07 11:07 ` Thippeswamy Havalige
@ 2023-08-07 11:07   ` Thippeswamy Havalige
  -1 siblings, 0 replies; 14+ messages in thread
From: Thippeswamy Havalige @ 2023-08-07 11:07 UTC (permalink / raw)
  To: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree
  Cc: lpieralisi, bharat.kumar.gogada, michal.simek, linux-arm-kernel,
	Thippeswamy Havalige

Update ECAM size in example to discover up to 256 buses.

Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
---
 Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
index 8976025..426f90a 100644
--- a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
@@ -118,7 +118,7 @@ examples:
             compatible = "xlnx,nwl-pcie-2.11";
             reg = <0x0 0xfd0e0000 0x0 0x1000>,
                   <0x0 0xfd480000 0x0 0x1000>,
-                  <0x80 0x00000000 0x0 0x1000000>;
+                  <0x80 0x00000000 0x0 0x10000000>;
             reg-names = "breg", "pcireg", "cfg";
             ranges = <0x02000000 0x0 0xe0000000 0x0 0xe0000000 0x0 0x10000000>,
                      <0x43000000 0x00000006 0x0 0x00000006 0x0 0x00000002 0x0>;
-- 
1.8.3.1


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

* [PATCH v1 2/2] dt-bindings: PCI: xilinx-nwl: Modify ECAM size in example.
@ 2023-08-07 11:07   ` Thippeswamy Havalige
  0 siblings, 0 replies; 14+ messages in thread
From: Thippeswamy Havalige @ 2023-08-07 11:07 UTC (permalink / raw)
  To: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree
  Cc: lpieralisi, bharat.kumar.gogada, michal.simek, linux-arm-kernel,
	Thippeswamy Havalige

Update ECAM size in example to discover up to 256 buses.

Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
---
 Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
index 8976025..426f90a 100644
--- a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
@@ -118,7 +118,7 @@ examples:
             compatible = "xlnx,nwl-pcie-2.11";
             reg = <0x0 0xfd0e0000 0x0 0x1000>,
                   <0x0 0xfd480000 0x0 0x1000>,
-                  <0x80 0x00000000 0x0 0x1000000>;
+                  <0x80 0x00000000 0x0 0x10000000>;
             reg-names = "breg", "pcireg", "cfg";
             ranges = <0x02000000 0x0 0xe0000000 0x0 0xe0000000 0x0 0x10000000>,
                      <0x43000000 0x00000006 0x0 0x00000006 0x0 0x00000002 0x0>;
-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
  2023-08-07 11:07   ` Thippeswamy Havalige
@ 2023-08-07 21:23     ` Bjorn Helgaas
  -1 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2023-08-07 21:23 UTC (permalink / raw)
  To: Thippeswamy Havalige
  Cc: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree, lpieralisi, bharat.kumar.gogada, michal.simek,
	linux-arm-kernel

Ideally the subject would include useful information about *why*
you're changing the ECAM value.  Drop the period at the end of the
subject.  E.g., something like:

  PCI: xilinx-nwl: Increase ECAM size to accommodate 256 buses

On Mon, Aug 07, 2023 at 04:37:32PM +0530, Thippeswamy Havalige wrote:
> Our controller is expecting ECAM size to be programmed by software.
> By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access up to
> 16MB ECAM region which is used to detect 16 buses, so by updating
> "NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to 256MB
> ECAM region to detect 256 buses.

Rob needs to ack this because it sounds like this change might make
the driver incompatible with DTs in the field, i.e., the user might be
forced to update the DT at the same time as picking up this driver
change.

> E_ECAM_CONTROL register from bit 16 to 20 uses this value as input
> to calculate ECAM Size.
> 
> The primary,secondary and sub-ordinate bus number registers are updated
> by Linux PCI core, so removing code which is updating primary,secondary
> and sub-ordinate bus numbers of type 1 header 18th offset of ECAM.

This code removal sounds like a separate logical change that could be
a separate patch.

s/primary,secondary/primary, secondary/ (twice)
s/removing/remove/

> Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> ---
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: 
> | https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
> | com/
> ---
>  drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 176686b..b515019 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -126,7 +126,7 @@
>  #define E_ECAM_CR_ENABLE		BIT(0)
>  #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
>  #define E_ECAM_SIZE_SHIFT		16
> -#define NWL_ECAM_VALUE_DEFAULT		12
> +#define NWL_ECAM_VALUE_DEFAULT		16
>  
>  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
>  #define CFG_PCIE_CACHE			GENMASK(7, 0)
> @@ -165,8 +165,6 @@ struct nwl_pcie {
>  	u32 ecam_size;
>  	int irq_intx;
>  	int irq_misc;
> -	u32 ecam_value;
> -	u8 last_busno;
>  	struct nwl_msi msi;
>  	struct irq_domain *legacy_irq_domain;
>  	struct clk *clk;
> @@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  {
>  	struct device *dev = pcie->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
> -	u32 breg_val, ecam_val, first_busno = 0;
> +	u32 breg_val, ecam_val;
>  	int err;
>  
>  	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) & BREG_PRESENT;
> @@ -675,7 +673,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
>  
>  	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
> -			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
> +			  (NWL_ECAM_VALUE_DEFAULT << E_ECAM_SIZE_SHIFT),
>  			  E_ECAM_CONTROL);
>  
>  	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
> @@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
>  			  E_ECAM_BASE_HI);
>  
> -	/* Get bus range */
> -	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
> -	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >> E_ECAM_SIZE_SHIFT;
> -	/* Write primary, secondary and subordinate bus numbers */
> -	ecam_val = first_busno;
> -	ecam_val |= (first_busno + 1) << 8;
> -	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
> -	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
> -
>  	if (nwl_pcie_link_up(pcie))
>  		dev_info(dev, "Link is UP\n");
>  	else
> @@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device *pdev)
>  	pcie = pci_host_bridge_priv(bridge);
>  
>  	pcie->dev = dev;
> -	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
>  
>  	err = nwl_pcie_parse_dt(pcie, pdev);
>  	if (err) {
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
@ 2023-08-07 21:23     ` Bjorn Helgaas
  0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Helgaas @ 2023-08-07 21:23 UTC (permalink / raw)
  To: Thippeswamy Havalige
  Cc: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree, lpieralisi, bharat.kumar.gogada, michal.simek,
	linux-arm-kernel

Ideally the subject would include useful information about *why*
you're changing the ECAM value.  Drop the period at the end of the
subject.  E.g., something like:

  PCI: xilinx-nwl: Increase ECAM size to accommodate 256 buses

On Mon, Aug 07, 2023 at 04:37:32PM +0530, Thippeswamy Havalige wrote:
> Our controller is expecting ECAM size to be programmed by software.
> By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access up to
> 16MB ECAM region which is used to detect 16 buses, so by updating
> "NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to 256MB
> ECAM region to detect 256 buses.

Rob needs to ack this because it sounds like this change might make
the driver incompatible with DTs in the field, i.e., the user might be
forced to update the DT at the same time as picking up this driver
change.

> E_ECAM_CONTROL register from bit 16 to 20 uses this value as input
> to calculate ECAM Size.
> 
> The primary,secondary and sub-ordinate bus number registers are updated
> by Linux PCI core, so removing code which is updating primary,secondary
> and sub-ordinate bus numbers of type 1 header 18th offset of ECAM.

This code removal sounds like a separate logical change that could be
a separate patch.

s/primary,secondary/primary, secondary/ (twice)
s/removing/remove/

> Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> ---
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: 
> | https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
> | com/
> ---
>  drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 176686b..b515019 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -126,7 +126,7 @@
>  #define E_ECAM_CR_ENABLE		BIT(0)
>  #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
>  #define E_ECAM_SIZE_SHIFT		16
> -#define NWL_ECAM_VALUE_DEFAULT		12
> +#define NWL_ECAM_VALUE_DEFAULT		16
>  
>  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
>  #define CFG_PCIE_CACHE			GENMASK(7, 0)
> @@ -165,8 +165,6 @@ struct nwl_pcie {
>  	u32 ecam_size;
>  	int irq_intx;
>  	int irq_misc;
> -	u32 ecam_value;
> -	u8 last_busno;
>  	struct nwl_msi msi;
>  	struct irq_domain *legacy_irq_domain;
>  	struct clk *clk;
> @@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  {
>  	struct device *dev = pcie->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
> -	u32 breg_val, ecam_val, first_busno = 0;
> +	u32 breg_val, ecam_val;
>  	int err;
>  
>  	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) & BREG_PRESENT;
> @@ -675,7 +673,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
>  
>  	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
> -			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
> +			  (NWL_ECAM_VALUE_DEFAULT << E_ECAM_SIZE_SHIFT),
>  			  E_ECAM_CONTROL);
>  
>  	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
> @@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
>  			  E_ECAM_BASE_HI);
>  
> -	/* Get bus range */
> -	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
> -	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >> E_ECAM_SIZE_SHIFT;
> -	/* Write primary, secondary and subordinate bus numbers */
> -	ecam_val = first_busno;
> -	ecam_val |= (first_busno + 1) << 8;
> -	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
> -	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
> -
>  	if (nwl_pcie_link_up(pcie))
>  		dev_info(dev, "Link is UP\n");
>  	else
> @@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device *pdev)
>  	pcie = pci_host_bridge_priv(bridge);
>  
>  	pcie->dev = dev;
> -	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
>  
>  	err = nwl_pcie_parse_dt(pcie, pdev);
>  	if (err) {
> -- 
> 1.8.3.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
  2023-08-07 21:23     ` Bjorn Helgaas
@ 2023-08-08 10:37       ` Havalige, Thippeswamy
  -1 siblings, 0 replies; 14+ messages in thread
From: Havalige, Thippeswamy @ 2023-08-08 10:37 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree, lpieralisi, Gogada, Bharat Kumar, Simek, Michal,
	linux-arm-kernel

Hi Bjorn,

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, August 8, 2023 2:53 AM
> To: Havalige, Thippeswamy <thippeswamy.havalige@amd.com>
> Cc: linux-kernel@vger.kernel.org; robh+dt@kernel.org;
> bhelgaas@google.com; krzysztof.kozlowski@linaro.org; linux-
> pci@vger.kernel.org; devicetree@vger.kernel.org; lpieralisi@kernel.org;
> Gogada, Bharat Kumar <bharat.kumar.gogada@amd.com>; Simek, Michal
> <michal.simek@amd.com>; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and
> remove unnecessary code.
> 
> Ideally the subject would include useful information about *why* you're
> changing the ECAM value.  Drop the period at the end of the subject.  E.g.,
> something like:
> 
>   PCI: xilinx-nwl: Increase ECAM size to accommodate 256 buses
- Agreed, I ll update in next patch.
> On Mon, Aug 07, 2023 at 04:37:32PM +0530, Thippeswamy Havalige wrote:
> > Our controller is expecting ECAM size to be programmed by software.
> > By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access
> up
> > to 16MB ECAM region which is used to detect 16 buses, so by updating
> > "NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to
> > 256MB ECAM region to detect 256 buses.
> Rob needs to ack this because it sounds like this change might make the
> driver incompatible with DTs in the field, i.e., the user might be forced to
> update the DT at the same time as picking up this driver change.
> 
> > E_ECAM_CONTROL register from bit 16 to 20 uses this value as input to
> > calculate ECAM Size.
> >
> > The primary,secondary and sub-ordinate bus number registers are
> > updated by Linux PCI core, so removing code which is updating
> > primary,secondary and sub-ordinate bus numbers of type 1 header 18th
> offset of ECAM.
> 
> This code removal sounds like a separate logical change that could be a
> separate patch.
> 
> s/primary,secondary/primary, secondary/ (twice) s/removing/remove/
- Agreed, I ll update in next patch.
> > Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> > ---
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes:
> > | https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
> > | com/
> > ---
> >  drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
> >  1 file changed, 3 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> > b/drivers/pci/controller/pcie-xilinx-nwl.c
> > index 176686b..b515019 100644
> > --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> > +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> > @@ -126,7 +126,7 @@
> >  #define E_ECAM_CR_ENABLE		BIT(0)
> >  #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
> >  #define E_ECAM_SIZE_SHIFT		16
> > -#define NWL_ECAM_VALUE_DEFAULT		12
> > +#define NWL_ECAM_VALUE_DEFAULT		16
> >
> >  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
> >  #define CFG_PCIE_CACHE			GENMASK(7, 0)
> > @@ -165,8 +165,6 @@ struct nwl_pcie {
> >  	u32 ecam_size;
> >  	int irq_intx;
> >  	int irq_misc;
> > -	u32 ecam_value;
> > -	u8 last_busno;
> >  	struct nwl_msi msi;
> >  	struct irq_domain *legacy_irq_domain;
> >  	struct clk *clk;
> > @@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie
> > *pcie)  {
> >  	struct device *dev = pcie->dev;
> >  	struct platform_device *pdev = to_platform_device(dev);
> > -	u32 breg_val, ecam_val, first_busno = 0;
> > +	u32 breg_val, ecam_val;
> >  	int err;
> >
> >  	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) &
> > BREG_PRESENT; @@ -675,7 +673,7 @@ static int
> nwl_pcie_bridge_init(struct nwl_pcie *pcie)
> >  			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
> >
> >  	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
> > -			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
> > +			  (NWL_ECAM_VALUE_DEFAULT <<
> E_ECAM_SIZE_SHIFT),
> >  			  E_ECAM_CONTROL);
> >
> >  	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
> > @@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie
> *pcie)
> >  	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
> >  			  E_ECAM_BASE_HI);
> >
> > -	/* Get bus range */
> > -	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
> > -	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >>
> E_ECAM_SIZE_SHIFT;
> > -	/* Write primary, secondary and subordinate bus numbers */
> > -	ecam_val = first_busno;
> > -	ecam_val |= (first_busno + 1) << 8;
> > -	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
> > -	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
> > -
> >  	if (nwl_pcie_link_up(pcie))
> >  		dev_info(dev, "Link is UP\n");
> >  	else
> > @@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device
> *pdev)
> >  	pcie = pci_host_bridge_priv(bridge);
> >
> >  	pcie->dev = dev;
> > -	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
> >
> >  	err = nwl_pcie_parse_dt(pcie, pdev);
> >  	if (err) {
> > --
> > 1.8.3.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
@ 2023-08-08 10:37       ` Havalige, Thippeswamy
  0 siblings, 0 replies; 14+ messages in thread
From: Havalige, Thippeswamy @ 2023-08-08 10:37 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree, lpieralisi, Gogada, Bharat Kumar, Simek, Michal,
	linux-arm-kernel

Hi Bjorn,

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@kernel.org>
> Sent: Tuesday, August 8, 2023 2:53 AM
> To: Havalige, Thippeswamy <thippeswamy.havalige@amd.com>
> Cc: linux-kernel@vger.kernel.org; robh+dt@kernel.org;
> bhelgaas@google.com; krzysztof.kozlowski@linaro.org; linux-
> pci@vger.kernel.org; devicetree@vger.kernel.org; lpieralisi@kernel.org;
> Gogada, Bharat Kumar <bharat.kumar.gogada@amd.com>; Simek, Michal
> <michal.simek@amd.com>; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and
> remove unnecessary code.
> 
> Ideally the subject would include useful information about *why* you're
> changing the ECAM value.  Drop the period at the end of the subject.  E.g.,
> something like:
> 
>   PCI: xilinx-nwl: Increase ECAM size to accommodate 256 buses
- Agreed, I ll update in next patch.
> On Mon, Aug 07, 2023 at 04:37:32PM +0530, Thippeswamy Havalige wrote:
> > Our controller is expecting ECAM size to be programmed by software.
> > By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access
> up
> > to 16MB ECAM region which is used to detect 16 buses, so by updating
> > "NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to
> > 256MB ECAM region to detect 256 buses.
> Rob needs to ack this because it sounds like this change might make the
> driver incompatible with DTs in the field, i.e., the user might be forced to
> update the DT at the same time as picking up this driver change.
> 
> > E_ECAM_CONTROL register from bit 16 to 20 uses this value as input to
> > calculate ECAM Size.
> >
> > The primary,secondary and sub-ordinate bus number registers are
> > updated by Linux PCI core, so removing code which is updating
> > primary,secondary and sub-ordinate bus numbers of type 1 header 18th
> offset of ECAM.
> 
> This code removal sounds like a separate logical change that could be a
> separate patch.
> 
> s/primary,secondary/primary, secondary/ (twice) s/removing/remove/
- Agreed, I ll update in next patch.
> > Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> > ---
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes:
> > | https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
> > | com/
> > ---
> >  drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
> >  1 file changed, 3 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> > b/drivers/pci/controller/pcie-xilinx-nwl.c
> > index 176686b..b515019 100644
> > --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> > +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> > @@ -126,7 +126,7 @@
> >  #define E_ECAM_CR_ENABLE		BIT(0)
> >  #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
> >  #define E_ECAM_SIZE_SHIFT		16
> > -#define NWL_ECAM_VALUE_DEFAULT		12
> > +#define NWL_ECAM_VALUE_DEFAULT		16
> >
> >  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
> >  #define CFG_PCIE_CACHE			GENMASK(7, 0)
> > @@ -165,8 +165,6 @@ struct nwl_pcie {
> >  	u32 ecam_size;
> >  	int irq_intx;
> >  	int irq_misc;
> > -	u32 ecam_value;
> > -	u8 last_busno;
> >  	struct nwl_msi msi;
> >  	struct irq_domain *legacy_irq_domain;
> >  	struct clk *clk;
> > @@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie
> > *pcie)  {
> >  	struct device *dev = pcie->dev;
> >  	struct platform_device *pdev = to_platform_device(dev);
> > -	u32 breg_val, ecam_val, first_busno = 0;
> > +	u32 breg_val, ecam_val;
> >  	int err;
> >
> >  	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) &
> > BREG_PRESENT; @@ -675,7 +673,7 @@ static int
> nwl_pcie_bridge_init(struct nwl_pcie *pcie)
> >  			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
> >
> >  	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
> > -			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
> > +			  (NWL_ECAM_VALUE_DEFAULT <<
> E_ECAM_SIZE_SHIFT),
> >  			  E_ECAM_CONTROL);
> >
> >  	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
> > @@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie
> *pcie)
> >  	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
> >  			  E_ECAM_BASE_HI);
> >
> > -	/* Get bus range */
> > -	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
> > -	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >>
> E_ECAM_SIZE_SHIFT;
> > -	/* Write primary, secondary and subordinate bus numbers */
> > -	ecam_val = first_busno;
> > -	ecam_val |= (first_busno + 1) << 8;
> > -	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
> > -	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
> > -
> >  	if (nwl_pcie_link_up(pcie))
> >  		dev_info(dev, "Link is UP\n");
> >  	else
> > @@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device
> *pdev)
> >  	pcie = pci_host_bridge_priv(bridge);
> >
> >  	pcie->dev = dev;
> > -	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
> >
> >  	err = nwl_pcie_parse_dt(pcie, pdev);
> >  	if (err) {
> > --
> > 1.8.3.1
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
  2023-08-07 11:04   ` Thippeswamy Havalige
@ 2023-08-07 11:14     ` Havalige, Thippeswamy
  -1 siblings, 0 replies; 14+ messages in thread
From: Havalige, Thippeswamy @ 2023-08-07 11:14 UTC (permalink / raw)
  To: Havalige, Thippeswamy, linux-kernel, robh+dt, bhelgaas,
	krzysztof.kozlowski, linux-pci, devicetree
  Cc: lpieralisi, Gogada, Bharat Kumar, Simek, Michal, linux-arm-kernel

Ignore this, as I missed adding one patch

> -----Original Message-----
> From: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Sent: Monday, August 7, 2023 4:35 PM
> To: linux-kernel@vger.kernel.org; robh+dt@kernel.org;
> bhelgaas@google.com; krzysztof.kozlowski@linaro.org; linux-
> pci@vger.kernel.org; devicetree@vger.kernel.org
> Cc: lpieralisi@kernel.org; Gogada, Bharat Kumar
> <bharat.kumar.gogada@amd.com>; Simek, Michal
> <michal.simek@amd.com>; linux-arm-kernel@lists.infradead.org; Havalige,
> Thippeswamy <thippeswamy.havalige@amd.com>
> Subject: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and
> remove unnecessary code.
> 
> Our controller is expecting ECAM size to be programmed by software.
> By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access up
> to 16MB ECAM region which is used to detect 16 buses, so by updating
> "NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to
> 256MB ECAM region to detect 256 buses.
> 
> E_ECAM_CONTROL register from bit 16 to 20 uses this value as input to
> calculate ECAM Size.
> 
> The primary,secondary and sub-ordinate bus number registers are updated
> by Linux PCI core, so removing code which is updating primary,secondary and
> sub-ordinate bus numbers of type 1 header 18th offset of ECAM.
> 
> Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> ---
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes:
> | https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
> | com/
> ---
>  drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 176686b..b515019 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -126,7 +126,7 @@
>  #define E_ECAM_CR_ENABLE		BIT(0)
>  #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
>  #define E_ECAM_SIZE_SHIFT		16
> -#define NWL_ECAM_VALUE_DEFAULT		12
> +#define NWL_ECAM_VALUE_DEFAULT		16
> 
>  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
>  #define CFG_PCIE_CACHE			GENMASK(7, 0)
> @@ -165,8 +165,6 @@ struct nwl_pcie {
>  	u32 ecam_size;
>  	int irq_intx;
>  	int irq_misc;
> -	u32 ecam_value;
> -	u8 last_busno;
>  	struct nwl_msi msi;
>  	struct irq_domain *legacy_irq_domain;
>  	struct clk *clk;
> @@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)  {
>  	struct device *dev = pcie->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
> -	u32 breg_val, ecam_val, first_busno = 0;
> +	u32 breg_val, ecam_val;
>  	int err;
> 
>  	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) &
> BREG_PRESENT; @@ -675,7 +673,7 @@ static int nwl_pcie_bridge_init(struct
> nwl_pcie *pcie)
>  			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
> 
>  	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
> -			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
> +			  (NWL_ECAM_VALUE_DEFAULT <<
> E_ECAM_SIZE_SHIFT),
>  			  E_ECAM_CONTROL);
> 
>  	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
> @@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
>  			  E_ECAM_BASE_HI);
> 
> -	/* Get bus range */
> -	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
> -	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >>
> E_ECAM_SIZE_SHIFT;
> -	/* Write primary, secondary and subordinate bus numbers */
> -	ecam_val = first_busno;
> -	ecam_val |= (first_busno + 1) << 8;
> -	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
> -	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
> -
>  	if (nwl_pcie_link_up(pcie))
>  		dev_info(dev, "Link is UP\n");
>  	else
> @@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device
> *pdev)
>  	pcie = pci_host_bridge_priv(bridge);
> 
>  	pcie->dev = dev;
> -	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
> 
>  	err = nwl_pcie_parse_dt(pcie, pdev);
>  	if (err) {
> --
> 1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
@ 2023-08-07 11:14     ` Havalige, Thippeswamy
  0 siblings, 0 replies; 14+ messages in thread
From: Havalige, Thippeswamy @ 2023-08-07 11:14 UTC (permalink / raw)
  To: Havalige, Thippeswamy, linux-kernel, robh+dt, bhelgaas,
	krzysztof.kozlowski, linux-pci, devicetree
  Cc: lpieralisi, Gogada, Bharat Kumar, Simek, Michal, linux-arm-kernel

Ignore this, as I missed adding one patch

> -----Original Message-----
> From: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Sent: Monday, August 7, 2023 4:35 PM
> To: linux-kernel@vger.kernel.org; robh+dt@kernel.org;
> bhelgaas@google.com; krzysztof.kozlowski@linaro.org; linux-
> pci@vger.kernel.org; devicetree@vger.kernel.org
> Cc: lpieralisi@kernel.org; Gogada, Bharat Kumar
> <bharat.kumar.gogada@amd.com>; Simek, Michal
> <michal.simek@amd.com>; linux-arm-kernel@lists.infradead.org; Havalige,
> Thippeswamy <thippeswamy.havalige@amd.com>
> Subject: [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and
> remove unnecessary code.
> 
> Our controller is expecting ECAM size to be programmed by software.
> By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access up
> to 16MB ECAM region which is used to detect 16 buses, so by updating
> "NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to
> 256MB ECAM region to detect 256 buses.
> 
> E_ECAM_CONTROL register from bit 16 to 20 uses this value as input to
> calculate ECAM Size.
> 
> The primary,secondary and sub-ordinate bus number registers are updated
> by Linux PCI core, so removing code which is updating primary,secondary and
> sub-ordinate bus numbers of type 1 header 18th offset of ECAM.
> 
> Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> ---
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes:
> | https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
> | com/
> ---
>  drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 176686b..b515019 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -126,7 +126,7 @@
>  #define E_ECAM_CR_ENABLE		BIT(0)
>  #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
>  #define E_ECAM_SIZE_SHIFT		16
> -#define NWL_ECAM_VALUE_DEFAULT		12
> +#define NWL_ECAM_VALUE_DEFAULT		16
> 
>  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
>  #define CFG_PCIE_CACHE			GENMASK(7, 0)
> @@ -165,8 +165,6 @@ struct nwl_pcie {
>  	u32 ecam_size;
>  	int irq_intx;
>  	int irq_misc;
> -	u32 ecam_value;
> -	u8 last_busno;
>  	struct nwl_msi msi;
>  	struct irq_domain *legacy_irq_domain;
>  	struct clk *clk;
> @@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)  {
>  	struct device *dev = pcie->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
> -	u32 breg_val, ecam_val, first_busno = 0;
> +	u32 breg_val, ecam_val;
>  	int err;
> 
>  	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) &
> BREG_PRESENT; @@ -675,7 +673,7 @@ static int nwl_pcie_bridge_init(struct
> nwl_pcie *pcie)
>  			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
> 
>  	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
> -			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
> +			  (NWL_ECAM_VALUE_DEFAULT <<
> E_ECAM_SIZE_SHIFT),
>  			  E_ECAM_CONTROL);
> 
>  	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
> @@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
>  			  E_ECAM_BASE_HI);
> 
> -	/* Get bus range */
> -	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
> -	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >>
> E_ECAM_SIZE_SHIFT;
> -	/* Write primary, secondary and subordinate bus numbers */
> -	ecam_val = first_busno;
> -	ecam_val |= (first_busno + 1) << 8;
> -	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
> -	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
> -
>  	if (nwl_pcie_link_up(pcie))
>  		dev_info(dev, "Link is UP\n");
>  	else
> @@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device
> *pdev)
>  	pcie = pci_host_bridge_priv(bridge);
> 
>  	pcie->dev = dev;
> -	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
> 
>  	err = nwl_pcie_parse_dt(pcie, pdev);
>  	if (err) {
> --
> 1.8.3.1


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

* [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
  2023-08-07 11:04 [PATCH v1 0/2] Fix ecam size value to discover 256 buses during Thippeswamy Havalige
@ 2023-08-07 11:04   ` Thippeswamy Havalige
  0 siblings, 0 replies; 14+ messages in thread
From: Thippeswamy Havalige @ 2023-08-07 11:04 UTC (permalink / raw)
  To: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree
  Cc: lpieralisi, bharat.kumar.gogada, michal.simek, linux-arm-kernel,
	Thippeswamy Havalige

Our controller is expecting ECAM size to be programmed by software.
By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access up to
16MB ECAM region which is used to detect 16 buses, so by updating
"NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to 256MB
ECAM region to detect 256 buses.

E_ECAM_CONTROL register from bit 16 to 20 uses this value as input
to calculate ECAM Size.

The primary,secondary and sub-ordinate bus number registers are updated
by Linux PCI core, so removing code which is updating primary,secondary
and sub-ordinate bus numbers of type 1 header 18th offset of ECAM.

Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
---
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: 
| https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
| com/
---
 drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 176686b..b515019 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -126,7 +126,7 @@
 #define E_ECAM_CR_ENABLE		BIT(0)
 #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
 #define E_ECAM_SIZE_SHIFT		16
-#define NWL_ECAM_VALUE_DEFAULT		12
+#define NWL_ECAM_VALUE_DEFAULT		16
 
 #define CFG_DMA_REG_BAR			GENMASK(2, 0)
 #define CFG_PCIE_CACHE			GENMASK(7, 0)
@@ -165,8 +165,6 @@ struct nwl_pcie {
 	u32 ecam_size;
 	int irq_intx;
 	int irq_misc;
-	u32 ecam_value;
-	u8 last_busno;
 	struct nwl_msi msi;
 	struct irq_domain *legacy_irq_domain;
 	struct clk *clk;
@@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
 	struct platform_device *pdev = to_platform_device(dev);
-	u32 breg_val, ecam_val, first_busno = 0;
+	u32 breg_val, ecam_val;
 	int err;
 
 	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) & BREG_PRESENT;
@@ -675,7 +673,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
 
 	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
-			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
+			  (NWL_ECAM_VALUE_DEFAULT << E_ECAM_SIZE_SHIFT),
 			  E_ECAM_CONTROL);
 
 	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
@@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
 			  E_ECAM_BASE_HI);
 
-	/* Get bus range */
-	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
-	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >> E_ECAM_SIZE_SHIFT;
-	/* Write primary, secondary and subordinate bus numbers */
-	ecam_val = first_busno;
-	ecam_val |= (first_busno + 1) << 8;
-	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
-	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
-
 	if (nwl_pcie_link_up(pcie))
 		dev_info(dev, "Link is UP\n");
 	else
@@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 	pcie = pci_host_bridge_priv(bridge);
 
 	pcie->dev = dev;
-	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
 
 	err = nwl_pcie_parse_dt(pcie, pdev);
 	if (err) {
-- 
1.8.3.1


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

* [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code.
@ 2023-08-07 11:04   ` Thippeswamy Havalige
  0 siblings, 0 replies; 14+ messages in thread
From: Thippeswamy Havalige @ 2023-08-07 11:04 UTC (permalink / raw)
  To: linux-kernel, robh+dt, bhelgaas, krzysztof.kozlowski, linux-pci,
	devicetree
  Cc: lpieralisi, bharat.kumar.gogada, michal.simek, linux-arm-kernel,
	Thippeswamy Havalige

Our controller is expecting ECAM size to be programmed by software.
By programming "NWL_ECAM_VALUE_DEFAULT  12" controller can access up to
16MB ECAM region which is used to detect 16 buses, so by updating
"NWL_ECAM_VALUE_DEFAULT " to 16 so that controller can access up to 256MB
ECAM region to detect 256 buses.

E_ECAM_CONTROL register from bit 16 to 20 uses this value as input
to calculate ECAM Size.

The primary,secondary and sub-ordinate bus number registers are updated
by Linux PCI core, so removing code which is updating primary,secondary
and sub-ordinate bus numbers of type 1 header 18th offset of ECAM.

Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
---
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: 
| https://lore.kernel.org/oe-kbuild-all/202308040330.eMTjX3tF-lkp@intel.
| com/
---
 drivers/pci/controller/pcie-xilinx-nwl.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 176686b..b515019 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -126,7 +126,7 @@
 #define E_ECAM_CR_ENABLE		BIT(0)
 #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
 #define E_ECAM_SIZE_SHIFT		16
-#define NWL_ECAM_VALUE_DEFAULT		12
+#define NWL_ECAM_VALUE_DEFAULT		16
 
 #define CFG_DMA_REG_BAR			GENMASK(2, 0)
 #define CFG_PCIE_CACHE			GENMASK(7, 0)
@@ -165,8 +165,6 @@ struct nwl_pcie {
 	u32 ecam_size;
 	int irq_intx;
 	int irq_misc;
-	u32 ecam_value;
-	u8 last_busno;
 	struct nwl_msi msi;
 	struct irq_domain *legacy_irq_domain;
 	struct clk *clk;
@@ -625,7 +623,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
 	struct platform_device *pdev = to_platform_device(dev);
-	u32 breg_val, ecam_val, first_busno = 0;
+	u32 breg_val, ecam_val;
 	int err;
 
 	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) & BREG_PRESENT;
@@ -675,7 +673,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 			  E_ECAM_CR_ENABLE, E_ECAM_CONTROL);
 
 	nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, E_ECAM_CONTROL) |
-			  (pcie->ecam_value << E_ECAM_SIZE_SHIFT),
+			  (NWL_ECAM_VALUE_DEFAULT << E_ECAM_SIZE_SHIFT),
 			  E_ECAM_CONTROL);
 
 	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_ecam_base),
@@ -683,15 +681,6 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
 	nwl_bridge_writel(pcie, upper_32_bits(pcie->phys_ecam_base),
 			  E_ECAM_BASE_HI);
 
-	/* Get bus range */
-	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CONTROL);
-	pcie->last_busno = (ecam_val & E_ECAM_SIZE_LOC) >> E_ECAM_SIZE_SHIFT;
-	/* Write primary, secondary and subordinate bus numbers */
-	ecam_val = first_busno;
-	ecam_val |= (first_busno + 1) << 8;
-	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
-	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
-
 	if (nwl_pcie_link_up(pcie))
 		dev_info(dev, "Link is UP\n");
 	else
@@ -792,7 +781,6 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 	pcie = pci_host_bridge_priv(bridge);
 
 	pcie->dev = dev;
-	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
 
 	err = nwl_pcie_parse_dt(pcie, pdev);
 	if (err) {
-- 
1.8.3.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2023-08-08 16:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-07 11:07 [PATCH v1 0/2] Fix ecam size value to discover 256 buses during Thippeswamy Havalige
2023-08-07 11:07 ` Thippeswamy Havalige
2023-08-07 11:07 ` [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code Thippeswamy Havalige
2023-08-07 11:07   ` Thippeswamy Havalige
2023-08-07 21:23   ` Bjorn Helgaas
2023-08-07 21:23     ` Bjorn Helgaas
2023-08-08 10:37     ` Havalige, Thippeswamy
2023-08-08 10:37       ` Havalige, Thippeswamy
2023-08-07 11:07 ` [PATCH v1 2/2] dt-bindings: PCI: xilinx-nwl: Modify ECAM size in example Thippeswamy Havalige
2023-08-07 11:07   ` Thippeswamy Havalige
  -- strict thread matches above, loose matches on Subject: below --
2023-08-07 11:04 [PATCH v1 0/2] Fix ecam size value to discover 256 buses during Thippeswamy Havalige
2023-08-07 11:04 ` [PATCH v1 1/2] PCI: xilinx-nwl: Update ECAM default value and remove unnecessary code Thippeswamy Havalige
2023-08-07 11:04   ` Thippeswamy Havalige
2023-08-07 11:14   ` Havalige, Thippeswamy
2023-08-07 11:14     ` Havalige, Thippeswamy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.