[PATCH v2 0/4] PCI: xgene: Cleanups

[PATCH v2 0/4] PCI: xgene: Cleanups

[Bjorn Helgaas]

  - Add local "dev" pointers to reduce repetition of things like
    "&pdev->dev".

  - Remove platform drvdata because it appears unused (we called
    platform_set_drvdata() but not platform_get_drvdata()).

  - Pass the struct xgene_pcie_port pointer, not addresses, to setup
    functions to enable simplifications.

  - Add register accessors to encapsulate usage of port->csr_base a bit.

Nothing here should change the behavior of the driver.

Changes from v1:
  I dropped the following patch because it was a lot of churn for
  questionable benefit:
    PCI: xgene: Name private struct pointer "xgene" consistently

---

Bjorn Helgaas (4):
      PCI: xgene: Add local struct device pointers
      PCI: xgene: Remove unused platform data
      PCI: xgene: Pass struct xgene_pcie_port to setup functions
      PCI: xgene: Add register accessors


 drivers/pci/host/pci-xgene.c |  151 +++++++++++++++++++++++-------------------
 1 file changed, 81 insertions(+), 70 deletions(-)

[PATCH v2 1/4] PCI: xgene: Add local struct device pointers

[Bjorn Helgaas]

Use a local "struct device *dev" for brevity and consistency with other
drivers.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-xgene.c |   43 +++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index a81273c..03d24e7 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -223,17 +223,18 @@ static void xgene_pcie_linkup(struct xgene_pcie_port *port,
 
 static int xgene_pcie_init_port(struct xgene_pcie_port *port)
 {
+	struct device *dev = port->dev;
 	int rc;
 
-	port->clk = clk_get(port->dev, NULL);
+	port->clk = clk_get(dev, NULL);
 	if (IS_ERR(port->clk)) {
-		dev_err(port->dev, "clock not available\n");
+		dev_err(dev, "clock not available\n");
 		return -ENODEV;
 	}
 
 	rc = clk_prepare_enable(port->clk);
 	if (rc) {
-		dev_err(port->dev, "clock enable failed\n");
+		dev_err(dev, "clock enable failed\n");
 		return rc;
 	}
 
@@ -243,15 +244,16 @@ static int xgene_pcie_init_port(struct xgene_pcie_port *port)
 static int xgene_pcie_map_reg(struct xgene_pcie_port *port,
 			      struct platform_device *pdev)
 {
+	struct device *dev = port->dev;
 	struct resource *res;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "csr");
-	port->csr_base = devm_ioremap_resource(port->dev, res);
+	port->csr_base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(port->csr_base))
 		return PTR_ERR(port->csr_base);
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg");
-	port->cfg_base = devm_ioremap_resource(port->dev, res);
+	port->cfg_base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(port->cfg_base))
 		return PTR_ERR(port->cfg_base);
 	port->cfg_addr = res->start;
@@ -264,6 +266,7 @@ static void xgene_pcie_setup_ob_reg(struct xgene_pcie_port *port,
 				    u64 cpu_addr, u64 pci_addr)
 {
 	void __iomem *base = port->csr_base + offset;
+	struct device *dev = port->dev;
 	resource_size_t size = resource_size(res);
 	u64 restype = resource_type(res);
 	u64 mask = 0;
@@ -280,7 +283,7 @@ static void xgene_pcie_setup_ob_reg(struct xgene_pcie_port *port,
 	if (size >= min_size)
 		mask = ~(size - 1) | flag;
 	else
-		dev_warn(port->dev, "res size 0x%llx less than minimum 0x%x\n",
+		dev_warn(dev, "res size 0x%llx less than minimum 0x%x\n",
 			 (u64)size, min_size);
 
 	writel(lower_32_bits(cpu_addr), base);
@@ -310,7 +313,7 @@ static int xgene_pcie_map_ranges(struct xgene_pcie_port *port,
 		struct resource *res = window->res;
 		u64 restype = resource_type(res);
 
-		dev_dbg(port->dev, "%pR\n", res);
+		dev_dbg(dev, "%pR\n", res);
 
 		switch (restype) {
 		case IORESOURCE_IO:
@@ -381,6 +384,7 @@ static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
 {
 	void __iomem *csr_base = port->csr_base;
 	void __iomem *cfg_base = port->cfg_base;
+	struct device *dev = port->dev;
 	void *bar_addr;
 	void *pim_addr;
 	u64 cpu_addr = range->cpu_addr;
@@ -393,7 +397,7 @@ static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
 
 	region = xgene_pcie_select_ib_reg(ib_reg_mask, range->size);
 	if (region < 0) {
-		dev_warn(port->dev, "invalid pcie dma-range config\n");
+		dev_warn(dev, "invalid pcie dma-range config\n");
 		return;
 	}
 
@@ -463,7 +467,7 @@ static int xgene_pcie_parse_map_dma_ranges(struct xgene_pcie_port *port)
 	for_each_of_pci_range(&parser, &range) {
 		u64 end = range.cpu_addr + range.size - 1;
 
-		dev_dbg(port->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
+		dev_dbg(dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n",
 			range.flags, range.cpu_addr, end, range.pci_addr);
 		xgene_pcie_setup_ib_reg(port, &range, &ib_reg_mask);
 	}
@@ -483,6 +487,7 @@ static int xgene_pcie_setup(struct xgene_pcie_port *port,
 			    struct list_head *res,
 			    resource_size_t io_base)
 {
+	struct device *dev = port->dev;
 	u32 val, lanes = 0, speed = 0;
 	int ret;
 
@@ -502,27 +507,28 @@ static int xgene_pcie_setup(struct xgene_pcie_port *port,
 
 	xgene_pcie_linkup(port, &lanes, &speed);
 	if (!port->link_up)
-		dev_info(port->dev, "(rc) link down\n");
+		dev_info(dev, "(rc) link down\n");
 	else
-		dev_info(port->dev, "(rc) x%d gen-%d link up\n",
-				lanes, speed + 1);
+		dev_info(dev, "(rc) x%d gen-%d link up\n", lanes, speed + 1);
 	return 0;
 }
 
 static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 {
-	struct device_node *dn = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct device_node *dn = dev->of_node;
 	struct xgene_pcie_port *port;
 	resource_size_t iobase = 0;
 	struct pci_bus *bus;
 	int ret;
 	LIST_HEAD(res);
 
-	port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
+	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
 	if (!port)
 		return -ENOMEM;
-	port->node = of_node_get(pdev->dev.of_node);
-	port->dev = &pdev->dev;
+
+	port->node = of_node_get(dn);
+	port->dev = dev;
 
 	port->version = XGENE_PCIE_IP_VER_UNKN;
 	if (of_device_is_compatible(port->node, "apm,xgene-pcie"))
@@ -540,7 +546,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ret = devm_request_pci_bus_resources(&pdev->dev, &res);
+	ret = devm_request_pci_bus_resources(dev, &res);
 	if (ret)
 		goto error;
 
@@ -548,8 +554,7 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 	if (ret)
 		goto error;
 
-	bus = pci_create_root_bus(&pdev->dev, 0,
-					&xgene_pcie_ops, port, &res);
+	bus = pci_create_root_bus(dev, 0, &xgene_pcie_ops, port, &res);
 	if (!bus) {
 		ret = -ENOMEM;
 		goto error;

[PATCH v2 2/4] PCI: xgene: Remove unused platform data

[Bjorn Helgaas]

The xgene driver never uses the platform drvdata pointer, so don't
bother setting it.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-xgene.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 03d24e7..7da3b09 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -563,8 +563,6 @@ static int xgene_pcie_probe_bridge(struct platform_device *pdev)
 	pci_scan_child_bus(bus);
 	pci_assign_unassigned_bus_resources(bus);
 	pci_bus_add_devices(bus);
-
-	platform_set_drvdata(pdev, port);
 	return 0;
 
 error:

[PATCH v2 3/4] PCI: xgene: Pass struct xgene_pcie_port to setup functions

[Bjorn Helgaas]

Pass the struct xgene_pcie_port pointer, not addresses, to setup functions.
This enables future simplifications.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-xgene.c |   36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index 7da3b09..f1481b2 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -179,9 +179,10 @@ static struct pci_ops xgene_pcie_ops = {
 	.write = pci_generic_config_write32,
 };
 
-static u64 xgene_pcie_set_ib_mask(void __iomem *csr_base, u32 addr,
+static u64 xgene_pcie_set_ib_mask(struct xgene_pcie_port *port, u32 addr,
 				  u32 flags, u64 size)
 {
+	void __iomem *csr_base = port->csr_base;
 	u64 mask = (~(size - 1) & PCI_BASE_ADDRESS_MEM_MASK) | flags;
 	u32 val32 = 0;
 	u32 val;
@@ -294,8 +295,11 @@ static void xgene_pcie_setup_ob_reg(struct xgene_pcie_port *port,
 	writel(upper_32_bits(pci_addr), base + 0x14);
 }
 
-static void xgene_pcie_setup_cfg_reg(void __iomem *csr_base, u64 addr)
+static void xgene_pcie_setup_cfg_reg(struct xgene_pcie_port *port)
 {
+	void __iomem *csr_base = port->csr_base;
+	u64 addr = port->cfg_addr;
+
 	writel(lower_32_bits(addr), csr_base + CFGBARL);
 	writel(upper_32_bits(addr), csr_base + CFGBARH);
 	writel(EN_REG, csr_base + CFGCTL);
@@ -342,17 +346,19 @@ static int xgene_pcie_map_ranges(struct xgene_pcie_port *port,
 			return -EINVAL;
 		}
 	}
-	xgene_pcie_setup_cfg_reg(port->csr_base, port->cfg_addr);
-
+	xgene_pcie_setup_cfg_reg(port);
 	return 0;
 }
 
-static void xgene_pcie_setup_pims(void *addr, u64 pim, u64 size)
+static void xgene_pcie_setup_pims(struct xgene_pcie_port *port, u32 pim_reg,
+				  u64 pim, u64 size)
 {
-	writel(lower_32_bits(pim), addr);
-	writel(upper_32_bits(pim) | EN_COHERENCY, addr + 0x04);
-	writel(lower_32_bits(size), addr + 0x10);
-	writel(upper_32_bits(size), addr + 0x14);
+	void __iomem *addr = port->csr_base;
+
+	writel(lower_32_bits(pim), addr + pim_reg);
+	writel(upper_32_bits(pim) | EN_COHERENCY, addr + pim_reg + 0x04);
+	writel(lower_32_bits(size), addr + pim_reg + 0x10);
+	writel(upper_32_bits(size), addr + pim_reg + 0x14);
 }
 
 /*
@@ -386,7 +392,7 @@ static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
 	void __iomem *cfg_base = port->cfg_base;
 	struct device *dev = port->dev;
 	void *bar_addr;
-	void *pim_addr;
+	u32 pim_reg;
 	u64 cpu_addr = range->cpu_addr;
 	u64 pci_addr = range->pci_addr;
 	u64 size = range->size;
@@ -407,17 +413,17 @@ static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
 	bar_low = pcie_bar_low_val((u32)cpu_addr, flags);
 	switch (region) {
 	case 0:
-		xgene_pcie_set_ib_mask(csr_base, BRIDGE_CFG_4, flags, size);
+		xgene_pcie_set_ib_mask(port, BRIDGE_CFG_4, flags, size);
 		bar_addr = cfg_base + PCI_BASE_ADDRESS_0;
 		writel(bar_low, bar_addr);
 		writel(upper_32_bits(cpu_addr), bar_addr + 0x4);
-		pim_addr = csr_base + PIM1_1L;
+		pim_reg = PIM1_1L;
 		break;
 	case 1:
 		bar_addr = csr_base + IBAR2;
 		writel(bar_low, bar_addr);
 		writel(lower_32_bits(mask), csr_base + IR2MSK);
-		pim_addr = csr_base + PIM2_1L;
+		pim_reg = PIM2_1L;
 		break;
 	case 2:
 		bar_addr = csr_base + IBAR3L;
@@ -425,11 +431,11 @@ static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
 		writel(upper_32_bits(cpu_addr), bar_addr + 0x4);
 		writel(lower_32_bits(mask), csr_base + IR3MSKL);
 		writel(upper_32_bits(mask), csr_base + IR3MSKL + 0x4);
-		pim_addr = csr_base + PIM3_1L;
+		pim_reg = PIM3_1L;
 		break;
 	}
 
-	xgene_pcie_setup_pims(pim_addr, pci_addr, ~(size - 1));
+	xgene_pcie_setup_pims(port, pim_reg, pci_addr, ~(size - 1));
 }
 
 static int pci_dma_range_parser_init(struct of_pci_range_parser *parser,

[PATCH v2 4/4] PCI: xgene: Add register accessors

[Bjorn Helgaas]

Add device-specific register accessors for consistency across host drivers.
No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pci-xgene.c |   86 +++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 42 deletions(-)

diff --git a/drivers/pci/host/pci-xgene.c b/drivers/pci/host/pci-xgene.c
index f1481b2..1de23d7 100644
--- a/drivers/pci/host/pci-xgene.c
+++ b/drivers/pci/host/pci-xgene.c
@@ -76,6 +76,16 @@ struct xgene_pcie_port {
 	u32			version;
 };
 
+static u32 xgene_pcie_readl(struct xgene_pcie_port *port, u32 reg)
+{
+	return readl(port->csr_base + reg);
+}
+
+static void xgene_pcie_writel(struct xgene_pcie_port *port, u32 reg, u32 val)
+{
+	writel(val, port->csr_base + reg);
+}
+
 static inline u32 pcie_bar_low_val(u32 addr, u32 flags)
 {
 	return (addr & PCI_BASE_ADDRESS_MEM_MASK) | flags;
@@ -112,9 +122,9 @@ static void xgene_pcie_set_rtdid_reg(struct pci_bus *bus, uint devfn)
 	if (!pci_is_root_bus(bus))
 		rtdid_val = (b << 8) | (d << 3) | f;
 
-	writel(rtdid_val, port->csr_base + RTDID);
+	xgene_pcie_writel(port, RTDID, rtdid_val);
 	/* read the register back to ensure flush */
-	readl(port->csr_base + RTDID);
+	xgene_pcie_readl(port, RTDID);
 }
 
 /*
@@ -182,26 +192,25 @@ static struct pci_ops xgene_pcie_ops = {
 static u64 xgene_pcie_set_ib_mask(struct xgene_pcie_port *port, u32 addr,
 				  u32 flags, u64 size)
 {
-	void __iomem *csr_base = port->csr_base;
 	u64 mask = (~(size - 1) & PCI_BASE_ADDRESS_MEM_MASK) | flags;
 	u32 val32 = 0;
 	u32 val;
 
-	val32 = readl(csr_base + addr);
+	val32 = xgene_pcie_readl(port, addr);
 	val = (val32 & 0x0000ffff) | (lower_32_bits(mask) << 16);
-	writel(val, csr_base + addr);
+	xgene_pcie_writel(port, addr, val);
 
-	val32 = readl(csr_base + addr + 0x04);
+	val32 = xgene_pcie_readl(port, addr + 0x04);
 	val = (val32 & 0xffff0000) | (lower_32_bits(mask) >> 16);
-	writel(val, csr_base + addr + 0x04);
+	xgene_pcie_writel(port, addr + 0x04, val);
 
-	val32 = readl(csr_base + addr + 0x04);
+	val32 = xgene_pcie_readl(port, addr + 0x04);
 	val = (val32 & 0x0000ffff) | (upper_32_bits(mask) << 16);
-	writel(val, csr_base + addr + 0x04);
+	xgene_pcie_writel(port, addr + 0x04, val);
 
-	val32 = readl(csr_base + addr + 0x08);
+	val32 = xgene_pcie_readl(port, addr + 0x08);
 	val = (val32 & 0xffff0000) | (upper_32_bits(mask) >> 16);
-	writel(val, csr_base + addr + 0x08);
+	xgene_pcie_writel(port, addr + 0x08, val);
 
 	return mask;
 }
@@ -209,15 +218,14 @@ static u64 xgene_pcie_set_ib_mask(struct xgene_pcie_port *port, u32 addr,
 static void xgene_pcie_linkup(struct xgene_pcie_port *port,
 				   u32 *lanes, u32 *speed)
 {
-	void __iomem *csr_base = port->csr_base;
 	u32 val32;
 
 	port->link_up = false;
-	val32 = readl(csr_base + PCIECORE_CTLANDSTATUS);
+	val32 = xgene_pcie_readl(port, PCIECORE_CTLANDSTATUS);
 	if (val32 & LINK_UP_MASK) {
 		port->link_up = true;
 		*speed = PIPE_PHY_RATE_RD(val32);
-		val32 = readl(csr_base + BRIDGE_STATUS_0);
+		val32 = xgene_pcie_readl(port, BRIDGE_STATUS_0);
 		*lanes = val32 >> 26;
 	}
 }
@@ -266,7 +274,6 @@ static void xgene_pcie_setup_ob_reg(struct xgene_pcie_port *port,
 				    struct resource *res, u32 offset,
 				    u64 cpu_addr, u64 pci_addr)
 {
-	void __iomem *base = port->csr_base + offset;
 	struct device *dev = port->dev;
 	resource_size_t size = resource_size(res);
 	u64 restype = resource_type(res);
@@ -287,22 +294,21 @@ static void xgene_pcie_setup_ob_reg(struct xgene_pcie_port *port,
 		dev_warn(dev, "res size 0x%llx less than minimum 0x%x\n",
 			 (u64)size, min_size);
 
-	writel(lower_32_bits(cpu_addr), base);
-	writel(upper_32_bits(cpu_addr), base + 0x04);
-	writel(lower_32_bits(mask), base + 0x08);
-	writel(upper_32_bits(mask), base + 0x0c);
-	writel(lower_32_bits(pci_addr), base + 0x10);
-	writel(upper_32_bits(pci_addr), base + 0x14);
+	xgene_pcie_writel(port, offset, lower_32_bits(cpu_addr));
+	xgene_pcie_writel(port, offset + 0x04, upper_32_bits(cpu_addr));
+	xgene_pcie_writel(port, offset + 0x08, lower_32_bits(mask));
+	xgene_pcie_writel(port, offset + 0x0c, upper_32_bits(mask));
+	xgene_pcie_writel(port, offset + 0x10, lower_32_bits(pci_addr));
+	xgene_pcie_writel(port, offset + 0x14, upper_32_bits(pci_addr));
 }
 
 static void xgene_pcie_setup_cfg_reg(struct xgene_pcie_port *port)
 {
-	void __iomem *csr_base = port->csr_base;
 	u64 addr = port->cfg_addr;
 
-	writel(lower_32_bits(addr), csr_base + CFGBARL);
-	writel(upper_32_bits(addr), csr_base + CFGBARH);
-	writel(EN_REG, csr_base + CFGCTL);
+	xgene_pcie_writel(port, CFGBARL, lower_32_bits(addr));
+	xgene_pcie_writel(port, CFGBARH, upper_32_bits(addr));
+	xgene_pcie_writel(port, CFGCTL, EN_REG);
 }
 
 static int xgene_pcie_map_ranges(struct xgene_pcie_port *port,
@@ -353,12 +359,11 @@ static int xgene_pcie_map_ranges(struct xgene_pcie_port *port,
 static void xgene_pcie_setup_pims(struct xgene_pcie_port *port, u32 pim_reg,
 				  u64 pim, u64 size)
 {
-	void __iomem *addr = port->csr_base;
-
-	writel(lower_32_bits(pim), addr + pim_reg);
-	writel(upper_32_bits(pim) | EN_COHERENCY, addr + pim_reg + 0x04);
-	writel(lower_32_bits(size), addr + pim_reg + 0x10);
-	writel(upper_32_bits(size), addr + pim_reg + 0x14);
+	xgene_pcie_writel(port, pim_reg, lower_32_bits(pim));
+	xgene_pcie_writel(port, pim_reg + 0x04,
+			  upper_32_bits(pim) | EN_COHERENCY);
+	xgene_pcie_writel(port, pim_reg + 0x10, lower_32_bits(size));
+	xgene_pcie_writel(port, pim_reg + 0x14, upper_32_bits(size));
 }
 
 /*
@@ -388,7 +393,6 @@ static int xgene_pcie_select_ib_reg(u8 *ib_reg_mask, u64 size)
 static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
 				    struct of_pci_range *range, u8 *ib_reg_mask)
 {
-	void __iomem *csr_base = port->csr_base;
 	void __iomem *cfg_base = port->cfg_base;
 	struct device *dev = port->dev;
 	void *bar_addr;
@@ -420,17 +424,15 @@ static void xgene_pcie_setup_ib_reg(struct xgene_pcie_port *port,
 		pim_reg = PIM1_1L;
 		break;
 	case 1:
-		bar_addr = csr_base + IBAR2;
-		writel(bar_low, bar_addr);
-		writel(lower_32_bits(mask), csr_base + IR2MSK);
+		xgene_pcie_writel(port, IBAR2, bar_low);
+		xgene_pcie_writel(port, IR2MSK, lower_32_bits(mask));
 		pim_reg = PIM2_1L;
 		break;
 	case 2:
-		bar_addr = csr_base + IBAR3L;
-		writel(bar_low, bar_addr);
-		writel(upper_32_bits(cpu_addr), bar_addr + 0x4);
-		writel(lower_32_bits(mask), csr_base + IR3MSKL);
-		writel(upper_32_bits(mask), csr_base + IR3MSKL + 0x4);
+		xgene_pcie_writel(port, IBAR3L, bar_low);
+		xgene_pcie_writel(port, IBAR3L + 0x4, upper_32_bits(cpu_addr));
+		xgene_pcie_writel(port, IR3MSKL, lower_32_bits(mask));
+		xgene_pcie_writel(port, IR3MSKL + 0x4, upper_32_bits(mask));
 		pim_reg = PIM3_1L;
 		break;
 	}
@@ -486,7 +488,7 @@ static void xgene_pcie_clear_config(struct xgene_pcie_port *port)
 	int i;
 
 	for (i = PIM1_1L; i <= CFGCTL; i += 4)
-		writel(0x0, port->csr_base + i);
+		xgene_pcie_writel(port, i, 0);
 }
 
 static int xgene_pcie_setup(struct xgene_pcie_port *port,
@@ -501,7 +503,7 @@ static int xgene_pcie_setup(struct xgene_pcie_port *port,
 
 	/* setup the vendor and device IDs correctly */
 	val = (XGENE_PCIE_DEVICEID << 16) | XGENE_PCIE_VENDORID;
-	writel(val, port->csr_base + BRIDGE_CFG_0);
+	xgene_pcie_writel(port, BRIDGE_CFG_0, val);
 
 	ret = xgene_pcie_map_ranges(port, res, io_base);
 	if (ret)

Re: [PATCH v2 0/4] PCI: xgene: Cleanups

[Bjorn Helgaas]

On Wed, Oct 12, 2016 at 08:07:35AM -0500, Bjorn Helgaas wrote:
>   - Add local "dev" pointers to reduce repetition of things like
>     "&pdev->dev".
> 
>   - Remove platform drvdata because it appears unused (we called
>     platform_set_drvdata() but not platform_get_drvdata()).
> 
>   - Pass the struct xgene_pcie_port pointer, not addresses, to setup
>     functions to enable simplifications.
> 
>   - Add register accessors to encapsulate usage of port->csr_base a bit.
> 
> Nothing here should change the behavior of the driver.
> 
> Changes from v1:
>   I dropped the following patch because it was a lot of churn for
>   questionable benefit:
>     PCI: xgene: Name private struct pointer "xgene" consistently
> 
> ---
> 
> Bjorn Helgaas (4):
>       PCI: xgene: Add local struct device pointers
>       PCI: xgene: Remove unused platform data
>       PCI: xgene: Pass struct xgene_pcie_port to setup functions
>       PCI: xgene: Add register accessors

I applied these to pci/host-xgene for v4.9.  I hope to ask Linus to
pull them tomorrow, so if you see any issues, let me know soon.

Re: [PATCH v2 0/4] PCI: xgene: Cleanups

[Duc Dang]

On Wed, Oct 12, 2016 at 9:03 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Oct 12, 2016 at 08:07:35AM -0500, Bjorn Helgaas wrote:
>>   - Add local "dev" pointers to reduce repetition of things like
>>     "&pdev->dev".
>>
>>   - Remove platform drvdata because it appears unused (we called
>>     platform_set_drvdata() but not platform_get_drvdata()).
>>
>>   - Pass the struct xgene_pcie_port pointer, not addresses, to setup
>>     functions to enable simplifications.
>>
>>   - Add register accessors to encapsulate usage of port->csr_base a bit.
>>
>> Nothing here should change the behavior of the driver.
>>
>> Changes from v1:
>>   I dropped the following patch because it was a lot of churn for
>>   questionable benefit:
>>     PCI: xgene: Name private struct pointer "xgene" consistently
>>
>> ---
>>
>> Bjorn Helgaas (4):
>>       PCI: xgene: Add local struct device pointers
>>       PCI: xgene: Remove unused platform data
>>       PCI: xgene: Pass struct xgene_pcie_port to setup functions
>>       PCI: xgene: Add register accessors
>
> I applied these to pci/host-xgene for v4.9.  I hope to ask Linus to
> pull them tomorrow, so if you see any issues, let me know soon.

Hi Bjorn,

I tested the v2 series on X-Gene 1 and X-Gene 2 and no surprise
happens. Please feel free to add my Acked-by/Tested-by.

Regards,
Duc Dang.