| Message ID | 20230718091425.307589-1-haotienh@nvidia.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v2] usb: xhci: tegra: Add shutdown callback for Tegra XUSB | expand |
On 18.07.23 11:14, Haotien Hsu wrote: > From: Henry Lin <henryl@nvidia.com> > > If memory accesses by the Tegra XUSB controller are translated through > the SMMU (System MMU), the hardware may continue accessing memory even > after the SMMU translations have been disabled during the shutdown > process and this can in turn cause unpredictable crashes. > Fix this by adding a shutdown implementation that ensures the hardware > is turned off during system reboot or shutdown. > > Signed-off-by: Henry Lin <henryl@nvidia.com> > Signed-off-by: Haotien Hsu <haotienh@nvidia.com> > Acked-by: Thierry Reding <treding@nvidia.com> > > --- > V1 -> V2: Resend for the typo in the mailing list > --- > drivers/usb/host/xhci-tegra.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > index a56cc81b9404..6ef2eac9835d 100644 > --- a/drivers/usb/host/xhci-tegra.c > +++ b/drivers/usb/host/xhci-tegra.c > @@ -1925,6 +1925,15 @@ static int tegra_xusb_probe(struct platform_device *pdev) > return err; > } > > +static void tegra_xusb_disable(struct tegra_xusb *tegra) > +{ > + tegra_xusb_powergate_partitions(tegra); > + tegra_xusb_powerdomain_remove(tegra->dev, tegra); > + tegra_xusb_phy_disable(tegra); > + tegra_xusb_clk_disable(tegra); > + regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); > +} > + > static int tegra_xusb_remove(struct platform_device *pdev) > { > struct tegra_xusb *tegra = platform_get_drvdata(pdev); > @@ -1947,18 +1956,22 @@ static int tegra_xusb_remove(struct platform_device *pdev) > > pm_runtime_put(&pdev->dev); > > - tegra_xusb_powergate_partitions(tegra); > - > - tegra_xusb_powerdomain_remove(&pdev->dev, tegra); > - > - tegra_xusb_phy_disable(tegra); > - tegra_xusb_clk_disable(tegra); > - regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); > + tegra_xusb_disable(tegra); > tegra_xusb_padctl_put(tegra->padctl); > > return 0; > } > > +static void tegra_xusb_shutdown(struct platform_device *pdev) > +{ > + struct tegra_xusb *tegra = platform_get_drvdata(pdev); > + > + pm_runtime_get_sync(&pdev->dev); Where is this balanced? Regards Oliver
On Tue, Jul 18, 2023 at 11:18:25AM +0200, Oliver Neukum wrote: > > > On 18.07.23 11:14, Haotien Hsu wrote: > > From: Henry Lin <henryl@nvidia.com> > > > > If memory accesses by the Tegra XUSB controller are translated through > > the SMMU (System MMU), the hardware may continue accessing memory even > > after the SMMU translations have been disabled during the shutdown > > process and this can in turn cause unpredictable crashes. > > Fix this by adding a shutdown implementation that ensures the hardware > > is turned off during system reboot or shutdown. > > > > Signed-off-by: Henry Lin <henryl@nvidia.com> > > Signed-off-by: Haotien Hsu <haotienh@nvidia.com> > > Acked-by: Thierry Reding <treding@nvidia.com> > > > > --- > > V1 -> V2: Resend for the typo in the mailing list > > --- > > drivers/usb/host/xhci-tegra.c | 28 +++++++++++++++++++++------- > > 1 file changed, 21 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > > index a56cc81b9404..6ef2eac9835d 100644 > > --- a/drivers/usb/host/xhci-tegra.c > > +++ b/drivers/usb/host/xhci-tegra.c > > @@ -1925,6 +1925,15 @@ static int tegra_xusb_probe(struct platform_device *pdev) > > return err; > > } > > +static void tegra_xusb_disable(struct tegra_xusb *tegra) > > +{ > > + tegra_xusb_powergate_partitions(tegra); > > + tegra_xusb_powerdomain_remove(tegra->dev, tegra); > > + tegra_xusb_phy_disable(tegra); > > + tegra_xusb_clk_disable(tegra); > > + regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); > > +} > > + > > static int tegra_xusb_remove(struct platform_device *pdev) > > { > > struct tegra_xusb *tegra = platform_get_drvdata(pdev); > > @@ -1947,18 +1956,22 @@ static int tegra_xusb_remove(struct platform_device *pdev) > > pm_runtime_put(&pdev->dev); > > - tegra_xusb_powergate_partitions(tegra); > > - > > - tegra_xusb_powerdomain_remove(&pdev->dev, tegra); > > - > > - tegra_xusb_phy_disable(tegra); > > - tegra_xusb_clk_disable(tegra); > > - regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); > > + tegra_xusb_disable(tegra); > > tegra_xusb_padctl_put(tegra->padctl); > > return 0; > > } > > +static void tegra_xusb_shutdown(struct platform_device *pdev) > > +{ > > + struct tegra_xusb *tegra = platform_get_drvdata(pdev); > > + > > + pm_runtime_get_sync(&pdev->dev); > > Where is this balanced? Well, I suppose we could add a pm_runtime_put() at the end of this function, but since we expect the system to shut down after this, at which point any runtime PM accounting is going to go bye-bye anyway, do we really want to bother? Thierry
On Tue, Jul 18, 2023 at 05:14:25PM +0800, Haotien Hsu wrote: > From: Henry Lin <henryl@nvidia.com> > > If memory accesses by the Tegra XUSB controller are translated through > the SMMU (System MMU), the hardware may continue accessing memory even > after the SMMU translations have been disabled during the shutdown > process and this can in turn cause unpredictable crashes. > Fix this by adding a shutdown implementation that ensures the hardware > is turned off during system reboot or shutdown. > > Signed-off-by: Henry Lin <henryl@nvidia.com> > Signed-off-by: Haotien Hsu <haotienh@nvidia.com> > Acked-by: Thierry Reding <treding@nvidia.com> > > --- > V1 -> V2: Resend for the typo in the mailing list > --- > drivers/usb/host/xhci-tegra.c | 28 +++++++++++++++++++++------- > 1 file changed, 21 insertions(+), 7 deletions(-) Does not apply against 6.5-rc4 at all :(
On Tue, 2023-07-25 at 18:19 +0200, Greg Kroah-Hartman wrote: > External email: Use caution opening links or attachments > > > On Tue, Jul 18, 2023 at 05:14:25PM +0800, Haotien Hsu wrote: > > From: Henry Lin <henryl@nvidia.com> > > > > If memory accesses by the Tegra XUSB controller are translated > > through > > the SMMU (System MMU), the hardware may continue accessing memory > > even > > after the SMMU translations have been disabled during the shutdown > > process and this can in turn cause unpredictable crashes. > > Fix this by adding a shutdown implementation that ensures the > > hardware > > is turned off during system reboot or shutdown. > > > > Signed-off-by: Henry Lin <henryl@nvidia.com> > > Signed-off-by: Haotien Hsu <haotienh@nvidia.com> > > Acked-by: Thierry Reding <treding@nvidia.com> > > > > --- > > V1 -> V2: Resend for the typo in the mailing list > > --- > > drivers/usb/host/xhci-tegra.c | 28 +++++++++++++++++++++------- > > 1 file changed, 21 insertions(+), 7 deletions(-) > > Does not apply against 6.5-rc4 at all :( I rebased the patch and sent it again. I am sorry about that.
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c index a56cc81b9404..6ef2eac9835d 100644 --- a/drivers/usb/host/xhci-tegra.c +++ b/drivers/usb/host/xhci-tegra.c @@ -1925,6 +1925,15 @@ static int tegra_xusb_probe(struct platform_device *pdev) return err; } +static void tegra_xusb_disable(struct tegra_xusb *tegra) +{ + tegra_xusb_powergate_partitions(tegra); + tegra_xusb_powerdomain_remove(tegra->dev, tegra); + tegra_xusb_phy_disable(tegra); + tegra_xusb_clk_disable(tegra); + regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); +} + static int tegra_xusb_remove(struct platform_device *pdev) { struct tegra_xusb *tegra = platform_get_drvdata(pdev); @@ -1947,18 +1956,22 @@ static int tegra_xusb_remove(struct platform_device *pdev) pm_runtime_put(&pdev->dev); - tegra_xusb_powergate_partitions(tegra); - - tegra_xusb_powerdomain_remove(&pdev->dev, tegra); - - tegra_xusb_phy_disable(tegra); - tegra_xusb_clk_disable(tegra); - regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); + tegra_xusb_disable(tegra); tegra_xusb_padctl_put(tegra->padctl); return 0; } +static void tegra_xusb_shutdown(struct platform_device *pdev) +{ + struct tegra_xusb *tegra = platform_get_drvdata(pdev); + + pm_runtime_get_sync(&pdev->dev); + disable_irq(tegra->xhci_irq); + xhci_shutdown(tegra->hcd); + tegra_xusb_disable(tegra); +} + static bool xhci_hub_ports_suspended(struct xhci_hub *hub) { struct device *dev = hub->hcd->self.controller; @@ -2666,6 +2679,7 @@ MODULE_DEVICE_TABLE(of, tegra_xusb_of_match); static struct platform_driver tegra_xusb_driver = { .probe = tegra_xusb_probe, .remove = tegra_xusb_remove, + .shutdown = tegra_xusb_shutdown, .driver = { .name = "tegra-xusb", .pm = &tegra_xusb_pm_ops,