Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1474553 > unrolled thread

[PATCH 0/9] PCI: rockchip: Cleanups against v10

Started byBjorn Helgaas <bhelgaas@google.com>
First post2016-09-01 18:50 +0200
Last post2016-09-01 23:20 +0200
Articles 7 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/9] PCI: rockchip: Cleanups against v10 Bjorn Helgaas <bhelgaas@google.com> - 2016-09-01 18:50 +0200
    [PATCH 4/9] Rename struct rockchip_pcie_port to struct  rockchip_pcie. Bjorn Helgaas <bhelgaas@google.com> - 2016-09-01 18:50 +0200
    [PATCH 9/9] Move msleeps to address Guenter's comments. Bjorn Helgaas <bhelgaas@google.com> - 2016-09-01 18:50 +0200
    [PATCH 5/9] Use a local "dev" to avoid repetition of  "rockchip->dev". Bjorn Helgaas <bhelgaas@google.com> - 2016-09-01 18:50 +0200
    [PATCH 7/9] Simplify the confusing HIWORD_UPDATE scheme. Bjorn Helgaas <bhelgaas@google.com> - 2016-09-01 18:50 +0200
    [PATCH 6/9] Add comment about why 32-bit read/modify/write isn't  safe. Bjorn Helgaas <bhelgaas@google.com> - 2016-09-01 18:50 +0200
    Re: [PATCH 0/9] PCI: rockchip: Cleanups against v10 Arnd Bergmann <arnd@arndb.de> - 2016-09-01 23:20 +0200

#1474553 — [PATCH 0/9] PCI: rockchip: Cleanups against v10

FromBjorn Helgaas <bhelgaas@google.com>
Date2016-09-01 18:50 +0200
Subject[PATCH 0/9] PCI: rockchip: Cleanups against v10
Message-ID<scDlD-4lf-17@gated-at.bofh.it>
These are cleanups against 2098142ae87d, the current pci/host-rockchip
head in my tree.  This is basically the v10 driver posting (it's not
identical because I had already tweaked a few things and squashed in
Brian's bug fix).

These haven't been compiled so I'm sure the 0-day builder will find some
errors.  After fixing those, I intend to squash everything into one commit,
since this driver hasn't been merged yet.  I'm just posting these split out
patches in case anybody wants to review them.

These address most of Guenter's review comments.  I didn't change anything
related to -EPROBE_DEFER because it wasn't obvious to me what to do.

I also made several other changes -- mostly renaming functions and
variables so they're more consistent with other PCI host drivers.

These are in git as pci/host-rockchip-wip.

---

Bjorn Helgaas (9):
      Remove unused symbols, unnecessary parens, other minor comments from
      Rename pcie_read() and pcie_write() to rockchip_pcie_read() and
      Always use "rockchip" as the pointer to per-device struct.
      Rename struct rockchip_pcie_port to struct rockchip_pcie.
      Use a local "dev" to avoid repetition of "rockchip->dev".
      Add comment about why 32-bit read/modify/write isn't safe.
      Simplify the confusing HIWORD_UPDATE scheme.
      Simplify testing of link status and speed testing.
      Move msleeps to address Guenter's comments.


 drivers/pci/host/pcie-rockchip.c |  680 +++++++++++++++++---------------------
 1 file changed, 307 insertions(+), 373 deletions(-)

[toc] | [next] | [standalone]

#1474554 — [PATCH 4/9] Rename struct rockchip_pcie_port to struct rockchip_pcie.

FromBjorn Helgaas <bhelgaas@google.com>
Date2016-09-01 18:50 +0200
Subject[PATCH 4/9] Rename struct rockchip_pcie_port to struct rockchip_pcie.
Message-ID<scDlE-4lf-49@gated-at.bofh.it>
In reply to#1474553
---
 drivers/pci/host/pcie-rockchip.c |   46 +++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 3cfb47a..63fb0ebc 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -178,7 +178,7 @@
 	PCIE_CLIENT_INT_LEGACY_DONE | PCIE_CLIENT_INT_LEGACY | \
 	PCIE_CLIENT_INT_PHY)
 
-struct rockchip_pcie_port {
+struct rockchip_pcie {
 	void	__iomem *reg_base;		/* DT axi-base */
 	void	__iomem *apb_base;		/* DT apb-base */
 	struct	phy *phy;
@@ -200,17 +200,17 @@ struct rockchip_pcie_port {
 	struct	irq_domain *irq_domain;
 };
 
-static u32 rockchip_pcie_read(struct rockchip_pcie_port *rockchip, u32 reg)
+static u32 rockchip_pcie_read(struct rockchip_pcie *rockchip, u32 reg)
 {
 	return readl(rockchip->apb_base + reg);
 }
 
-static void rockchip_pcie_write(struct rockchip_pcie_port *rockchip, u32 val, u32 reg)
+static void rockchip_pcie_write(struct rockchip_pcie *rockchip, u32 val, u32 reg)
 {
 	writel(val, rockchip->apb_base + reg);
 }
 
-static void rockchip_pcie_enable_bw_int(struct rockchip_pcie_port *rockchip)
+static void rockchip_pcie_enable_bw_int(struct rockchip_pcie *rockchip)
 {
 	u32 status;
 
@@ -219,7 +219,7 @@ static void rockchip_pcie_enable_bw_int(struct rockchip_pcie_port *rockchip)
 	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_BASE + PCIE_RC_CONFIG_LCSR);
 }
 
-static void rockchip_pcie_clr_bw_int(struct rockchip_pcie_port *rockchip)
+static void rockchip_pcie_clr_bw_int(struct rockchip_pcie *rockchip)
 {
 	u32 status;
 
@@ -228,7 +228,7 @@ static void rockchip_pcie_clr_bw_int(struct rockchip_pcie_port *rockchip)
 	rockchip_pcie_write(rockchip, status, PCIE_RC_CONFIG_BASE + PCIE_RC_CONFIG_LCSR);
 }
 
-static int rockchip_pcie_valid_device(struct rockchip_pcie_port *rockchip,
+static int rockchip_pcie_valid_device(struct rockchip_pcie *rockchip,
 				      struct pci_bus *bus, int dev)
 {
 	/* access only one slot on each root port */
@@ -245,7 +245,7 @@ static int rockchip_pcie_valid_device(struct rockchip_pcie_port *rockchip,
 	return 1;
 }
 
-static int rockchip_pcie_rd_own_conf(struct rockchip_pcie_port *rockchip,
+static int rockchip_pcie_rd_own_conf(struct rockchip_pcie *rockchip,
 				     int where, int size, u32 *val)
 {
 	void __iomem *addr = rockchip->apb_base + PCIE_RC_CONFIG_BASE + where;
@@ -268,7 +268,7 @@ static int rockchip_pcie_rd_own_conf(struct rockchip_pcie_port *rockchip,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static int rockchip_pcie_wr_own_conf(struct rockchip_pcie_port *rockchip,
+static int rockchip_pcie_wr_own_conf(struct rockchip_pcie *rockchip,
 				     int where, int size, u32 val)
 {
 	u32 mask, tmp, offset;
@@ -289,7 +289,7 @@ static int rockchip_pcie_wr_own_conf(struct rockchip_pcie_port *rockchip,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static int rockchip_pcie_rd_other_conf(struct rockchip_pcie_port *rockchip,
+static int rockchip_pcie_rd_other_conf(struct rockchip_pcie *rockchip,
 				       struct pci_bus *bus, u32 devfn,
 				       int where, int size, u32 *val)
 {
@@ -316,7 +316,7 @@ static int rockchip_pcie_rd_other_conf(struct rockchip_pcie_port *rockchip,
 	return PCIBIOS_SUCCESSFUL;
 }
 
-static int rockchip_pcie_wr_other_conf(struct rockchip_pcie_port *rockchip,
+static int rockchip_pcie_wr_other_conf(struct rockchip_pcie *rockchip,
 				       struct pci_bus *bus, u32 devfn,
 				       int where, int size, u32 val)
 {
@@ -342,7 +342,7 @@ static int rockchip_pcie_wr_other_conf(struct rockchip_pcie_port *rockchip,
 static int rockchip_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 				 int size, u32 *val)
 {
-	struct rockchip_pcie_port *rockchip = bus->sysdata;
+	struct rockchip_pcie *rockchip = bus->sysdata;
 
 	if (!rockchip_pcie_valid_device(rockchip, bus, PCI_SLOT(devfn))) {
 		*val = 0xffffffff;
@@ -358,7 +358,7 @@ static int rockchip_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 static int rockchip_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 				 int where, int size, u32 val)
 {
-	struct rockchip_pcie_port *rockchip = bus->sysdata;
+	struct rockchip_pcie *rockchip = bus->sysdata;
 
 	if (!rockchip_pcie_valid_device(rockchip, bus, PCI_SLOT(devfn)))
 		return PCIBIOS_DEVICE_NOT_FOUND;
@@ -378,7 +378,7 @@ static struct pci_ops rockchip_pcie_ops = {
  * rockchip_pcie_init_port - Initialize hardware
  * @rockchip: PCIe port information
  */
-static int rockchip_pcie_init_port(struct rockchip_pcie_port *rockchip)
+static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 {
 	int err;
 	u32 status;
@@ -579,7 +579,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie_port *rockchip)
 
 static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
 {
-	struct rockchip_pcie_port *rockchip = arg;
+	struct rockchip_pcie *rockchip = arg;
 	u32 reg;
 	u32 sub_reg;
 
@@ -642,7 +642,7 @@ static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
 
 static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
 {
-	struct rockchip_pcie_port *rockchip = arg;
+	struct rockchip_pcie *rockchip = arg;
 	u32 reg;
 
 	reg = rockchip_pcie_read(rockchip, PCIE_CLIENT_INT_STATUS);
@@ -684,7 +684,7 @@ static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
 static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
-	struct rockchip_pcie_port *rockchip;
+	struct rockchip_pcie *rockchip;
 	u32 reg;
 	u32 hwirq;
 	u32 virq;
@@ -717,7 +717,7 @@ static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
  *
  * Return: '0' on success and error value on failure
  */
-static int rockchip_pcie_parse_dt(struct rockchip_pcie_port *rockchip)
+static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 {
 	struct device *dev = rockchip->dev;
 	struct platform_device *pdev = to_platform_device(dev);
@@ -874,7 +874,7 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie_port *rockchip)
 	return 0;
 }
 
-static int rockchip_pcie_set_vpcie(struct rockchip_pcie_port *rockchip)
+static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
 {
 	int err;
 
@@ -914,7 +914,7 @@ err_out:
 	return err;
 }
 
-static void rockchip_pcie_enable_interrupts(struct rockchip_pcie_port *rockchip)
+static void rockchip_pcie_enable_interrupts(struct rockchip_pcie *rockchip)
 {
 	rockchip_pcie_write(rockchip, (PCIE_CLIENT_INT_CLI << 16) &
 		   (~PCIE_CLIENT_INT_CLI), PCIE_CLIENT_INT_MASK);
@@ -936,7 +936,7 @@ static const struct irq_domain_ops intx_domain_ops = {
 	.map = rockchip_pcie_intx_map,
 };
 
-static int rockchip_pcie_init_irq_domain(struct rockchip_pcie_port *rockchip)
+static int rockchip_pcie_init_irq_domain(struct rockchip_pcie *rockchip)
 {
 	struct device *dev = rockchip->dev;
 	struct device_node *intc = of_get_next_child(dev->of_node, NULL);
@@ -956,7 +956,7 @@ static int rockchip_pcie_init_irq_domain(struct rockchip_pcie_port *rockchip)
 	return 0;
 }
 
-static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie_port *rockchip,
+static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie *rockchip,
 				     int region_no, int type, u8 num_pass_bits,
 				     u32 lower_addr, u32 upper_addr)
 {
@@ -996,7 +996,7 @@ static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie_port *rockchip,
 	return 0;
 }
 
-static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie_port *rockchip,
+static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie *rockchip,
 				     int region_no, u8 num_pass_bits,
 				     u32 lower_addr, u32 upper_addr)
 {
@@ -1026,7 +1026,7 @@ static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie_port *rockchip,
 
 static int rockchip_pcie_probe(struct platform_device *pdev)
 {
-	struct rockchip_pcie_port *rockchip;
+	struct rockchip_pcie *rockchip;
 	struct device *dev = &pdev->dev;
 	struct pci_bus *bus, *child;
 	struct resource_entry *win;

[toc] | [prev] | [next] | [standalone]

#1474557 — [PATCH 9/9] Move msleeps to address Guenter's comments.

FromBjorn Helgaas <bhelgaas@google.com>
Date2016-09-01 18:50 +0200
Subject[PATCH 9/9] Move msleeps to address Guenter's comments.
Message-ID<scDlE-4lf-53@gated-at.bofh.it>
In reply to#1474553
---
 drivers/pci/host/pcie-rockchip.c |   34 +++++++---------------------------
 1 file changed, 7 insertions(+), 27 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 61b0630..6623598 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -476,23 +476,12 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 			break;
 		}
 
-		msleep(20);
-
-		if (!time_before(jiffies, timeout)) {
-			err = -ETIMEDOUT;
-			break;
-		}
-	}
-
-	/* Double check gen1 training */
-	if (err) {
-		status = rockchip_pcie_read(rockchip, PCIE_CLIENT_BASIC_STATUS1);
-		err = ((status & PCIE_CLIENT_LINK_STATUS_MASK) ==
-		       PCIE_CLIENT_LINK_STATUS_UP) ? 0 : -ETIMEDOUT;
-		if (err) {
+		if (time_after(jiffies, timeout)) {
 			dev_err(dev, "PCIe link training gen1 timeout!\n");
-			return err;
+			return -ETIMEDOUT;
 		}
+
+		msleep(20);
 	}
 
 	/*
@@ -514,21 +503,12 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 			break;
 		}
 
-		msleep(20);
-
-		if (!time_before(jiffies, timeout)) {
-			err = -ETIMEDOUT;
+		if (time_after(jiffies, timeout)) {
+			dev_dbg(dev, "PCIe link training gen2 timeout, fall back to gen1!\n");
 			break;
 		}
-	}
 
-	/* Double check gen2 training */
-	if (err) {
-		status = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL_MGMT_BASE);
-		err = ((status & PCIE_CORE_PL_CONF_SPEED_MASK) ==
-		       PCIE_CORE_PL_CONF_SPEED_5G) ? 0 : -ETIMEDOUT;
-		if (err)
-			dev_dbg(dev, "PCIe link training gen2 timeout, fall back to gen1!\n");
+		msleep(20);
 	}
 
 	/* Check the final link width from negotiated lane counter from MGMT */

[toc] | [prev] | [next] | [standalone]

#1474561 — [PATCH 5/9] Use a local "dev" to avoid repetition of "rockchip->dev".

FromBjorn Helgaas <bhelgaas@google.com>
Date2016-09-01 18:50 +0200
Subject[PATCH 5/9] Use a local "dev" to avoid repetition of "rockchip->dev".
Message-ID<scDlE-4lf-63@gated-at.bofh.it>
In reply to#1474553
---
 drivers/pci/host/pcie-rockchip.c |   95 ++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 45 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 63fb0ebc..17612e5 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -380,6 +380,7 @@ static struct pci_ops rockchip_pcie_ops = {
  */
 static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 {
+	struct device *dev = rockchip->dev;
 	int err;
 	u32 status;
 	unsigned long timeout;
@@ -388,31 +389,31 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 
 	err = phy_init(rockchip->phy);
 	if (err < 0) {
-		dev_err(rockchip->dev, "fail to init phy, err %d\n", err);
+		dev_err(dev, "fail to init phy, err %d\n", err);
 		return err;
 	}
 
 	err = reset_control_assert(rockchip->core_rst);
 	if (err) {
-		dev_err(rockchip->dev, "assert core_rst err %d\n", err);
+		dev_err(dev, "assert core_rst err %d\n", err);
 		return err;
 	}
 
 	err = reset_control_assert(rockchip->mgmt_rst);
 	if (err) {
-		dev_err(rockchip->dev, "assert mgmt_rst err %d\n", err);
+		dev_err(dev, "assert mgmt_rst err %d\n", err);
 		return err;
 	}
 
 	err = reset_control_assert(rockchip->mgmt_sticky_rst);
 	if (err) {
-		dev_err(rockchip->dev, "assert mgmt_sticky_rst err %d\n", err);
+		dev_err(dev, "assert mgmt_sticky_rst err %d\n", err);
 		return err;
 	}
 
 	err = reset_control_assert(rockchip->pipe_rst);
 	if (err) {
-		dev_err(rockchip->dev, "assert pipe_rst err %d\n", err);
+		dev_err(dev, "assert pipe_rst err %d\n", err);
 		return err;
 	}
 
@@ -436,31 +437,31 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 
 	err = phy_power_on(rockchip->phy);
 	if (err) {
-		dev_err(rockchip->dev, "fail to power on phy, err %d\n", err);
+		dev_err(dev, "fail to power on phy, err %d\n", err);
 		return err;
 	}
 
 	err = reset_control_deassert(rockchip->core_rst);
 	if (err) {
-		dev_err(rockchip->dev, "deassert core_rst err %d\n", err);
+		dev_err(dev, "deassert core_rst err %d\n", err);
 		return err;
 	}
 
 	err = reset_control_deassert(rockchip->mgmt_rst);
 	if (err) {
-		dev_err(rockchip->dev, "deassert mgmt_rst err %d\n", err);
+		dev_err(dev, "deassert mgmt_rst err %d\n", err);
 		return err;
 	}
 
 	err = reset_control_deassert(rockchip->mgmt_sticky_rst);
 	if (err) {
-		dev_err(rockchip->dev, "deassert mgmt_sticky_rst err %d\n", err);
+		dev_err(dev, "deassert mgmt_sticky_rst err %d\n", err);
 		return err;
 	}
 
 	err = reset_control_deassert(rockchip->pipe_rst);
 	if (err) {
-		dev_err(rockchip->dev, "deassert pipe_rst err %d\n", err);
+		dev_err(dev, "deassert pipe_rst err %d\n", err);
 		return err;
 	}
 
@@ -492,7 +493,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		if (((status >> PCIE_CLIENT_LINK_STATUS_SHIFT) &
 		      PCIE_CLIENT_LINK_STATUS_MASK) ==
 		      PCIE_CLIENT_LINK_STATUS_UP) {
-			dev_dbg(rockchip->dev, "PCIe link training gen1 pass!\n");
+			dev_dbg(dev, "PCIe link training gen1 pass!\n");
 			break;
 		}
 
@@ -511,7 +512,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 			PCIE_CLIENT_LINK_STATUS_MASK) ==
 			PCIE_CLIENT_LINK_STATUS_UP) ? 0 : -ETIMEDOUT;
 		if (err) {
-			dev_err(rockchip->dev, "PCIe link training gen1 timeout!\n");
+			dev_err(dev, "PCIe link training gen1 timeout!\n");
 			return err;
 		}
 	}
@@ -532,7 +533,7 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 		if (((status >> PCIE_CORE_PL_CONF_SPEED_SHIFT) &
 		     PCIE_CORE_PL_CONF_SPEED_MASK) ==
 		     PCIE_CORE_PL_CONF_SPEED_5G) {
-			dev_dbg(rockchip->dev, "PCIe link training gen2 pass!\n");
+			dev_dbg(dev, "PCIe link training gen2 pass!\n");
 			break;
 		}
 
@@ -551,14 +552,14 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 			PCIE_CORE_PL_CONF_SPEED_MASK) ==
 			PCIE_CORE_PL_CONF_SPEED_5G) ? 0 : -ETIMEDOUT;
 		if (err)
-			dev_dbg(rockchip->dev, "PCIe link training gen2 timeout, fall back to gen1!\n");
+			dev_dbg(dev, "PCIe link training gen2 timeout, fall back to gen1!\n");
 	}
 
 	/* Check the final link width from negotiated lane counter from MGMT */
 	status = rockchip_pcie_read(rockchip, PCIE_CORE_CTRL_MGMT_BASE);
 	status =  0x1 << ((status >> PCIE_CORE_PL_CONF_LANE_SHIFT) &
 			   PCIE_CORE_PL_CONF_LANE_MASK);
-	dev_dbg(rockchip->dev, "current link width is x%d\n", status);
+	dev_dbg(dev, "current link width is x%d\n", status);
 
 	rockchip_pcie_write(rockchip, ROCKCHIP_VENDOR_ID, PCIE_RC_CONFIG_BASE);
 	rockchip_pcie_write(rockchip, PCI_CLASS_BRIDGE_PCI << PCIE_CORE_RC_CONF_SCC_SHIFT,
@@ -580,58 +581,59 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
 {
 	struct rockchip_pcie *rockchip = arg;
+	struct device *dev = rockchip->dev;
 	u32 reg;
 	u32 sub_reg;
 
 	reg = rockchip_pcie_read(rockchip, PCIE_CLIENT_INT_STATUS);
 	if (reg & PCIE_CLIENT_INT_LOCAL) {
-		dev_dbg(rockchip->dev, "local interrupt received\n");
+		dev_dbg(dev, "local interrupt received\n");
 		sub_reg = rockchip_pcie_read(rockchip, PCIE_CORE_INT_STATUS);
 		if (sub_reg & PCIE_CORE_INT_PRFPE)
-			dev_dbg(rockchip->dev, "parity error detected while reading from the PNP receive FIFO RAM\n");
+			dev_dbg(dev, "parity error detected while reading from the PNP receive FIFO RAM\n");
 
 		if (sub_reg & PCIE_CORE_INT_CRFPE)
-			dev_dbg(rockchip->dev, "parity error detected while reading from the Completion Receive FIFO RAM\n");
+			dev_dbg(dev, "parity error detected while reading from the Completion Receive FIFO RAM\n");
 
 		if (sub_reg & PCIE_CORE_INT_RRPE)
-			dev_dbg(rockchip->dev, "parity error detected while reading from replay buffer RAM\n");
+			dev_dbg(dev, "parity error detected while reading from replay buffer RAM\n");
 
 		if (sub_reg & PCIE_CORE_INT_PRFO)
-			dev_dbg(rockchip->dev, "overflow occurred in the PNP receive FIFO\n");
+			dev_dbg(dev, "overflow occurred in the PNP receive FIFO\n");
 
 		if (sub_reg & PCIE_CORE_INT_CRFO)
-			dev_dbg(rockchip->dev, "overflow occurred in the completion receive FIFO\n");
+			dev_dbg(dev, "overflow occurred in the completion receive FIFO\n");
 
 		if (sub_reg & PCIE_CORE_INT_RT)
-			dev_dbg(rockchip->dev, "replay timer timed out\n");
+			dev_dbg(dev, "replay timer timed out\n");
 
 		if (sub_reg & PCIE_CORE_INT_RTR)
-			dev_dbg(rockchip->dev, "replay timer rolled over after 4 transmissions of the same TLP\n");
+			dev_dbg(dev, "replay timer rolled over after 4 transmissions of the same TLP\n");
 
 		if (sub_reg & PCIE_CORE_INT_PE)
-			dev_dbg(rockchip->dev, "phy error detected on receive side\n");
+			dev_dbg(dev, "phy error detected on receive side\n");
 
 		if (sub_reg & PCIE_CORE_INT_MTR)
-			dev_dbg(rockchip->dev, "malformed TLP received from the link\n");
+			dev_dbg(dev, "malformed TLP received from the link\n");
 
 		if (sub_reg & PCIE_CORE_INT_UCR)
-			dev_dbg(rockchip->dev, "malformed TLP received from the link\n");
+			dev_dbg(dev, "malformed TLP received from the link\n");
 
 		if (sub_reg & PCIE_CORE_INT_FCE)
-			dev_dbg(rockchip->dev, "an error was observed in the flow control advertisements from the other side\n");
+			dev_dbg(dev, "an error was observed in the flow control advertisements from the other side\n");
 
 		if (sub_reg & PCIE_CORE_INT_CT)
-			dev_dbg(rockchip->dev, "a request timed out waiting for completion\n");
+			dev_dbg(dev, "a request timed out waiting for completion\n");
 
 		if (sub_reg & PCIE_CORE_INT_UTC)
-			dev_dbg(rockchip->dev, "unmapped TC error\n");
+			dev_dbg(dev, "unmapped TC error\n");
 
 		if (sub_reg & PCIE_CORE_INT_MMVC)
-			dev_dbg(rockchip->dev, "MSI mask register changes\n");
+			dev_dbg(dev, "MSI mask register changes\n");
 
 		rockchip_pcie_write(rockchip, sub_reg, PCIE_CORE_INT_STATUS);
 	} else if (reg & PCIE_CLIENT_INT_PHY) {
-		dev_dbg(rockchip->dev, "phy link changes\n");
+		dev_dbg(dev, "phy link changes\n");
 		rockchip_pcie_clr_bw_int(rockchip);
 	}
 
@@ -643,32 +645,33 @@ static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
 static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
 {
 	struct rockchip_pcie *rockchip = arg;
+	struct device *dev = rockchip->dev;
 	u32 reg;
 
 	reg = rockchip_pcie_read(rockchip, PCIE_CLIENT_INT_STATUS);
 	if (reg & PCIE_CLIENT_INT_LEGACY_DONE)
-		dev_dbg(rockchip->dev, "legacy done interrupt received\n");
+		dev_dbg(dev, "legacy done interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_MSG)
-		dev_dbg(rockchip->dev, "message done interrupt received\n");
+		dev_dbg(dev, "message done interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_HOT_RST)
-		dev_dbg(rockchip->dev, "hot reset interrupt received\n");
+		dev_dbg(dev, "hot reset interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_DPA)
-		dev_dbg(rockchip->dev, "dpa interrupt received\n");
+		dev_dbg(dev, "dpa interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_FATAL_ERR)
-		dev_dbg(rockchip->dev, "fatal error interrupt received\n");
+		dev_dbg(dev, "fatal error interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_NFATAL_ERR)
-		dev_dbg(rockchip->dev, "no fatal error interrupt received\n");
+		dev_dbg(dev, "no fatal error interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_CORR_ERR)
-		dev_dbg(rockchip->dev, "correctable error interrupt received\n");
+		dev_dbg(dev, "correctable error interrupt received\n");
 
 	if (reg & PCIE_CLIENT_INT_PHY)
-		dev_dbg(rockchip->dev, "phy interrupt received\n");
+		dev_dbg(dev, "phy interrupt received\n");
 
 	rockchip_pcie_write(rockchip, reg & (PCIE_CLIENT_INT_LEGACY_DONE |
 			      PCIE_CLIENT_INT_MSG | PCIE_CLIENT_INT_HOT_RST |
@@ -685,6 +688,7 @@ static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct rockchip_pcie *rockchip;
+	struct device *dev = rockchip->dev;
 	u32 reg;
 	u32 hwirq;
 	u32 virq;
@@ -704,7 +708,7 @@ static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
 		if (virq)
 			generic_handle_irq(virq);
 		else
-			dev_err(rockchip->dev, "unexpected IRQ, INT%d\n", hwirq);
+			dev_err(dev, "unexpected IRQ, INT%d\n", hwirq);
 	}
 
 	chained_irq_exit(chip, desc);
@@ -876,12 +880,13 @@ static int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
 
 static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
 {
+	struct device *dev = rockchip->dev;
 	int err;
 
 	if (!IS_ERR(rockchip->vpcie3v3)) {
 		err = regulator_enable(rockchip->vpcie3v3);
 		if (err) {
-			dev_err(rockchip->dev, "fail to enable vpcie3v3 regulator\n");
+			dev_err(dev, "fail to enable vpcie3v3 regulator\n");
 			goto err_out;
 		}
 	}
@@ -889,7 +894,7 @@ static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
 	if (!IS_ERR(rockchip->vpcie1v8)) {
 		err = regulator_enable(rockchip->vpcie1v8);
 		if (err) {
-			dev_err(rockchip->dev, "fail to enable vpcie1v8 regulator\n");
+			dev_err(dev, "fail to enable vpcie1v8 regulator\n");
 			goto err_disable_3v3;
 		}
 	}
@@ -897,7 +902,7 @@ static int rockchip_pcie_set_vpcie(struct rockchip_pcie *rockchip)
 	if (!IS_ERR(rockchip->vpcie0v9)) {
 		err = regulator_enable(rockchip->vpcie0v9);
 		if (err) {
-			dev_err(rockchip->dev, "fail to enable vpcie0v9 regulator\n");
+			dev_err(dev, "fail to enable vpcie0v9 regulator\n");
 			goto err_disable_1v8;
 		}
 	}
@@ -1082,7 +1087,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 
 	err = rockchip_pcie_set_vpcie(rockchip);
 	if (err) {
-		dev_err(rockchip->dev, "failed to set vpcie regulator\n");
+		dev_err(dev, "failed to set vpcie regulator\n");
 		goto err_set_vpcie;
 	}
 
@@ -1117,7 +1122,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 			io_bus_addr = io->start - win->offset;
 			err = pci_remap_iospace(io, io_base);
 			if (err) {
-				dev_warn(rockchip->dev, "error %d: failed to map resource %pR\n",
+				dev_warn(dev, "error %d: failed to map resource %pR\n",
 					 err, io);
 				continue;
 			}

[toc] | [prev] | [next] | [standalone]

#1474563 — [PATCH 7/9] Simplify the confusing HIWORD_UPDATE scheme.

FromBjorn Helgaas <bhelgaas@google.com>
Date2016-09-01 18:50 +0200
Subject[PATCH 7/9] Simplify the confusing HIWORD_UPDATE scheme.
Message-ID<scDlE-4lf-65@gated-at.bofh.it>
In reply to#1474553
---
 drivers/pci/host/pcie-rockchip.c |   69 +++++++++++++-------------------------
 1 file changed, 23 insertions(+), 46 deletions(-)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 33bf2e1..7f6fe7d 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -115,36 +115,25 @@
 	  (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
 	   PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
 
-/*
-  * The higher 16-bit of this register is used for write protection
-  * only if BIT(x + 16) set to 1 the BIT(x) can be written.
-  */
-#define HIWORD_UPDATE(val, mask, shift) \
-	((val) << (shift) | (mask) << ((shift) + 16))
-
 #define RC_REGION_0_ADDR_TRANS_H		0x00000000
 #define RC_REGION_0_ADDR_TRANS_L		0x00000000
 #define RC_REGION_0_PASS_BITS			(25 - 1)
 #define MAX_AXI_WRAPPER_REGION_NUM		33
 #define PCIE_CORE_LCSR_RETRAIN_LINK		BIT(5)
-#define PCIE_CLIENT_CONF_ENABLE			BIT(0)
-#define PCIE_CLIENT_CONF_ENABLE_SHIFT		0
-#define PCIE_CLIENT_CONF_ENABLE_MASK		0x1
-#define PCIE_CLIENT_LINK_TRAIN_ENABLE		1
-#define PCIE_CLIENT_LINK_TRAIN_SHIFT		1
-#define PCIE_CLIENT_LINK_TRAIN_MASK		0x1
-#define PCIE_CLIENT_ARI_ENABLE			BIT(0)
-#define PCIE_CLIENT_ARI_ENABLE_SHIFT		3
-#define PCIE_CLIENT_ARI_ENABLE_MASK		0x1
-#define PCIE_CLIENT_CONF_LANE_NUM(x)		(x / 2)
-#define PCIE_CLIENT_CONF_LANE_NUM_SHIFT		4
-#define PCIE_CLIENT_CONF_LANE_NUM_MASK		0x3
-#define PCIE_CLIENT_MODE_RC			BIT(0)
-#define PCIE_CLIENT_MODE_SHIFT			6
-#define PCIE_CLIENT_MODE_MASK			0x1
-#define PCIE_CLIENT_GEN_SEL_2			1
-#define PCIE_CLIENT_GEN_SEL_SHIFT		7
-#define PCIE_CLIENT_GEN_SEL_MASK		0x1
+
+/*
+ * The higher 16-bit of this register is used for write protection
+ * only if BIT(x + 16) set to 1 the BIT(x) can be written.
+ */
+#define PCIE_CLIENT_CONF_ENABLE		(0x00010000 | 0x0001)
+#define PCIE_CLIENT_LINK_TRAIN_ENABLE	(0x00020000 | 0x0002)
+#define PCIE_CLIENT_ARI_ENABLE		(0x00080000 | 0x0008)
+#define PCIE_CLIENT_CONF_LANE_NUM(x)	(0x00300000 | (((x >> 1) & 3) << 4)
+#define PCIE_CLIENT_MODE_RC		(0x00400000 | 0x0040)
+#define PCIE_CLIENT_GEN_SEL(x)		(0x00800000 | ((x & 1) << 7)
+#define  PCIE_CLIENT_GEN_SEL_0			0
+#define  PCIE_CLIENT_GEN_SEL_2			1
+
 #define PCIE_CLIENT_LINK_STATUS_UP		0x3
 #define PCIE_CLIENT_LINK_STATUS_SHIFT		20
 #define PCIE_CLIENT_LINK_STATUS_MASK		0x3
@@ -423,22 +412,13 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 	}
 
 	rockchip_pcie_write(rockchip,
-		   HIWORD_UPDATE(PCIE_CLIENT_CONF_ENABLE,
-				 PCIE_CLIENT_CONF_ENABLE_MASK,
-				 PCIE_CLIENT_CONF_ENABLE_SHIFT) |
-		   HIWORD_UPDATE(PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes),
-				 PCIE_CLIENT_CONF_LANE_NUM_MASK,
-				 PCIE_CLIENT_CONF_LANE_NUM_SHIFT) |
-		   HIWORD_UPDATE(PCIE_CLIENT_MODE_RC,
-				 PCIE_CLIENT_MODE_MASK,
-				 PCIE_CLIENT_MODE_SHIFT) |
-		   HIWORD_UPDATE(PCIE_CLIENT_ARI_ENABLE,
-				 PCIE_CLIENT_ARI_ENABLE_MASK,
-				 PCIE_CLIENT_ARI_ENABLE_SHIFT) |
-		   HIWORD_UPDATE(PCIE_CLIENT_GEN_SEL_2,
-				 PCIE_CLIENT_GEN_SEL_MASK,
-				 PCIE_CLIENT_GEN_SEL_SHIFT),
-		   PCIE_CLIENT_BASE);
+			    PCIE_CLIENT_CONF_ENABLE |
+			    PCIE_CLIENT_LINK_TRAIN_ENABLE |
+			    PCIE_CLIENT_ARI_ENABLE |
+			    PCIE_CLIENT_CONF_LANE_NUM(rockchip->lanes) |
+			    PCIE_CLIENT_MODE_RC |
+			    PCIE_CLIENT_GEN_SEL(PCIE_CLIENT_GEN_SEL_2),
+			    PCIE_CLIENT_BASE);
 
 	err = phy_power_on(rockchip->phy);
 	if (err) {
@@ -482,11 +462,8 @@ static int rockchip_pcie_init_port(struct rockchip_pcie *rockchip)
 				 PCIE_RC_CONFIG_L1_SUBSTATE_CTRL2);
 
 	/* Enable Gen1 training */
-	rockchip_pcie_write(rockchip,
-		   HIWORD_UPDATE(PCIE_CLIENT_LINK_TRAIN_ENABLE,
-				 PCIE_CLIENT_LINK_TRAIN_MASK,
-				 PCIE_CLIENT_LINK_TRAIN_SHIFT),
-		   PCIE_CLIENT_BASE);
+	rockchip_pcie_write(rockchip, PCIE_CLIENT_LINK_TRAIN_ENABLE,
+			    PCIE_CLIENT_BASE);
 
 	gpiod_set_value(rockchip->ep_gpio, 1);

[toc] | [prev] | [next] | [standalone]

#1474566 — [PATCH 6/9] Add comment about why 32-bit read/modify/write isn't safe.

FromBjorn Helgaas <bhelgaas@google.com>
Date2016-09-01 18:50 +0200
Subject[PATCH 6/9] Add comment about why 32-bit read/modify/write isn't safe.
Message-ID<scDlF-4lf-71@gated-at.bofh.it>
In reply to#1474553
---
 drivers/pci/host/pcie-rockchip.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
index 17612e5..33bf2e1 100644
--- a/drivers/pci/host/pcie-rockchip.c
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -282,6 +282,11 @@ static int rockchip_pcie_wr_own_conf(struct rockchip_pcie *rockchip,
 
 	mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
 
+	/*
+	 * N.B. This read/modify/write isn't safe in general because it can
+	 * corrupt RW1C bits in adjacent registers.  But the hardware
+	 * doesn't support smaller writes.
+	 */
 	tmp = readl(rockchip->apb_base + PCIE_RC_CONFIG_BASE + offset) & mask;
 	tmp |= val << ((where & 0x3) * 8);
 	writel(tmp, rockchip->apb_base + PCIE_RC_CONFIG_BASE + offset);

[toc] | [prev] | [next] | [standalone]

#1474623

FromArnd Bergmann <arnd@arndb.de>
Date2016-09-01 23:20 +0200
Message-ID<scHyX-75a-47@gated-at.bofh.it>
In reply to#1474553
On Thursday, September 1, 2016 11:43:52 AM CEST Bjorn Helgaas wrote:
> These are cleanups against 2098142ae87d, the current pci/host-rockchip
> head in my tree.  This is basically the v10 driver posting (it's not
> identical because I had already tweaked a few things and squashed in
> Brian's bug fix).

I looked over the series and everything looks good, I particularly
like the way that 	patches 7 and 8 make the code so much more readable.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web