drivers/pci/controller/pcie-xilinx-nwl.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
The primary,secondary and sub-ordinate bus number registers are updated by
Linux PCI core, so remove code which updates respective fields of type 1
header.
Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
---
changes in v3:
- Remove unnecessary period at end of subject line.
- Updated commit message.
changes in v2:
- Code increasing ECAM Size value is added into a seperate patch.
- Modified commit messages.
changes in v1:
- Modified commit messages.
---
drivers/pci/controller/pcie-xilinx-nwl.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 176686bdb15c..d8a3a08be1d5 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -166,7 +166,6 @@ struct nwl_pcie {
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 +624,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;
@@ -683,15 +682,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
--
2.17.1Current 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. Update ecam size to 256MB in device tree binding example. Thippeswamy Havalige (2): dt-bindings: PCI: xilinx-nwl: Modify ECAM size in example PCI: xilinx-nwl: Increase ECAM size to accommodate 256 buses Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 2 +- drivers/pci/controller/pcie-xilinx-nwl.c | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) -- 2.17.1
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>
---
changes in v2:
None.
changes in v1:
None.
---
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 897602559b37..426f90a47f35 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>;
--
2.17.1On Thu, 10 Aug 2023 17:50:01 +0530, Thippeswamy Havalige wrote: > 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> > --- > changes in v2: > None. > changes in v1: > None. > --- > Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Acked-by: Rob Herring <robh@kernel.org>
On Thu, 10 Aug 2023 17:50:01 +0530, Thippeswamy Havalige wrote: > 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> > --- > changes in v2: > None. > changes in v1: > None. > --- > Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230810122002.133531-3-thippeswamy.havalige@amd.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
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.
Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
---
changes in v3:
- Remove unnecessary period at end of subject line.
changes in v2:
- Update this changes in a seperate patch.
---
drivers/pci/controller/pcie-xilinx-nwl.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index d8a3a08be1d5..b51501921d3b 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,7 +165,6 @@ struct nwl_pcie {
u32 ecam_size;
int irq_intx;
int irq_misc;
- u32 ecam_value;
struct nwl_msi msi;
struct irq_domain *legacy_irq_domain;
struct clk *clk;
@@ -674,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),
@@ -782,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) {
--
2.17.1On Thu, Aug 10, 2023 at 05:50:02PM +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.
What happens when your DT has the smaller size and the kernel configures
the larger size? Seems like you could have an ABI issue.
>
> Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> ---
> changes in v3:
> - Remove unnecessary period at end of subject line.
> changes in v2:
> - Update this changes in a seperate patch.
> ---
> drivers/pci/controller/pcie-xilinx-nwl.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index d8a3a08be1d5..b51501921d3b 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
Not really a meaningful name. It doesn't explain what '16' means.
> #define CFG_DMA_REG_BAR GENMASK(2, 0)
> #define CFG_PCIE_CACHE GENMASK(7, 0)
> @@ -165,7 +165,6 @@ struct nwl_pcie {
> u32 ecam_size;
> int irq_intx;
> int irq_misc;
> - u32 ecam_value;
> struct nwl_msi msi;
> struct irq_domain *legacy_irq_domain;
> struct clk *clk;
> @@ -674,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),
> @@ -782,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) {
> --
> 2.17.1
>
Hi Rob,
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Friday, August 11, 2023 2:47 AM
> To: Havalige, Thippeswamy <thippeswamy.havalige@amd.com>
> Cc: linux-kernel@vger.kernel.org; bhelgaas@google.com;
> krzysztof.kozlowski@linaro.org; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; conor+dt@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 v3 2/2] PCI: xilinx-nwl: Increase ECAM size to
> accommodate 256 buses
>
> On Thu, Aug 10, 2023 at 05:50:02PM +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.
>
> What happens when your DT has the smaller size and the kernel configures
> the larger size? Seems like you could have an ABI issue.
- Here we are enabling hardware to support maximum buses. In this case kernel can enumerate up to device tree exposed ECAM size.
We will not face any issue.
> >
> > Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> > ---
> > changes in v3:
> > - Remove unnecessary period at end of subject line.
> > changes in v2:
> > - Update this changes in a seperate patch.
> > ---
> > drivers/pci/controller/pcie-xilinx-nwl.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> > b/drivers/pci/controller/pcie-xilinx-nwl.c
> > index d8a3a08be1d5..b51501921d3b 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
- Agreed, ll fix it in next patch.
> Not really a meaningful name. It doesn't explain what '16' means.
>
> > #define CFG_DMA_REG_BAR GENMASK(2, 0)
> > #define CFG_PCIE_CACHE GENMASK(7, 0)
> > @@ -165,7 +165,6 @@ struct nwl_pcie {
> > u32 ecam_size;
> > int irq_intx;
> > int irq_misc;
> > - u32 ecam_value;
> > struct nwl_msi msi;
> > struct irq_domain *legacy_irq_domain;
> > struct clk *clk;
> > @@ -674,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),
> > @@ -782,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) {
> > --
> > 2.17.1
> >
On Fri, Aug 11, 2023 at 05:07:09AM +0000, Havalige, Thippeswamy wrote:
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > On Thu, Aug 10, 2023 at 05:50:02PM +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.
> >
> > What happens when your DT has the smaller size and the kernel configures
> > the larger size? Seems like you could have an ABI issue.
>
> - Here we are enabling hardware to support maximum buses. In this
> case kernel can enumerate up to device tree exposed ECAM size. We
> will not face any issue.
So IIUC, if you have a DT with the smaller size and you boot a kernel
that includes this change, nothing will break, but the kernel will
only be able to use 16 buses.
Conversely, if you have a DT with the larger size and boot a kernel
that does not include change, nothing will break, but the kernel will
still only be able to use 16 buses.
Probably worth capturing this in the commit log somehow, especially
the first case.
> > > Signed-off-by: Thippeswamy Havalige <thippeswamy.havalige@amd.com>
> > > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@amd.com>
> > > ---
> > > changes in v3:
> > > - Remove unnecessary period at end of subject line.
> > > changes in v2:
> > > - Update this changes in a seperate patch.
> > > ---
> > > drivers/pci/controller/pcie-xilinx-nwl.c | 6 ++----
> > > 1 file changed, 2 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> > > b/drivers/pci/controller/pcie-xilinx-nwl.c
> > > index d8a3a08be1d5..b51501921d3b 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
> - Agreed, ll fix it in next patch.
> > Not really a meaningful name. It doesn't explain what '16' means.
> >
> > > #define CFG_DMA_REG_BAR GENMASK(2, 0)
> > > #define CFG_PCIE_CACHE GENMASK(7, 0)
> > > @@ -165,7 +165,6 @@ struct nwl_pcie {
> > > u32 ecam_size;
> > > int irq_intx;
> > > int irq_misc;
> > > - u32 ecam_value;
> > > struct nwl_msi msi;
> > > struct irq_domain *legacy_irq_domain;
> > > struct clk *clk;
> > > @@ -674,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),
> > > @@ -782,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) {
> > > --
> > > 2.17.1
> > >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
© 2016 - 2025 Red Hat, Inc.