[PATCH v2 0/7] PCI: xilinx-nwl: Add phy support

[PATCH v2 0/7] PCI: xilinx-nwl: Add phy support

[Sean Anderson]

Add phy subsystem support for the xilinx-nwl PCIe controller. This
series also includes several small fixes and improvements.

Changes in v2:
- Remove phy-names
- Add an example
- Get phys by index and not by name

Sean Anderson (7):
  dt-bindings: pci: xilinx-nwl: Add phys
  PCI: xilinx-nwl: Fix off-by-one
  PCI: xilinx-nwl: Fix register misspelling
  PCI: xilinx-nwl: Rate-limit misc interrupt messages
  PCI: xilinx-nwl: Clean up clock on probe failure/removal
  PCI: xilinx-nwl: Add phy support
  arm64: zynqmp: Add PCIe phys

 .../bindings/pci/xlnx,nwl-pcie.yaml           |   6 +
 .../boot/dts/xilinx/zynqmp-zcu102-revA.dts    |   1 +
 drivers/pci/controller/pcie-xilinx-nwl.c      | 122 ++++++++++++++----
 3 files changed, 106 insertions(+), 23 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty


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

[PATCH v2 1/7] dt-bindings: pci: xilinx-nwl: Add phys

[Sean Anderson]

Add phys properties so Linux can power-on/configure the GTR
transcievers.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

Changes in v2:
- Remove phy-names
- Add an example

 Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
index 426f90a47f35..693b29039a9b 100644
--- a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
@@ -61,6 +61,10 @@ properties:
   interrupt-map:
     maxItems: 4
 
+  phys:
+    minItems: 1
+    maxItems: 4
+
   power-domains:
     maxItems: 1
 
@@ -110,6 +114,7 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/phy/phy.h>
     #include <dt-bindings/power/xlnx-zynqmp-power.h>
     soc {
         #address-cells = <2>;
@@ -138,6 +143,7 @@ examples:
                             <0x0 0x0 0x0 0x3 &pcie_intc 0x3>,
                             <0x0 0x0 0x0 0x4 &pcie_intc 0x4>;
             msi-parent = <&nwl_pcie>;
+            phys = <&psgtr 0 PHY_TYPE_PCIE 0 0>;
             power-domains = <&zynqmp_firmware PD_PCIE>;
             iommus = <&smmu 0x4d0>;
             pcie_intc: legacy-interrupt-controller {
-- 
2.35.1.1320.gc452695387.dirty


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

[PATCH v2 2/7] PCI: xilinx-nwl: Fix off-by-one

[Sean Anderson]

IRQs start at 0, so we don't need to subtract 1.

Fixes: 9a181e1093af ("PCI: xilinx-nwl: Modify IRQ chip for legacy interrupts")
Cc: <stable@vger.kernel.org>
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

(no changes since v1)

 drivers/pci/controller/pcie-xilinx-nwl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 0408f4d612b5..437927e3bcca 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -371,7 +371,7 @@ static void nwl_mask_intx_irq(struct irq_data *data)
 	u32 mask;
 	u32 val;
 
-	mask = 1 << (data->hwirq - 1);
+	mask = 1 << data->hwirq;
 	raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
 	val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
 	nwl_bridge_writel(pcie, (val & (~mask)), MSGF_LEG_MASK);
@@ -385,7 +385,7 @@ static void nwl_unmask_intx_irq(struct irq_data *data)
 	u32 mask;
 	u32 val;
 
-	mask = 1 << (data->hwirq - 1);
+	mask = 1 << data->hwirq;
 	raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
 	val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
 	nwl_bridge_writel(pcie, (val | mask), MSGF_LEG_MASK);
-- 
2.35.1.1320.gc452695387.dirty


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

[PATCH v2 3/7] PCI: xilinx-nwl: Fix register misspelling

[Sean Anderson]

MSIC -> MISC

Fixes: c2a7ff18edcd ("PCI: xilinx-nwl: Expand error logging")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

(no changes since v1)

 drivers/pci/controller/pcie-xilinx-nwl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 437927e3bcca..ce881baac6d8 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -80,8 +80,8 @@
 #define MSGF_MISC_SR_NON_FATAL_DEV	BIT(22)
 #define MSGF_MISC_SR_FATAL_DEV		BIT(23)
 #define MSGF_MISC_SR_LINK_DOWN		BIT(24)
-#define MSGF_MSIC_SR_LINK_AUTO_BWIDTH	BIT(25)
-#define MSGF_MSIC_SR_LINK_BWIDTH	BIT(26)
+#define MSGF_MISC_SR_LINK_AUTO_BWIDTH	BIT(25)
+#define MSGF_MISC_SR_LINK_BWIDTH	BIT(26)
 
 #define MSGF_MISC_SR_MASKALL		(MSGF_MISC_SR_RXMSG_AVAIL | \
 					MSGF_MISC_SR_RXMSG_OVER | \
@@ -96,8 +96,8 @@
 					MSGF_MISC_SR_NON_FATAL_DEV | \
 					MSGF_MISC_SR_FATAL_DEV | \
 					MSGF_MISC_SR_LINK_DOWN | \
-					MSGF_MSIC_SR_LINK_AUTO_BWIDTH | \
-					MSGF_MSIC_SR_LINK_BWIDTH)
+					MSGF_MISC_SR_LINK_AUTO_BWIDTH | \
+					MSGF_MISC_SR_LINK_BWIDTH)
 
 /* Legacy interrupt status mask bits */
 #define MSGF_LEG_SR_INTA		BIT(0)
@@ -299,10 +299,10 @@ static irqreturn_t nwl_pcie_misc_handler(int irq, void *data)
 	if (misc_stat & MSGF_MISC_SR_FATAL_DEV)
 		dev_err(dev, "Fatal Error Detected\n");
 
-	if (misc_stat & MSGF_MSIC_SR_LINK_AUTO_BWIDTH)
+	if (misc_stat & MSGF_MISC_SR_LINK_AUTO_BWIDTH)
 		dev_info(dev, "Link Autonomous Bandwidth Management Status bit set\n");
 
-	if (misc_stat & MSGF_MSIC_SR_LINK_BWIDTH)
+	if (misc_stat & MSGF_MISC_SR_LINK_BWIDTH)
 		dev_info(dev, "Link Bandwidth Management Status bit set\n");
 
 	/* Clear misc interrupt status */
-- 
2.35.1.1320.gc452695387.dirty


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

[PATCH v2 4/7] PCI: xilinx-nwl: Rate-limit misc interrupt messages

[Sean Anderson]

The conditions logged by the misc interrupt can occur repeatedly and
continuously. Avoid rendering the console unusable by rate-limiting
these messages.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

(no changes since v1)

 drivers/pci/controller/pcie-xilinx-nwl.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index ce881baac6d8..c0a60cebdb2e 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -267,37 +267,37 @@ static irqreturn_t nwl_pcie_misc_handler(int irq, void *data)
 		return IRQ_NONE;
 
 	if (misc_stat & MSGF_MISC_SR_RXMSG_OVER)
-		dev_err(dev, "Received Message FIFO Overflow\n");
+		dev_err_ratelimited(dev, "Received Message FIFO Overflow\n");
 
 	if (misc_stat & MSGF_MISC_SR_SLAVE_ERR)
-		dev_err(dev, "Slave error\n");
+		dev_err_ratelimited(dev, "Slave error\n");
 
 	if (misc_stat & MSGF_MISC_SR_MASTER_ERR)
-		dev_err(dev, "Master error\n");
+		dev_err_ratelimited(dev, "Master error\n");
 
 	if (misc_stat & MSGF_MISC_SR_I_ADDR_ERR)
-		dev_err(dev, "In Misc Ingress address translation error\n");
+		dev_err_ratelimited(dev, "In Misc Ingress address translation error\n");
 
 	if (misc_stat & MSGF_MISC_SR_E_ADDR_ERR)
-		dev_err(dev, "In Misc Egress address translation error\n");
+		dev_err_ratelimited(dev, "In Misc Egress address translation error\n");
 
 	if (misc_stat & MSGF_MISC_SR_FATAL_AER)
-		dev_err(dev, "Fatal Error in AER Capability\n");
+		dev_err_ratelimited(dev, "Fatal Error in AER Capability\n");
 
 	if (misc_stat & MSGF_MISC_SR_NON_FATAL_AER)
-		dev_err(dev, "Non-Fatal Error in AER Capability\n");
+		dev_err_ratelimited(dev, "Non-Fatal Error in AER Capability\n");
 
 	if (misc_stat & MSGF_MISC_SR_CORR_AER)
-		dev_err(dev, "Correctable Error in AER Capability\n");
+		dev_err_ratelimited(dev, "Correctable Error in AER Capability\n");
 
 	if (misc_stat & MSGF_MISC_SR_UR_DETECT)
-		dev_err(dev, "Unsupported request Detected\n");
+		dev_err_ratelimited(dev, "Unsupported request Detected\n");
 
 	if (misc_stat & MSGF_MISC_SR_NON_FATAL_DEV)
-		dev_err(dev, "Non-Fatal Error Detected\n");
+		dev_err_ratelimited(dev, "Non-Fatal Error Detected\n");
 
 	if (misc_stat & MSGF_MISC_SR_FATAL_DEV)
-		dev_err(dev, "Fatal Error Detected\n");
+		dev_err_ratelimited(dev, "Fatal Error Detected\n");
 
 	if (misc_stat & MSGF_MISC_SR_LINK_AUTO_BWIDTH)
 		dev_info(dev, "Link Autonomous Bandwidth Management Status bit set\n");
-- 
2.35.1.1320.gc452695387.dirty


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

[PATCH v2 5/7] PCI: xilinx-nwl: Clean up clock on probe failure/removal

[Sean Anderson]

Make sure we turn off the clock on probe failure and device removal.

Fixes: de0a01f52966 ("PCI: xilinx-nwl: Enable the clock through CCF")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

(no changes since v1)

 drivers/pci/controller/pcie-xilinx-nwl.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index c0a60cebdb2e..424cc5a1b4d1 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -779,6 +779,7 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	pcie = pci_host_bridge_priv(bridge);
+	platform_set_drvdata(pdev, pcie);
 
 	pcie->dev = dev;
 
@@ -801,13 +802,13 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 	err = nwl_pcie_bridge_init(pcie);
 	if (err) {
 		dev_err(dev, "HW Initialization failed\n");
-		return err;
+		goto err_clk;
 	}
 
 	err = nwl_pcie_init_irq_domain(pcie);
 	if (err) {
 		dev_err(dev, "Failed creating IRQ Domain\n");
-		return err;
+		goto err_clk;
 	}
 
 	bridge->sysdata = pcie;
@@ -817,11 +818,23 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 		err = nwl_pcie_enable_msi(pcie);
 		if (err < 0) {
 			dev_err(dev, "failed to enable MSI support: %d\n", err);
-			return err;
+			goto err_clk;
 		}
 	}
 
-	return pci_host_probe(bridge);
+	err = pci_host_probe(bridge);
+
+err_clk:
+	if (err)
+		clk_disable_unprepare(pcie->clk);
+	return err;
+}
+
+static void nwl_pcie_remove(struct platform_device *pdev)
+{
+	struct nwl_pcie *pcie = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(pcie->clk);
 }
 
 static struct platform_driver nwl_pcie_driver = {
@@ -831,5 +844,6 @@ static struct platform_driver nwl_pcie_driver = {
 		.of_match_table = nwl_pcie_of_match,
 	},
 	.probe = nwl_pcie_probe,
+	.remove_new = nwl_pcie_remove,
 };
 builtin_platform_driver(nwl_pcie_driver);
-- 
2.35.1.1320.gc452695387.dirty


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

[PATCH v2 6/7] PCI: xilinx-nwl: Add phy support

[Sean Anderson]

Add support for enabling/disabling PCIe phys. We can't really do
anything about failures in the disable/remove path, so just warn.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

Changes in v2:
- Get phys by index and not by name

 drivers/pci/controller/pcie-xilinx-nwl.c | 68 ++++++++++++++++++++++--
 1 file changed, 65 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 424cc5a1b4d1..d32cf4247836 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -19,6 +19,7 @@
 #include <linux/of_platform.h>
 #include <linux/pci.h>
 #include <linux/pci-ecam.h>
+#include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/irqchip/chained_irq.h>
 
@@ -157,6 +158,7 @@ struct nwl_pcie {
 	void __iomem *breg_base;
 	void __iomem *pcireg_base;
 	void __iomem *ecam_base;
+	struct phy *phy[4];
 	phys_addr_t phys_breg_base;	/* Physical Bridge Register Base */
 	phys_addr_t phys_pcie_reg_base;	/* Physical PCIe Controller Base */
 	phys_addr_t phys_ecam_base;	/* Physical Configuration Base */
@@ -521,6 +523,43 @@ static int nwl_pcie_init_msi_irq_domain(struct nwl_pcie *pcie)
 	return 0;
 }
 
+static int nwl_pcie_phy_enable(struct nwl_pcie *pcie)
+{
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(pcie->phy); i++) {
+		ret = phy_init(pcie->phy[i]);
+		if (ret)
+			goto err;
+
+		ret = phy_power_on(pcie->phy[i]);
+		if (ret) {
+			WARN_ON(phy_exit(pcie->phy[i]));
+			goto err;
+		}
+	}
+
+	return 0;
+
+err:
+	while (--i) {
+		WARN_ON(phy_power_off(pcie->phy[i]));
+		WARN_ON(phy_exit(pcie->phy[i]));
+	}
+
+	return ret;
+}
+
+static void nwl_pcie_phy_disable(struct nwl_pcie *pcie)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pcie->phy); i++) {
+		WARN_ON(phy_power_off(pcie->phy[i]));
+		WARN_ON(phy_exit(pcie->phy[i]));
+	}
+}
+
 static int nwl_pcie_init_irq_domain(struct nwl_pcie *pcie)
 {
 	struct device *dev = pcie->dev;
@@ -732,6 +771,7 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
 {
 	struct device *dev = pcie->dev;
 	struct resource *res;
+	int i;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "breg");
 	pcie->breg_base = devm_ioremap_resource(dev, res);
@@ -759,6 +799,18 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
 	irq_set_chained_handler_and_data(pcie->irq_intx,
 					 nwl_pcie_leg_handler, pcie);
 
+
+	for (i = 0; i < ARRAY_SIZE(pcie->phy); i++) {
+		pcie->phy[i] = devm_of_phy_get_by_index(dev, dev->of_node, i);
+		if (PTR_ERR(pcie->phy[i]) == -ENODEV) {
+			pcie->phy[i] = NULL;
+			break;
+		}
+
+		if (IS_ERR(pcie->phy[i]))
+			return PTR_ERR(pcie->phy[i]);
+	}
+
 	return 0;
 }
 
@@ -799,16 +851,22 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	err = nwl_pcie_phy_enable(pcie);
+	if (err) {
+		dev_err(dev, "could not enable PHYs\n");
+		goto err_clk;
+	}
+
 	err = nwl_pcie_bridge_init(pcie);
 	if (err) {
 		dev_err(dev, "HW Initialization failed\n");
-		goto err_clk;
+		goto err_phy;
 	}
 
 	err = nwl_pcie_init_irq_domain(pcie);
 	if (err) {
 		dev_err(dev, "Failed creating IRQ Domain\n");
-		goto err_clk;
+		goto err_phy;
 	}
 
 	bridge->sysdata = pcie;
@@ -818,12 +876,15 @@ static int nwl_pcie_probe(struct platform_device *pdev)
 		err = nwl_pcie_enable_msi(pcie);
 		if (err < 0) {
 			dev_err(dev, "failed to enable MSI support: %d\n", err);
-			goto err_clk;
+			goto err_phy;
 		}
 	}
 
 	err = pci_host_probe(bridge);
 
+err_phy:
+	if (err)
+		nwl_pcie_phy_disable(pcie);
 err_clk:
 	if (err)
 		clk_disable_unprepare(pcie->clk);
@@ -834,6 +895,7 @@ static void nwl_pcie_remove(struct platform_device *pdev)
 {
 	struct nwl_pcie *pcie = platform_get_drvdata(pdev);
 
+	nwl_pcie_phy_disable(pcie);
 	clk_disable_unprepare(pcie->clk);
 }
 
-- 
2.35.1.1320.gc452695387.dirty


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

[PATCH v2 7/7] arm64: zynqmp: Add PCIe phys

[Sean Anderson]

Add PCIe phy bindings for the ZCU102.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
Tested-by: thippeswamy.havalige@amd.com
---

Changes in v2:
- Remove phy-names

 arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
index ad8f23a0ec67..d2175f3dd099 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu102-revA.dts
@@ -941,6 +941,7 @@ conf-pull-none {
 
 &pcie {
 	status = "okay";
+	phys = <&psgtr 0 PHY_TYPE_PCIE 0 0>;
 };
 
 &psgtr {
-- 
2.35.1.1320.gc452695387.dirty


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

Re: [PATCH v2 1/7] dt-bindings: pci: xilinx-nwl: Add phys

[Rob Herring]

On Mon, May 06, 2024 at 12:15:04PM -0400, Sean Anderson wrote:
> Add phys properties so Linux can power-on/configure the GTR
> transcievers.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
> Changes in v2:
> - Remove phy-names
> - Add an example
> 
>  Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
> index 426f90a47f35..693b29039a9b 100644
> --- a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
> @@ -61,6 +61,10 @@ properties:
>    interrupt-map:
>      maxItems: 4
>  
> +  phys:
> +    minItems: 1
> +    maxItems: 4

I assume this is 1 phy per lane, but don't make me assume and define it.

Rob

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

Re: [PATCH v2 1/7] dt-bindings: pci: xilinx-nwl: Add phys

[Sean Anderson]

On 5/7/24 16:06, Rob Herring wrote:
> On Mon, May 06, 2024 at 12:15:04PM -0400, Sean Anderson wrote:
>> Add phys properties so Linux can power-on/configure the GTR
>> transcievers.
>> 
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>> 
>> Changes in v2:
>> - Remove phy-names
>> - Add an example
>> 
>>  Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
>> index 426f90a47f35..693b29039a9b 100644
>> --- a/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
>> +++ b/Documentation/devicetree/bindings/pci/xlnx,nwl-pcie.yaml
>> @@ -61,6 +61,10 @@ properties:
>>    interrupt-map:
>>      maxItems: 4
>>  
>> +  phys:
>> +    minItems: 1
>> +    maxItems: 4
> 
> I assume this is 1 phy per lane, but don't make me assume and define it.
> 
> Rob

It's one per lane. I'll add that to the description.

--Sean

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

Re: [PATCH v2 2/7] PCI: xilinx-nwl: Fix off-by-one

[Bjorn Helgaas]

Maybe the subject could include something about why this is important,
e.g., it's IRQ-related, we mask/unmask the wrong thing, etc?

On Mon, May 06, 2024 at 12:15:05PM -0400, Sean Anderson wrote:
> IRQs start at 0, so we don't need to subtract 1.

What does "IRQ" refer to here?  Something to do with INTx, I guess,
but apparently not PCI_INTERRUPT_PIN, since 0 in that register means
the device doesn't use INTx, and 1=INTA, 2=INTB, etc.

I assume this fixes a bug, e.g., we mask/unmask the wrong INTx?  What
does this look like for a user?  Unexpected IRQs?

9a181e1093af is from seven years ago.  Should we be surprised that we
haven't tripped over this before?

> Fixes: 9a181e1093af ("PCI: xilinx-nwl: Modify IRQ chip for legacy interrupts")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
> (no changes since v1)
> 
>  drivers/pci/controller/pcie-xilinx-nwl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 0408f4d612b5..437927e3bcca 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -371,7 +371,7 @@ static void nwl_mask_intx_irq(struct irq_data *data)
>  	u32 mask;
>  	u32 val;
>  
> -	mask = 1 << (data->hwirq - 1);
> +	mask = 1 << data->hwirq;
>  	raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
>  	val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
>  	nwl_bridge_writel(pcie, (val & (~mask)), MSGF_LEG_MASK);
> @@ -385,7 +385,7 @@ static void nwl_unmask_intx_irq(struct irq_data *data)
>  	u32 mask;
>  	u32 val;
>  
> -	mask = 1 << (data->hwirq - 1);
> +	mask = 1 << data->hwirq;
>  	raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
>  	val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
>  	nwl_bridge_writel(pcie, (val | mask), MSGF_LEG_MASK);
> -- 
> 2.35.1.1320.gc452695387.dirty
> 

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

Re: [PATCH v2 2/7] PCI: xilinx-nwl: Fix off-by-one

[Sean Anderson]

On 5/7/24 21:59, Bjorn Helgaas wrote:
> Maybe the subject could include something about why this is important,
> e.g., it's IRQ-related, we mask/unmask the wrong thing, etc?
> 
> On Mon, May 06, 2024 at 12:15:05PM -0400, Sean Anderson wrote:
>> IRQs start at 0, so we don't need to subtract 1.
> 
> What does "IRQ" refer to here?  Something to do with INTx, I guess,
> but apparently not PCI_INTERRUPT_PIN, since 0 in that register means
> the device doesn't use INTx, and 1=INTA, 2=INTB, etc.

This refers to INTx. MSGF_LEG_MASK is laid out with INTA in bit 0, INTB
in bit 1, INTC in bit 2, and INTD in bit 3. Hardware IRQ numbers start
at 0, and we register PCI_NUM_INTX irqs. So by subtracting 1, we try to
set the -1st bit when enabling INTA.

> I assume this fixes a bug, e.g., we mask/unmask the wrong INTx?  What
> does this look like for a user?  Unexpected IRQs?

Without this patch I get the following splat:

[    5.037483] ================================================================================
[    5.046260] UBSAN: shift-out-of-bounds in ../drivers/pci/controller/pcie-xilinx-nwl.c:389:11
[    5.054983] shift exponent 18446744073709551615 is too large for 32-bit type 'int'
[    5.062813] CPU: 1 PID: 61 Comm: kworker/u10:1 Not tainted 6.6.20+ #268
[    5.070008] Hardware name: xlnx,zynqmp (DT)
[    5.074348] Workqueue: events_unbound deferred_probe_work_func
[    5.080410] Call trace:
[    5.082958] dump_backtrace (arch/arm64/kernel/stacktrace.c:235) 
[    5.086850] show_stack (arch/arm64/kernel/stacktrace.c:242) 
[    5.090292] dump_stack_lvl (lib/dump_stack.c:107) 
[    5.094095] dump_stack (lib/dump_stack.c:114) 
[    5.097540] __ubsan_handle_shift_out_of_bounds (lib/ubsan.c:218 lib/ubsan.c:387) 
[    5.103227] nwl_unmask_leg_irq (drivers/pci/controller/pcie-xilinx-nwl.c:389 (discriminator 1)) 
[    5.107386] irq_enable (kernel/irq/internals.h:234 kernel/irq/chip.c:170 kernel/irq/chip.c:439 kernel/irq/chip.c:432 kernel/irq/chip.c:345) 
[    5.110838] __irq_startup (kernel/irq/internals.h:239 kernel/irq/chip.c:180 kernel/irq/chip.c:250) 
[    5.114552] irq_startup (kernel/irq/chip.c:270) 
[    5.118266] __setup_irq (kernel/irq/manage.c:1800) 
[    5.121982] request_threaded_irq (kernel/irq/manage.c:2206) 
[    5.126412] pcie_pme_probe (include/linux/interrupt.h:168 drivers/pci/pcie/pme.c:348) 
[    5.130303] pcie_port_probe_service (drivers/pci/pcie/portdrv.c:528) 
[    5.134915] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658) 
[    5.138720] __driver_probe_device (drivers/base/dd.c:800) 
[    5.143236] driver_probe_device (drivers/base/dd.c:830) 
[    5.147571] __device_attach_driver (drivers/base/dd.c:959) 
[    5.152179] bus_for_each_drv (drivers/base/bus.c:457) 
[    5.156163] __device_attach (drivers/base/dd.c:1032) 
[    5.160147] device_initial_probe (drivers/base/dd.c:1080) 
[    5.164488] bus_probe_device (drivers/base/bus.c:532) 
[    5.168471] device_add (drivers/base/core.c:3638) 
[    5.172098] device_register (drivers/base/core.c:3714) 
[    5.175994] pcie_portdrv_probe (drivers/pci/pcie/portdrv.c:309 drivers/pci/pcie/portdrv.c:363 drivers/pci/pcie/portdrv.c:695) 
[    5.180338] pci_device_probe (drivers/pci/pci-driver.c:324 drivers/pci/pci-driver.c:392 drivers/pci/pci-driver.c:417 drivers/pci/pci-driver.c:460) 
[    5.184410] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658) 
[    5.188213] __driver_probe_device (drivers/base/dd.c:800) 
[    5.192729] driver_probe_device (drivers/base/dd.c:830) 
[    5.197064] __device_attach_driver (drivers/base/dd.c:959) 
[    5.201672] bus_for_each_drv (drivers/base/bus.c:457) 
[    5.205657] __device_attach (drivers/base/dd.c:1032) 
[    5.209641] device_attach (drivers/base/dd.c:1074) 
[    5.213357] pci_bus_add_device (drivers/pci/bus.c:352) 
[    5.217518] pci_bus_add_devices (drivers/pci/bus.c:371 (discriminator 2)) 
[    5.221774] pci_host_probe (drivers/pci/probe.c:3099) 
[    5.225581] nwl_pcie_probe (drivers/pci/controller/pcie-xilinx-nwl.c:938) 
[    5.229562] platform_probe (drivers/base/platform.c:1404) 
[    5.233367] really_probe (drivers/base/dd.c:579 drivers/base/dd.c:658) 
[    5.237169] __driver_probe_device (drivers/base/dd.c:800) 
[    5.241685] driver_probe_device (drivers/base/dd.c:830) 
[    5.246020] __device_attach_driver (drivers/base/dd.c:959) 
[    5.250628] bus_for_each_drv (drivers/base/bus.c:457) 
[    5.254612] __device_attach (drivers/base/dd.c:1032) 
[    5.258596] device_initial_probe (drivers/base/dd.c:1080) 
[    5.262938] bus_probe_device (drivers/base/bus.c:532) 
[    5.266920] deferred_probe_work_func (drivers/base/dd.c:124) 
[    5.271619] process_one_work (arch/arm64/include/asm/jump_label.h:21 include/linux/jump_label.h:207 include/trace/events/workqueue.h:108 kernel/workqueue.c:2632) 
[    5.275788] worker_thread (kernel/workqueue.c:2694 (discriminator 2) kernel/workqueue.c:2781 (discriminator 2)) 
[    5.279686] kthread (kernel/kthread.c:388) 
[    5.283048] ret_from_fork (arch/arm64/kernel/entry.S:862) 
[    5.286765] ================================================================================

> 9a181e1093af is from seven years ago.  Should we be surprised that we
> haven't tripped over this before?

I suppose no one enables UBSAN on this platform.

--Sean

>> Fixes: 9a181e1093af ("PCI: xilinx-nwl: Modify IRQ chip for legacy interrupts")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> ---
>> 
>> (no changes since v1)
>> 
>>  drivers/pci/controller/pcie-xilinx-nwl.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
>> index 0408f4d612b5..437927e3bcca 100644
>> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
>> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
>> @@ -371,7 +371,7 @@ static void nwl_mask_intx_irq(struct irq_data *data)
>>  	u32 mask;
>>  	u32 val;
>>  
>> -	mask = 1 << (data->hwirq - 1);
>> +	mask = 1 << data->hwirq;
>>  	raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
>>  	val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
>>  	nwl_bridge_writel(pcie, (val & (~mask)), MSGF_LEG_MASK);
>> @@ -385,7 +385,7 @@ static void nwl_unmask_intx_irq(struct irq_data *data)
>>  	u32 mask;
>>  	u32 val;
>>  
>> -	mask = 1 << (data->hwirq - 1);
>> +	mask = 1 << data->hwirq;
>>  	raw_spin_lock_irqsave(&pcie->leg_mask_lock, flags);
>>  	val = nwl_bridge_readl(pcie, MSGF_LEG_MASK);
>>  	nwl_bridge_writel(pcie, (val | mask), MSGF_LEG_MASK);
>> -- 
>> 2.35.1.1320.gc452695387.dirty
>> 

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