On Tue, Nov 02, 2021 at 09:48:01PM +0300, Dmitry Osipenko wrote:
> 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: <stable@xxxxxxxxxxxxxxx> # 5.14+
> Tested-by: Nicolas Chauvet <kwizart@xxxxxxxxx>
> Reported-by: Nicolas Chauvet <kwizart@xxxxxxxxx>
> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> ---
>
> 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(-)
I like this version much better. Two minor nits:
>
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 1bf494b649bd..0a7ab596be85 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -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.
> 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);
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 <treding@xxxxxxxxxx>
I've also run this through our GVS test farm, and didn't spot any
regressions, so:
Tested-by: Thierry Reding <treding@xxxxxxxxxx>
Thierry
> }
>
> 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);
> --
> 2.33.1
>
Attachment:
signature.asc
Description: PGP signature
