diff mbox series

[v1,21/30] usb: host: ehci-tegra: Support OPP and SoC core voltage scaling

Message ID 20201104234427.26477-22-digetx@gmail.com
State New
Headers show
Series Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs | expand

Commit Message

Dmitry Osipenko Nov. 4, 2020, 11:44 p.m. UTC
Add initial OPP and SoC core voltage scaling support to the Tegra EHCI
driver. This is required for enabling system-wide DVFS on older Tegra
SoCs.

Tested-by: Peter Geis <pgwipeout@gmail.com>
Tested-by: Nicolas Chauvet <kwizart@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/usb/host/Kconfig      |  1 +
 drivers/usb/host/ehci-tegra.c | 79 +++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

Comments

Alan Stern Nov. 5, 2020, 4:07 p.m. UTC | #1
On Thu, Nov 05, 2020 at 02:44:18AM +0300, Dmitry Osipenko wrote:
> Add initial OPP and SoC core voltage scaling support to the Tegra EHCI

> driver. This is required for enabling system-wide DVFS on older Tegra

> SoCs.

> 

> Tested-by: Peter Geis <pgwipeout@gmail.com>

> Tested-by: Nicolas Chauvet <kwizart@gmail.com>

> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

> ---


I'm no expert on OPP stuff, but some of what you have done here looks 
peculiar.

> diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c

> index 869d9c4de5fc..0976577f54b4 100644

> --- a/drivers/usb/host/ehci-tegra.c

> +++ b/drivers/usb/host/ehci-tegra.c

> @@ -17,6 +17,7 @@

>  #include <linux/of_device.h>

>  #include <linux/of_gpio.h>

>  #include <linux/platform_device.h>

> +#include <linux/pm_opp.h>

>  #include <linux/pm_runtime.h>

>  #include <linux/reset.h>

>  #include <linux/slab.h>

> @@ -364,6 +365,79 @@ static void tegra_ehci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)

>  	free_dma_aligned_buffer(urb);

>  }

>  

> +static void tegra_ehci_deinit_opp_table(void *data)

> +{

> +	struct device *dev = data;

> +	struct opp_table *opp_table;

> +

> +	opp_table = dev_pm_opp_get_opp_table(dev);

> +	dev_pm_opp_of_remove_table(dev);

> +	dev_pm_opp_put_regulators(opp_table);

> +	dev_pm_opp_put_opp_table(opp_table);

> +}

> +

> +static int devm_tegra_ehci_init_opp_table(struct device *dev)

> +{

> +	unsigned long rate = ULONG_MAX;

> +	struct opp_table *opp_table;

> +	const char *rname = "core";

> +	struct dev_pm_opp *opp;

> +	int err;

> +

> +	/* legacy device-trees don't have OPP table */

> +	if (!device_property_present(dev, "operating-points-v2"))

> +		return 0;

> +

> +	/* voltage scaling is optional */

> +	if (device_property_present(dev, "core-supply"))

> +		opp_table = dev_pm_opp_set_regulators(dev, &rname, 1);

> +	else

> +		opp_table = dev_pm_opp_get_opp_table(dev);

> +

> +	if (IS_ERR(opp_table))

> +		return dev_err_probe(dev, PTR_ERR(opp_table),

> +				     "failed to prepare OPP table\n");

> +

> +	err = dev_pm_opp_of_add_table(dev);

> +	if (err) {

> +		dev_err(dev, "failed to add OPP table: %d\n", err);

> +		goto put_table;

> +	}

> +

> +	/* find suitable OPP for the maximum clock rate */

> +	opp = dev_pm_opp_find_freq_floor(dev, &rate);

> +	err = PTR_ERR_OR_ZERO(opp);

> +	if (err) {

> +		dev_err(dev, "failed to get OPP: %d\n", err);

> +		goto remove_table;

> +	}

> +

> +	dev_pm_opp_put(opp);

> +

> +	/*

> +	 * First dummy rate-set initializes voltage vote by setting voltage

> +	 * in accordance to the clock rate.

> +	 */

> +	err = dev_pm_opp_set_rate(dev, rate);

> +	if (err) {

> +		dev_err(dev, "failed to initialize OPP clock: %d\n", err);

> +		goto remove_table;

> +	}

> +

> +	err = devm_add_action(dev, tegra_ehci_deinit_opp_table, dev);

> +	if (err)

> +		goto remove_table;

> +

> +	return 0;

> +

> +remove_table:

> +	dev_pm_opp_of_remove_table(dev);

> +put_table:

> +	dev_pm_opp_put_regulators(opp_table);


Do you really want to use the same error unwinding for opp_table values 
obtained from dev_pm_opp_set_regulators() as from 
dev_pm_opp_get_opp_table()?

> +

> +	return err;

> +}

> +

>  static const struct tegra_ehci_soc_config tegra30_soc_config = {

>  	.has_hostpc = true,

>  };

> @@ -431,6 +505,11 @@ static int tegra_ehci_probe(struct platform_device *pdev)

>  		goto cleanup_hcd_create;

>  	}

>  

> +	err = devm_tegra_ehci_init_opp_table(&pdev->dev);

> +	if (err)

> +		return dev_err_probe(&pdev->dev, err,

> +				     "Failed to initialize OPP\n");


Why log a second error message?  Just return err.

Alan Stern
Dmitry Osipenko Nov. 5, 2020, 5:54 p.m. UTC | #2
05.11.2020 19:07, Alan Stern пишет:
> Do you really want to use the same error unwinding for opp_table values 

> obtained from dev_pm_opp_set_regulators() as from 

> dev_pm_opp_get_opp_table()?


They both are pointing at the same opp_table, which is refcounted.

The dev_pm_opp_set_regulators() is dev_pm_opp_get_opp_table() + it sets
regulator for the table.

https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/opp/core.c#L1756
Dmitry Osipenko Nov. 5, 2020, 6:02 p.m. UTC | #3
05.11.2020 19:07, Alan Stern пишет:
>> +	err = devm_tegra_ehci_init_opp_table(&pdev->dev);
>> +	if (err)
>> +		return dev_err_probe(&pdev->dev, err,
>> +				     "Failed to initialize OPP\n");
> Why log a second error message?  Just return err.

Indeed, thanks.
diff mbox series

Patch

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index ab12c4bf0ef1..35c42bc05c5a 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -278,6 +278,7 @@  config USB_EHCI_TEGRA
 	depends on ARCH_TEGRA
 	select USB_EHCI_ROOT_HUB_TT
 	select USB_TEGRA_PHY
+	select PM_OPP
 	help
 	  This driver enables support for the internal USB Host Controllers
 	  found in NVIDIA Tegra SoCs. The controllers are EHCI compliant.
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 869d9c4de5fc..0976577f54b4 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -17,6 +17,7 @@ 
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_device.h>
+#include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
@@ -364,6 +365,79 @@  static void tegra_ehci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
 	free_dma_aligned_buffer(urb);
 }
 
+static void tegra_ehci_deinit_opp_table(void *data)
+{
+	struct device *dev = data;
+	struct opp_table *opp_table;
+
+	opp_table = dev_pm_opp_get_opp_table(dev);
+	dev_pm_opp_of_remove_table(dev);
+	dev_pm_opp_put_regulators(opp_table);
+	dev_pm_opp_put_opp_table(opp_table);
+}
+
+static int devm_tegra_ehci_init_opp_table(struct device *dev)
+{
+	unsigned long rate = ULONG_MAX;
+	struct opp_table *opp_table;
+	const char *rname = "core";
+	struct dev_pm_opp *opp;
+	int err;
+
+	/* legacy device-trees don't have OPP table */
+	if (!device_property_present(dev, "operating-points-v2"))
+		return 0;
+
+	/* voltage scaling is optional */
+	if (device_property_present(dev, "core-supply"))
+		opp_table = dev_pm_opp_set_regulators(dev, &rname, 1);
+	else
+		opp_table = dev_pm_opp_get_opp_table(dev);
+
+	if (IS_ERR(opp_table))
+		return dev_err_probe(dev, PTR_ERR(opp_table),
+				     "failed to prepare OPP table\n");
+
+	err = dev_pm_opp_of_add_table(dev);
+	if (err) {
+		dev_err(dev, "failed to add OPP table: %d\n", err);
+		goto put_table;
+	}
+
+	/* find suitable OPP for the maximum clock rate */
+	opp = dev_pm_opp_find_freq_floor(dev, &rate);
+	err = PTR_ERR_OR_ZERO(opp);
+	if (err) {
+		dev_err(dev, "failed to get OPP: %d\n", err);
+		goto remove_table;
+	}
+
+	dev_pm_opp_put(opp);
+
+	/*
+	 * First dummy rate-set initializes voltage vote by setting voltage
+	 * in accordance to the clock rate.
+	 */
+	err = dev_pm_opp_set_rate(dev, rate);
+	if (err) {
+		dev_err(dev, "failed to initialize OPP clock: %d\n", err);
+		goto remove_table;
+	}
+
+	err = devm_add_action(dev, tegra_ehci_deinit_opp_table, dev);
+	if (err)
+		goto remove_table;
+
+	return 0;
+
+remove_table:
+	dev_pm_opp_of_remove_table(dev);
+put_table:
+	dev_pm_opp_put_regulators(opp_table);
+
+	return err;
+}
+
 static const struct tegra_ehci_soc_config tegra30_soc_config = {
 	.has_hostpc = true,
 };
@@ -431,6 +505,11 @@  static int tegra_ehci_probe(struct platform_device *pdev)
 		goto cleanup_hcd_create;
 	}
 
+	err = devm_tegra_ehci_init_opp_table(&pdev->dev);
+	if (err)
+		return dev_err_probe(&pdev->dev, err,
+				     "Failed to initialize OPP\n");
+
 	err = clk_prepare_enable(tegra->clk);
 	if (err)
 		goto cleanup_hcd_create;