Re: [PATCH 22/31] soc/tegra: Migrate to dev_pm_opp_set_config()

From: Viresh Kumar <viresh.kumar@linaro.org>
To: Dmitry Osipenko <digetx@gmail.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Jonathan Hunter <jonathanh@nvidia.com>
Cc: linux-pm@vger.kernel.org,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Rafael Wysocki <rjw@rjwysocki.net>,
	Stephen Boyd <sboyd@kernel.org>, Nishanth Menon <nm@ti.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 22/31] soc/tegra: Migrate to dev_pm_opp_set_config()
Date: Fri, 24 Jun 2022 06:18:31 +0530	[thread overview]
Message-ID: <20220624004831.po35sowzfo4c47b3@vireshk-i7> (raw)
In-Reply-To: <449b344f037c7ef1970bc84d31e0d4c4cb4d2951.1653564321.git.viresh.kumar@linaro.org>

On 26-05-22, 17:12, Viresh Kumar wrote:
> The OPP core now provides a unified API for setting all configuration
> types, i.e. dev_pm_opp_set_config().
> 
> Lets start using it.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/soc/tegra/common.c | 8 ++++++--
>  drivers/soc/tegra/pmc.c    | 8 ++++++--
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
> index 49a5360f4507..7ba15cb836e8 100644
> --- a/drivers/soc/tegra/common.c
> +++ b/drivers/soc/tegra/common.c
> @@ -107,6 +107,10 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
>  {
>  	u32 hw_version;
>  	int err;
> +	struct dev_pm_opp_config config = {
> +		.supported_hw = &hw_version,
> +		.supported_hw_count = 1,
> +	};
>  
>  	/* Tegra114+ doesn't support OPP yet */
>  	if (!of_machine_is_compatible("nvidia,tegra20") &&
> @@ -118,9 +122,9 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
>  	else
>  		hw_version = BIT(tegra_sku_info.soc_speedo_id);
>  
> -	err = devm_pm_opp_set_supported_hw(dev, &hw_version, 1);
> +	err = devm_pm_opp_set_config(dev, &config);
>  	if (err) {
> -		dev_err(dev, "failed to set OPP supported HW: %d\n", err);
> +		dev_err(dev, "failed to set OPP config: %d\n", err);
>  		return err;
>  	}

Jon/Dmitry,

Because of the update [1] to previous patch 21/31, I am updating this
file as (fresh diff):

diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
index 9f3fdeb1a11c..cd53e46c4058 100644
--- a/drivers/soc/tegra/common.c
+++ b/drivers/soc/tegra/common.c
@@ -107,36 +107,42 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
 {
        u32 hw_version;
        int err;
-
-       /*
-        * For some devices we don't have any OPP table in the DT, and in order
-        * to use the same code path for all the devices, we create a dummy OPP
-        * table for them via this call. The dummy OPP table is only capable of
-        * doing clk_set_rate() on invocation of dev_pm_opp_set_rate() and
-        * doesn't provide any other functionality.
-        */
-       err = devm_pm_opp_set_clkname(dev, NULL);
-       if (err) {
-               dev_err(dev, "failed to set OPP clk: %d\n", err);
-               return err;
-       }
-
-       /* Tegra114+ doesn't support OPP yet */
-       if (!of_machine_is_compatible("nvidia,tegra20") &&
-           !of_machine_is_compatible("nvidia,tegra30"))
-               return -ENODEV;
-
-       if (of_machine_is_compatible("nvidia,tegra20"))
+       struct dev_pm_opp_config config = {
+               /*
+                * For some devices we don't have any OPP table in the DT, and
+                * in order to use the same code path for all the devices, we
+                * create a dummy OPP table for them via this. The dummy OPP
+                * table is only capable of doing clk_set_rate() on invocation
+                * of dev_pm_opp_set_rate() and doesn't provide any other
+                * functionality.
+                */
+               .clk_names = NULL,
+               .clk_count = 1,
+       };
+
+       if (of_machine_is_compatible("nvidia,tegra20")) {
                hw_version = BIT(tegra_sku_info.soc_process_id);
-       else
+               config.supported_hw = &hw_version;
+               config.supported_hw_count = 1;
+       } else if (of_machine_is_compatible("nvidia,tegra30")) {
                hw_version = BIT(tegra_sku_info.soc_speedo_id);
+               config.supported_hw = &hw_version;
+               config.supported_hw_count = 1;
+       }

-       err = devm_pm_opp_set_supported_hw(dev, &hw_version, 1);
+       err = devm_pm_opp_set_config(dev, &config);
        if (err) {
-               dev_err(dev, "failed to set OPP supported HW: %d\n", err);
+               dev_err(dev, "failed to set OPP config: %d\n", err);
                return err;
        }

+       /*
+        * Tegra114+ doesn't support OPP yet, return early for non tegra20/30
+        * case.
+        */
+       if (!config.supported_hw)
+               return -ENODEV;
+
        /*
         * Older device-trees have an empty OPP table, we will get
         * -ENODEV from devm_pm_opp_of_add_table() in this case.

-------------------------8<-------------------------

The idea here is to always set the clk name (to NULL) and skip other
stuff for SoCs other than tegra 20/30.

Just see if you can find something odd with the review of it, I will
resend it properly later once the issues are settled.

-- 
viresh

[1] https://lore.kernel.org/lkml/20220624002805.anv62ufihdrncwus@vireshk-i7/
