[”PATCH” 0/5] Asynchronous linkdown recovery

[”PATCH” 0/5] Asynchronous linkdown recovery

[bpeled]

From: Ben Peled <bpeled@marvell.com>

The following patches implement the required procedure to handle and recover from asynchronous PCIE link down events on Armada SoCs.

The procedure is defined as the following:
1) Prevent new access to the PCI-E I/F by disabling the LTSSM
2) Flush all pending transaction/access to the PCI-E I/F
3) HW reset the PCIE end point device (based on board support)
4) Reset the PCIE MAC
5) Reinitialize the PCIE root complex and enable the LTSSM

The execution of this procedure is triggered by the PCIE RST_LINK_DOWN interrupt

Ben Peled (5):
  PCI: armada8k: Disable LTSSM on link down interrupts
  PCI: armada8k: Add link-down handle
  PCI: armada8k: add device reset to link-down handle
  dt-bindings: pci: add system controller and MAC reset bit to    
    Armada 7K/8K controller bindings
  arm64: dts: marvell: add pcie mac reset to pcie

 Documentation/devicetree/bindings/pci/pci-armada8k.txt |   6 +
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi          |   7 ++
 drivers/pci/controller/dwc/pcie-armada8k.c             | 126 ++++++++++++++++++++
 3 files changed, 139 insertions(+)

-- 
2.7.4

[”PATCH” 0/5] Asynchronous linkdown recovery

[bpeled]

From: Ben Peled <bpeled@marvell.com>

The following patches implement the required procedure to handle and recover from asynchronous PCIE link down events on Armada SoCs.

The procedure is defined as the following:
1) Prevent new access to the PCI-E I/F by disabling the LTSSM
2) Flush all pending transaction/access to the PCI-E I/F
3) HW reset the PCIE end point device (based on board support)
4) Reset the PCIE MAC
5) Reinitialize the PCIE root complex and enable the LTSSM

The execution of this procedure is triggered by the PCIE RST_LINK_DOWN interrupt

Ben Peled (5):
  PCI: armada8k: Disable LTSSM on link down interrupts
  PCI: armada8k: Add link-down handle
  PCI: armada8k: add device reset to link-down handle
  dt-bindings: pci: add system controller and MAC reset bit to    
    Armada 7K/8K controller bindings
  arm64: dts: marvell: add pcie mac reset to pcie

 Documentation/devicetree/bindings/pci/pci-armada8k.txt |   6 +
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi          |   7 ++
 drivers/pci/controller/dwc/pcie-armada8k.c             | 126 ++++++++++++++++++++
 3 files changed, 139 insertions(+)

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[”PATCH” 1/5] PCI: armada8k: Disable LTSSM on link down interrupts

[bpeled]

From: Ben Peled <bpeled@marvell.com>

When a PCI link down condition is detected, the link training state
machine must be disabled immediately.

Signed-off-by: Marc St-Amand <mstamand@ciena.com>
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 38 ++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 13901f3..b2278b1 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -54,6 +54,10 @@ struct armada8k_pcie {
 #define PCIE_INT_C_ASSERT_MASK		BIT(11)
 #define PCIE_INT_D_ASSERT_MASK		BIT(12)
 
+#define PCIE_GLOBAL_INT_CAUSE2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x24)
+#define PCIE_GLOBAL_INT_MASK2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x28)
+#define PCIE_INT2_PHY_RST_LINK_DOWN	BIT(1)
+
 #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)
@@ -193,6 +197,11 @@ static void armada8k_pcie_establish_link(struct armada8k_pcie *pcie)
 	       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
 	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
 
+	/* Also enable link down interrupts */
+	reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG);
+	reg |= PCIE_INT2_PHY_RST_LINK_DOWN;
+	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG, reg);
+
 	if (!dw_pcie_link_up(pci)) {
 		/* Configuration done. Start LTSSM */
 		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
@@ -230,6 +239,35 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG);
 	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG, val);
 
+	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE2_REG);
+
+	if (PCIE_INT2_PHY_RST_LINK_DOWN & val) {
+		u32 ctrl_reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
+		/*
+		 * The link went down. Disable LTSSM immediately. This
+		 * unlocks the root complex config registers. Downstream
+		 * device accesses will return all-Fs
+		 */
+		ctrl_reg &= ~(PCIE_APP_LTSSM_EN);
+		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, ctrl_reg);
+		/*
+		 * Mask link down interrupts. They can be re-enabled once
+		 * the link is retrained.
+		 */
+		ctrl_reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG);
+		ctrl_reg &= ~PCIE_INT2_PHY_RST_LINK_DOWN;
+		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG, ctrl_reg);
+		/*
+		 * At this point a worker thread can be triggered to
+		 * initiate a link retrain. If link retrains were
+		 * possible, that is.
+		 */
+		dev_dbg(pci->dev, "%s: link went down\n", __func__);
+	}
+
+	/* Now clear the second interrupt cause. */
+	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE2_REG, val);
+
 	return IRQ_HANDLED;
 }
 
-- 
2.7.4

[”PATCH” 1/5] PCI: armada8k: Disable LTSSM on link down interrupts

[bpeled]

From: Ben Peled <bpeled@marvell.com>

When a PCI link down condition is detected, the link training state
machine must be disabled immediately.

Signed-off-by: Marc St-Amand <mstamand@ciena.com>
Signed-off-by: Konstantin Porotchkin <kostap@marvell.com>
Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 38 ++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 13901f3..b2278b1 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -54,6 +54,10 @@ struct armada8k_pcie {
 #define PCIE_INT_C_ASSERT_MASK		BIT(11)
 #define PCIE_INT_D_ASSERT_MASK		BIT(12)
 
+#define PCIE_GLOBAL_INT_CAUSE2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x24)
+#define PCIE_GLOBAL_INT_MASK2_REG	(PCIE_VENDOR_REGS_OFFSET + 0x28)
+#define PCIE_INT2_PHY_RST_LINK_DOWN	BIT(1)
+
 #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)
@@ -193,6 +197,11 @@ static void armada8k_pcie_establish_link(struct armada8k_pcie *pcie)
 	       PCIE_INT_C_ASSERT_MASK | PCIE_INT_D_ASSERT_MASK;
 	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK1_REG, reg);
 
+	/* Also enable link down interrupts */
+	reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG);
+	reg |= PCIE_INT2_PHY_RST_LINK_DOWN;
+	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG, reg);
+
 	if (!dw_pcie_link_up(pci)) {
 		/* Configuration done. Start LTSSM */
 		reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
@@ -230,6 +239,35 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG);
 	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE1_REG, val);
 
+	val = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_CAUSE2_REG);
+
+	if (PCIE_INT2_PHY_RST_LINK_DOWN & val) {
+		u32 ctrl_reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_CONTROL_REG);
+		/*
+		 * The link went down. Disable LTSSM immediately. This
+		 * unlocks the root complex config registers. Downstream
+		 * device accesses will return all-Fs
+		 */
+		ctrl_reg &= ~(PCIE_APP_LTSSM_EN);
+		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_CONTROL_REG, ctrl_reg);
+		/*
+		 * Mask link down interrupts. They can be re-enabled once
+		 * the link is retrained.
+		 */
+		ctrl_reg = dw_pcie_readl_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG);
+		ctrl_reg &= ~PCIE_INT2_PHY_RST_LINK_DOWN;
+		dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_MASK2_REG, ctrl_reg);
+		/*
+		 * At this point a worker thread can be triggered to
+		 * initiate a link retrain. If link retrains were
+		 * possible, that is.
+		 */
+		dev_dbg(pci->dev, "%s: link went down\n", __func__);
+	}
+
+	/* Now clear the second interrupt cause. */
+	dw_pcie_writel_dbi(pci, PCIE_GLOBAL_INT_CAUSE2_REG, val);
+
 	return IRQ_HANDLED;
 }
 
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[”PATCH” 2/5] PCI: armada8k: Add link-down handle

[bpeled]

From: Ben Peled <bpeled@marvell.com>

In PCIE ISR routine caused by RST_LINK_DOWN
we schedule work to handle the link-down procedure.
Link-down procedure will:
1. Remove PCIe bus
2. Reset the MAC
3. Reconfigure link back up
4. Rescan PCIe bus

Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 68 ++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index b2278b1..4eb8607 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -22,6 +22,8 @@
 #include <linux/resource.h>
 #include <linux/of_pci.h>
 #include <linux/of_irq.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #include "pcie-designware.h"
 
@@ -33,6 +35,9 @@ struct armada8k_pcie {
 	struct clk *clk_reg;
 	struct phy *phy[ARMADA8K_PCIE_MAX_LANES];
 	unsigned int phy_count;
+	struct regmap *sysctrl_base;
+	u32 mac_rest_bitmask;
+	struct work_struct recover_link_work;
 };
 
 #define PCIE_VENDOR_REGS_OFFSET		0x8000
@@ -73,6 +78,8 @@ struct armada8k_pcie {
 #define AX_USER_DOMAIN_MASK		0x3
 #define AX_USER_DOMAIN_SHIFT		4
 
+#define UNIT_SOFT_RESET_CONFIG_REG	0x268
+
 #define to_armada8k_pcie(x)	dev_get_drvdata((x)->dev)
 
 static void armada8k_pcie_disable_phys(struct armada8k_pcie *pcie)
@@ -224,6 +231,49 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
 
 	return 0;
 }
+static void armada8k_pcie_recover_link(struct work_struct *ws)
+{
+	struct armada8k_pcie *pcie = container_of(ws, struct armada8k_pcie, recover_link_work);
+	struct pcie_port *pp = &pcie->pci->pp;
+	struct pci_bus *bus = pp->bridge->bus;
+	struct pci_dev *root_port;
+	int ret;
+
+	root_port = pci_get_slot(bus, 0);
+	if (!root_port) {
+		dev_err(pcie->pci->dev, "failed to get root port\n");
+		return;
+	}
+	pci_lock_rescan_remove();
+	pci_stop_and_remove_bus_device(root_port);
+	/*
+	 * Sleep needed to make sure all pcie transactions and access
+	 * are flushed before resetting the mac
+	 */
+	msleep(100);
+
+	/* Reset mac */
+	regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG,
+				pcie->mac_rest_bitmask, 0, NULL, false, true);
+	udelay(1);
+	regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG,
+				pcie->mac_rest_bitmask, pcie->mac_rest_bitmask,
+				NULL, false, true);
+	udelay(1);
+	ret = armada8k_pcie_host_init(pp);
+	if (ret) {
+		dev_err(pcie->pci->dev, "failed to initialize host: %d\n", ret);
+		pci_unlock_rescan_remove();
+		pci_dev_put(root_port);
+		return;
+	}
+
+	bus = NULL;
+	while ((bus = pci_find_next_bus(bus)) != NULL)
+		pci_rescan_bus(bus);
+	pci_unlock_rescan_remove();
+	pci_dev_put(root_port);
+}
 
 static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 {
@@ -262,6 +312,9 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 		 * initiate a link retrain. If link retrains were
 		 * possible, that is.
 		 */
+		if (pcie->sysctrl_base && pcie->mac_rest_bitmask)
+			schedule_work(&pcie->recover_link_work);
+
 		dev_dbg(pci->dev, "%s: link went down\n", __func__);
 	}
 
@@ -330,6 +383,8 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 
 	pcie->pci = pci;
 
+	INIT_WORK(&pcie->recover_link_work, armada8k_pcie_recover_link);
+
 	pcie->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(pcie->clk))
 		return PTR_ERR(pcie->clk);
@@ -357,6 +412,19 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 		goto fail_clkreg;
 	}
 
+	pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+						       "marvell,system-controller");
+	if (IS_ERR(pcie->sysctrl_base)) {
+		dev_warn(dev, "failed to find marvell,system-controller\n");
+		pcie->sysctrl_base = 0x0;
+	}
+
+	ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-bit-mask",
+				   &pcie->mac_rest_bitmask);
+	if (ret < 0) {
+		dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret);
+		pcie->mac_rest_bitmask = 0x0;
+	}
 	ret = armada8k_pcie_setup_phys(pcie);
 	if (ret)
 		goto fail_clkreg;
-- 
2.7.4

[”PATCH” 2/5] PCI: armada8k: Add link-down handle

[bpeled]

From: Ben Peled <bpeled@marvell.com>

In PCIE ISR routine caused by RST_LINK_DOWN
we schedule work to handle the link-down procedure.
Link-down procedure will:
1. Remove PCIe bus
2. Reset the MAC
3. Reconfigure link back up
4. Rescan PCIe bus

Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 68 ++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index b2278b1..4eb8607 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -22,6 +22,8 @@
 #include <linux/resource.h>
 #include <linux/of_pci.h>
 #include <linux/of_irq.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #include "pcie-designware.h"
 
@@ -33,6 +35,9 @@ struct armada8k_pcie {
 	struct clk *clk_reg;
 	struct phy *phy[ARMADA8K_PCIE_MAX_LANES];
 	unsigned int phy_count;
+	struct regmap *sysctrl_base;
+	u32 mac_rest_bitmask;
+	struct work_struct recover_link_work;
 };
 
 #define PCIE_VENDOR_REGS_OFFSET		0x8000
@@ -73,6 +78,8 @@ struct armada8k_pcie {
 #define AX_USER_DOMAIN_MASK		0x3
 #define AX_USER_DOMAIN_SHIFT		4
 
+#define UNIT_SOFT_RESET_CONFIG_REG	0x268
+
 #define to_armada8k_pcie(x)	dev_get_drvdata((x)->dev)
 
 static void armada8k_pcie_disable_phys(struct armada8k_pcie *pcie)
@@ -224,6 +231,49 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
 
 	return 0;
 }
+static void armada8k_pcie_recover_link(struct work_struct *ws)
+{
+	struct armada8k_pcie *pcie = container_of(ws, struct armada8k_pcie, recover_link_work);
+	struct pcie_port *pp = &pcie->pci->pp;
+	struct pci_bus *bus = pp->bridge->bus;
+	struct pci_dev *root_port;
+	int ret;
+
+	root_port = pci_get_slot(bus, 0);
+	if (!root_port) {
+		dev_err(pcie->pci->dev, "failed to get root port\n");
+		return;
+	}
+	pci_lock_rescan_remove();
+	pci_stop_and_remove_bus_device(root_port);
+	/*
+	 * Sleep needed to make sure all pcie transactions and access
+	 * are flushed before resetting the mac
+	 */
+	msleep(100);
+
+	/* Reset mac */
+	regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG,
+				pcie->mac_rest_bitmask, 0, NULL, false, true);
+	udelay(1);
+	regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG,
+				pcie->mac_rest_bitmask, pcie->mac_rest_bitmask,
+				NULL, false, true);
+	udelay(1);
+	ret = armada8k_pcie_host_init(pp);
+	if (ret) {
+		dev_err(pcie->pci->dev, "failed to initialize host: %d\n", ret);
+		pci_unlock_rescan_remove();
+		pci_dev_put(root_port);
+		return;
+	}
+
+	bus = NULL;
+	while ((bus = pci_find_next_bus(bus)) != NULL)
+		pci_rescan_bus(bus);
+	pci_unlock_rescan_remove();
+	pci_dev_put(root_port);
+}
 
 static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 {
@@ -262,6 +312,9 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
 		 * initiate a link retrain. If link retrains were
 		 * possible, that is.
 		 */
+		if (pcie->sysctrl_base && pcie->mac_rest_bitmask)
+			schedule_work(&pcie->recover_link_work);
+
 		dev_dbg(pci->dev, "%s: link went down\n", __func__);
 	}
 
@@ -330,6 +383,8 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 
 	pcie->pci = pci;
 
+	INIT_WORK(&pcie->recover_link_work, armada8k_pcie_recover_link);
+
 	pcie->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(pcie->clk))
 		return PTR_ERR(pcie->clk);
@@ -357,6 +412,19 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
 		goto fail_clkreg;
 	}
 
+	pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+						       "marvell,system-controller");
+	if (IS_ERR(pcie->sysctrl_base)) {
+		dev_warn(dev, "failed to find marvell,system-controller\n");
+		pcie->sysctrl_base = 0x0;
+	}
+
+	ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-bit-mask",
+				   &pcie->mac_rest_bitmask);
+	if (ret < 0) {
+		dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret);
+		pcie->mac_rest_bitmask = 0x0;
+	}
 	ret = armada8k_pcie_setup_phys(pcie);
 	if (ret)
 		goto fail_clkreg;
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [”PATCH” 2/5] PCI: armada8k: Add link-down handle

[Jonathan Cameron]

On Mon, 12 Apr 2021 18:30:53 +0300
<bpeled@marvell.com> wrote:

> From: Ben Peled <bpeled@marvell.com>
> 
> In PCIE ISR routine caused by RST_LINK_DOWN
> we schedule work to handle the link-down procedure.
> Link-down procedure will:
> 1. Remove PCIe bus
> 2. Reset the MAC
> 3. Reconfigure link back up
> 4. Rescan PCIe bus
> 
> Signed-off-by: Ben Peled <bpeled@marvell.com>

Trivial comment inline.

Also, something odd with quotes around PATCH in the title you probably
want to clean up.

> ---
>  drivers/pci/controller/dwc/pcie-armada8k.c | 68 ++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index b2278b1..4eb8607 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -22,6 +22,8 @@
>  #include <linux/resource.h>
>  #include <linux/of_pci.h>
>  #include <linux/of_irq.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -33,6 +35,9 @@ struct armada8k_pcie {
>  	struct clk *clk_reg;
>  	struct phy *phy[ARMADA8K_PCIE_MAX_LANES];
>  	unsigned int phy_count;
> +	struct regmap *sysctrl_base;
> +	u32 mac_rest_bitmask;
> +	struct work_struct recover_link_work;
>  };
>  
>  #define PCIE_VENDOR_REGS_OFFSET		0x8000
> @@ -73,6 +78,8 @@ struct armada8k_pcie {
>  #define AX_USER_DOMAIN_MASK		0x3
>  #define AX_USER_DOMAIN_SHIFT		4
>  
> +#define UNIT_SOFT_RESET_CONFIG_REG	0x268
> +
>  #define to_armada8k_pcie(x)	dev_get_drvdata((x)->dev)
>  
>  static void armada8k_pcie_disable_phys(struct armada8k_pcie *pcie)
> @@ -224,6 +231,49 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
>  
>  	return 0;
>  }
> +static void armada8k_pcie_recover_link(struct work_struct *ws)
> +{
> +	struct armada8k_pcie *pcie = container_of(ws, struct armada8k_pcie, recover_link_work);
> +	struct pcie_port *pp = &pcie->pci->pp;
> +	struct pci_bus *bus = pp->bridge->bus;
> +	struct pci_dev *root_port;
> +	int ret;
> +
> +	root_port = pci_get_slot(bus, 0);
> +	if (!root_port) {
> +		dev_err(pcie->pci->dev, "failed to get root port\n");
> +		return;
> +	}
> +	pci_lock_rescan_remove();
> +	pci_stop_and_remove_bus_device(root_port);
> +	/*
> +	 * Sleep needed to make sure all pcie transactions and access
> +	 * are flushed before resetting the mac
> +	 */
> +	msleep(100);
> +
> +	/* Reset mac */
> +	regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG,
> +				pcie->mac_rest_bitmask, 0, NULL, false, true);
> +	udelay(1);
> +	regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG,
> +				pcie->mac_rest_bitmask, pcie->mac_rest_bitmask,
> +				NULL, false, true);
> +	udelay(1);
> +	ret = armada8k_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(pcie->pci->dev, "failed to initialize host: %d\n", ret);
> +		pci_unlock_rescan_remove();
> +		pci_dev_put(root_port);
> +		return;
> +	}
> +
> +	bus = NULL;
> +	while ((bus = pci_find_next_bus(bus)) != NULL)
> +		pci_rescan_bus(bus);
> +	pci_unlock_rescan_remove();
> +	pci_dev_put(root_port);
> +}
>  
>  static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
>  {
> @@ -262,6 +312,9 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
>  		 * initiate a link retrain. If link retrains were
>  		 * possible, that is.
>  		 */
> +		if (pcie->sysctrl_base && pcie->mac_rest_bitmask)
> +			schedule_work(&pcie->recover_link_work);
> +
>  		dev_dbg(pci->dev, "%s: link went down\n", __func__);
>  	}
>  
> @@ -330,6 +383,8 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  
>  	pcie->pci = pci;
>  
> +	INIT_WORK(&pcie->recover_link_work, armada8k_pcie_recover_link);
> +
>  	pcie->clk = devm_clk_get(dev, NULL);
>  	if (IS_ERR(pcie->clk))
>  		return PTR_ERR(pcie->clk);
> @@ -357,6 +412,19 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  		goto fail_clkreg;
>  	}
>  
> +	pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +						       "marvell,system-controller");
> +	if (IS_ERR(pcie->sysctrl_base)) {
> +		dev_warn(dev, "failed to find marvell,system-controller\n");
> +		pcie->sysctrl_base = 0x0;

= NULL; ?

> +	}
> +
> +	ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-bit-mask",
> +				   &pcie->mac_rest_bitmask);
> +	if (ret < 0) {
> +		dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret);
> +		pcie->mac_rest_bitmask = 0x0;
> +	}
>  	ret = armada8k_pcie_setup_phys(pcie);
>  	if (ret)
>  		goto fail_clkreg;

Re: [”PATCH” 2/5] PCI: armada8k: Add link-down handle

[Jonathan Cameron]

On Mon, 12 Apr 2021 18:30:53 +0300
<bpeled@marvell.com> wrote:

> From: Ben Peled <bpeled@marvell.com>
> 
> In PCIE ISR routine caused by RST_LINK_DOWN
> we schedule work to handle the link-down procedure.
> Link-down procedure will:
> 1. Remove PCIe bus
> 2. Reset the MAC
> 3. Reconfigure link back up
> 4. Rescan PCIe bus
> 
> Signed-off-by: Ben Peled <bpeled@marvell.com>

Trivial comment inline.

Also, something odd with quotes around PATCH in the title you probably
want to clean up.

> ---
>  drivers/pci/controller/dwc/pcie-armada8k.c | 68 ++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
> index b2278b1..4eb8607 100644
> --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> @@ -22,6 +22,8 @@
>  #include <linux/resource.h>
>  #include <linux/of_pci.h>
>  #include <linux/of_irq.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -33,6 +35,9 @@ struct armada8k_pcie {
>  	struct clk *clk_reg;
>  	struct phy *phy[ARMADA8K_PCIE_MAX_LANES];
>  	unsigned int phy_count;
> +	struct regmap *sysctrl_base;
> +	u32 mac_rest_bitmask;
> +	struct work_struct recover_link_work;
>  };
>  
>  #define PCIE_VENDOR_REGS_OFFSET		0x8000
> @@ -73,6 +78,8 @@ struct armada8k_pcie {
>  #define AX_USER_DOMAIN_MASK		0x3
>  #define AX_USER_DOMAIN_SHIFT		4
>  
> +#define UNIT_SOFT_RESET_CONFIG_REG	0x268
> +
>  #define to_armada8k_pcie(x)	dev_get_drvdata((x)->dev)
>  
>  static void armada8k_pcie_disable_phys(struct armada8k_pcie *pcie)
> @@ -224,6 +231,49 @@ static int armada8k_pcie_host_init(struct pcie_port *pp)
>  
>  	return 0;
>  }
> +static void armada8k_pcie_recover_link(struct work_struct *ws)
> +{
> +	struct armada8k_pcie *pcie = container_of(ws, struct armada8k_pcie, recover_link_work);
> +	struct pcie_port *pp = &pcie->pci->pp;
> +	struct pci_bus *bus = pp->bridge->bus;
> +	struct pci_dev *root_port;
> +	int ret;
> +
> +	root_port = pci_get_slot(bus, 0);
> +	if (!root_port) {
> +		dev_err(pcie->pci->dev, "failed to get root port\n");
> +		return;
> +	}
> +	pci_lock_rescan_remove();
> +	pci_stop_and_remove_bus_device(root_port);
> +	/*
> +	 * Sleep needed to make sure all pcie transactions and access
> +	 * are flushed before resetting the mac
> +	 */
> +	msleep(100);
> +
> +	/* Reset mac */
> +	regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG,
> +				pcie->mac_rest_bitmask, 0, NULL, false, true);
> +	udelay(1);
> +	regmap_update_bits_base(pcie->sysctrl_base, UNIT_SOFT_RESET_CONFIG_REG,
> +				pcie->mac_rest_bitmask, pcie->mac_rest_bitmask,
> +				NULL, false, true);
> +	udelay(1);
> +	ret = armada8k_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(pcie->pci->dev, "failed to initialize host: %d\n", ret);
> +		pci_unlock_rescan_remove();
> +		pci_dev_put(root_port);
> +		return;
> +	}
> +
> +	bus = NULL;
> +	while ((bus = pci_find_next_bus(bus)) != NULL)
> +		pci_rescan_bus(bus);
> +	pci_unlock_rescan_remove();
> +	pci_dev_put(root_port);
> +}
>  
>  static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
>  {
> @@ -262,6 +312,9 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
>  		 * initiate a link retrain. If link retrains were
>  		 * possible, that is.
>  		 */
> +		if (pcie->sysctrl_base && pcie->mac_rest_bitmask)
> +			schedule_work(&pcie->recover_link_work);
> +
>  		dev_dbg(pci->dev, "%s: link went down\n", __func__);
>  	}
>  
> @@ -330,6 +383,8 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  
>  	pcie->pci = pci;
>  
> +	INIT_WORK(&pcie->recover_link_work, armada8k_pcie_recover_link);
> +
>  	pcie->clk = devm_clk_get(dev, NULL);
>  	if (IS_ERR(pcie->clk))
>  		return PTR_ERR(pcie->clk);
> @@ -357,6 +412,19 @@ static int armada8k_pcie_probe(struct platform_device *pdev)
>  		goto fail_clkreg;
>  	}
>  
> +	pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +						       "marvell,system-controller");
> +	if (IS_ERR(pcie->sysctrl_base)) {
> +		dev_warn(dev, "failed to find marvell,system-controller\n");
> +		pcie->sysctrl_base = 0x0;

= NULL; ?

> +	}
> +
> +	ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-bit-mask",
> +				   &pcie->mac_rest_bitmask);
> +	if (ret < 0) {
> +		dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret);
> +		pcie->mac_rest_bitmask = 0x0;
> +	}
>  	ret = armada8k_pcie_setup_phys(pcie);
>  	if (ret)
>  		goto fail_clkreg;


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [EXT] Re: [”PATCH” 2/5] PCI: armada8k: Add link-down handle

[Ben Peled]

Hi Jonathan,


> > In PCIE ISR routine caused by RST_LINK_DOWN we schedule work to handle
> > the link-down procedure.
> > Link-down procedure will:
> > 1. Remove PCIe bus
> > 2. Reset the MAC
> > 3. Reconfigure link back up
> > 4. Rescan PCIe bus
> >
> > Signed-off-by: Ben Peled <bpeled@marvell.com>
> 
> Trivial comment inline.
> 
> Also, something odd with quotes around PATCH in the title you probably
> want to clean up.
> 

I'll change the comment to more informative comments

> > ---
> >  drivers/pci/controller/dwc/pcie-armada8k.c | 68 ++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c
> > b/drivers/pci/controller/dwc/pcie-armada8k.c
> > index b2278b1..4eb8607 100644
> > --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> > +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> > @@ -22,6 +22,8 @@
> >  #include <linux/resource.h>
> >  #include <linux/of_pci.h>
> >  #include <linux/of_irq.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/regmap.h>
> >
> >  #include "pcie-designware.h"
> >
> > @@ -33,6 +35,9 @@ struct armada8k_pcie {
> >  	struct clk *clk_reg;
> >  	struct phy *phy[ARMADA8K_PCIE_MAX_LANES];
> >  	unsigned int phy_count;
> > +	struct regmap *sysctrl_base;
> > +	u32 mac_rest_bitmask;
> > +	struct work_struct recover_link_work;
> >  };
> >
> >  #define PCIE_VENDOR_REGS_OFFSET		0x8000
> > @@ -73,6 +78,8 @@ struct armada8k_pcie {
> >  #define AX_USER_DOMAIN_MASK		0x3
> >  #define AX_USER_DOMAIN_SHIFT		4
> >
> > +#define UNIT_SOFT_RESET_CONFIG_REG	0x268
> > +
> >  #define to_armada8k_pcie(x)	dev_get_drvdata((x)->dev)
> >
> >  static void armada8k_pcie_disable_phys(struct armada8k_pcie *pcie) @@
> > -224,6 +231,49 @@ static int armada8k_pcie_host_init(struct pcie_port
> > *pp)
> >
> >  	return 0;
> >  }
> > +static void armada8k_pcie_recover_link(struct work_struct *ws) {
> > +	struct armada8k_pcie *pcie = container_of(ws, struct
> armada8k_pcie, recover_link_work);
> > +	struct pcie_port *pp = &pcie->pci->pp;
> > +	struct pci_bus *bus = pp->bridge->bus;
> > +	struct pci_dev *root_port;
> > +	int ret;
> > +
> > +	root_port = pci_get_slot(bus, 0);
> > +	if (!root_port) {
> > +		dev_err(pcie->pci->dev, "failed to get root port\n");
> > +		return;
> > +	}
> > +	pci_lock_rescan_remove();
> > +	pci_stop_and_remove_bus_device(root_port);
> > +	/*
> > +	 * Sleep needed to make sure all pcie transactions and access
> > +	 * are flushed before resetting the mac
> > +	 */
> > +	msleep(100);
> > +
> > +	/* Reset mac */
> > +	regmap_update_bits_base(pcie->sysctrl_base,
> UNIT_SOFT_RESET_CONFIG_REG,
> > +				pcie->mac_rest_bitmask, 0, NULL, false, true);
> > +	udelay(1);
> > +	regmap_update_bits_base(pcie->sysctrl_base,
> UNIT_SOFT_RESET_CONFIG_REG,
> > +				pcie->mac_rest_bitmask, pcie-
> >mac_rest_bitmask,
> > +				NULL, false, true);
> > +	udelay(1);
> > +	ret = armada8k_pcie_host_init(pp);
> > +	if (ret) {
> > +		dev_err(pcie->pci->dev, "failed to initialize host: %d\n", ret);
> > +		pci_unlock_rescan_remove();
> > +		pci_dev_put(root_port);
> > +		return;
> > +	}
> > +
> > +	bus = NULL;
> > +	while ((bus = pci_find_next_bus(bus)) != NULL)
> > +		pci_rescan_bus(bus);
> > +	pci_unlock_rescan_remove();
> > +	pci_dev_put(root_port);
> > +}
> >
> >  static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)  {
> > @@ -262,6 +312,9 @@ static irqreturn_t armada8k_pcie_irq_handler(int
> irq, void *arg)
> >  		 * initiate a link retrain. If link retrains were
> >  		 * possible, that is.
> >  		 */
> > +		if (pcie->sysctrl_base && pcie->mac_rest_bitmask)
> > +			schedule_work(&pcie->recover_link_work);
> > +
> >  		dev_dbg(pci->dev, "%s: link went down\n", __func__);
> >  	}
> >
> > @@ -330,6 +383,8 @@ static int armada8k_pcie_probe(struct
> > platform_device *pdev)
> >
> >  	pcie->pci = pci;
> >
> > +	INIT_WORK(&pcie->recover_link_work,
> armada8k_pcie_recover_link);
> > +
> >  	pcie->clk = devm_clk_get(dev, NULL);
> >  	if (IS_ERR(pcie->clk))
> >  		return PTR_ERR(pcie->clk);
> > @@ -357,6 +412,19 @@ static int armada8k_pcie_probe(struct
> platform_device *pdev)
> >  		goto fail_clkreg;
> >  	}
> >
> > +	pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev-
> >dev.of_node,
> > +						       "marvell,system-
> controller");
> > +	if (IS_ERR(pcie->sysctrl_base)) {
> > +		dev_warn(dev, "failed to find marvell,system-controller\n");
> > +		pcie->sysctrl_base = 0x0;
> 
> = NULL; ?
> 
I will change this to NULL in v3
> > +	}
> > +
> > +	ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-
> bit-mask",
> > +				   &pcie->mac_rest_bitmask);
> > +	if (ret < 0) {
> > +		dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret);
> > +		pcie->mac_rest_bitmask = 0x0;
> > +	}
> >  	ret = armada8k_pcie_setup_phys(pcie);
> >  	if (ret)
> >  		goto fail_clkreg;

Thanks,
Ben

RE: [EXT] Re: [”PATCH” 2/5] PCI: armada8k: Add link-down handle

[Ben Peled]

Hi Jonathan,


> > In PCIE ISR routine caused by RST_LINK_DOWN we schedule work to handle
> > the link-down procedure.
> > Link-down procedure will:
> > 1. Remove PCIe bus
> > 2. Reset the MAC
> > 3. Reconfigure link back up
> > 4. Rescan PCIe bus
> >
> > Signed-off-by: Ben Peled <bpeled@marvell.com>
> 
> Trivial comment inline.
> 
> Also, something odd with quotes around PATCH in the title you probably
> want to clean up.
> 

I'll change the comment to more informative comments

> > ---
> >  drivers/pci/controller/dwc/pcie-armada8k.c | 68 ++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c
> > b/drivers/pci/controller/dwc/pcie-armada8k.c
> > index b2278b1..4eb8607 100644
> > --- a/drivers/pci/controller/dwc/pcie-armada8k.c
> > +++ b/drivers/pci/controller/dwc/pcie-armada8k.c
> > @@ -22,6 +22,8 @@
> >  #include <linux/resource.h>
> >  #include <linux/of_pci.h>
> >  #include <linux/of_irq.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/regmap.h>
> >
> >  #include "pcie-designware.h"
> >
> > @@ -33,6 +35,9 @@ struct armada8k_pcie {
> >  	struct clk *clk_reg;
> >  	struct phy *phy[ARMADA8K_PCIE_MAX_LANES];
> >  	unsigned int phy_count;
> > +	struct regmap *sysctrl_base;
> > +	u32 mac_rest_bitmask;
> > +	struct work_struct recover_link_work;
> >  };
> >
> >  #define PCIE_VENDOR_REGS_OFFSET		0x8000
> > @@ -73,6 +78,8 @@ struct armada8k_pcie {
> >  #define AX_USER_DOMAIN_MASK		0x3
> >  #define AX_USER_DOMAIN_SHIFT		4
> >
> > +#define UNIT_SOFT_RESET_CONFIG_REG	0x268
> > +
> >  #define to_armada8k_pcie(x)	dev_get_drvdata((x)->dev)
> >
> >  static void armada8k_pcie_disable_phys(struct armada8k_pcie *pcie) @@
> > -224,6 +231,49 @@ static int armada8k_pcie_host_init(struct pcie_port
> > *pp)
> >
> >  	return 0;
> >  }
> > +static void armada8k_pcie_recover_link(struct work_struct *ws) {
> > +	struct armada8k_pcie *pcie = container_of(ws, struct
> armada8k_pcie, recover_link_work);
> > +	struct pcie_port *pp = &pcie->pci->pp;
> > +	struct pci_bus *bus = pp->bridge->bus;
> > +	struct pci_dev *root_port;
> > +	int ret;
> > +
> > +	root_port = pci_get_slot(bus, 0);
> > +	if (!root_port) {
> > +		dev_err(pcie->pci->dev, "failed to get root port\n");
> > +		return;
> > +	}
> > +	pci_lock_rescan_remove();
> > +	pci_stop_and_remove_bus_device(root_port);
> > +	/*
> > +	 * Sleep needed to make sure all pcie transactions and access
> > +	 * are flushed before resetting the mac
> > +	 */
> > +	msleep(100);
> > +
> > +	/* Reset mac */
> > +	regmap_update_bits_base(pcie->sysctrl_base,
> UNIT_SOFT_RESET_CONFIG_REG,
> > +				pcie->mac_rest_bitmask, 0, NULL, false, true);
> > +	udelay(1);
> > +	regmap_update_bits_base(pcie->sysctrl_base,
> UNIT_SOFT_RESET_CONFIG_REG,
> > +				pcie->mac_rest_bitmask, pcie-
> >mac_rest_bitmask,
> > +				NULL, false, true);
> > +	udelay(1);
> > +	ret = armada8k_pcie_host_init(pp);
> > +	if (ret) {
> > +		dev_err(pcie->pci->dev, "failed to initialize host: %d\n", ret);
> > +		pci_unlock_rescan_remove();
> > +		pci_dev_put(root_port);
> > +		return;
> > +	}
> > +
> > +	bus = NULL;
> > +	while ((bus = pci_find_next_bus(bus)) != NULL)
> > +		pci_rescan_bus(bus);
> > +	pci_unlock_rescan_remove();
> > +	pci_dev_put(root_port);
> > +}
> >
> >  static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)  {
> > @@ -262,6 +312,9 @@ static irqreturn_t armada8k_pcie_irq_handler(int
> irq, void *arg)
> >  		 * initiate a link retrain. If link retrains were
> >  		 * possible, that is.
> >  		 */
> > +		if (pcie->sysctrl_base && pcie->mac_rest_bitmask)
> > +			schedule_work(&pcie->recover_link_work);
> > +
> >  		dev_dbg(pci->dev, "%s: link went down\n", __func__);
> >  	}
> >
> > @@ -330,6 +383,8 @@ static int armada8k_pcie_probe(struct
> > platform_device *pdev)
> >
> >  	pcie->pci = pci;
> >
> > +	INIT_WORK(&pcie->recover_link_work,
> armada8k_pcie_recover_link);
> > +
> >  	pcie->clk = devm_clk_get(dev, NULL);
> >  	if (IS_ERR(pcie->clk))
> >  		return PTR_ERR(pcie->clk);
> > @@ -357,6 +412,19 @@ static int armada8k_pcie_probe(struct
> platform_device *pdev)
> >  		goto fail_clkreg;
> >  	}
> >
> > +	pcie->sysctrl_base = syscon_regmap_lookup_by_phandle(pdev-
> >dev.of_node,
> > +						       "marvell,system-
> controller");
> > +	if (IS_ERR(pcie->sysctrl_base)) {
> > +		dev_warn(dev, "failed to find marvell,system-controller\n");
> > +		pcie->sysctrl_base = 0x0;
> 
> = NULL; ?
> 
I will change this to NULL in v3
> > +	}
> > +
> > +	ret = of_property_read_u32(pdev->dev.of_node, "marvell,mac-reset-
> bit-mask",
> > +				   &pcie->mac_rest_bitmask);
> > +	if (ret < 0) {
> > +		dev_warn(dev, "couldn't find mac reset bit mask: %d\n", ret);
> > +		pcie->mac_rest_bitmask = 0x0;
> > +	}
> >  	ret = armada8k_pcie_setup_phys(pcie);
> >  	if (ret)
> >  		goto fail_clkreg;

Thanks,
Ben




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[”PATCH” 3/5] dt-bindings: pci: add system controller and MAC reset bit to Armada 7K/8K controller bindings

[bpeled]

From: Ben Peled <bpeled@marvell.com>

Adding optional system-controller and mac-reset-bit-mask
needed for linkdown procedure.

Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 Documentation/devicetree/bindings/pci/pci-armada8k.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
index 7a813d0..2696e79 100644
--- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
+++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
@@ -24,6 +24,10 @@ Optional properties:
 - phy-names: names of the PHYs corresponding to the number of lanes.
 	Must be "cp0-pcie0-x4-lane0-phy", "cp0-pcie0-x4-lane1-phy" for
 	2 PHYs.
+- marvell,system-controller: address of system controller needed
+	in order to reset MAC used by link-down handle
+- marvell,mac-reset-bit-mask: MAC reset bit of system controller
+	needed in order to reset MAC used by link-down handle
 
 Example:
 
@@ -45,4 +49,6 @@ Example:
 		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 		num-lanes = <1>;
 		clocks = <&cpm_syscon0 1 13>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		marvell,mac-reset-bit-mask = <CP11X_PCIEx_MAC_RESET_BIT_MASK(1)>;
 	};
-- 
2.7.4

[”PATCH” 3/5] dt-bindings: pci: add system controller and MAC reset bit to Armada 7K/8K controller bindings

[bpeled]

From: Ben Peled <bpeled@marvell.com>

Adding optional system-controller and mac-reset-bit-mask
needed for linkdown procedure.

Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 Documentation/devicetree/bindings/pci/pci-armada8k.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
index 7a813d0..2696e79 100644
--- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
+++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
@@ -24,6 +24,10 @@ Optional properties:
 - phy-names: names of the PHYs corresponding to the number of lanes.
 	Must be "cp0-pcie0-x4-lane0-phy", "cp0-pcie0-x4-lane1-phy" for
 	2 PHYs.
+- marvell,system-controller: address of system controller needed
+	in order to reset MAC used by link-down handle
+- marvell,mac-reset-bit-mask: MAC reset bit of system controller
+	needed in order to reset MAC used by link-down handle
 
 Example:
 
@@ -45,4 +49,6 @@ Example:
 		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 		num-lanes = <1>;
 		clocks = <&cpm_syscon0 1 13>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		marvell,mac-reset-bit-mask = <CP11X_PCIEx_MAC_RESET_BIT_MASK(1)>;
 	};
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [”PATCH” 3/5] dt-bindings: pci: add system controller and MAC reset bit to Armada 7K/8K controller bindings

[Rob Herring]

On Mon, Apr 12, 2021 at 06:30:54PM +0300, bpeled@marvell.com wrote:
> From: Ben Peled <bpeled@marvell.com>
> 
> Adding optional system-controller and mac-reset-bit-mask
> needed for linkdown procedure.
> 
> Signed-off-by: Ben Peled <bpeled@marvell.com>
> ---
>  Documentation/devicetree/bindings/pci/pci-armada8k.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> index 7a813d0..2696e79 100644
> --- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> @@ -24,6 +24,10 @@ Optional properties:
>  - phy-names: names of the PHYs corresponding to the number of lanes.
>  	Must be "cp0-pcie0-x4-lane0-phy", "cp0-pcie0-x4-lane1-phy" for
>  	2 PHYs.
> +- marvell,system-controller: address of system controller needed
> +	in order to reset MAC used by link-down handle
> +- marvell,mac-reset-bit-mask: MAC reset bit of system controller
> +	needed in order to reset MAC used by link-down handle

Seems like this should use the reset controller binding instead.

If not, this can be a single property with a phandle plus arg.

Rob

Re: [”PATCH” 3/5] dt-bindings: pci: add system controller and MAC reset bit to Armada 7K/8K controller bindings

[Rob Herring]

On Mon, Apr 12, 2021 at 06:30:54PM +0300, bpeled@marvell.com wrote:
> From: Ben Peled <bpeled@marvell.com>
> 
> Adding optional system-controller and mac-reset-bit-mask
> needed for linkdown procedure.
> 
> Signed-off-by: Ben Peled <bpeled@marvell.com>
> ---
>  Documentation/devicetree/bindings/pci/pci-armada8k.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> index 7a813d0..2696e79 100644
> --- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> @@ -24,6 +24,10 @@ Optional properties:
>  - phy-names: names of the PHYs corresponding to the number of lanes.
>  	Must be "cp0-pcie0-x4-lane0-phy", "cp0-pcie0-x4-lane1-phy" for
>  	2 PHYs.
> +- marvell,system-controller: address of system controller needed
> +	in order to reset MAC used by link-down handle
> +- marvell,mac-reset-bit-mask: MAC reset bit of system controller
> +	needed in order to reset MAC used by link-down handle

Seems like this should use the reset controller binding instead.

If not, this can be a single property with a phandle plus arg.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [EXT] Re: [”PATCH” 3/5] dt-bindings: pci: add system controller and MAC reset bit to Armada 7K/8K controller bindings

[Ben Peled]

Hi Rob,
Sorry I missed it.

> > diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> > b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> > index 7a813d0..2696e79 100644
> > --- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> > +++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> > @@ -24,6 +24,10 @@ Optional properties:
> >  - phy-names: names of the PHYs corresponding to the number of lanes.
> >  	Must be "cp0-pcie0-x4-lane0-phy", "cp0-pcie0-x4-lane1-phy" for
> >  	2 PHYs.
> > +- marvell,system-controller: address of system controller needed
> > +	in order to reset MAC used by link-down handle
> > +- marvell,mac-reset-bit-mask: MAC reset bit of system controller
> > +	needed in order to reset MAC used by link-down handle
> 
> Seems like this should use the reset controller binding instead.
> 
> If not, this can be a single property with a phandle plus arg.
> 
> Rob

I will fix it v3.
Thanks

RE: [EXT] Re: [”PATCH” 3/5] dt-bindings: pci: add system controller and MAC reset bit to Armada 7K/8K controller bindings

[Ben Peled]

Hi Rob,
Sorry I missed it.

> > diff --git a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> > b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> > index 7a813d0..2696e79 100644
> > --- a/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> > +++ b/Documentation/devicetree/bindings/pci/pci-armada8k.txt
> > @@ -24,6 +24,10 @@ Optional properties:
> >  - phy-names: names of the PHYs corresponding to the number of lanes.
> >  	Must be "cp0-pcie0-x4-lane0-phy", "cp0-pcie0-x4-lane1-phy" for
> >  	2 PHYs.
> > +- marvell,system-controller: address of system controller needed
> > +	in order to reset MAC used by link-down handle
> > +- marvell,mac-reset-bit-mask: MAC reset bit of system controller
> > +	needed in order to reset MAC used by link-down handle
> 
> Seems like this should use the reset controller binding instead.
> 
> If not, this can be a single property with a phandle plus arg.
> 
> Rob

I will fix it v3.
Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[”PATCH” 4/5] arm64: dts: marvell: add pcie mac reset to pcie

[bpeled]

From: Ben Peled <bpeled@marvell.com>

Add system controller and reset bit to each pcie to enable pcie mac reset

Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 9dcf16b..eb60e73 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -11,6 +11,7 @@
 #include "armada-common.dtsi"
 
 #define CP11X_PCIEx_CONF_BASE(iface)	(CP11X_PCIEx_MEM_BASE(iface) + CP11X_PCIEx_MEM_SIZE(iface))
+#define CP11X_PCIEx_MAC_RESET_BIT_MASK(n)	(0x1 << 11 + ((n + 2) % 3))
 
 / {
 	/*
@@ -513,6 +514,8 @@
 		num-lanes = <1>;
 		clock-names = "core", "reg";
 		clocks = <&CP11X_LABEL(clk) 1 13>, <&CP11X_LABEL(clk) 1 14>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		marvell,mac-reset-bit-mask = <CP11X_PCIEx_MAC_RESET_BIT_MASK(0)>;
 		status = "disabled";
 	};
 
@@ -538,6 +541,8 @@
 		num-lanes = <1>;
 		clock-names = "core", "reg";
 		clocks = <&CP11X_LABEL(clk) 1 11>, <&CP11X_LABEL(clk) 1 14>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		marvell,mac-reset-bit-mask = <CP11X_PCIEx_MAC_RESET_BIT_MASK(1)>;
 		status = "disabled";
 	};
 
@@ -563,6 +568,8 @@
 		num-lanes = <1>;
 		clock-names = "core", "reg";
 		clocks = <&CP11X_LABEL(clk) 1 12>, <&CP11X_LABEL(clk) 1 14>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		marvell,mac-reset-bit-mask = <CP11X_PCIEx_MAC_RESET_BIT_MASK(2)>;
 		status = "disabled";
 	};
 };
-- 
2.7.4

[”PATCH” 4/5] arm64: dts: marvell: add pcie mac reset to pcie

[bpeled]

From: Ben Peled <bpeled@marvell.com>

Add system controller and reset bit to each pcie to enable pcie mac reset

Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 9dcf16b..eb60e73 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -11,6 +11,7 @@
 #include "armada-common.dtsi"
 
 #define CP11X_PCIEx_CONF_BASE(iface)	(CP11X_PCIEx_MEM_BASE(iface) + CP11X_PCIEx_MEM_SIZE(iface))
+#define CP11X_PCIEx_MAC_RESET_BIT_MASK(n)	(0x1 << 11 + ((n + 2) % 3))
 
 / {
 	/*
@@ -513,6 +514,8 @@
 		num-lanes = <1>;
 		clock-names = "core", "reg";
 		clocks = <&CP11X_LABEL(clk) 1 13>, <&CP11X_LABEL(clk) 1 14>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		marvell,mac-reset-bit-mask = <CP11X_PCIEx_MAC_RESET_BIT_MASK(0)>;
 		status = "disabled";
 	};
 
@@ -538,6 +541,8 @@
 		num-lanes = <1>;
 		clock-names = "core", "reg";
 		clocks = <&CP11X_LABEL(clk) 1 11>, <&CP11X_LABEL(clk) 1 14>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		marvell,mac-reset-bit-mask = <CP11X_PCIEx_MAC_RESET_BIT_MASK(1)>;
 		status = "disabled";
 	};
 
@@ -563,6 +568,8 @@
 		num-lanes = <1>;
 		clock-names = "core", "reg";
 		clocks = <&CP11X_LABEL(clk) 1 12>, <&CP11X_LABEL(clk) 1 14>;
+		marvell,system-controller = <&CP11X_LABEL(syscon0)>;
+		marvell,mac-reset-bit-mask = <CP11X_PCIEx_MAC_RESET_BIT_MASK(2)>;
 		status = "disabled";
 	};
 };
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[”PATCH” 5/5] PCI: armada8k: add device reset to link-down handle

[bpeled]

From: Ben Peled <bpeled@marvell.com>

Added pcie reset via gpio support as described in the
designware-pcie.txt DT binding document.
In cases link down cause still exist in device.
The device need to be reset to reestablish the link.
If reset-gpio pin provided in the device tree, then the linkdown
handle resets the device before reestablishing link.

Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 4eb8607..83ac91e 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -24,6 +24,7 @@
 #include <linux/of_irq.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/of_gpio.h>
 
 #include "pcie-designware.h"
 
@@ -38,6 +39,7 @@ struct armada8k_pcie {
 	struct regmap *sysctrl_base;
 	u32 mac_rest_bitmask;
 	struct work_struct recover_link_work;
+	enum of_gpio_flags flags;
 };
 
 #define PCIE_VENDOR_REGS_OFFSET		0x8000
-- 
2.7.4

[”PATCH” 5/5] PCI: armada8k: add device reset to link-down handle

[bpeled]

From: Ben Peled <bpeled@marvell.com>

Added pcie reset via gpio support as described in the
designware-pcie.txt DT binding document.
In cases link down cause still exist in device.
The device need to be reset to reestablish the link.
If reset-gpio pin provided in the device tree, then the linkdown
handle resets the device before reestablishing link.

Signed-off-by: Ben Peled <bpeled@marvell.com>
---
 drivers/pci/controller/dwc/pcie-armada8k.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 4eb8607..83ac91e 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -24,6 +24,7 @@
 #include <linux/of_irq.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/of_gpio.h>
 
 #include "pcie-designware.h"
 
@@ -38,6 +39,7 @@ struct armada8k_pcie {
 	struct regmap *sysctrl_base;
 	u32 mac_rest_bitmask;
 	struct work_struct recover_link_work;
+	enum of_gpio_flags flags;
 };
 
 #define PCIE_VENDOR_REGS_OFFSET		0x8000
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [”PATCH” 0/5] Asynchronous linkdown recovery

[Ben Peled]

Hi all,
Please ignore this patch list there is a small change missing. 

-----Original Message-----
From: bpeled@marvell.com <bpeled@marvell.com> 
Sent: Monday, April 12, 2021 6:31 PM
To: thomas.petazzoni@bootlin.com; lorenzo.pieralisi@arm.com; bhelgaas@google.com
Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-pci@vger.kernel.org; sebastian.hesselbarth@gmail.com; gregory.clement@bootlin.com; andrew@lunn.ch; robh+dt@kernel.org; mw@semihalf.com; jaz@semihalf.com; Kostya Porotchkin <kostap@marvell.com>; Nadav Haklai <nadavh@marvell.com>; Stefan Chulski <stefanc@marvell.com>; Ofer Heifetz <oferh@marvell.com>; Ben Peled <bpeled@marvell.com>
Subject: [”PATCH” 0/5] Asynchronous linkdown recovery

From: Ben Peled <bpeled@marvell.com>

The following patches implement the required procedure to handle and recover from asynchronous PCIE link down events on Armada SoCs.

The procedure is defined as the following:
1) Prevent new access to the PCI-E I/F by disabling the LTSSM
2) Flush all pending transaction/access to the PCI-E I/F
3) HW reset the PCIE end point device (based on board support)
4) Reset the PCIE MAC
5) Reinitialize the PCIE root complex and enable the LTSSM

The execution of this procedure is triggered by the PCIE RST_LINK_DOWN interrupt

Ben Peled (5):
  PCI: armada8k: Disable LTSSM on link down interrupts
  PCI: armada8k: Add link-down handle
  PCI: armada8k: add device reset to link-down handle
  dt-bindings: pci: add system controller and MAC reset bit to    
    Armada 7K/8K controller bindings
  arm64: dts: marvell: add pcie mac reset to pcie

 Documentation/devicetree/bindings/pci/pci-armada8k.txt |   6 +
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi          |   7 ++
 drivers/pci/controller/dwc/pcie-armada8k.c             | 126 ++++++++++++++++++++
 3 files changed, 139 insertions(+)

-- 
2.7.4

RE: [”PATCH” 0/5] Asynchronous linkdown recovery

[Ben Peled]

Hi all,
Please ignore this patch list there is a small change missing. 

-----Original Message-----
From: bpeled@marvell.com <bpeled@marvell.com> 
Sent: Monday, April 12, 2021 6:31 PM
To: thomas.petazzoni@bootlin.com; lorenzo.pieralisi@arm.com; bhelgaas@google.com
Cc: linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-pci@vger.kernel.org; sebastian.hesselbarth@gmail.com; gregory.clement@bootlin.com; andrew@lunn.ch; robh+dt@kernel.org; mw@semihalf.com; jaz@semihalf.com; Kostya Porotchkin <kostap@marvell.com>; Nadav Haklai <nadavh@marvell.com>; Stefan Chulski <stefanc@marvell.com>; Ofer Heifetz <oferh@marvell.com>; Ben Peled <bpeled@marvell.com>
Subject: [”PATCH” 0/5] Asynchronous linkdown recovery

From: Ben Peled <bpeled@marvell.com>

The following patches implement the required procedure to handle and recover from asynchronous PCIE link down events on Armada SoCs.

The procedure is defined as the following:
1) Prevent new access to the PCI-E I/F by disabling the LTSSM
2) Flush all pending transaction/access to the PCI-E I/F
3) HW reset the PCIE end point device (based on board support)
4) Reset the PCIE MAC
5) Reinitialize the PCIE root complex and enable the LTSSM

The execution of this procedure is triggered by the PCIE RST_LINK_DOWN interrupt

Ben Peled (5):
  PCI: armada8k: Disable LTSSM on link down interrupts
  PCI: armada8k: Add link-down handle
  PCI: armada8k: add device reset to link-down handle
  dt-bindings: pci: add system controller and MAC reset bit to    
    Armada 7K/8K controller bindings
  arm64: dts: marvell: add pcie mac reset to pcie

 Documentation/devicetree/bindings/pci/pci-armada8k.txt |   6 +
 arch/arm64/boot/dts/marvell/armada-cp11x.dtsi          |   7 ++
 drivers/pci/controller/dwc/pcie-armada8k.c             | 126 ++++++++++++++++++++
 3 files changed, 139 insertions(+)

-- 
2.7.4

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel