diff mbox series

[v2] usb: xhci: tegra: Add shutdown callback for Tegra XUSB

Message ID 20230718091425.307589-1-haotienh@nvidia.com
State New
Headers
Series [v2] usb: xhci: tegra: Add shutdown callback for Tegra XUSB |

Commit Message

HaoTien Hsu July 18, 2023, 9:14 a.m. UTC 
  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(-)

Comments

Thierry Reding July 18, 2023, 11:35 a.m. UTC | #1 
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
Greg KH July 25, 2023, 4:19 p.m. UTC | #2 
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 :(
HaoTien Hsu July 27, 2023, 8:08 a.m. UTC | #3 
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 mbox series

Patch

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,