[PATCH V2 0/5] Tegra xHCI genpd support

[PATCH V2 0/5] Tegra xHCI genpd support

[Jon Hunter]

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

[PATCH V2 1/5] dt-bindings: usb: xhci-tegra: Add power-domain details

[Jon Hunter]

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 <jonathanh@nvidia.com>
---
 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

Re: [PATCH V2 1/5] dt-bindings: usb: xhci-tegra: Add power-domain details

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]

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 <jonathanh@nvidia.com>
> ---
>  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 <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 1/5] dt-bindings: usb: xhci-tegra: Add power-domain details

[Jon Hunter]

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 <jonathanh@nvidia.com>
>> ---
>>  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 <treding@nvidia.com>

Thanks!
Jon

-- 
nvpublic

Re: [PATCH V2 1/5] dt-bindings: usb: xhci-tegra: Add power-domain details

[Rob Herring]

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 <jonathanh@nvidia.com>
> ---
>  Documentation/devicetree/bindings/usb/nvidia,tegra124-xusb.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH V2 2/5] usb: xhci: tegra: Power-off power-domains on removal

[Jon Hunter]

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 <jonathanh@nvidia.com>
---
 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

Re: [PATCH V2 2/5] usb: xhci: tegra: Power-off power-domains on removal

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 542 bytes --]

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 <jonathanh@nvidia.com>
> ---
>  drivers/usb/host/xhci-tegra.c | 5 +++++
>  1 file changed, 5 insertions(+)

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH V2 3/5] usb: xhci: tegra: Add genpd support

[Jon Hunter]

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 <jonathanh@nvidia.com>
---
 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

Re: [PATCH V2 3/5] usb: xhci: tegra: Add genpd support

[Ulf Hansson]

[...]

>  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

Re: [PATCH V2 3/5] usb: xhci: tegra: Add genpd support

[Jon Hunter]

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

Re: [PATCH V2 3/5] usb: xhci: tegra: Add genpd support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

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 <jonathanh@nvidia.com>
> ---
>  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 <treding@nvidia.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 3/5] usb: xhci: tegra: Add genpd support

[Jon Hunter]

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 <jonathanh@nvidia.com>
>> ---
>>  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 <treding@nvidia.com>

Thanks!
Jon

-- 
nvpublic

Re: [PATCH V2 3/5] usb: xhci: tegra: Add genpd support

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1694 bytes --]

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 <jonathanh@nvidia.com>
> >> ---
> >>  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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 3/5] usb: xhci: tegra: Add genpd support

[Ulf Hansson]

On 28 September 2018 at 16:11, Jon Hunter <jonathanh@nvidia.com> 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 <jonathanh@nvidia.com>

If not too late, feel free to add:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

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
>

[PATCH V2 4/5] soc/tegra: pmc: Don't power-up XUSB power-domains

[Jon Hunter]

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 <jonathanh@nvidia.com>
---
 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

Re: [PATCH V2 4/5] soc/tegra: pmc: Don't power-up XUSB power-domains

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

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 <jonathanh@nvidia.com>
> ---
>  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 <treding@nvidia.com>

If not, I'll pick this up into the Tegra tree after patches 1-3 have
been merged.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH V2 5/5] arm64: dts: tegra210: Add power-domains for xHCI

[Jon Hunter]

Populate the power-domain properties for the xHCI device for Tegra210.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 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

Re: [PATCH V2 5/5] arm64: dts: tegra210: Add power-domains for xHCI

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

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 <jonathanh@nvidia.com>
> ---
>  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 <treding@nvidia.com>

Otherwise I'll pick this up when the rest have gone in.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2 0/5] Tegra xHCI genpd support

[Jon Hunter]

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

Re: [PATCH V2 0/5] Tegra xHCI genpd support

[Mathias Nyman]

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