On 18.07.23 11:14, Haotien Hsu wrote:
> From: Henry Lin <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Haotien Hsu <[email protected]>
> Acked-by: Thierry Reding <[email protected]>
>
> ---
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Haotien Hsu <[email protected]>
> > Acked-by: Thierry Reding <[email protected]>
> >
> > ---
> > 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