usb: xhci: tegra: Check padctrl interrupt presence in device tree

PATCH Superseded Dmitry Osipenko 2 participants 2 messages Nov 2, 2021 – Nov 4, 2021

Subsystems (3)

USB Tegra architecture support USB xHCI driver

lore

Share & tools

lore

Dmitry Osipenko

author

2021-11-02 18:49 UTC #0 lore raw

PATCH v2 1/1 usb: xhci: tegra: Check padctrl interrupt presence in device tree

Older device-trees don't specify padctrl interrupt and xhci-tegra driver now fails to probe with -EINVAL using those device-trees. Check interrupt presence and keep runtime PM disabled if it's missing to fix the trouble.

Fixes 971ee247060d ("usb: xhci: tegra: Enable ELPG for runtime/system PM")

Cc <[email protected]> # 5.14+

Tested-by Nicolas Chauvet <[email protected]>

Reported-by Nicolas Chauvet <[email protected]>

Signed-off-by Dmitry Osipenko <[email protected]>

---

Changelog:

v2: - Use of_irq_parse_one() to check interrupt presence status in device-tree,
instead of checking interrupt properties directly.

- USB wakeup and runtime PM are kept disabled if interrupt is missing, instead of returning -EOPNOTSUPP from RPM-suspend callback.

- Added debug message, telling about the missing interrupt.

 drivers/usb/host/xhci-tegra.c | 40 ++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 12 deletions(-)

drivers/usb/host/xhci-tegra.c +28 ~~-12~~

index 1bf494b649bd..0a7ab596be85 100644
@@ -1400,6 +1400,7 @@ static void tegra_xusb_deinit_usb_phy(struct tegra_xusb *tegra)
 
 static int tegra_xusb_probe(struct platform_device *pdev)
 {
+	struct of_phandle_args irq_arg;
 	struct tegra_xusb *tegra;
 	struct device_node *np;
 	struct resource *regs;

@@ -1454,10 +1455,16 @@ static int tegra_xusb_probe(struct platform_device *pdev)
 		goto put_padctl;
 	}
 
-	tegra->padctl_irq = of_irq_get(np, 0);
-	if (tegra->padctl_irq <= 0) {
-		err = (tegra->padctl_irq == 0) ? -ENODEV : tegra->padctl_irq;
-		goto put_padctl;
+	/* Older device-trees don't have padctrl interrupt */
+	err = of_irq_parse_one(np, 0, &irq_arg);
+	if (!err) {
+		tegra->padctl_irq = of_irq_get(np, 0);
+		if (tegra->padctl_irq <= 0) {
+			err = (tegra->padctl_irq == 0) ? -ENODEV : tegra->padctl_irq;
+			goto put_padctl;
+		}
+	} else {
+		dev_dbg(&pdev->dev, "%pOF doesn't have interrupt\n", np);
 	}
 
 	tegra->host_clk = devm_clk_get(&pdev->dev, "xusb_host");

@@ -1696,11 +1703,15 @@ static int tegra_xusb_probe(struct platform_device *pdev)
 		goto remove_usb3;
 	}
 
-	err = devm_request_threaded_irq(&pdev->dev, tegra->padctl_irq, NULL, tegra_xusb_padctl_irq,
-					IRQF_ONESHOT, dev_name(&pdev->dev), tegra);
-	if (err < 0) {
-		dev_err(&pdev->dev, "failed to request padctl IRQ: %d\n", err);
-		goto remove_usb3;
+	if (tegra->padctl_irq) {
+		err = devm_request_threaded_irq(&pdev->dev, tegra->padctl_irq,
+						NULL, tegra_xusb_padctl_irq,
+						IRQF_ONESHOT, dev_name(&pdev->dev),
+						tegra);
+		if (err < 0) {
+			dev_err(&pdev->dev, "failed to request padctl IRQ: %d\n", err);
+			goto remove_usb3;
+		}
 	}
 
 	err = tegra_xusb_enable_firmware_messages(tegra);

@@ -1718,13 +1729,16 @@ static int tegra_xusb_probe(struct platform_device *pdev)
 	/* Enable wake for both USB 2.0 and USB 3.0 roothubs */
 	device_init_wakeup(&tegra->hcd->self.root_hub->dev, true);
 	device_init_wakeup(&xhci->shared_hcd->self.root_hub->dev, true);
-	device_init_wakeup(tegra->dev, true);
 
 	pm_runtime_use_autosuspend(tegra->dev);
 	pm_runtime_set_autosuspend_delay(tegra->dev, 2000);
 	pm_runtime_mark_last_busy(tegra->dev);
 	pm_runtime_set_active(tegra->dev);
-	pm_runtime_enable(tegra->dev);
+
+	if (tegra->padctl_irq) {
+		device_init_wakeup(tegra->dev, true);
+		pm_runtime_enable(tegra->dev);
+	}
 
 	return 0;

@@ -1772,7 +1786,9 @@ static int tegra_xusb_remove(struct platform_device *pdev)
 	dma_free_coherent(&pdev->dev, tegra->fw.size, tegra->fw.virt,
 			  tegra->fw.phys);
 
-	pm_runtime_disable(&pdev->dev);
+	if (tegra->padctl_irq)
+		pm_runtime_disable(&pdev->dev);
+
 	pm_runtime_put(&pdev->dev);
 
 	tegra_xusb_powergate_partitions(tegra);

Signature · 1 line

2.33.1

Thierry Reding

Acked-by Tested-by on v2 1/1

2021-11-04 08:29 UTC #1 lore raw

Quoted 16 lines · Dmitry Osipenko

Older device-trees don't specify padctrl interrupt and xhci-tegra driver now fails to probe with -EINVAL using those device-trees. Check interrupt presence and keep runtime PM disabled if it's missing to fix the trouble.
Fixes 971ee247060d ("usb: xhci: tegra: Enable ELPG for runtime/system PM")
Cc <[email protected]> # 5.14+
Tested-by Nicolas Chauvet <[email protected]>
Reported-by Nicolas Chauvet <[email protected]>
Signed-off-by Dmitry Osipenko <[email protected]>
---
Changelog:
v2: - Use of_irq_parse_one() to check interrupt presence status in device-tree,
instead of checking interrupt properties directly.

Quoted 7 lines

- USB wakeup and runtime PM are kept disabled if interrupt is missing, instead of returning -EOPNOTSUPP from RPM-suspend callback.
- Added debug message, telling about the missing interrupt.
 drivers/usb/host/xhci-tegra.c | 40 ++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 12 deletions(-)

I like this version much better. Two minor nits:

drivers/usb/host/xhci-tegra.c static void tegra_xusb_deinit_usb_phy(struct tegra_xusb *tegra) · line 1403

Quoted 10 lines

drivers/usb/host/xhci-tegra.c +1 -0

index 1bf494b649bd..0a7ab596be85 100644
@@ -1400,6 +1400,7 @@ static void tegra_xusb_deinit_usb_phy(struct tegra_xusb *tegra)
 
 static int tegra_xusb_probe(struct platform_device *pdev)
 {
+	struct of_phandle_args irq_arg;

Could've been just "args". There's no other "arg" variable here, so no need for an irq_ prefix to differentiate.

Quoted 11 lines

drivers/usb/host/xhci-tegra.c +0 -4

 	struct tegra_xusb *tegra;
 	struct device_node *np;
 	struct resource *regs;

@@ -1454,10 +1455,16 @@ static int tegra_xusb_probe(struct platform_device *pdev)
 		goto put_padctl;
 	}
 
-	tegra->padctl_irq = of_irq_get(np, 0);
-	if (tegra->padctl_irq <= 0) {
-		err = (tegra->padctl_irq == 0) ? -ENODEV : tegra->padctl_irq;
-		goto put_padctl;

drivers/usb/host/xhci-tegra.c static int tegra_xusb_probe(struct platform_device *pdev) · line 1467

Quoted 10 lines

drivers/usb/host/xhci-tegra.c +10 -0

+	/* Older device-trees don't have padctrl interrupt */
+	err = of_irq_parse_one(np, 0, &irq_arg);
+	if (!err) {
+		tegra->padctl_irq = of_irq_get(np, 0);
+		if (tegra->padctl_irq <= 0) {
+			err = (tegra->padctl_irq == 0) ? -ENODEV : tegra->padctl_irq;
+			goto put_padctl;
+		}
+	} else {
+		dev_dbg(&pdev->dev, "%pOF doesn't have interrupt\n", np);

This seems a bit vague. I think it'd be better to include information about the consequence of this interrupt being missing and/or some hint about what should be done about it. Perhaps something like:

		dev_dbg(&pdev->dev, "%pOF is missing an interrupt, disabling PM support\n", np);

With that fixed:

Acked-by Thierry Reding <[email protected]>

I've also run this through our GVS test farm, and didn't spot any regressions, so:

Tested-by Thierry Reding <[email protected]>

Thierry

Quoted 57 lines

drivers/usb/host/xhci-tegra.c +17 -8

 	}
 
 	tegra->host_clk = devm_clk_get(&pdev->dev, "xusb_host");

@@ -1696,11 +1703,15 @@ static int tegra_xusb_probe(struct platform_device *pdev)
 		goto remove_usb3;
 	}
 
-	err = devm_request_threaded_irq(&pdev->dev, tegra->padctl_irq, NULL, tegra_xusb_padctl_irq,
-					IRQF_ONESHOT, dev_name(&pdev->dev), tegra);
-	if (err < 0) {
-		dev_err(&pdev->dev, "failed to request padctl IRQ: %d\n", err);
-		goto remove_usb3;
+	if (tegra->padctl_irq) {
+		err = devm_request_threaded_irq(&pdev->dev, tegra->padctl_irq,
+						NULL, tegra_xusb_padctl_irq,
+						IRQF_ONESHOT, dev_name(&pdev->dev),
+						tegra);
+		if (err < 0) {
+			dev_err(&pdev->dev, "failed to request padctl IRQ: %d\n", err);
+			goto remove_usb3;
+		}
 	}
 
 	err = tegra_xusb_enable_firmware_messages(tegra);

@@ -1718,13 +1729,16 @@ static int tegra_xusb_probe(struct platform_device *pdev)
 	/* Enable wake for both USB 2.0 and USB 3.0 roothubs */
 	device_init_wakeup(&tegra->hcd->self.root_hub->dev, true);
 	device_init_wakeup(&xhci->shared_hcd->self.root_hub->dev, true);
-	device_init_wakeup(tegra->dev, true);
 
 	pm_runtime_use_autosuspend(tegra->dev);
 	pm_runtime_set_autosuspend_delay(tegra->dev, 2000);
 	pm_runtime_mark_last_busy(tegra->dev);
 	pm_runtime_set_active(tegra->dev);
-	pm_runtime_enable(tegra->dev);
+
+	if (tegra->padctl_irq) {
+		device_init_wakeup(tegra->dev, true);
+		pm_runtime_enable(tegra->dev);
+	}
 
 	return 0;

@@ -1772,7 +1786,9 @@ static int tegra_xusb_remove(struct platform_device *pdev)
 	dma_free_coherent(&pdev->dev, tegra->fw.size, tegra->fw.virt,
 			  tegra->fw.phys);
 
-	pm_runtime_disable(&pdev->dev);
+	if (tegra->padctl_irq)
+		pm_runtime_disable(&pdev->dev);
+
 	pm_runtime_put(&pdev->dev);
 
 	tegra_xusb_powergate_partitions(tegra);

Signature · 1 line

2.33.1