All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently
@ 2016-10-07 16:31 Bjorn Helgaas
  2016-10-07 16:31 ` [PATCH 2/8] PCI: armada: Add register accessors Bjorn Helgaas
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-07 16:31 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-pci

Use a device-specific name, "armada8k", for struct armada8k_pcie pointers
to hint that this is device-specific information.  No functional change
intended.

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

diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
index 0f4f570..f88a790 100644
--- a/drivers/pci/host/pcie-armada8k.c
+++ b/drivers/pci/host/pcie-armada8k.c
@@ -72,12 +72,11 @@ struct armada8k_pcie {
 
 static int armada8k_pcie_link_up(struct pcie_port *pp)
 {
-	struct armada8k_pcie *pcie = to_armada8k_pcie(pp);
+	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
 	u32 reg;
 	u32 mask = PCIE_GLB_STS_RDLH_LINK_UP | PCIE_GLB_STS_PHY_LINK_UP;
 
-	reg = readl(pcie->base + PCIE_GLOBAL_STATUS_REG);
-
+	reg = readl(armada8k->base + PCIE_GLOBAL_STATUS_REG);
 	if ((reg & mask) == mask)
 		return 1;
 
@@ -87,8 +86,8 @@ static int armada8k_pcie_link_up(struct pcie_port *pp)
 
 static void armada8k_pcie_establish_link(struct pcie_port *pp)
 {
-	struct armada8k_pcie *pcie = to_armada8k_pcie(pp);
-	void __iomem *base = pcie->base;
+	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
+	void __iomem *base = armada8k->base;
 	u32 reg;
 
 	if (!dw_pcie_link_up(pp)) {
@@ -146,8 +145,8 @@ static void armada8k_pcie_host_init(struct pcie_port *pp)
 static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 {
 	struct pcie_port *pp = arg;
-	struct armada8k_pcie *pcie = to_armada8k_pcie(pp);
-	void __iomem *base = pcie->base;
+	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
+	void __iomem *base = armada8k->base;
 	u32 val;
 
 	/*
@@ -199,25 +198,25 @@ static int armada8k_add_pcie_port(struct pcie_port *pp,
 
 static int armada8k_pcie_probe(struct platform_device *pdev)
 {
-	struct armada8k_pcie *pcie;
+	struct armada8k_pcie *armada8k;
 	struct pcie_port *pp;
 	struct device *dev = &pdev->dev;
 	struct resource *base;
 	int ret;
 
-	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
-	if (!pcie)
+	armada8k = devm_kzalloc(dev, sizeof(*armada8k), GFP_KERNEL);
+	if (!armada8k)
 		return -ENOMEM;
 
-	pcie->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(pcie->clk))
-		return PTR_ERR(pcie->clk);
+	armada8k->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(armada8k->clk))
+		return PTR_ERR(armada8k->clk);
 
-	clk_prepare_enable(pcie->clk);
+	clk_prepare_enable(armada8k->clk);
 
-	pp = &pcie->pp;
+	pp = &armada8k->pp;
 	pp->dev = dev;
-	platform_set_drvdata(pdev, pcie);
+	platform_set_drvdata(pdev, armada8k);
 
 	/* Get the dw-pcie unit configuration/control registers base. */
 	base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
@@ -228,7 +227,7 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	pcie->base = pp->dbi_base + PCIE_VENDOR_REGS_OFFSET;
+	armada8k->base = pp->dbi_base + PCIE_VENDOR_REGS_OFFSET;
 
 	ret = armada8k_add_pcie_port(pp, pdev);
 	if (ret)
@@ -237,8 +236,8 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 	return 0;
 
 fail:
-	if (!IS_ERR(pcie->clk))
-		clk_disable_unprepare(pcie->clk);
+	if (!IS_ERR(armada8k->clk))
+		clk_disable_unprepare(armada8k->clk);
 
 	return ret;
 }


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

* [PATCH 2/8] PCI: armada: Add register accessors
  2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
@ 2016-10-07 16:31 ` Bjorn Helgaas
  2016-10-07 16:31 ` [PATCH 3/8] PCI: armada: Remove redundant struct armada8k_pcie.base Bjorn Helgaas
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-07 16:31 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-pci

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/pcie-armada8k.c |   46 ++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
index f88a790..d0a3ab4 100644
--- a/drivers/pci/host/pcie-armada8k.c
+++ b/drivers/pci/host/pcie-armada8k.c
@@ -70,13 +70,23 @@ struct armada8k_pcie {
 
 #define to_armada8k_pcie(x)	container_of(x, struct armada8k_pcie, pp)
 
+static u32 armada8k_readl(struct armada8k_pcie *armada8k, u32 offset)
+{
+	return readl(armada8k->base + offset);
+}
+
+static void armada8k_writel(struct armada8k_pcie *armada8k, u32 offset, u32 val)
+{
+	writel(val, armada8k->base + offset);
+}
+
 static int armada8k_pcie_link_up(struct pcie_port *pp)
 {
 	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
 	u32 reg;
 	u32 mask = PCIE_GLB_STS_RDLH_LINK_UP | PCIE_GLB_STS_PHY_LINK_UP;
 
-	reg = readl(armada8k->base + PCIE_GLOBAL_STATUS_REG);
+	reg = armada8k_readl(armada8k, PCIE_GLOBAL_STATUS_REG);
 	if ((reg & mask) == mask)
 		return 1;
 
@@ -87,48 +97,47 @@ static int armada8k_pcie_link_up(struct pcie_port *pp)
 static void armada8k_pcie_establish_link(struct pcie_port *pp)
 {
 	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
-	void __iomem *base = armada8k->base;
 	u32 reg;
 
 	if (!dw_pcie_link_up(pp)) {
 		/* Disable LTSSM state machine to enable configuration */
-		reg = readl(base + PCIE_GLOBAL_CONTROL_REG);
+		reg = armada8k_readl(armada8k, PCIE_GLOBAL_CONTROL_REG);
 		reg &= ~(PCIE_APP_LTSSM_EN);
-		writel(reg, base + PCIE_GLOBAL_CONTROL_REG);
+		armada8k_writel(armada8k, PCIE_GLOBAL_CONTROL_REG, reg);
 	}
 
 	/* Set the device to root complex mode */
-	reg = readl(base + PCIE_GLOBAL_CONTROL_REG);
+	reg = armada8k_readl(armada8k, PCIE_GLOBAL_CONTROL_REG);
 	reg &= ~(PCIE_DEVICE_TYPE_MASK << PCIE_DEVICE_TYPE_SHIFT);
 	reg |= PCIE_DEVICE_TYPE_RC << PCIE_DEVICE_TYPE_SHIFT;
-	writel(reg, base + PCIE_GLOBAL_CONTROL_REG);
+	armada8k_writel(armada8k, PCIE_GLOBAL_CONTROL_REG, reg);
 
 	/* Set the PCIe master AxCache attributes */
-	writel(ARCACHE_DEFAULT_VALUE, base + PCIE_ARCACHE_TRC_REG);
-	writel(AWCACHE_DEFAULT_VALUE, base + PCIE_AWCACHE_TRC_REG);
+	armada8k_writel(armada8k, PCIE_ARCACHE_TRC_REG, ARCACHE_DEFAULT_VALUE);
+	armada8k_writel(armada8k, PCIE_AWCACHE_TRC_REG, AWCACHE_DEFAULT_VALUE);
 
 	/* Set the PCIe master AxDomain attributes */
-	reg = readl(base + PCIE_ARUSER_REG);
+	reg = armada8k_readl(armada8k, PCIE_ARUSER_REG);
 	reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
 	reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
-	writel(reg, base + PCIE_ARUSER_REG);
+	armada8k_writel(armada8k, PCIE_ARUSER_REG, reg);
 
-	reg = readl(base + PCIE_AWUSER_REG);
+	reg = armada8k_readl(armada8k, PCIE_AWUSER_REG);
 	reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
 	reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
-	writel(reg, base + PCIE_AWUSER_REG);
+	armada8k_writel(armada8k, PCIE_AWUSER_REG, reg);
 
 	/* Enable INT A-D interrupts */
-	reg = readl(base + PCIE_GLOBAL_INT_MASK1_REG);
+	reg = armada8k_readl(armada8k, PCIE_GLOBAL_INT_MASK1_REG);
 	reg |= PCIE_INT_A_ASSERT_MASK | PCIE_INT_B_ASSERT_MASK |
 	       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
-	writel(reg, base + PCIE_GLOBAL_INT_MASK1_REG);
+	armada8k_writel(armada8k, PCIE_GLOBAL_INT_MASK1_REG, reg);
 
 	if (!dw_pcie_link_up(pp)) {
 		/* Configuration done. Start LTSSM */
-		reg = readl(base + PCIE_GLOBAL_CONTROL_REG);
+		reg = armada8k_readl(armada8k, PCIE_GLOBAL_CONTROL_REG);
 		reg |= PCIE_APP_LTSSM_EN;
-		writel(reg, base + PCIE_GLOBAL_CONTROL_REG);
+		armada8k_writel(armada8k, PCIE_GLOBAL_CONTROL_REG, reg);
 	}
 
 	/* Wait until the link becomes active again */
@@ -146,7 +155,6 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 {
 	struct pcie_port *pp = arg;
 	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
-	void __iomem *base = armada8k->base;
 	u32 val;
 
 	/*
@@ -154,8 +162,8 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 	 * PCI device. However, they are also latched into the PCIe
 	 * controller, so we simply discard them.
 	 */
-	val = readl(base + PCIE_GLOBAL_INT_CAUSE1_REG);
-	writel(val, base + PCIE_GLOBAL_INT_CAUSE1_REG);
+	val = armada8k_readl(armada8k, PCIE_GLOBAL_INT_CAUSE1_REG);
+	armada8k_writel(armada8k, PCIE_GLOBAL_INT_CAUSE1_REG, val);
 
 	return IRQ_HANDLED;
 }


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

* [PATCH 3/8] PCI: armada: Remove redundant struct armada8k_pcie.base
  2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
  2016-10-07 16:31 ` [PATCH 2/8] PCI: armada: Add register accessors Bjorn Helgaas
@ 2016-10-07 16:31 ` Bjorn Helgaas
  2016-10-07 16:31 ` [PATCH 4/8] PCI: armada: Pass device-specific struct to internal functions Bjorn Helgaas
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-07 16:31 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-pci

The struct armada8k_pcie.base pointer is always a constant offset from
struct pcie_port.dbi_base.  Encode that offset in the register macros so we
don't need to maintain the armada8k_pcie.base pointer.  No functional
change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-armada8k.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
index d0a3ab4..ed96857 100644
--- a/drivers/pci/host/pcie-armada8k.c
+++ b/drivers/pci/host/pcie-armada8k.c
@@ -29,34 +29,33 @@
 #include "pcie-designware.h"
 
 struct armada8k_pcie {
-	void __iomem *base;
 	struct clk *clk;
 	struct pcie_port pp;
 };
 
 #define PCIE_VENDOR_REGS_OFFSET		0x8000
 
-#define PCIE_GLOBAL_CONTROL_REG		0x0
+#define PCIE_GLOBAL_CONTROL_REG		(PCIE_VENDOR_REGS_OFFSET + 0x0)
 #define PCIE_APP_LTSSM_EN		BIT(2)
 #define PCIE_DEVICE_TYPE_SHIFT		4
 #define PCIE_DEVICE_TYPE_MASK		0xF
 #define PCIE_DEVICE_TYPE_RC		0x4 /* Root complex */
 
-#define PCIE_GLOBAL_STATUS_REG		0x8
+#define PCIE_GLOBAL_STATUS_REG		(PCIE_VENDOR_REGS_OFFSET + 0x8)
 #define PCIE_GLB_STS_RDLH_LINK_UP	BIT(1)
 #define PCIE_GLB_STS_PHY_LINK_UP	BIT(9)
 
-#define PCIE_GLOBAL_INT_CAUSE1_REG	0x1C
-#define PCIE_GLOBAL_INT_MASK1_REG	0x20
+#define PCIE_GLOBAL_INT_CAUSE1_REG	(PCIE_VENDOR_REGS_OFFSET + 0x1C)
+#define PCIE_GLOBAL_INT_MASK1_REG	(PCIE_VENDOR_REGS_OFFSET + 0x20)
 #define PCIE_INT_A_ASSERT_MASK		BIT(9)
 #define PCIE_INT_B_ASSERT_MASK		BIT(10)
 #define PCIE_INT_C_ASSERT_MASK		BIT(11)
 #define PCIE_INT_D_ASSERT_MASK		BIT(12)
 
-#define PCIE_ARCACHE_TRC_REG		0x50
-#define PCIE_AWCACHE_TRC_REG		0x54
-#define PCIE_ARUSER_REG			0x5C
-#define PCIE_AWUSER_REG			0x60
+#define PCIE_ARCACHE_TRC_REG		(PCIE_VENDOR_REGS_OFFSET + 0x50)
+#define PCIE_AWCACHE_TRC_REG		(PCIE_VENDOR_REGS_OFFSET + 0x54)
+#define PCIE_ARUSER_REG			(PCIE_VENDOR_REGS_OFFSET + 0x5C)
+#define PCIE_AWUSER_REG			(PCIE_VENDOR_REGS_OFFSET + 0x60)
 /*
  * AR/AW Cache defauls: Normal memory, Write-Back, Read / Write
  * allocate
@@ -72,12 +71,12 @@ struct armada8k_pcie {
 
 static u32 armada8k_readl(struct armada8k_pcie *armada8k, u32 offset)
 {
-	return readl(armada8k->base + offset);
+	return readl(armada8k->pp.dbi_base + offset);
 }
 
 static void armada8k_writel(struct armada8k_pcie *armada8k, u32 offset, u32 val)
 {
-	writel(val, armada8k->base + offset);
+	writel(val, armada8k->pp.dbi_base + offset);
 }
 
 static int armada8k_pcie_link_up(struct pcie_port *pp)
@@ -235,8 +234,6 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	armada8k->base = pp->dbi_base + PCIE_VENDOR_REGS_OFFSET;
-
 	ret = armada8k_add_pcie_port(pp, pdev);
 	if (ret)
 		goto fail;


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

* [PATCH 4/8] PCI: armada: Pass device-specific struct to internal functions
  2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
  2016-10-07 16:31 ` [PATCH 2/8] PCI: armada: Add register accessors Bjorn Helgaas
  2016-10-07 16:31 ` [PATCH 3/8] PCI: armada: Remove redundant struct armada8k_pcie.base Bjorn Helgaas
@ 2016-10-07 16:31 ` Bjorn Helgaas
  2016-10-07 16:31 ` [PATCH 5/8] PCI: armada: Reorder struct armada8k_pcie Bjorn Helgaas
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-07 16:31 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-pci

Only interfaces used from outside the driver, e.g., those called by the
DesignWare core, need to accept pointers to the generic struct pcie_port.
Internal interfaces can accept pointers to the device-specific struct,
which makes them more straightforward.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-armada8k.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
index ed96857..16fcb43 100644
--- a/drivers/pci/host/pcie-armada8k.c
+++ b/drivers/pci/host/pcie-armada8k.c
@@ -93,9 +93,9 @@ static int armada8k_pcie_link_up(struct pcie_port *pp)
 	return 0;
 }
 
-static void armada8k_pcie_establish_link(struct pcie_port *pp)
+static void armada8k_pcie_establish_link(struct armada8k_pcie *armada8k)
 {
-	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
+	struct pcie_port *pp = &armada8k->pp;
 	u32 reg;
 
 	if (!dw_pcie_link_up(pp)) {
@@ -146,14 +146,15 @@ static void armada8k_pcie_establish_link(struct pcie_port *pp)
 
 static void armada8k_pcie_host_init(struct pcie_port *pp)
 {
+	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
+
 	dw_pcie_setup_rc(pp);
-	armada8k_pcie_establish_link(pp);
+	armada8k_pcie_establish_link(armada8k);
 }
 
 static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 {
-	struct pcie_port *pp = arg;
-	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
+	struct armada8k_pcie *armada8k = arg;
 	u32 val;
 
 	/*
@@ -172,9 +173,10 @@ static struct pcie_host_ops armada8k_pcie_host_ops = {
 	.host_init = armada8k_pcie_host_init,
 };
 
-static int armada8k_add_pcie_port(struct pcie_port *pp,
+static int armada8k_add_pcie_port(struct armada8k_pcie *armada8k,
 				  struct platform_device *pdev)
 {
+	struct pcie_port *pp = &armada8k->pp;
 	struct device *dev = &pdev->dev;
 	int ret;
 
@@ -188,7 +190,7 @@ static int armada8k_add_pcie_port(struct pcie_port *pp,
 	}
 
 	ret = devm_request_irq(dev, pp->irq, armada8k_pcie_irq_handler,
-			       IRQF_SHARED, "armada8k-pcie", pp);
+			       IRQF_SHARED, "armada8k-pcie", armada8k);
 	if (ret) {
 		dev_err(dev, "failed to request irq %d\n", pp->irq);
 		return ret;
@@ -234,7 +236,7 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 		goto fail;
 	}
 
-	ret = armada8k_add_pcie_port(pp, pdev);
+	ret = armada8k_add_pcie_port(armada8k, pdev);
 	if (ret)
 		goto fail;
 


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

* [PATCH 5/8] PCI: armada: Reorder struct armada8k_pcie
  2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
                   ` (2 preceding siblings ...)
  2016-10-07 16:31 ` [PATCH 4/8] PCI: armada: Pass device-specific struct to internal functions Bjorn Helgaas
@ 2016-10-07 16:31 ` Bjorn Helgaas
  2016-10-07 16:32 ` [PATCH 6/8] PCI: armada: Replace armada8k_readl() with dw_pcie_readl_rc() Bjorn Helgaas
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-07 16:31 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-pci

Reorder the device-specific struct to put the DesignWare generic struct
pcie_port first.  No functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-armada8k.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
index 16fcb43..276125f 100644
--- a/drivers/pci/host/pcie-armada8k.c
+++ b/drivers/pci/host/pcie-armada8k.c
@@ -29,8 +29,8 @@
 #include "pcie-designware.h"
 
 struct armada8k_pcie {
+	struct pcie_port pp;		/* pp.dbi_base is DT ctrl */
 	struct clk *clk;
-	struct pcie_port pp;
 };
 
 #define PCIE_VENDOR_REGS_OFFSET		0x8000


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

* [PATCH 6/8] PCI: armada: Replace armada8k_readl() with dw_pcie_readl_rc()
  2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
                   ` (3 preceding siblings ...)
  2016-10-07 16:31 ` [PATCH 5/8] PCI: armada: Reorder struct armada8k_pcie Bjorn Helgaas
@ 2016-10-07 16:32 ` Bjorn Helgaas
  2016-10-07 16:32 ` [PATCH 7/8] PCI: armada: Reorder local declaration Bjorn Helgaas
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-07 16:32 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-pci

The dw_pcie_readl_rc() and dw_pcie_writel_rc() interfaces do the same as
armada8k_readl() and armada8k_writel(), and they also give us a clue that
we're using the DesignWare-generic functionality.  Use the dw_*()
interfaces and remove the armada8k-specific ones.  No functional change
intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/host/pcie-armada8k.c |   46 +++++++++++++++-----------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
index 276125f..19086a4 100644
--- a/drivers/pci/host/pcie-armada8k.c
+++ b/drivers/pci/host/pcie-armada8k.c
@@ -69,23 +69,12 @@ struct armada8k_pcie {
 
 #define to_armada8k_pcie(x)	container_of(x, struct armada8k_pcie, pp)
 
-static u32 armada8k_readl(struct armada8k_pcie *armada8k, u32 offset)
-{
-	return readl(armada8k->pp.dbi_base + offset);
-}
-
-static void armada8k_writel(struct armada8k_pcie *armada8k, u32 offset, u32 val)
-{
-	writel(val, armada8k->pp.dbi_base + offset);
-}
-
 static int armada8k_pcie_link_up(struct pcie_port *pp)
 {
-	struct armada8k_pcie *armada8k = to_armada8k_pcie(pp);
 	u32 reg;
 	u32 mask = PCIE_GLB_STS_RDLH_LINK_UP | PCIE_GLB_STS_PHY_LINK_UP;
 
-	reg = armada8k_readl(armada8k, PCIE_GLOBAL_STATUS_REG);
+	reg = dw_pcie_readl_rc(pp, PCIE_GLOBAL_STATUS_REG);
 	if ((reg & mask) == mask)
 		return 1;
 
@@ -100,43 +89,43 @@ static void armada8k_pcie_establish_link(struct armada8k_pcie *armada8k)
 
 	if (!dw_pcie_link_up(pp)) {
 		/* Disable LTSSM state machine to enable configuration */
-		reg = armada8k_readl(armada8k, PCIE_GLOBAL_CONTROL_REG);
+		reg = dw_pcie_readl_rc(pp, PCIE_GLOBAL_CONTROL_REG);
 		reg &= ~(PCIE_APP_LTSSM_EN);
-		armada8k_writel(armada8k, PCIE_GLOBAL_CONTROL_REG, reg);
+		dw_pcie_writel_rc(pp, PCIE_GLOBAL_CONTROL_REG, reg);
 	}
 
 	/* Set the device to root complex mode */
-	reg = armada8k_readl(armada8k, PCIE_GLOBAL_CONTROL_REG);
+	reg = dw_pcie_readl_rc(pp, PCIE_GLOBAL_CONTROL_REG);
 	reg &= ~(PCIE_DEVICE_TYPE_MASK << PCIE_DEVICE_TYPE_SHIFT);
 	reg |= PCIE_DEVICE_TYPE_RC << PCIE_DEVICE_TYPE_SHIFT;
-	armada8k_writel(armada8k, PCIE_GLOBAL_CONTROL_REG, reg);
+	dw_pcie_writel_rc(pp, PCIE_GLOBAL_CONTROL_REG, reg);
 
 	/* Set the PCIe master AxCache attributes */
-	armada8k_writel(armada8k, PCIE_ARCACHE_TRC_REG, ARCACHE_DEFAULT_VALUE);
-	armada8k_writel(armada8k, PCIE_AWCACHE_TRC_REG, AWCACHE_DEFAULT_VALUE);
+	dw_pcie_writel_rc(pp, PCIE_ARCACHE_TRC_REG, ARCACHE_DEFAULT_VALUE);
+	dw_pcie_writel_rc(pp, PCIE_AWCACHE_TRC_REG, AWCACHE_DEFAULT_VALUE);
 
 	/* Set the PCIe master AxDomain attributes */
-	reg = armada8k_readl(armada8k, PCIE_ARUSER_REG);
+	reg = dw_pcie_readl_rc(pp, PCIE_ARUSER_REG);
 	reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
 	reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
-	armada8k_writel(armada8k, PCIE_ARUSER_REG, reg);
+	dw_pcie_writel_rc(pp, PCIE_ARUSER_REG, reg);
 
-	reg = armada8k_readl(armada8k, PCIE_AWUSER_REG);
+	reg = dw_pcie_readl_rc(pp, PCIE_AWUSER_REG);
 	reg &= ~(AX_USER_DOMAIN_MASK << AX_USER_DOMAIN_SHIFT);
 	reg |= DOMAIN_OUTER_SHAREABLE << AX_USER_DOMAIN_SHIFT;
-	armada8k_writel(armada8k, PCIE_AWUSER_REG, reg);
+	dw_pcie_writel_rc(pp, PCIE_AWUSER_REG, reg);
 
 	/* Enable INT A-D interrupts */
-	reg = armada8k_readl(armada8k, PCIE_GLOBAL_INT_MASK1_REG);
+	reg = dw_pcie_readl_rc(pp, PCIE_GLOBAL_INT_MASK1_REG);
 	reg |= PCIE_INT_A_ASSERT_MASK | PCIE_INT_B_ASSERT_MASK |
 	       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
-	armada8k_writel(armada8k, PCIE_GLOBAL_INT_MASK1_REG, reg);
+	dw_pcie_writel_rc(pp, PCIE_GLOBAL_INT_MASK1_REG, reg);
 
 	if (!dw_pcie_link_up(pp)) {
 		/* Configuration done. Start LTSSM */
-		reg = armada8k_readl(armada8k, PCIE_GLOBAL_CONTROL_REG);
+		reg = dw_pcie_readl_rc(pp, PCIE_GLOBAL_CONTROL_REG);
 		reg |= PCIE_APP_LTSSM_EN;
-		armada8k_writel(armada8k, PCIE_GLOBAL_CONTROL_REG, reg);
+		dw_pcie_writel_rc(pp, PCIE_GLOBAL_CONTROL_REG, reg);
 	}
 
 	/* Wait until the link becomes active again */
@@ -155,6 +144,7 @@ static void armada8k_pcie_host_init(struct pcie_port *pp)
 static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 {
 	struct armada8k_pcie *armada8k = arg;
+	struct pcie_port *pp = &armada8k->pp;
 	u32 val;
 
 	/*
@@ -162,8 +152,8 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 	 * PCI device. However, they are also latched into the PCIe
 	 * controller, so we simply discard them.
 	 */
-	val = armada8k_readl(armada8k, PCIE_GLOBAL_INT_CAUSE1_REG);
-	armada8k_writel(armada8k, PCIE_GLOBAL_INT_CAUSE1_REG, val);
+	val = dw_pcie_readl_rc(pp, PCIE_GLOBAL_INT_CAUSE1_REG);
+	dw_pcie_writel_rc(pp, PCIE_GLOBAL_INT_CAUSE1_REG, val);
 
 	return IRQ_HANDLED;
 }


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

* [PATCH 7/8] PCI: armada: Reorder local declaration
  2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
                   ` (4 preceding siblings ...)
  2016-10-07 16:32 ` [PATCH 6/8] PCI: armada: Replace armada8k_readl() with dw_pcie_readl_rc() Bjorn Helgaas
@ 2016-10-07 16:32 ` Bjorn Helgaas
  2016-10-07 16:32 ` [PATCH 8/8] PCI: armada: Remove unused platform data Bjorn Helgaas
  2016-10-07 20:17 ` [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Thomas Petazzoni
  7 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-07 16:32 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-pci

Move initialized local variable declaration to top and set drvdata pointer
only when the probe function succeeds.  No functional change intended.

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

diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
index 19086a4..36002e4 100644
--- a/drivers/pci/host/pcie-armada8k.c
+++ b/drivers/pci/host/pcie-armada8k.c
@@ -197,9 +197,9 @@ static int armada8k_add_pcie_port(struct armada8k_pcie *armada8k,
 
 static int armada8k_pcie_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct armada8k_pcie *armada8k;
 	struct pcie_port *pp;
-	struct device *dev = &pdev->dev;
 	struct resource *base;
 	int ret;
 
@@ -215,7 +215,6 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 
 	pp = &armada8k->pp;
 	pp->dev = dev;
-	platform_set_drvdata(pdev, armada8k);
 
 	/* Get the dw-pcie unit configuration/control registers base. */
 	base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl");
@@ -230,6 +229,7 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto fail;
 
+	platform_set_drvdata(pdev, armada8k);
 	return 0;
 
 fail:


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

* [PATCH 8/8] PCI: armada: Remove unused platform data
  2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
                   ` (5 preceding siblings ...)
  2016-10-07 16:32 ` [PATCH 7/8] PCI: armada: Reorder local declaration Bjorn Helgaas
@ 2016-10-07 16:32 ` Bjorn Helgaas
  2016-10-07 20:17 ` [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Thomas Petazzoni
  7 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-07 16:32 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-pci

The armada 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/pcie-armada8k.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/host/pcie-armada8k.c b/drivers/pci/host/pcie-armada8k.c
index 36002e4..890c171 100644
--- a/drivers/pci/host/pcie-armada8k.c
+++ b/drivers/pci/host/pcie-armada8k.c
@@ -229,7 +229,6 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 	if (ret)
 		goto fail;
 
-	platform_set_drvdata(pdev, armada8k);
 	return 0;
 
 fail:


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

* Re: [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently
  2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
                   ` (6 preceding siblings ...)
  2016-10-07 16:32 ` [PATCH 8/8] PCI: armada: Remove unused platform data Bjorn Helgaas
@ 2016-10-07 20:17 ` Thomas Petazzoni
  2016-10-10 17:17   ` Bjorn Helgaas
  7 siblings, 1 reply; 10+ messages in thread
From: Thomas Petazzoni @ 2016-10-07 20:17 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci

Hello,

On Fri, 07 Oct 2016 11:31:18 -0500, Bjorn Helgaas wrote:
> Use a device-specific name, "armada8k", for struct armada8k_pcie pointers
> to hint that this is device-specific information.  No functional change
> intended.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/host/pcie-armada8k.c |   37 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)

For the entire series:

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

However, I don't have access to HW right now to test. I can give some
testing when I get back to the office.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* Re: [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently
  2016-10-07 20:17 ` [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Thomas Petazzoni
@ 2016-10-10 17:17   ` Bjorn Helgaas
  0 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2016-10-10 17:17 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: Bjorn Helgaas, linux-pci

On Fri, Oct 07, 2016 at 10:17:17PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 07 Oct 2016 11:31:18 -0500, Bjorn Helgaas wrote:
> > Use a device-specific name, "armada8k", for struct armada8k_pcie pointers
> > to hint that this is device-specific information.  No functional change
> > intended.
> > 
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> >  drivers/pci/host/pcie-armada8k.c |   37 ++++++++++++++++++-------------------
> >  1 file changed, 18 insertions(+), 19 deletions(-)
> 
> For the entire series:
> 
> Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> However, I don't have access to HW right now to test. I can give some
> testing when I get back to the office.

I made tweaks similar to the aardvark ones to this branch.

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

end of thread, other threads:[~2016-10-10 17:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-07 16:31 [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Bjorn Helgaas
2016-10-07 16:31 ` [PATCH 2/8] PCI: armada: Add register accessors Bjorn Helgaas
2016-10-07 16:31 ` [PATCH 3/8] PCI: armada: Remove redundant struct armada8k_pcie.base Bjorn Helgaas
2016-10-07 16:31 ` [PATCH 4/8] PCI: armada: Pass device-specific struct to internal functions Bjorn Helgaas
2016-10-07 16:31 ` [PATCH 5/8] PCI: armada: Reorder struct armada8k_pcie Bjorn Helgaas
2016-10-07 16:32 ` [PATCH 6/8] PCI: armada: Replace armada8k_readl() with dw_pcie_readl_rc() Bjorn Helgaas
2016-10-07 16:32 ` [PATCH 7/8] PCI: armada: Reorder local declaration Bjorn Helgaas
2016-10-07 16:32 ` [PATCH 8/8] PCI: armada: Remove unused platform data Bjorn Helgaas
2016-10-07 20:17 ` [PATCH 1/8] PCI: armada: Name private struct pointer "armada8k" consistently Thomas Petazzoni
2016-10-10 17:17   ` Bjorn Helgaas

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.