| Message ID | 20200831044043.1561074-12-jckuo@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Tegra XHCI controller ELPG support | expand |
On Mon, Aug 31, 2020 at 12:40:42PM +0800, JC Kuo wrote: > This commit unlinks xhci-tegra platform device with ss/host power > domain devices. Reasons for this change is - at elpg entry, phy > sleepwalk and wake configuration need to be done before powering > down ss/host partitions, and phy need be powered off after powering > down ss/host partitions. Sequence looks like roughly below: > > tegra_xusb_enter_elpg() -> xhci_suspend() > -> enable phy sleepwalk and wake if needed > -> power down ss/host partitions > -> power down phy > > If ss/host power domains are linked to xhci-tegra platform device, we > are not able to perform the sequence like above. > > This commit introduces: > 1. tegra_xusb_unpowergate_partitions() to power up ss and host > partitions together. If ss/host power domain devices are > available, it invokes pm_runtime_get_sync() to request power > driver to power up partitions; If power domain devices are not > available, tegra_powergate_sequence_power_up() will be used to > power up partitions. > > 2. tegra_xusb_powergate_partitions() to power down ss and host > partitions together. If ss/host power domain devices are > available, it invokes pm_runtime_put_sync() to request power > driver to power down partitions; If power domain devices are not > available, tegra_powergate_power_off() will be used to power down > partitions. > > Signed-off-by: JC Kuo <jckuo@nvidia.com> > --- > drivers/usb/host/xhci-tegra.c | 202 +++++++++++++++++++--------------- > 1 file changed, 111 insertions(+), 91 deletions(-) > > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c > index 934be1686352..ce6526c2caf6 100644 > --- a/drivers/usb/host/xhci-tegra.c > +++ b/drivers/usb/host/xhci-tegra.c > @@ -249,8 +249,6 @@ struct tegra_xusb { > > struct device *genpd_dev_host; > struct device *genpd_dev_ss; > - struct device_link *genpd_dl_host; > - struct device_link *genpd_dl_ss; > > struct phy **phys; > unsigned int num_phys; > @@ -814,36 +812,12 @@ static void tegra_xusb_phy_disable(struct tegra_xusb *tegra) > > static int tegra_xusb_runtime_suspend(struct device *dev) > { > - struct tegra_xusb *tegra = dev_get_drvdata(dev); > - > - regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); > - tegra_xusb_clk_disable(tegra); > - > return 0; > } > > static int tegra_xusb_runtime_resume(struct device *dev) > { > - struct tegra_xusb *tegra = dev_get_drvdata(dev); > - int err; > - > - err = tegra_xusb_clk_enable(tegra); > - if (err) { > - dev_err(dev, "failed to enable clocks: %d\n", err); > - return err; > - } > - > - err = regulator_bulk_enable(tegra->soc->num_supplies, tegra->supplies); > - if (err) { > - dev_err(dev, "failed to enable regulators: %d\n", err); > - goto disable_clk; > - } > - > return 0; > - > -disable_clk: > - tegra_xusb_clk_disable(tegra); > - return err; > } > > #ifdef CONFIG_PM_SLEEP > @@ -1019,10 +993,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra) > static void tegra_xusb_powerdomain_remove(struct device *dev, > struct tegra_xusb *tegra) > { > - if (tegra->genpd_dl_ss) > - device_link_del(tegra->genpd_dl_ss); > - if (tegra->genpd_dl_host) > - device_link_del(tegra->genpd_dl_host); > if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) > dev_pm_domain_detach(tegra->genpd_dev_ss, true); > if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) > @@ -1048,20 +1018,88 @@ static int tegra_xusb_powerdomain_init(struct device *dev, > return err; > } > > - tegra->genpd_dl_host = device_link_add(dev, tegra->genpd_dev_host, > - DL_FLAG_PM_RUNTIME | > - DL_FLAG_STATELESS); > - if (!tegra->genpd_dl_host) { > - dev_err(dev, "adding host device link failed!\n"); > - return -ENODEV; > + return 0; > +} > + > +static int tegra_xusb_unpowergate_partitions(struct tegra_xusb *tegra) > +{ > + struct device *dev = tegra->dev; > + bool use_genpd; > + int rc; > + > + use_genpd = of_property_read_bool(dev->of_node, "power-domains"); I don't think that's technically correct. Just because a "power-domains" property exists in DT doesn't mean any power domains are necessarily attached to the device. I think you'll need to check for something like if (dev->pm_domain) here. Thierry
On 8/31/20 8:42 PM, Thierry Reding wrote: > On Mon, Aug 31, 2020 at 12:40:42PM +0800, JC Kuo wrote: >> This commit unlinks xhci-tegra platform device with ss/host power >> domain devices. Reasons for this change is - at elpg entry, phy >> sleepwalk and wake configuration need to be done before powering >> down ss/host partitions, and phy need be powered off after powering >> down ss/host partitions. Sequence looks like roughly below: >> >> tegra_xusb_enter_elpg() -> xhci_suspend() >> -> enable phy sleepwalk and wake if needed >> -> power down ss/host partitions >> -> power down phy >> >> If ss/host power domains are linked to xhci-tegra platform device, we >> are not able to perform the sequence like above. >> >> This commit introduces: >> 1. tegra_xusb_unpowergate_partitions() to power up ss and host >> partitions together. If ss/host power domain devices are >> available, it invokes pm_runtime_get_sync() to request power >> driver to power up partitions; If power domain devices are not >> available, tegra_powergate_sequence_power_up() will be used to >> power up partitions. >> >> 2. tegra_xusb_powergate_partitions() to power down ss and host >> partitions together. If ss/host power domain devices are >> available, it invokes pm_runtime_put_sync() to request power >> driver to power down partitions; If power domain devices are not >> available, tegra_powergate_power_off() will be used to power down >> partitions. >> >> Signed-off-by: JC Kuo <jckuo@nvidia.com> >> --- >> drivers/usb/host/xhci-tegra.c | 202 +++++++++++++++++++--------------- >> 1 file changed, 111 insertions(+), 91 deletions(-) >> >> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c >> index 934be1686352..ce6526c2caf6 100644 >> --- a/drivers/usb/host/xhci-tegra.c >> +++ b/drivers/usb/host/xhci-tegra.c >> @@ -249,8 +249,6 @@ struct tegra_xusb { >> >> struct device *genpd_dev_host; >> struct device *genpd_dev_ss; >> - struct device_link *genpd_dl_host; >> - struct device_link *genpd_dl_ss; >> >> struct phy **phys; >> unsigned int num_phys; >> @@ -814,36 +812,12 @@ static void tegra_xusb_phy_disable(struct tegra_xusb *tegra) >> >> static int tegra_xusb_runtime_suspend(struct device *dev) >> { >> - struct tegra_xusb *tegra = dev_get_drvdata(dev); >> - >> - regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); >> - tegra_xusb_clk_disable(tegra); >> - >> return 0; >> } >> >> static int tegra_xusb_runtime_resume(struct device *dev) >> { >> - struct tegra_xusb *tegra = dev_get_drvdata(dev); >> - int err; >> - >> - err = tegra_xusb_clk_enable(tegra); >> - if (err) { >> - dev_err(dev, "failed to enable clocks: %d\n", err); >> - return err; >> - } >> - >> - err = regulator_bulk_enable(tegra->soc->num_supplies, tegra->supplies); >> - if (err) { >> - dev_err(dev, "failed to enable regulators: %d\n", err); >> - goto disable_clk; >> - } >> - >> return 0; >> - >> -disable_clk: >> - tegra_xusb_clk_disable(tegra); >> - return err; >> } >> >> #ifdef CONFIG_PM_SLEEP >> @@ -1019,10 +993,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra) >> static void tegra_xusb_powerdomain_remove(struct device *dev, >> struct tegra_xusb *tegra) >> { >> - if (tegra->genpd_dl_ss) >> - device_link_del(tegra->genpd_dl_ss); >> - if (tegra->genpd_dl_host) >> - device_link_del(tegra->genpd_dl_host); >> if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) >> dev_pm_domain_detach(tegra->genpd_dev_ss, true); >> if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) >> @@ -1048,20 +1018,88 @@ static int tegra_xusb_powerdomain_init(struct device *dev, >> return err; >> } >> >> - tegra->genpd_dl_host = device_link_add(dev, tegra->genpd_dev_host, >> - DL_FLAG_PM_RUNTIME | >> - DL_FLAG_STATELESS); >> - if (!tegra->genpd_dl_host) { >> - dev_err(dev, "adding host device link failed!\n"); >> - return -ENODEV; >> + return 0; >> +} >> + >> +static int tegra_xusb_unpowergate_partitions(struct tegra_xusb *tegra) >> +{ >> + struct device *dev = tegra->dev; >> + bool use_genpd; >> + int rc; >> + >> + use_genpd = of_property_read_bool(dev->of_node, "power-domains"); > > I don't think that's technically correct. Just because a "power-domains" > property exists in DT doesn't mean any power domains are necessarily > attached to the device. I think you'll need to check for something like > > if (dev->pm_domain) > > here. > Thanks Thierry. I will do so in the next revision. > Thierry >
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c index 934be1686352..ce6526c2caf6 100644 --- a/drivers/usb/host/xhci-tegra.c +++ b/drivers/usb/host/xhci-tegra.c @@ -249,8 +249,6 @@ struct tegra_xusb { struct device *genpd_dev_host; struct device *genpd_dev_ss; - struct device_link *genpd_dl_host; - struct device_link *genpd_dl_ss; struct phy **phys; unsigned int num_phys; @@ -814,36 +812,12 @@ static void tegra_xusb_phy_disable(struct tegra_xusb *tegra) static int tegra_xusb_runtime_suspend(struct device *dev) { - struct tegra_xusb *tegra = dev_get_drvdata(dev); - - regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); - tegra_xusb_clk_disable(tegra); - return 0; } static int tegra_xusb_runtime_resume(struct device *dev) { - struct tegra_xusb *tegra = dev_get_drvdata(dev); - int err; - - err = tegra_xusb_clk_enable(tegra); - if (err) { - dev_err(dev, "failed to enable clocks: %d\n", err); - return err; - } - - err = regulator_bulk_enable(tegra->soc->num_supplies, tegra->supplies); - if (err) { - dev_err(dev, "failed to enable regulators: %d\n", err); - goto disable_clk; - } - return 0; - -disable_clk: - tegra_xusb_clk_disable(tegra); - return err; } #ifdef CONFIG_PM_SLEEP @@ -1019,10 +993,6 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra) static void tegra_xusb_powerdomain_remove(struct device *dev, struct tegra_xusb *tegra) { - if (tegra->genpd_dl_ss) - device_link_del(tegra->genpd_dl_ss); - if (tegra->genpd_dl_host) - device_link_del(tegra->genpd_dl_host); if (!IS_ERR_OR_NULL(tegra->genpd_dev_ss)) dev_pm_domain_detach(tegra->genpd_dev_ss, true); if (!IS_ERR_OR_NULL(tegra->genpd_dev_host)) @@ -1048,20 +1018,88 @@ static int tegra_xusb_powerdomain_init(struct device *dev, return err; } - tegra->genpd_dl_host = device_link_add(dev, tegra->genpd_dev_host, - DL_FLAG_PM_RUNTIME | - DL_FLAG_STATELESS); - if (!tegra->genpd_dl_host) { - dev_err(dev, "adding host device link failed!\n"); - return -ENODEV; + return 0; +} + +static int tegra_xusb_unpowergate_partitions(struct tegra_xusb *tegra) +{ + struct device *dev = tegra->dev; + bool use_genpd; + int rc; + + use_genpd = of_property_read_bool(dev->of_node, "power-domains"); + + if (use_genpd) { + rc = pm_runtime_get_sync(tegra->genpd_dev_ss); + if (rc < 0) { + dev_err(dev, "failed to enable XUSB SS partition\n"); + return rc; + } + + rc = pm_runtime_get_sync(tegra->genpd_dev_host); + if (rc < 0) { + dev_err(dev, "failed to enable XUSB Host partition\n"); + pm_runtime_put_sync(tegra->genpd_dev_ss); + return rc; + } + } else { + rc = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_XUSBA, + tegra->ss_clk, + tegra->ss_rst); + if (rc < 0) { + dev_err(dev, "failed to enable XUSB SS partition\n"); + return rc; + } + + rc = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_XUSBC, + tegra->host_clk, + tegra->host_rst); + if (rc < 0) { + dev_err(dev, "failed to enable XUSB Host partition\n"); + tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA); + return rc; + } } - tegra->genpd_dl_ss = device_link_add(dev, tegra->genpd_dev_ss, - DL_FLAG_PM_RUNTIME | - DL_FLAG_STATELESS); - if (!tegra->genpd_dl_ss) { - dev_err(dev, "adding superspeed device link failed!\n"); - return -ENODEV; + return 0; +} + +static int tegra_xusb_powergate_partitions(struct tegra_xusb *tegra) +{ + struct device *dev = tegra->dev; + bool use_genpd; + int rc; + + use_genpd = of_property_read_bool(dev->of_node, "power-domains"); + + if (use_genpd) { + rc = pm_runtime_put_sync(tegra->genpd_dev_host); + if (rc < 0) { + dev_err(dev, "failed to disable XUSB Host partition\n"); + return rc; + } + + rc = pm_runtime_put_sync(tegra->genpd_dev_ss); + if (rc < 0) { + dev_err(dev, "failed to disable XUSB SS partition\n"); + pm_runtime_get_sync(tegra->genpd_dev_host); + return rc; + } + } else { + rc = tegra_powergate_power_off(TEGRA_POWERGATE_XUSBC); + if (rc < 0) { + dev_err(dev, "failed to disable XUSB Host partition\n"); + return rc; + } + + rc = tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA); + if (rc < 0) { + dev_err(dev, "failed to disable XUSB SS partition\n"); + tegra_powergate_sequence_power_up(TEGRA_POWERGATE_XUSBC, + tegra->host_clk, + tegra->host_rst); + return rc; + } } return 0; @@ -1425,25 +1463,6 @@ static int tegra_xusb_probe(struct platform_device *pdev) err); goto put_padctl; } - - err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_XUSBA, - tegra->ss_clk, - tegra->ss_rst); - if (err) { - dev_err(&pdev->dev, - "failed to enable XUSBA domain: %d\n", err); - goto put_padctl; - } - - err = tegra_powergate_sequence_power_up(TEGRA_POWERGATE_XUSBC, - tegra->host_clk, - tegra->host_rst); - if (err) { - tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA); - dev_err(&pdev->dev, - "failed to enable XUSBC domain: %d\n", err); - goto put_padctl; - } } else { err = tegra_xusb_powerdomain_init(&pdev->dev, tegra); if (err) @@ -1518,10 +1537,22 @@ static int tegra_xusb_probe(struct platform_device *pdev) */ platform_set_drvdata(pdev, tegra); + err = tegra_xusb_clk_enable(tegra); + if (err) { + dev_err(tegra->dev, "failed to enable clocks: %d\n", err); + goto put_hcd; + } + + err = regulator_bulk_enable(tegra->soc->num_supplies, tegra->supplies); + if (err) { + dev_err(tegra->dev, "failed to enable regulators: %d\n", err); + goto disable_clk; + } + err = tegra_xusb_phy_enable(tegra); if (err < 0) { dev_err(&pdev->dev, "failed to enable PHYs: %d\n", err); - goto put_hcd; + goto disable_regulator; } /* @@ -1540,30 +1571,22 @@ static int tegra_xusb_probe(struct platform_device *pdev) goto disable_phy; } - pm_runtime_enable(&pdev->dev); - - if (!pm_runtime_enabled(&pdev->dev)) - err = tegra_xusb_runtime_resume(&pdev->dev); - else - err = pm_runtime_get_sync(&pdev->dev); - - if (err < 0) { - dev_err(&pdev->dev, "failed to enable device: %d\n", err); + err = tegra_xusb_unpowergate_partitions(tegra); + if (err) goto free_firmware; - } tegra_xusb_config(tegra); err = tegra_xusb_load_firmware(tegra); if (err < 0) { dev_err(&pdev->dev, "failed to load firmware: %d\n", err); - goto put_rpm; + goto powergate; } err = usb_add_hcd(tegra->hcd, tegra->xhci_irq, IRQF_SHARED); if (err < 0) { dev_err(&pdev->dev, "failed to add USB HCD: %d\n", err); - goto put_rpm; + goto powergate; } device_wakeup_enable(tegra->hcd->self.controller); @@ -1615,24 +1638,21 @@ static int tegra_xusb_probe(struct platform_device *pdev) usb_put_hcd(xhci->shared_hcd); remove_usb2: usb_remove_hcd(tegra->hcd); -put_rpm: - if (!pm_runtime_status_suspended(&pdev->dev)) - tegra_xusb_runtime_suspend(&pdev->dev); -put_hcd: - usb_put_hcd(tegra->hcd); +powergate: + tegra_xusb_powergate_partitions(tegra); free_firmware: dma_free_coherent(&pdev->dev, tegra->fw.size, tegra->fw.virt, tegra->fw.phys); disable_phy: tegra_xusb_phy_disable(tegra); - pm_runtime_disable(&pdev->dev); +disable_regulator: + regulator_bulk_disable(tegra->soc->num_supplies, tegra->supplies); +disable_clk: + tegra_xusb_clk_disable(tegra); +put_hcd: + usb_put_hcd(tegra->hcd); put_powerdomains: - if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) { - tegra_powergate_power_off(TEGRA_POWERGATE_XUSBC); - tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA); - } else { - tegra_xusb_powerdomain_remove(&pdev->dev, tegra); - } + tegra_xusb_powerdomain_remove(&pdev->dev, tegra); put_padctl: tegra_xusb_padctl_put(tegra->padctl); return err; @@ -1657,15 +1677,15 @@ static int tegra_xusb_remove(struct platform_device *pdev) pm_runtime_put_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); - if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) { - tegra_powergate_power_off(TEGRA_POWERGATE_XUSBC); - tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA); - } else { + tegra_xusb_powergate_partitions(tegra); + + if (of_property_read_bool(pdev->dev.of_node, "power-domains")) { 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_padctl_put(tegra->padctl); return 0;
This commit unlinks xhci-tegra platform device with ss/host power domain devices. Reasons for this change is - at elpg entry, phy sleepwalk and wake configuration need to be done before powering down ss/host partitions, and phy need be powered off after powering down ss/host partitions. Sequence looks like roughly below: tegra_xusb_enter_elpg() -> xhci_suspend() -> enable phy sleepwalk and wake if needed -> power down ss/host partitions -> power down phy If ss/host power domains are linked to xhci-tegra platform device, we are not able to perform the sequence like above. This commit introduces: 1. tegra_xusb_unpowergate_partitions() to power up ss and host partitions together. If ss/host power domain devices are available, it invokes pm_runtime_get_sync() to request power driver to power up partitions; If power domain devices are not available, tegra_powergate_sequence_power_up() will be used to power up partitions. 2. tegra_xusb_powergate_partitions() to power down ss and host partitions together. If ss/host power domain devices are available, it invokes pm_runtime_put_sync() to request power driver to power down partitions; If power domain devices are not available, tegra_powergate_power_off() will be used to power down partitions. Signed-off-by: JC Kuo <jckuo@nvidia.com> --- drivers/usb/host/xhci-tegra.c | 202 +++++++++++++++++++--------------- 1 file changed, 111 insertions(+), 91 deletions(-)