This series add genpd support for the Tegra xHCI device that requires
more than one power-domain to operate.
This series is making changes to more than one subsystem and at first
glance may look like a maintainers nightmare. However, my proposal is
this, once reviewed and people are happy ...
1. Patches 1-3 should be merged first. Patches 1 can go via the Tegra
tree and 2-3 via the USB tree.
2. Once patches 1-3 are accepted, then 4 and 5 can be merged via the
Tegra tree.
Changes since V1:
- Dropped unneeded patch to export symbol for genpd API
- Added patch to power-off XUSB domains
- Slight re-work to Tegra xHCI driver changes to simplify clean-up path.
Jon Hunter (5):
dt-bindings: usb: xhci-tegra: Add power-domain details
usb: xhci: tegra: Power-off power-domains on removal
usb: xhci: tegra: Add genpd support
soc/tegra: pmc: Don't power-up XUSB power-domains
arm64: dts: tegra210: Add power-domains for xHCI
.../bindings/usb/nvidia,tegra124-xusb.txt | 8 ++
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 +
drivers/soc/tegra/pmc.c | 16 ----
drivers/usb/host/xhci-tegra.c | 92 +++++++++++++++++++---
4 files changed, 91 insertions(+), 27 deletions(-)
--
2.7.4
Add details for power-domains to the Tegra xHCI bindings so that
generic power-domains can be used for inconjunction with the xHCI
driver.
Signed-off-by: Jon Hunter <[email protected]>
---
Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
index 3eee9e505400..4156c3e181c5 100644
--- a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
+++ b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
@@ -59,6 +59,14 @@ For Tegra210:
- avdd-pll-uerefe-supply: PLLE reference PLL power supply. Must supply 1.05 V.
- dvdd-pex-pll-supply: PCIe/USB3 PLL power supply. Must supply 1.05 V.
- hvdd-pex-pll-e-supply: High-voltage PLLE power supply. Must supply 1.8 V.
+- power-domains: A list of PM domain specifiers that reference each power-domain
+ used by the xHCI controller. This list must comprise of a specifier for the
+ XUSBA and XUSBC power-domains. See ../power/power_domain.txt and
+ ../arm/tegra/nvidia,tegra20-pmc.txt for details.
+- power-domain-names: A list of names that represent each of the specifiers in
+ the 'power-domains' property. Must include 'xusb_ss' and 'xusb_host' which
+ represent the power-domains XUSBA and XUSBC, respectively. See
+ ../power/power_domain.txt for details.
Optional properties:
--------------------
--
2.7.4
Populate the power-domain properties for the xHCI device for Tegra210.
Signed-off-by: Jon Hunter <[email protected]>
---
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
index 8fe47d6445a5..2205d66b0443 100644
--- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
@@ -879,6 +879,8 @@
resets = <&tegra_car 89>, <&tegra_car 156>,
<&tegra_car 143>;
reset-names = "xusb_host", "xusb_ss", "xusb_src";
+ power-domains = <&pd_xusbhost>, <&pd_xusbss>;
+ power-domain-names = "xusb_host", "xusb_ss";
nvidia,xusb-padctl = <&padctl>;
--
2.7.4
Now that the Tegra xHCI driver manages the XUSB power-domains itself,
remove the code to power-up the power-domains used by the xHCI device
from the PMC driver on boot.
Signed-off-by: Jon Hunter <[email protected]>
---
drivers/soc/tegra/pmc.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index ab719fa90150..a68b4476b4ee 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -847,22 +847,6 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, struct device_node *np)
goto remove_resets;
}
- /*
- * FIXME: If XHCI is enabled for Tegra, then power-up the XUSB
- * host and super-speed partitions. Once the XHCI driver
- * manages the partitions itself this code can be removed. Note
- * that we don't register these partitions with the genpd core
- * to avoid it from powering down the partitions as they appear
- * to be unused.
- */
- if (IS_ENABLED(CONFIG_USB_XHCI_TEGRA) &&
- (id == TEGRA_POWERGATE_XUSBA || id == TEGRA_POWERGATE_XUSBC)) {
- if (off)
- WARN_ON(tegra_powergate_power_up(pg, true));
-
- goto remove_resets;
- }
-
err = pm_genpd_init(&pg->genpd, NULL, off);
if (err < 0) {
pr_err("failed to initialise PM domain %s: %d\n", np->name,
--
2.7.4
The generic power-domain framework has been updated to allow devices
that require more than one power-domain to create a new device for
each power-domain required and then link these new power-domain
devices to the consumer device.
Update the Tegra xHCI driver to use the new APIs provided by the
generic power-domain framework so we can use the generic power-domain
framework for managing the xHCI controllers power-domains. Please
note that to maintain backward compatibility with older device-tree
blobs these new generic power-domain APIs are only used if the
'power-domains' property is present and otherwise we fall back to
using the legacy Tegra APIs for managing the power-domains.
Signed-off-by: Jon Hunter <[email protected]>
---
drivers/usb/host/xhci-tegra.c | 89 +++++++++++++++++++++++++++++++++++++------
1 file changed, 77 insertions(+), 12 deletions(-)
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index a586a7930a3d..696008ecc26e 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -18,6 +18,7 @@
#include <linux/phy/tegra/xusb.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
+#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
#include <linux/reset.h>
@@ -194,6 +195,11 @@ struct tegra_xusb {
struct reset_control *host_rst;
struct reset_control *ss_rst;
+ 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;
@@ -928,6 +934,57 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
return 0;
}
+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 (tegra->genpd_dev_ss)
+ dev_pm_domain_detach(tegra->genpd_dev_ss, true);
+ if (tegra->genpd_dev_host)
+ dev_pm_domain_detach(tegra->genpd_dev_host, true);
+}
+
+static int tegra_xusb_powerdomain_init(struct device *dev,
+ struct tegra_xusb *tegra)
+{
+ int err;
+
+ tegra->genpd_dev_host = dev_pm_domain_attach_by_name(dev, "xusb_host");
+ if (IS_ERR(tegra->genpd_dev_host)) {
+ err = PTR_ERR(tegra->genpd_dev_host);
+ dev_err(dev, "failed to get host pm-domain: %d\n", err);
+ return err;
+ }
+
+ tegra->genpd_dev_ss = dev_pm_domain_attach_by_name(dev, "xusb_ss");
+ if (IS_ERR(tegra->genpd_dev_ss)) {
+ err = PTR_ERR(tegra->genpd_dev_ss);
+ dev_err(dev, "failed to get superspeed pm-domain: %d\n", err);
+ 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;
+ }
+
+ 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_probe(struct platform_device *pdev)
{
struct tegra_xusb_mbox_msg msg;
@@ -1038,7 +1095,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
goto put_padctl;
}
- if (!pdev->dev.pm_domain) {
+ if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) {
tegra->host_rst = devm_reset_control_get(&pdev->dev,
"xusb_host");
if (IS_ERR(tegra->host_rst)) {
@@ -1069,17 +1126,22 @@ static int tegra_xusb_probe(struct platform_device *pdev)
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 disable_xusba;
+ goto put_padctl;
}
+ } else {
+ err = tegra_xusb_powerdomain_init(&pdev->dev, tegra);
+ if (err)
+ goto put_powerdomains;
}
tegra->supplies = devm_kcalloc(&pdev->dev, tegra->soc->num_supplies,
sizeof(*tegra->supplies), GFP_KERNEL);
if (!tegra->supplies) {
err = -ENOMEM;
- goto disable_xusbc;
+ goto put_powerdomains;
}
for (i = 0; i < tegra->soc->num_supplies; i++)
@@ -1089,7 +1151,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
tegra->supplies);
if (err) {
dev_err(&pdev->dev, "failed to get regulators: %d\n", err);
- goto disable_xusbc;
+ goto put_powerdomains;
}
for (i = 0; i < tegra->soc->num_types; i++)
@@ -1099,7 +1161,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
sizeof(*tegra->phys), GFP_KERNEL);
if (!tegra->phys) {
err = -ENOMEM;
- goto disable_xusbc;
+ goto put_powerdomains;
}
for (i = 0, k = 0; i < tegra->soc->num_types; i++) {
@@ -1115,7 +1177,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
"failed to get PHY %s: %ld\n", prop,
PTR_ERR(phy));
err = PTR_ERR(phy);
- goto disable_xusbc;
+ goto put_powerdomains;
}
tegra->phys[k++] = phy;
@@ -1126,7 +1188,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
dev_name(&pdev->dev));
if (!tegra->hcd) {
err = -ENOMEM;
- goto disable_xusbc;
+ goto put_powerdomains;
}
/*
@@ -1222,12 +1284,13 @@ static int tegra_xusb_probe(struct platform_device *pdev)
disable_rpm:
pm_runtime_disable(&pdev->dev);
usb_put_hcd(tegra->hcd);
-disable_xusbc:
- if (!pdev->dev.pm_domain)
+put_powerdomains:
+ if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) {
tegra_powergate_power_off(TEGRA_POWERGATE_XUSBC);
-disable_xusba:
- if (!pdev->dev.pm_domain)
tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA);
+ } else {
+ tegra_xusb_powerdomain_remove(&pdev->dev, tegra);
+ }
put_padctl:
tegra_xusb_padctl_put(tegra->padctl);
return err;
@@ -1249,9 +1312,11 @@ static int tegra_xusb_remove(struct platform_device *pdev)
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
- if (!pdev->dev.pm_domain) {
+ 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_padctl_put(tegra->padctl);
--
2.7.4
Currently the XUSB power domains used by the Tegra xHCI controller are
never powered off on the removal of the driver, however, they will be
powered off on probe failure. Update the removal code to be consistent
with the probe failure path to power off the XUSB power domains.
Signed-off-by: Jon Hunter <[email protected]>
---
drivers/usb/host/xhci-tegra.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 4b463e5202a4..a586a7930a3d 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -1249,6 +1249,11 @@ static int tegra_xusb_remove(struct platform_device *pdev)
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
+ if (!pdev->dev.pm_domain) {
+ tegra_powergate_power_off(TEGRA_POWERGATE_XUSBC);
+ tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA);
+ }
+
tegra_xusb_padctl_put(tegra->padctl);
return 0;
--
2.7.4
[...]
> static int tegra_xusb_probe(struct platform_device *pdev)
> {
> struct tegra_xusb_mbox_msg msg;
> @@ -1038,7 +1095,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> goto put_padctl;
> }
>
> - if (!pdev->dev.pm_domain) {
> + if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) {
I am assuming the original check was because allowing the two
power-domains to be (wrongly) modeled as one (or as a
master+subdomain)?
I was thinking that, perhaps we should add a new OF helper function,
where one can get the number of specifiers being listed in the
power-domains property. Would that help to easier distinguish what to
do when dealing with backwards compatibility?
> tegra->host_rst = devm_reset_control_get(&pdev->dev,
> "xusb_host");
> if (IS_ERR(tegra->host_rst)) {
> @@ -1069,17 +1126,22 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> 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 disable_xusba;
> + goto put_padctl;
> }
> + } else {
> + err = tegra_xusb_powerdomain_init(&pdev->dev, tegra);
> + if (err)
> + goto put_powerdomains;
> }
>
> tegra->supplies = devm_kcalloc(&pdev->dev, tegra->soc->num_supplies,
> sizeof(*tegra->supplies), GFP_KERNEL);
> if (!tegra->supplies) {
> err = -ENOMEM;
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> for (i = 0; i < tegra->soc->num_supplies; i++)
> @@ -1089,7 +1151,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> tegra->supplies);
> if (err) {
> dev_err(&pdev->dev, "failed to get regulators: %d\n", err);
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> for (i = 0; i < tegra->soc->num_types; i++)
> @@ -1099,7 +1161,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> sizeof(*tegra->phys), GFP_KERNEL);
> if (!tegra->phys) {
> err = -ENOMEM;
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> for (i = 0, k = 0; i < tegra->soc->num_types; i++) {
> @@ -1115,7 +1177,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> "failed to get PHY %s: %ld\n", prop,
> PTR_ERR(phy));
> err = PTR_ERR(phy);
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> tegra->phys[k++] = phy;
> @@ -1126,7 +1188,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> dev_name(&pdev->dev));
> if (!tegra->hcd) {
> err = -ENOMEM;
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> /*
> @@ -1222,12 +1284,13 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> disable_rpm:
> pm_runtime_disable(&pdev->dev);
> usb_put_hcd(tegra->hcd);
> -disable_xusbc:
> - if (!pdev->dev.pm_domain)
> +put_powerdomains:
> + if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) {
> tegra_powergate_power_off(TEGRA_POWERGATE_XUSBC);
> -disable_xusba:
> - if (!pdev->dev.pm_domain)
> tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA);
> + } else {
> + tegra_xusb_powerdomain_remove(&pdev->dev, tegra);
> + }
> put_padctl:
> tegra_xusb_padctl_put(tegra->padctl);
> return err;
[...]
Kind regards
Uffe
On 03/10/18 10:52, Ulf Hansson wrote:
> [...]
>
>> static int tegra_xusb_probe(struct platform_device *pdev)
>> {
>> struct tegra_xusb_mbox_msg msg;
>> @@ -1038,7 +1095,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
>> goto put_padctl;
>> }
>>
>> - if (!pdev->dev.pm_domain) {
>> + if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) {
>
> I am assuming the original check was because allowing the two
> power-domains to be (wrongly) modeled as one (or as a
> master+subdomain)?
Actually, the original check was added to prepare for supporting
multiple power-domains and that once we had proper support in place the
'pdev->dev.domain' would then be populated. However, given that this is
not used in the case of multiple power-domains, I simply changed the test.
> I was thinking that, perhaps we should add a new OF helper function,
> where one can get the number of specifiers being listed in the
> power-domains property. Would that help to easier distinguish what to
> do when dealing with backwards compatibility?
We could do, but it is not necessary here, because we never had any form
of genpd support for the Tegra xHCI driver.
Cheers
Jon
--
nvpublic
On Fri, Sep 28, 2018 at 03:11:48PM +0100, Jon Hunter wrote:
> The generic power-domain framework has been updated to allow devices
> that require more than one power-domain to create a new device for
> each power-domain required and then link these new power-domain
> devices to the consumer device.
>
> Update the Tegra xHCI driver to use the new APIs provided by the
> generic power-domain framework so we can use the generic power-domain
> framework for managing the xHCI controllers power-domains. Please
> note that to maintain backward compatibility with older device-tree
> blobs these new generic power-domain APIs are only used if the
> 'power-domains' property is present and otherwise we fall back to
> using the legacy Tegra APIs for managing the power-domains.
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> drivers/usb/host/xhci-tegra.c | 89 +++++++++++++++++++++++++++++++++++++------
> 1 file changed, 77 insertions(+), 12 deletions(-)
It'd be nice if we could eventually get rid of the legacy Tegra APIs,
but that's a separate issue, and this patch looks fine as-is:
Acked-by: Thierry Reding <[email protected]>
On Fri, Sep 28, 2018 at 03:11:47PM +0100, Jon Hunter wrote:
> Currently the XUSB power domains used by the Tegra xHCI controller are
> never powered off on the removal of the driver, however, they will be
> powered off on probe failure. Update the removal code to be consistent
> with the probe failure path to power off the XUSB power domains.
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> drivers/usb/host/xhci-tegra.c | 5 +++++
> 1 file changed, 5 insertions(+)
Acked-by: Thierry Reding <[email protected]>
On Fri, Sep 28, 2018 at 03:11:50PM +0100, Jon Hunter wrote:
> Populate the power-domain properties for the xHCI device for Tegra210.
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
Again, if somebody wants to pick up the series as a whole:
Acked-by: Thierry Reding <[email protected]>
Otherwise I'll pick this up when the rest have gone in.
Thierry
On Fri, Sep 28, 2018 at 03:11:49PM +0100, Jon Hunter wrote:
> Now that the Tegra xHCI driver manages the XUSB power-domains itself,
> remove the code to power-up the power-domains used by the xHCI device
> from the PMC driver on boot.
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> drivers/soc/tegra/pmc.c | 16 ----------------
> 1 file changed, 16 deletions(-)
In case somebody wants to pick this up along with the earlier patches:
Acked-by: Thierry Reding <[email protected]>
If not, I'll pick this up into the Tegra tree after patches 1-3 have
been merged.
Thierry
On Fri, Sep 28, 2018 at 03:11:46PM +0100, Jon Hunter wrote:
> Add details for power-domains to the Tegra xHCI bindings so that
> generic power-domains can be used for inconjunction with the xHCI
> driver.
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
> index 3eee9e505400..4156c3e181c5 100644
> --- a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
> @@ -59,6 +59,14 @@ For Tegra210:
> - avdd-pll-uerefe-supply: PLLE reference PLL power supply. Must supply 1.05 V.
> - dvdd-pex-pll-supply: PCIe/USB3 PLL power supply. Must supply 1.05 V.
> - hvdd-pex-pll-e-supply: High-voltage PLLE power supply. Must supply 1.8 V.
> +- power-domains: A list of PM domain specifiers that reference each power-domain
> + used by the xHCI controller. This list must comprise of a specifier for the
> + XUSBA and XUSBC power-domains. See ../power/power_domain.txt and
> + ../arm/tegra/nvidia,tegra20-pmc.txt for details.
> +- power-domain-names: A list of names that represent each of the specifiers in
> + the 'power-domains' property. Must include 'xusb_ss' and 'xusb_host' which
Total bike-shed comment: maybe call these "superspeed" and "host" or
something. The xusb_ prefix is kind of redundent because of the context.
On the other hand, those are the names by which the power partitions are
referred to, so either way:
Acked-by: Thierry Reding <[email protected]>
On 11/10/18 17:49, Thierry Reding wrote:
> On Fri, Sep 28, 2018 at 03:11:46PM +0100, Jon Hunter wrote:
>> Add details for power-domains to the Tegra xHCI bindings so that
>> generic power-domains can be used for inconjunction with the xHCI
>> driver.
>>
>> Signed-off-by: Jon Hunter <[email protected]>
>> ---
>> Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
>> index 3eee9e505400..4156c3e181c5 100644
>> --- a/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
>> +++ b/Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt
>> @@ -59,6 +59,14 @@ For Tegra210:
>> - avdd-pll-uerefe-supply: PLLE reference PLL power supply. Must supply 1.05 V.
>> - dvdd-pex-pll-supply: PCIe/USB3 PLL power supply. Must supply 1.05 V.
>> - hvdd-pex-pll-e-supply: High-voltage PLLE power supply. Must supply 1.8 V.
>> +- power-domains: A list of PM domain specifiers that reference each power-domain
>> + used by the xHCI controller. This list must comprise of a specifier for the
>> + XUSBA and XUSBC power-domains. See ../power/power_domain.txt and
>> + ../arm/tegra/nvidia,tegra20-pmc.txt for details.
>> +- power-domain-names: A list of names that represent each of the specifiers in
>> + the 'power-domains' property. Must include 'xusb_ss' and 'xusb_host' which
>
> Total bike-shed comment: maybe call these "superspeed" and "host" or
> something. The xusb_ prefix is kind of redundent because of the context.
> On the other hand, those are the names by which the power partitions are
> referred to, so either way:
I choose these names because they align with what we already have for
clocks ...
- clock-names: Must include the following entries:
- xusb_host
- xusb_host_src
- xusb_falcon_src
- xusb_ss
- xusb_ss_src
- xusb_ss_div2
- xusb_hs_src
- xusb_fs_src
- pll_u_480m
- clk_m
- pll_e
However, I don't have a strong preference.
> Acked-by: Thierry Reding <[email protected]>
Thanks!
Jon
--
nvpublic
On 11/10/18 17:47, Thierry Reding wrote:
> On Fri, Sep 28, 2018 at 03:11:48PM +0100, Jon Hunter wrote:
>> The generic power-domain framework has been updated to allow devices
>> that require more than one power-domain to create a new device for
>> each power-domain required and then link these new power-domain
>> devices to the consumer device.
>>
>> Update the Tegra xHCI driver to use the new APIs provided by the
>> generic power-domain framework so we can use the generic power-domain
>> framework for managing the xHCI controllers power-domains. Please
>> note that to maintain backward compatibility with older device-tree
>> blobs these new generic power-domain APIs are only used if the
>> 'power-domains' property is present and otherwise we fall back to
>> using the legacy Tegra APIs for managing the power-domains.
>>
>> Signed-off-by: Jon Hunter <[email protected]>
>> ---
>> drivers/usb/host/xhci-tegra.c | 89 +++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 77 insertions(+), 12 deletions(-)
>
> It'd be nice if we could eventually get rid of the legacy Tegra APIs,
> but that's a separate issue, and this patch looks fine as-is:
Unfortunately, I don't think it is possible as it will break DT backward
compatibility. However, one way to do it would be to force on all power
domains on boot if PM is not supported.
> Acked-by: Thierry Reding <[email protected]>
Thanks!
Jon
--
nvpublic
On Fri, Oct 12, 2018 at 09:41:35AM +0100, Jon Hunter wrote:
>
> On 11/10/18 17:47, Thierry Reding wrote:
> > On Fri, Sep 28, 2018 at 03:11:48PM +0100, Jon Hunter wrote:
> >> The generic power-domain framework has been updated to allow devices
> >> that require more than one power-domain to create a new device for
> >> each power-domain required and then link these new power-domain
> >> devices to the consumer device.
> >>
> >> Update the Tegra xHCI driver to use the new APIs provided by the
> >> generic power-domain framework so we can use the generic power-domain
> >> framework for managing the xHCI controllers power-domains. Please
> >> note that to maintain backward compatibility with older device-tree
> >> blobs these new generic power-domain APIs are only used if the
> >> 'power-domains' property is present and otherwise we fall back to
> >> using the legacy Tegra APIs for managing the power-domains.
> >>
> >> Signed-off-by: Jon Hunter <[email protected]>
> >> ---
> >> drivers/usb/host/xhci-tegra.c | 89 +++++++++++++++++++++++++++++++++++++------
> >> 1 file changed, 77 insertions(+), 12 deletions(-)
> >
> > It'd be nice if we could eventually get rid of the legacy Tegra APIs,
> > but that's a separate issue, and this patch looks fine as-is:
>
> Unfortunately, I don't think it is possible as it will break DT backward
> compatibility. However, one way to do it would be to force on all power
> domains on boot if PM is not supported.
Yeah, that sounds like a good option. If we don't support PM the I
imagine there's little use in keeping some of the partitions disabled.
But let's investigate that at a later time.
Thierry
On Fri, 28 Sep 2018 15:11:46 +0100, Jon Hunter wrote:
> Add details for power-domains to the Tegra xHCI bindings so that
> generic power-domains can be used for inconjunction with the xHCI
> driver.
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
Reviewed-by: Rob Herring <[email protected]>
Hi Mathias,
On 28/09/18 15:11, Jon Hunter wrote:
> This series add genpd support for the Tegra xHCI device that requires
> more than one power-domain to operate.
>
> This series is making changes to more than one subsystem and at first
> glance may look like a maintainers nightmare. However, my proposal is
> this, once reviewed and people are happy ...
> 1. Patches 1-3 should be merged first. Patches 1 can go via the Tegra
> tree and 2-3 via the USB tree.
> 2. Once patches 1-3 are accepted, then 4 and 5 can be merged via the
> Tegra tree.
>
> Changes since V1:
> - Dropped unneeded patch to export symbol for genpd API
> - Added patch to power-off XUSB domains
> - Slight re-work to Tegra xHCI driver changes to simplify clean-up path.
>
> Jon Hunter (5):
> dt-bindings: usb: xhci-tegra: Add power-domain details
> usb: xhci: tegra: Power-off power-domains on removal
> usb: xhci: tegra: Add genpd support
> soc/tegra: pmc: Don't power-up XUSB power-domains
> arm64: dts: tegra210: Add power-domains for xHCI
Let me know you are ok to pick up patches 2 and 3 for v4.20.
Cheers
Jon
--
nvpublic
On 28 September 2018 at 16:11, Jon Hunter <[email protected]> wrote:
> The generic power-domain framework has been updated to allow devices
> that require more than one power-domain to create a new device for
> each power-domain required and then link these new power-domain
> devices to the consumer device.
>
> Update the Tegra xHCI driver to use the new APIs provided by the
> generic power-domain framework so we can use the generic power-domain
> framework for managing the xHCI controllers power-domains. Please
> note that to maintain backward compatibility with older device-tree
> blobs these new generic power-domain APIs are only used if the
> 'power-domains' property is present and otherwise we fall back to
> using the legacy Tegra APIs for managing the power-domains.
>
> Signed-off-by: Jon Hunter <[email protected]>
If not too late, feel free to add:
Reviewed-by: Ulf Hansson <[email protected]>
Kind regards
Uffe
> ---
> drivers/usb/host/xhci-tegra.c | 89 +++++++++++++++++++++++++++++++++++++------
> 1 file changed, 77 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index a586a7930a3d..696008ecc26e 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -18,6 +18,7 @@
> #include <linux/phy/tegra/xusb.h>
> #include <linux/platform_device.h>
> #include <linux/pm.h>
> +#include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
> #include <linux/reset.h>
> @@ -194,6 +195,11 @@ struct tegra_xusb {
> struct reset_control *host_rst;
> struct reset_control *ss_rst;
>
> + 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;
>
> @@ -928,6 +934,57 @@ static int tegra_xusb_load_firmware(struct tegra_xusb *tegra)
> return 0;
> }
>
> +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 (tegra->genpd_dev_ss)
> + dev_pm_domain_detach(tegra->genpd_dev_ss, true);
> + if (tegra->genpd_dev_host)
> + dev_pm_domain_detach(tegra->genpd_dev_host, true);
> +}
> +
> +static int tegra_xusb_powerdomain_init(struct device *dev,
> + struct tegra_xusb *tegra)
> +{
> + int err;
> +
> + tegra->genpd_dev_host = dev_pm_domain_attach_by_name(dev, "xusb_host");
> + if (IS_ERR(tegra->genpd_dev_host)) {
> + err = PTR_ERR(tegra->genpd_dev_host);
> + dev_err(dev, "failed to get host pm-domain: %d\n", err);
> + return err;
> + }
> +
> + tegra->genpd_dev_ss = dev_pm_domain_attach_by_name(dev, "xusb_ss");
> + if (IS_ERR(tegra->genpd_dev_ss)) {
> + err = PTR_ERR(tegra->genpd_dev_ss);
> + dev_err(dev, "failed to get superspeed pm-domain: %d\n", err);
> + 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;
> + }
> +
> + 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_probe(struct platform_device *pdev)
> {
> struct tegra_xusb_mbox_msg msg;
> @@ -1038,7 +1095,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> goto put_padctl;
> }
>
> - if (!pdev->dev.pm_domain) {
> + if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) {
> tegra->host_rst = devm_reset_control_get(&pdev->dev,
> "xusb_host");
> if (IS_ERR(tegra->host_rst)) {
> @@ -1069,17 +1126,22 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> 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 disable_xusba;
> + goto put_padctl;
> }
> + } else {
> + err = tegra_xusb_powerdomain_init(&pdev->dev, tegra);
> + if (err)
> + goto put_powerdomains;
> }
>
> tegra->supplies = devm_kcalloc(&pdev->dev, tegra->soc->num_supplies,
> sizeof(*tegra->supplies), GFP_KERNEL);
> if (!tegra->supplies) {
> err = -ENOMEM;
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> for (i = 0; i < tegra->soc->num_supplies; i++)
> @@ -1089,7 +1151,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> tegra->supplies);
> if (err) {
> dev_err(&pdev->dev, "failed to get regulators: %d\n", err);
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> for (i = 0; i < tegra->soc->num_types; i++)
> @@ -1099,7 +1161,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> sizeof(*tegra->phys), GFP_KERNEL);
> if (!tegra->phys) {
> err = -ENOMEM;
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> for (i = 0, k = 0; i < tegra->soc->num_types; i++) {
> @@ -1115,7 +1177,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> "failed to get PHY %s: %ld\n", prop,
> PTR_ERR(phy));
> err = PTR_ERR(phy);
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> tegra->phys[k++] = phy;
> @@ -1126,7 +1188,7 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> dev_name(&pdev->dev));
> if (!tegra->hcd) {
> err = -ENOMEM;
> - goto disable_xusbc;
> + goto put_powerdomains;
> }
>
> /*
> @@ -1222,12 +1284,13 @@ static int tegra_xusb_probe(struct platform_device *pdev)
> disable_rpm:
> pm_runtime_disable(&pdev->dev);
> usb_put_hcd(tegra->hcd);
> -disable_xusbc:
> - if (!pdev->dev.pm_domain)
> +put_powerdomains:
> + if (!of_property_read_bool(pdev->dev.of_node, "power-domains")) {
> tegra_powergate_power_off(TEGRA_POWERGATE_XUSBC);
> -disable_xusba:
> - if (!pdev->dev.pm_domain)
> tegra_powergate_power_off(TEGRA_POWERGATE_XUSBA);
> + } else {
> + tegra_xusb_powerdomain_remove(&pdev->dev, tegra);
> + }
> put_padctl:
> tegra_xusb_padctl_put(tegra->padctl);
> return err;
> @@ -1249,9 +1312,11 @@ static int tegra_xusb_remove(struct platform_device *pdev)
> pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
>
> - if (!pdev->dev.pm_domain) {
> + 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_padctl_put(tegra->padctl);
> --
> 2.7.4
>
On 15.10.2018 15:40, Jon Hunter wrote:
> Hi Mathias,
>
> On 28/09/18 15:11, Jon Hunter wrote:
>> This series add genpd support for the Tegra xHCI device that requires
>> more than one power-domain to operate.
>>
>> This series is making changes to more than one subsystem and at first
>> glance may look like a maintainers nightmare. However, my proposal is
>> this, once reviewed and people are happy ...
>> 1. Patches 1-3 should be merged first. Patches 1 can go via the Tegra
>> tree and 2-3 via the USB tree.
>> 2. Once patches 1-3 are accepted, then 4 and 5 can be merged via the
>> Tegra tree.
>>
>> Changes since V1:
>> - Dropped unneeded patch to export symbol for genpd API
>> - Added patch to power-off XUSB domains
>> - Slight re-work to Tegra xHCI driver changes to simplify clean-up path.
>>
>> Jon Hunter (5):
>> dt-bindings: usb: xhci-tegra: Add power-domain details
>> usb: xhci: tegra: Power-off power-domains on removal
>> usb: xhci: tegra: Add genpd support
>> soc/tegra: pmc: Don't power-up XUSB power-domains
>> arm64: dts: tegra210: Add power-domains for xHCI
>
> Let me know you are ok to pick up patches 2 and 3 for v4.20.
>
Patches 2 and 3 look good to me.
I'll resend them to Greg
-Mathias