Re: [PATCH v2 28/48] soc/tegra: Introduce core power domain driver

On Thu, 17 Dec 2020 at 19:07, Dmitry Osipenko digetx@gmail.com wrote:

NVIDIA Tegra SoCs have multiple power domains, each domain corresponds to an external SoC power rail. Core power domain covers vast majority of hardware blocks within a Tegra SoC. The voltage of a power domain should be set to a value which satisfies all devices within a power domain. Add driver for the core power domain in order to manage the voltage state of the domain. This allows us to support a system-wide DVFS on Tegra.

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

FYI: from a genpd provider driver point of view, this looks good to me. However, withholding my ack for the next version, just to make sure I take a second look.

Kind regards Uffe

---

drivers/soc/tegra/Kconfig | 6 ++ drivers/soc/tegra/Makefile | 1 + drivers/soc/tegra/core-power-domain.c | 125 ++++++++++++++++++++++++++ include/soc/tegra/common.h | 6 ++ 4 files changed, 138 insertions(+) create mode 100644 drivers/soc/tegra/core-power-domain.c

diff --git a/drivers/soc/tegra/Kconfig b/drivers/soc/tegra/Kconfig index bcd61ae59ba3..fccbc168dd87 100644 --- a/drivers/soc/tegra/Kconfig +++ b/drivers/soc/tegra/Kconfig @@ -16,6 +16,7 @@ config ARCH_TEGRA_2x_SOC select SOC_TEGRA_COMMON select SOC_TEGRA_FLOWCTRL select SOC_TEGRA_PMC

•   select SOC_TEGRA_CORE_POWER_DOMAIN
    select SOC_TEGRA20_VOLTAGE_COUPLER
    select TEGRA_TIMER
    help
  

@@ -31,6 +32,7 @@ config ARCH_TEGRA_3x_SOC select SOC_TEGRA_COMMON select SOC_TEGRA_FLOWCTRL select SOC_TEGRA_PMC

•   select SOC_TEGRA_CORE_POWER_DOMAIN
    select SOC_TEGRA30_VOLTAGE_COUPLER
    select TEGRA_TIMER
    help
  

@@ -170,3 +172,7 @@ config SOC_TEGRA20_VOLTAGE_COUPLER config SOC_TEGRA30_VOLTAGE_COUPLER bool "Voltage scaling support for Tegra30 SoCs" depends on ARCH_TEGRA_3x_SOC || COMPILE_TEST

+config SOC_TEGRA_CORE_POWER_DOMAIN

•   bool
  

•   select PM_GENERIC_DOMAINS
  

diff --git a/drivers/soc/tegra/Makefile b/drivers/soc/tegra/Makefile index 9c809c1814bd..8f1294f954b4 100644 --- a/drivers/soc/tegra/Makefile +++ b/drivers/soc/tegra/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_SOC_TEGRA_PMC) += pmc.o obj-$(CONFIG_SOC_TEGRA_POWERGATE_BPMP) += powergate-bpmp.o obj-$(CONFIG_SOC_TEGRA20_VOLTAGE_COUPLER) += regulators-tegra20.o obj-$(CONFIG_SOC_TEGRA30_VOLTAGE_COUPLER) += regulators-tegra30.o +obj-$(CONFIG_SOC_TEGRA_CORE_POWER_DOMAIN) += core-power-domain.o diff --git a/drivers/soc/tegra/core-power-domain.c b/drivers/soc/tegra/core-power-domain.c new file mode 100644 index 000000000000..7c0cec8c79fd --- /dev/null +++ b/drivers/soc/tegra/core-power-domain.c @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • NVIDIA Tegra SoC Core Power Domain Driver
• */

+#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pm_domain.h> +#include <linux/pm_opp.h> +#include <linux/slab.h>

+#include <soc/tegra/common.h>

+static struct lock_class_key tegra_core_domain_lock_class; +static bool tegra_core_domain_state_synced;

+static int tegra_genpd_set_performance_state(struct generic_pm_domain *genpd,

•                                        unsigned int level)
  

+{

•   struct dev_pm_opp *opp;
  

•   int err;
  

•   opp = dev_pm_opp_find_level_ceil(&genpd->dev, &level);
  

•   if (IS_ERR(opp)) {
  

•           dev_err(&genpd->dev, "failed to find OPP for level %u: %pe\n",
  

•                   level, opp);
  

•           return PTR_ERR(opp);
  

•   }
  

•   err = dev_pm_opp_set_voltage(&genpd->dev, opp);
  

•   dev_pm_opp_put(opp);
  

•   if (err) {
  

•           dev_err(&genpd->dev, "failed to set voltage to %duV: %d\n",
  

•                   level, err);
  

•           return err;
  

•   }
  

•   return 0;
  

+}

+static unsigned int +tegra_genpd_opp_to_performance_state(struct generic_pm_domain *genpd,

•                                struct dev_pm_opp *opp)
  

+{

•   return dev_pm_opp_get_level(opp);
  

+}

+static int tegra_core_domain_probe(struct platform_device *pdev) +{

•   struct generic_pm_domain *genpd;
  

•   struct opp_table *opp_table;
  

•   const char *rname = "power";
  

•   int err;
  

•   genpd = devm_kzalloc(&pdev->dev, sizeof(*genpd), GFP_KERNEL);
  

•   if (!genpd)
  

•           return -ENOMEM;
  

•   genpd->name = pdev->dev.of_node->name;
  

•   genpd->set_performance_state = tegra_genpd_set_performance_state;
  

•   genpd->opp_to_performance_state = tegra_genpd_opp_to_performance_state;
  

•   opp_table = devm_pm_opp_set_regulators(&pdev->dev, &rname, 1);
  

•   if (IS_ERR(opp_table))
  

•           return dev_err_probe(&pdev->dev, PTR_ERR(opp_table),
  

•                                "failed to set OPP regulator\n");
  

•   err = pm_genpd_init(genpd, NULL, false);
  

•   if (err) {
  

•           dev_err(&pdev->dev, "failed to init genpd: %d\n", err);
  

•           return err;
  

•   }
  

•   /*
  

•    * We have a "PMC -> Core" hierarchy of the power domains where
  

•    * PMC needs to resume and change performance (voltage) of the
  

•    * Core domain from the PMC GENPD on/off callbacks, hence we need
  

•    * to annotate the lock in order to remove confusion from the
  

•    * lockdep checker when a nested access happens.
  

•    */
  

•   lockdep_set_class(&genpd->mlock, &tegra_core_domain_lock_class);
  

•   err = of_genpd_add_provider_simple(pdev->dev.of_node, genpd);
  

•   if (err) {
  

•           dev_err(&pdev->dev, "failed to add genpd: %d\n", err);
  

•           goto remove_genpd;
  

•   }
  

•   return 0;
  

+remove_genpd:

•   pm_genpd_remove(genpd);
  

•   return err;
  

+}

+bool tegra_soc_core_domain_state_synced(void) +{

•   return tegra_core_domain_state_synced;
  

+}

+static void tegra_core_domain_sync_state(struct device *dev) +{

•   tegra_core_domain_state_synced = true;
  

•   dev_pm_opp_sync_regulators(dev);
  

+}

+static const struct of_device_id tegra_core_domain_match[] = {

•   { .compatible = "nvidia,tegra20-core-domain", },
  

•   { .compatible = "nvidia,tegra30-core-domain", },
  

•   { }
  

+};

+static struct platform_driver tegra_core_domain_driver = {

•   .driver = {
  

•           .name = "tegra-core-power",
  

•           .of_match_table = tegra_core_domain_match,
  

•           .suppress_bind_attrs = true,
  

•           .sync_state = tegra_core_domain_sync_state,
  

•   },
  

•   .probe = tegra_core_domain_probe,
  

+}; +builtin_platform_driver(tegra_core_domain_driver); diff --git a/include/soc/tegra/common.h b/include/soc/tegra/common.h index 57b56793a9e5..6c2ccbbbf073 100644 --- a/include/soc/tegra/common.h +++ b/include/soc/tegra/common.h @@ -27,6 +27,7 @@ struct tegra_core_opp_params {

#ifdef CONFIG_ARCH_TEGRA bool soc_is_tegra(void); +bool tegra_soc_core_domain_state_synced(void); int devm_tegra_core_dev_init_opp_table(struct device *dev, struct tegra_core_opp_params *cfg); #else @@ -35,6 +36,11 @@ static inline bool soc_is_tegra(void) return false; }

+static inline bool tegra_soc_core_domain_state_synced(void) +{

•   return false;
  

+}

static inline int devm_tegra_core_dev_init_opp_table(struct device *dev, struct tegra_core_opp_params *cfg) -- 2.29.2