diff mbox series

[v4,09/18] PCI: dwc: Discard IP-core version checking on unrolled iATU detection

Message ID 20220610082535.12802-10-Sergey.Semin@baikalelectronics.ru
State New
Headers show
Series PCI: dwc: Various fixes and cleanups | expand

Commit Message

Serge Semin June 10, 2022, 8:25 a.m. UTC
It's pretty much pointless. Even though unrolled version of the internal
ATU has been indeed available since DWC PCIe v4.80a IP-core, there is no
guarantee it was enabled during the IP-core configuration (Synopsys
suggests to contact the Solvnet support for guidance of how to do that for
the newer IP-cores). So the only reliable way to find out the unrolled
iATU feature availability is indeed to check the iATU viewport register
content. In accordance with the reference manual [1] if the register
doesn't exist (unrolled iATU is enabled) it's content is fixed with
0xff-s, otherwise it will contain some zeros. So we can freely drop the
IP-core version checking in this matter then and use the
dw_pcie_iatu_unroll_enabled() method only to detect whether iATU/eDMA
space is unrolled.

[1] DesignWare Cores, PCI Express Controller, Register Desciptions,
v.4.90a, December 2016, p.855

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 drivers/pci/controller/dwc/pcie-designware.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Rob Herring (Arm) June 13, 2022, 8:20 p.m. UTC | #1
On Fri, Jun 10, 2022 at 11:25:25AM +0300, Serge Semin wrote:
> It's pretty much pointless. Even though unrolled version of the internal
> ATU has been indeed available since DWC PCIe v4.80a IP-core, there is no
> guarantee it was enabled during the IP-core configuration (Synopsys
> suggests to contact the Solvnet support for guidance of how to do that for
> the newer IP-cores). So the only reliable way to find out the unrolled
> iATU feature availability is indeed to check the iATU viewport register
> content. In accordance with the reference manual [1] if the register
> doesn't exist (unrolled iATU is enabled) it's content is fixed with
> 0xff-s, otherwise it will contain some zeros. So we can freely drop the
> IP-core version checking in this matter then and use the
> dw_pcie_iatu_unroll_enabled() method only to detect whether iATU/eDMA
> space is unrolled.

Are you sure that pre v4.80a, it is safe to read the register address? 
Seems unlikely that the all 1s guarantee would be valid before the 
feature ever existed. 


> [1] DesignWare Cores, PCI Express Controller, Register Desciptions,
> v.4.90a, December 2016, p.855
> 
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  drivers/pci/controller/dwc/pcie-designware.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

Assuming this works,

Reviewed-by: Rob Herring <robh@kernel.org>
Serge Semin June 14, 2022, 8:14 p.m. UTC | #2
On Mon, Jun 13, 2022 at 02:20:47PM -0600, Rob Herring wrote:
> On Fri, Jun 10, 2022 at 11:25:25AM +0300, Serge Semin wrote:
> > It's pretty much pointless. Even though unrolled version of the internal
> > ATU has been indeed available since DWC PCIe v4.80a IP-core, there is no
> > guarantee it was enabled during the IP-core configuration (Synopsys
> > suggests to contact the Solvnet support for guidance of how to do that for
> > the newer IP-cores). So the only reliable way to find out the unrolled
> > iATU feature availability is indeed to check the iATU viewport register
> > content. In accordance with the reference manual [1] if the register
> > doesn't exist (unrolled iATU is enabled) it's content is fixed with
> > 0xff-s, otherwise it will contain some zeros. So we can freely drop the
> > IP-core version checking in this matter then and use the
> > dw_pcie_iatu_unroll_enabled() method only to detect whether iATU/eDMA
> > space is unrolled.
> 

> Are you sure that pre v4.80a, it is safe to read the register address? 

v4.60a ref manual says that the register exists in case if
(!CX_PL_REG_DISABLE && CX_INTERNAL_ATU_ENABLE)
and no word regarding all 1s. Most likely it shall have all zeros by
default and if there is no iATU available.

> Seems unlikely that the all 1s guarantee would be valid before the 
> feature ever existed. 

Right, all 1s is the marker of the unrolled iATU mapping, which has
been available since v4.80a. So before that version we were never
supposed to meet all 1s in that registers.

> 
> 
> > [1] DesignWare Cores, PCI Express Controller, Register Desciptions,
> > v.4.90a, December 2016, p.855
> > 
> > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Tested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware.c | 11 +++++------
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> Assuming this works,
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

Thanks.

-Sergey
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index ac966ed28c5b..e5c30695f664 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -600,15 +600,15 @@  static void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
 
 }
 
-static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
+static bool dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
 {
 	u32 val;
 
 	val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT);
 	if (val == 0xffffffff)
-		return 1;
+		return true;
 
-	return 0;
+	return false;
 }
 
 static void dw_pcie_iatu_detect_regions_unroll(struct dw_pcie *pci)
@@ -680,9 +680,8 @@  void dw_pcie_iatu_detect(struct dw_pcie *pci)
 	struct device *dev = pci->dev;
 	struct platform_device *pdev = to_platform_device(dev);
 
-	if (pci->version >= 0x480A || (!pci->version &&
-				       dw_pcie_iatu_unroll_enabled(pci))) {
-		pci->iatu_unroll_enabled = true;
+	pci->iatu_unroll_enabled = dw_pcie_iatu_unroll_enabled(pci);
+	if (pci->iatu_unroll_enabled) {
 		if (!pci->atu_base) {
 			struct resource *res =
 				platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");