| Message ID | 20201104234427.26477-22-digetx@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs | expand |
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
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
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 --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;