[PATCH 00/31] OPP: Add new configuration interface: dev_pm_opp_set_config()

[PATCH 00/31] OPP: Add new configuration interface: dev_pm_opp_set_config()

[Viresh Kumar]

Hello,

We have too many configuration specific APIs currently, six of them already,
like dev_pm_opp_set_regulators(). This makes it complex/messy for both the OPP
core and its users to manage. There is also code redundancy in these API, in the
way they add/manage the OPP table specific stuff.

This patch series is an attempt to simplify these interfaces by adding a single
interface, dev_pm_opp_set_config(), which replaces all the existing ones. This
also migrates the users to the new API.

The first two patches help get the API in place, followed by patches to migrate
the end users. Once all the users are migrated, the last few patches remove the
now unused interfaces.

I have lightly tested this on Hikey960 for now and also getting help from
various build/boot bots, gitlab and lkp, to get these tested. It would be
helpful if someone with access to the affected platforms can give it a try.

This is pushed here:

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/config

The entire patchset shall get merged via the OPP tree in 5.20-rc1, please do not
merge individual patches.

Thanks.

--
Viresh

Viresh Kumar (31):
  OPP: Track if clock name is configured by platform
  OPP: Add dev_pm_opp_set_config() and friends
  cpufreq: dt: Migrate to dev_pm_opp_set_config()
  cpufreq: imx: Migrate to dev_pm_opp_set_config()
  cpufreq: qcom-nvmem: Migrate to dev_pm_opp_set_config()
  cpufreq: sti: Migrate to dev_pm_opp_set_config()
  cpufreq: sun50i: Migrate to dev_pm_opp_set_config()
  cpufreq: tegra20: Migrate to dev_pm_opp_set_config()
  cpufreq: ti: Migrate to dev_pm_opp_set_config()
  devfreq: exynos: Migrate to dev_pm_opp_set_config()
  devfreq: sun8i: Migrate to dev_pm_opp_set_config()
  devfreq: tegra30: Migrate to dev_pm_opp_set_config()
  drm/lima: Migrate to dev_pm_opp_set_config()
  drm/msm: Migrate to dev_pm_opp_set_config()
  drm/panfrost: Migrate to dev_pm_opp_set_config()
  drm/tegra: Migrate to dev_pm_opp_set_config()
  media: venus: Migrate to dev_pm_opp_set_config()
  media: tegra: Migrate to dev_pm_opp_set_config()
  mmc: sdhci-msm: Migrate to dev_pm_opp_set_config()
  OPP: ti: Migrate to dev_pm_opp_set_config()
  soc/tegra: Remove the call to devm_pm_opp_set_clkname()
  soc/tegra: Migrate to dev_pm_opp_set_config()
  spi: qcom: Migrate to dev_pm_opp_set_config()
  serial: qcom: Migrate to dev_pm_opp_set_config()
  OPP: Remove dev_pm_opp_set_regulators() and friends
  OPP: Remove dev_pm_opp_set_supported_hw() and friends
  OPP: Remove dev_pm_opp_set_clkname() and friends
  OPP: Remove dev_pm_opp_register_set_opp_helper() and friends
  OPP: Remove dev_pm_opp_attach_genpd() and friends
  OPP: Remove dev_pm_opp_set_prop_name() and friends
  OPP: Rearrange dev_pm_opp_set_config() and friends

 drivers/cpufreq/cpufreq-dt.c                  |  14 +-
 drivers/cpufreq/imx-cpufreq-dt.c              |  12 +-
 drivers/cpufreq/qcom-cpufreq-nvmem.c          | 107 +---
 drivers/cpufreq/sti-cpufreq.c                 |  22 +-
 drivers/cpufreq/sun50i-cpufreq-nvmem.c        |  11 +-
 drivers/cpufreq/tegra20-cpufreq.c             |  12 +-
 drivers/cpufreq/ti-cpufreq.c                  |  38 +-
 drivers/devfreq/exynos-bus.c                  |  14 +-
 drivers/devfreq/sun8i-a33-mbus.c              |   7 +-
 drivers/devfreq/tegra30-devfreq.c             |   8 +-
 drivers/gpu/drm/lima/lima_devfreq.c           |  11 +-
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c         |   8 +-
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c         |  10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |   5 +-
 drivers/gpu/drm/msm/dp/dp_ctrl.c              |   5 +-
 drivers/gpu/drm/msm/dsi/dsi_host.c            |   5 +-
 drivers/gpu/drm/panfrost/panfrost_devfreq.c   |   9 +-
 drivers/gpu/drm/tegra/gr3d.c                  |   6 +-
 .../media/platform/qcom/venus/pm_helpers.c    |  16 +-
 drivers/memory/tegra/tegra124-emc.c           |  14 +-
 drivers/mmc/host/sdhci-msm.c                  |   5 +-
 drivers/opp/core.c                            | 540 +++++++-----------
 drivers/opp/opp.h                             |   2 +
 drivers/opp/ti-opp-supply.c                   |   6 +-
 drivers/soc/tegra/common.c                    |  14 +-
 drivers/soc/tegra/pmc.c                       |   8 +-
 drivers/spi/spi-geni-qcom.c                   |   5 +-
 drivers/spi/spi-qcom-qspi.c                   |   5 +-
 drivers/tty/serial/qcom_geni_serial.c         |   5 +-
 include/linux/pm_opp.h                        | 118 ++--
 30 files changed, 444 insertions(+), 598 deletions(-)

-- 
2.31.1.272.g89b43f80a514

[PATCH 08/31] cpufreq: tegra20: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

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/cpufreq/tegra20-cpufreq.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
index e8db3d75be25..2c73623e3abb 100644
--- a/drivers/cpufreq/tegra20-cpufreq.c
+++ b/drivers/cpufreq/tegra20-cpufreq.c
@@ -34,7 +34,7 @@ static bool cpu0_node_has_opp_v2_prop(void)
 
 static void tegra20_cpufreq_put_supported_hw(void *opp_table)
 {
-	dev_pm_opp_put_supported_hw(opp_table);
+	dev_pm_opp_clear_config(opp_table);
 }
 
 static void tegra20_cpufreq_dt_unregister(void *cpufreq_dt)
@@ -49,6 +49,10 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
 	struct device *cpu_dev;
 	u32 versions[2];
 	int err;
+	struct dev_pm_opp_config config = {
+		.supported_hw = versions,
+		.supported_hw_count = ARRAY_SIZE(versions),
+	};
 
 	if (!cpu0_node_has_opp_v2_prop()) {
 		dev_err(&pdev->dev, "operating points not found\n");
@@ -71,10 +75,10 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
 	if (WARN_ON(!cpu_dev))
 		return -ENODEV;
 
-	opp_table = dev_pm_opp_set_supported_hw(cpu_dev, versions, 2);
-	err = PTR_ERR_OR_ZERO(opp_table);
+	opp_table = dev_pm_opp_set_config(cpu_dev, &config);
+	err = PTR_ERR(opp_table);
 	if (err) {
-		dev_err(&pdev->dev, "failed to set supported hw: %d\n", err);
+		dev_err(&pdev->dev, "failed to set OPP config: %d\n", err);
 		return err;
 	}
 
-- 
2.31.1.272.g89b43f80a514

[PATCH 12/31] devfreq: tegra30: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

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/devfreq/tegra30-devfreq.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 65ecf17a36f4..30382bdfc655 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -830,6 +830,10 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	unsigned int i;
 	long rate;
 	int err;
+	struct dev_pm_opp_config config = {
+		.supported_hw = &hw_version,
+		.supported_hw_count = 1,
+	};
 
 	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
 	if (!tegra)
@@ -874,9 +878,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	err = devm_pm_opp_set_supported_hw(&pdev->dev, &hw_version, 1);
+	err = devm_pm_opp_set_config(&pdev->dev, &config);
 	if (err) {
-		dev_err(&pdev->dev, "Failed to set supported HW: %d\n", err);
+		dev_err(&pdev->dev, "Failed to set OPP config: %d\n", err);
 		return err;
 	}
 
-- 
2.31.1.272.g89b43f80a514

[PATCH 16/31] drm/tegra: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

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/gpu/drm/tegra/gr3d.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/gr3d.c b/drivers/gpu/drm/tegra/gr3d.c
index a1fd3113ea96..05c45c104e13 100644
--- a/drivers/gpu/drm/tegra/gr3d.c
+++ b/drivers/gpu/drm/tegra/gr3d.c
@@ -389,6 +389,10 @@ static int gr3d_init_power(struct device *dev, struct gr3d *gr3d)
 	struct device_link *link;
 	unsigned int i;
 	int err;
+	struct dev_pm_opp_config config = {
+		.genpd_names = opp_genpd_names,
+		.virt_devs = &opp_virt_devs,
+	};
 
 	err = of_count_phandle_with_args(dev->of_node, "power-domains",
 					 "#power-domain-cells");
@@ -421,7 +425,7 @@ static int gr3d_init_power(struct device *dev, struct gr3d *gr3d)
 	if (dev->pm_domain)
 		return 0;
 
-	err = devm_pm_opp_attach_genpd(dev, opp_genpd_names, &opp_virt_devs);
+	err = devm_pm_opp_set_config(dev, &config);
 	if (err)
 		return err;
 
-- 
2.31.1.272.g89b43f80a514

[PATCH 18/31] media: tegra: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

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/memory/tegra/tegra124-emc.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
index 908f8d5392b2..d1e8f9ffef63 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -1395,13 +1395,17 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
 static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 {
 	u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
-	struct opp_table *hw_opp_table;
+	struct opp_table *opp_table;
 	int err;
+	struct dev_pm_opp_config config = {
+		.supported_hw = &hw_version,
+		.supported_hw_count = 1,
+	};
 
-	hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
-	err = PTR_ERR_OR_ZERO(hw_opp_table);
+	opp_table = dev_pm_opp_set_config(emc->dev, &config);
+	err = PTR_ERR_OR_ZERO(opp_table);
 	if (err) {
-		dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
+		dev_err(emc->dev, "failed to set OPP config: %d\n", err);
 		return err;
 	}
 
@@ -1430,7 +1434,7 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 remove_table:
 	dev_pm_opp_of_remove_table(emc->dev);
 put_hw_table:
-	dev_pm_opp_put_supported_hw(hw_opp_table);
+	dev_pm_opp_clear_config(opp_table);
 
 	return err;
 }
-- 
2.31.1.272.g89b43f80a514

[PATCH 21/31] soc/tegra: Remove the call to devm_pm_opp_set_clkname()

[Viresh Kumar]

The OPP core already performs devm_pm_opp_set_clkname() with name as
NULL, the callers shouldn't be doing the same unless they have a
different clock name to add here.

Drop the call.

Cc: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/soc/tegra/common.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
index 32c346b72635..49a5360f4507 100644
--- a/drivers/soc/tegra/common.c
+++ b/drivers/soc/tegra/common.c
@@ -108,12 +108,6 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
 	u32 hw_version;
 	int err;
 
-	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"))
-- 
2.31.1.272.g89b43f80a514

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

[Viresh Kumar]

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;
 	}
 
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index fdf508e03400..01ec76dd433d 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1366,6 +1366,10 @@ static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
 	struct generic_pm_domain *genpd;
 	const char *rname = "core";
 	int err;
+	struct dev_pm_opp_config config = {
+		.regulator_names = &rname,
+		.regulator_count = 1,
+	};
 
 	genpd = devm_kzalloc(pmc->dev, sizeof(*genpd), GFP_KERNEL);
 	if (!genpd)
@@ -1375,10 +1379,10 @@ static int tegra_pmc_core_pd_add(struct tegra_pmc *pmc, struct device_node *np)
 	genpd->set_performance_state = tegra_pmc_core_pd_set_performance_state;
 	genpd->opp_to_performance_state = tegra_pmc_core_pd_opp_to_performance_state;
 
-	err = devm_pm_opp_set_regulators(pmc->dev, &rname, 1);
+	err = devm_pm_opp_set_config(pmc->dev, &config);
 	if (err)
 		return dev_err_probe(pmc->dev, err,
-				     "failed to set core OPP regulator\n");
+				     "failed to set OPP config\n");
 
 	err = pm_genpd_init(genpd, NULL, false);
 	if (err) {
-- 
2.31.1.272.g89b43f80a514

Re: [PATCH 18/31] media: tegra: Migrate to dev_pm_opp_set_config()

[Krzysztof Kozlowski]

On 26/05/2022 13:42, 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/memory/tegra/tegra124-emc.c | 14 +++++++++-----

Wrong subject prefix - it's memory.



Best regards,
Krzysztof

Re: [PATCH 21/31] soc/tegra: Remove the call to devm_pm_opp_set_clkname()

[Dmitry Osipenko]

On 5/26/22 14:42, Viresh Kumar wrote:
> The OPP core already performs devm_pm_opp_set_clkname() with name as
> NULL, the callers shouldn't be doing the same unless they have a
> different clock name to add here.
> 
> Drop the call.
> 
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/soc/tegra/common.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
> index 32c346b72635..49a5360f4507 100644
> --- a/drivers/soc/tegra/common.c
> +++ b/drivers/soc/tegra/common.c
> @@ -108,12 +108,6 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
>  	u32 hw_version;
>  	int err;
>  
> -	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"))

I can't see where OPP core performs devm_pm_opp_set_clkname().

-- 
Best regards,
Dmitry

Re: [PATCH 18/31] media: tegra: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

On 26-05-22, 14:57, Krzysztof Kozlowski wrote:
> On 26/05/2022 13:42, 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/memory/tegra/tegra124-emc.c | 14 +++++++++-----
> 
> Wrong subject prefix - it's memory.

Oops, fixed it, thanks.

-- 
viresh

Re: [PATCH 21/31] soc/tegra: Remove the call to devm_pm_opp_set_clkname()

[Viresh Kumar]

On 26-05-22, 20:57, Dmitry Osipenko wrote:
> On 5/26/22 14:42, Viresh Kumar wrote:
> > The OPP core already performs devm_pm_opp_set_clkname() with name as
> > NULL, the callers shouldn't be doing the same unless they have a
> > different clock name to add here.

This is confusing. Updated this as:

    The OPP core already performs clk_get(dev, NULL) by default for everyone
    and the callers shouldn't try to set clkname unless they have an actual
    clock name to add here.


> > 
> > Drop the call.
> > 
> > Cc: Dmitry Osipenko <digetx@gmail.com>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> >  drivers/soc/tegra/common.c | 6 ------
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
> > index 32c346b72635..49a5360f4507 100644
> > --- a/drivers/soc/tegra/common.c
> > +++ b/drivers/soc/tegra/common.c
> > @@ -108,12 +108,6 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
> >  	u32 hw_version;
> >  	int err;
> >  
> > -	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"))
> 
> I can't see where OPP core performs devm_pm_opp_set_clkname().

Sorry about that, it is clk_get() it performs from
_update_opp_table_clk().

I don't think you need to call devm_pm_opp_set_clkname() here, but
lets see if this breaks anything for you.

-- 
viresh

Re: [PATCH 08/31] cpufreq: tegra20: Migrate to dev_pm_opp_set_config()

[Dmitry Osipenko]

On 5/26/22 14:42, 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/cpufreq/tegra20-cpufreq.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
> index e8db3d75be25..2c73623e3abb 100644
> --- a/drivers/cpufreq/tegra20-cpufreq.c
> +++ b/drivers/cpufreq/tegra20-cpufreq.c
> @@ -34,7 +34,7 @@ static bool cpu0_node_has_opp_v2_prop(void)
>  
>  static void tegra20_cpufreq_put_supported_hw(void *opp_table)
>  {
> -	dev_pm_opp_put_supported_hw(opp_table);
> +	dev_pm_opp_clear_config(opp_table);
>  }
>  
>  static void tegra20_cpufreq_dt_unregister(void *cpufreq_dt)
> @@ -49,6 +49,10 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
>  	struct device *cpu_dev;
>  	u32 versions[2];
>  	int err;
> +	struct dev_pm_opp_config config = {
> +		.supported_hw = versions,
> +		.supported_hw_count = ARRAY_SIZE(versions),
> +	};
>  
>  	if (!cpu0_node_has_opp_v2_prop()) {
>  		dev_err(&pdev->dev, "operating points not found\n");
> @@ -71,10 +75,10 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
>  	if (WARN_ON(!cpu_dev))
>  		return -ENODEV;
>  
> -	opp_table = dev_pm_opp_set_supported_hw(cpu_dev, versions, 2);
> -	err = PTR_ERR_OR_ZERO(opp_table);
> +	opp_table = dev_pm_opp_set_config(cpu_dev, &config);
> +	err = PTR_ERR(opp_table);

Please keep the PTR_ERR_OR_ZERO.

tegra20-cpufreq tegra20-cpufreq: failed to set OPP config: -1042688000

-- 
Best regards,
Dmitry

Re: [PATCH 08/31] cpufreq: tegra20: Migrate to dev_pm_opp_set_config()

[Dmitry Osipenko]

On 5/29/22 19:19, Dmitry Osipenko wrote:
> On 5/26/22 14:42, 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/cpufreq/tegra20-cpufreq.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
>> index e8db3d75be25..2c73623e3abb 100644
>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>> @@ -34,7 +34,7 @@ static bool cpu0_node_has_opp_v2_prop(void)
>>  
>>  static void tegra20_cpufreq_put_supported_hw(void *opp_table)
>>  {
>> -	dev_pm_opp_put_supported_hw(opp_table);
>> +	dev_pm_opp_clear_config(opp_table);
>>  }
>>  
>>  static void tegra20_cpufreq_dt_unregister(void *cpufreq_dt)
>> @@ -49,6 +49,10 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
>>  	struct device *cpu_dev;
>>  	u32 versions[2];
>>  	int err;
>> +	struct dev_pm_opp_config config = {
>> +		.supported_hw = versions,
>> +		.supported_hw_count = ARRAY_SIZE(versions),
>> +	};
>>  
>>  	if (!cpu0_node_has_opp_v2_prop()) {
>>  		dev_err(&pdev->dev, "operating points not found\n");
>> @@ -71,10 +75,10 @@ static int tegra20_cpufreq_probe(struct platform_device *pdev)
>>  	if (WARN_ON(!cpu_dev))
>>  		return -ENODEV;
>>  
>> -	opp_table = dev_pm_opp_set_supported_hw(cpu_dev, versions, 2);
>> -	err = PTR_ERR_OR_ZERO(opp_table);
>> +	opp_table = dev_pm_opp_set_config(cpu_dev, &config);
>> +	err = PTR_ERR(opp_table);
> 
> Please keep the PTR_ERR_OR_ZERO.
> 
> tegra20-cpufreq tegra20-cpufreq: failed to set OPP config: -1042688000
> 

With that fixed, now there is another error:

[    1.761945] cpu cpu0: _of_add_opp_table_v2: no supported OPPs
[    1.761960] cpu cpu0: OPP table can't be empty

I see this on Tegra30, but not on Tegra20. Apparently OPP table
refcounting is broken on Tegra30 by this patchset. To make it clear,
there are no error without these OPP patches applied. I may take a
closer look if will be needed, just ping me.

-- 
Best regards,
Dmitry

Re: [PATCH 08/31] cpufreq: tegra20: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

On 29-05-22, 19:59, Dmitry Osipenko wrote:
> > Please keep the PTR_ERR_OR_ZERO.

Ahh, sorry about that. Fixed.

> > tegra20-cpufreq tegra20-cpufreq: failed to set OPP config: -1042688000
> 
> With that fixed, now there is another error:
> 
> [    1.761945] cpu cpu0: _of_add_opp_table_v2: no supported OPPs
> [    1.761960] cpu cpu0: OPP table can't be empty

So we failed to find any OPPs which work with the hardware version of
updated with dev_pm_opp_set_config(). I tried to follow the path and
see if there is something wrong here. Failed to find that :(

> I see this on Tegra30, but not on Tegra20. Apparently OPP table
> refcounting is broken on Tegra30 by this patchset. To make it clear,
> there are no error without these OPP patches applied. I may take a
> closer look if will be needed, just ping me.

Yes, it would be very helpful as I don't have the necessary hardware.

-- 
viresh

Re: [PATCH 08/31] cpufreq: tegra20: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

On 30-05-22, 13:22, Viresh Kumar wrote:
> On 29-05-22, 19:59, Dmitry Osipenko wrote:
> > > Please keep the PTR_ERR_OR_ZERO.
> 
> Ahh, sorry about that. Fixed.
> 
> > > tegra20-cpufreq tegra20-cpufreq: failed to set OPP config: -1042688000
> > 
> > With that fixed, now there is another error:
> > 
> > [    1.761945] cpu cpu0: _of_add_opp_table_v2: no supported OPPs
> > [    1.761960] cpu cpu0: OPP table can't be empty
> 
> So we failed to find any OPPs which work with the hardware version of
> updated with dev_pm_opp_set_config(). I tried to follow the path and
> see if there is something wrong here. Failed to find that :(
> 
> > I see this on Tegra30, but not on Tegra20. Apparently OPP table
> > refcounting is broken on Tegra30 by this patchset. To make it clear,
> > there are no error without these OPP patches applied. I may take a
> > closer look if will be needed, just ping me.
> 
> Yes, it would be very helpful as I don't have the necessary hardware.

Hey, any updates on this ? I am looking to resend the series soon, would be nice
to fix this before that.

-- 
viresh

Re: [PATCH 08/31] cpufreq: tegra20: Migrate to dev_pm_opp_set_config()

[Dmitry Osipenko]

On 6/7/22 11:43, Viresh Kumar wrote:
> On 30-05-22, 13:22, Viresh Kumar wrote:
>> On 29-05-22, 19:59, Dmitry Osipenko wrote:
>>>> Please keep the PTR_ERR_OR_ZERO.
>>
>> Ahh, sorry about that. Fixed.
>>
>>>> tegra20-cpufreq tegra20-cpufreq: failed to set OPP config: -1042688000
>>>
>>> With that fixed, now there is another error:
>>>
>>> [    1.761945] cpu cpu0: _of_add_opp_table_v2: no supported OPPs
>>> [    1.761960] cpu cpu0: OPP table can't be empty
>>
>> So we failed to find any OPPs which work with the hardware version of
>> updated with dev_pm_opp_set_config(). I tried to follow the path and
>> see if there is something wrong here. Failed to find that :(
>>
>>> I see this on Tegra30, but not on Tegra20. Apparently OPP table
>>> refcounting is broken on Tegra30 by this patchset. To make it clear,
>>> there are no error without these OPP patches applied. I may take a
>>> closer look if will be needed, just ping me.
>>
>> Yes, it would be very helpful as I don't have the necessary hardware.
> 
> Hey, any updates on this ? I am looking to resend the series soon, would be nice
> to fix this before that.
> 

I'll take a look over this weekend. Sorry for the delay.

-- 
Best regards,
Dmitry

Re: [PATCH 21/31] soc/tegra: Remove the call to devm_pm_opp_set_clkname()

[Jon Hunter]

Hi Viresh,

On 26/05/2022 12:42, Viresh Kumar wrote:
> The OPP core already performs devm_pm_opp_set_clkname() with name as
> NULL, the callers shouldn't be doing the same unless they have a
> different clock name to add here.
> 
> Drop the call.
> 
> Cc: Dmitry Osipenko <digetx@gmail.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/soc/tegra/common.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
> index 32c346b72635..49a5360f4507 100644
> --- a/drivers/soc/tegra/common.c
> +++ b/drivers/soc/tegra/common.c
> @@ -108,12 +108,6 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
>   	u32 hw_version;
>   	int err;
>   
> -	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"))


This appears to be breaking a few Tegra drivers. For example, on Tegra210
Jetson TX1 I am seeing the following and the eMMC is no longer working ...

[    0.526729] sdhci-tegra 700b0600.mmc: dev_pm_opp_set_rate: device's opp table doesn't exist
[    0.526733] sdhci-tegra 700b0600.mmc: failed to set clk rate to 400000Hz: -19
[    0.528830] sdhci-tegra 700b0600.mmc: dev_pm_opp_set_rate: device's opp table doesn't exist
[    0.528833] sdhci-tegra 700b0600.mmc: failed to set clk rate to 400000Hz: -19

I have seen another instance of this on Jetson Xavier NX ...

[   12.301336] tegra-pwm 32d0000.pwm: dev_pm_opp_set_rate: device's opp table doesn't exist
[   12.301350] tegra-pwm 32d0000.pwm: Failed to set max frequency: -19

Bisect is point to this commit and so something is not working as
expected.

Cheers
Jon

-- 
nvpublic

Re: [PATCH 21/31] soc/tegra: Remove the call to devm_pm_opp_set_clkname()

[Viresh Kumar]

On 23-06-22, 22:15, Jon Hunter wrote:
> On 26/05/2022 12:42, Viresh Kumar wrote:
> > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
> > index 32c346b72635..49a5360f4507 100644
> > --- a/drivers/soc/tegra/common.c
> > +++ b/drivers/soc/tegra/common.c
> > @@ -108,12 +108,6 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
> >   	u32 hw_version;
> >   	int err;
> > -	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"))
> 
> 
> This appears to be breaking a few Tegra drivers. For example, on Tegra210
> Jetson TX1 I am seeing the following and the eMMC is no longer working ...
> 
> [    0.526729] sdhci-tegra 700b0600.mmc: dev_pm_opp_set_rate: device's opp table doesn't exist
> [    0.526733] sdhci-tegra 700b0600.mmc: failed to set clk rate to 400000Hz: -19
> [    0.528830] sdhci-tegra 700b0600.mmc: dev_pm_opp_set_rate: device's opp table doesn't exist
> [    0.528833] sdhci-tegra 700b0600.mmc: failed to set clk rate to 400000Hz: -19
> 
> I have seen another instance of this on Jetson Xavier NX ...
> 
> [   12.301336] tegra-pwm 32d0000.pwm: dev_pm_opp_set_rate: device's opp table doesn't exist
> [   12.301350] tegra-pwm 32d0000.pwm: Failed to set max frequency: -19
> 
> Bisect is point to this commit and so something is not working as
> expected.

Thanks again Jon.

This is what happens when the special code doesn't have a comment
attached with it. Neither the reviewer, nor the author remember why
the special piece was required :)

I had to go through the whole sequence, along with DT to understand
what might have broken this stuff :)

I will drop this patch and add this comment in its place:

diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
index 32c346b72635..9f3fdeb1a11c 100644
--- a/drivers/soc/tegra/common.c
+++ b/drivers/soc/tegra/common.c
@@ -108,6 +108,13 @@ 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);


Though there will still be a problem here with my changes, we don't
accept NULL clkname anymore for the set-clkname API. And tegra does
this to pick the first clock available in DT (at index 0) I think.
Other drivers (mostly qcom) who need such dummy OPP table, provide a
real clock name instead. Will it be possible to pass that here somehow
?

-- 
viresh

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

[Viresh Kumar]

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/

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

[Viresh Kumar]

On 24-06-22, 06:18, Viresh Kumar wrote:
> +       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,
> +       };

Slight modification here, sorry about that. We just need to set the
name as NULL and not the array itself.

diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
index cd53e46c4058..6a099d764cce 100644
--- a/drivers/soc/tegra/common.c
+++ b/drivers/soc/tegra/common.c
@@ -116,7 +116,7 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
                 * of dev_pm_opp_set_rate() and doesn't provide any other
                 * functionality.
                 */
-               .clk_names = NULL,
+               .clk_names = (const char *[]){ NULL },
                .clk_count = 1,
        };

-- 
viresh

Re: [PATCH 21/31] soc/tegra: Remove the call to devm_pm_opp_set_clkname()

[Viresh Kumar]

On 24-06-22, 05:58, Viresh Kumar wrote:
> Though there will still be a problem here with my changes, we don't
> accept NULL clkname anymore for the set-clkname API. And tegra does
> this to pick the first clock available in DT (at index 0) I think.
> Other drivers (mostly qcom) who need such dummy OPP table, provide a
> real clock name instead. Will it be possible to pass that here somehow
> ?

Jon,

Okay, I was able to handle it without making any further updates to
the OPP core. Nothing else required from your side on this.

I have pushed the updates (to this patch and 22/31) to opp/linux-next
branch. You can try it now and it should just work. I have only build
tested it though.

Thanks.

-- 
viresh

Re: [PATCH 08/31] cpufreq: tegra20: Migrate to dev_pm_opp_set_config()

[Viresh Kumar]

On 29-05-22, 19:59, Dmitry Osipenko wrote:
> With that fixed, now there is another error:
> 
> [    1.761945] cpu cpu0: _of_add_opp_table_v2: no supported OPPs
> [    1.761960] cpu cpu0: OPP table can't be empty
> 
> I see this on Tegra30, but not on Tegra20. Apparently OPP table
> refcounting is broken on Tegra30 by this patchset. To make it clear,
> there are no error without these OPP patches applied. I may take a
> closer look if will be needed, just ping me.

Hi Jon,

Dmitry reported this on Tegra30 earlier, do you also see such a
failure ? Would be helpful to get this fixed as well, if it still
exists.

-- 
viresh

Re: [PATCH 08/31] cpufreq: tegra20: Migrate to dev_pm_opp_set_config()

[Jon Hunter]

On 24/06/2022 06:38, Viresh Kumar wrote:
> On 29-05-22, 19:59, Dmitry Osipenko wrote:
>> With that fixed, now there is another error:
>>
>> [    1.761945] cpu cpu0: _of_add_opp_table_v2: no supported OPPs
>> [    1.761960] cpu cpu0: OPP table can't be empty
>>
>> I see this on Tegra30, but not on Tegra20. Apparently OPP table
>> refcounting is broken on Tegra30 by this patchset. To make it clear,
>> there are no error without these OPP patches applied. I may take a
>> closer look if will be needed, just ping me.
> 
> Hi Jon,
> 
> Dmitry reported this on Tegra30 earlier, do you also see such a
> failure ? Would be helpful to get this fixed as well, if it still
> exists.


Yes I am seeing the same issue on Tegra30 ...

[    2.177437] cpu cpu0: _of_add_opp_table_v2: no supported OPPs
[    2.177455] cpu cpu0: OPP table can't be empty

And the cpufreq test we are running is failing.

Thanks
Jon

-- 
nvpublic

Re: [PATCH 21/31] soc/tegra: Remove the call to devm_pm_opp_set_clkname()

[Jon Hunter]

On 24/06/2022 01:59, Viresh Kumar wrote:
> On 24-06-22, 05:58, Viresh Kumar wrote:
>> Though there will still be a problem here with my changes, we don't
>> accept NULL clkname anymore for the set-clkname API. And tegra does
>> this to pick the first clock available in DT (at index 0) I think.
>> Other drivers (mostly qcom) who need such dummy OPP table, provide a
>> real clock name instead. Will it be possible to pass that here somehow
>> ?
> 
> Jon,
> 
> Okay, I was able to handle it without making any further updates to
> the OPP core. Nothing else required from your side on this.
> 
> I have pushed the updates (to this patch and 22/31) to opp/linux-next
> branch. You can try it now and it should just work. I have only build
> tested it though.


Thanks. I have reverted the previous versions of 21 and 22, and pulled 
in the latest and these are working.

Cheers
Jon

-- 
nvpublic

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

[Dmitry Osipenko]

24.06.2022 03:57, Viresh Kumar пишет:
> On 24-06-22, 06:18, Viresh Kumar wrote:
>> +       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,
>> +       };
> 
> Slight modification here, sorry about that. We just need to set the
> name as NULL and not the array itself.
> 
> diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c
> index cd53e46c4058..6a099d764cce 100644
> --- a/drivers/soc/tegra/common.c
> +++ b/drivers/soc/tegra/common.c
> @@ -116,7 +116,7 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,
>                  * of dev_pm_opp_set_rate() and doesn't provide any other
>                  * functionality.
>                  */
> -               .clk_names = NULL,
> +               .clk_names = (const char *[]){ NULL },
>                 .clk_count = 1,
>         };
> 

Looks okay. If you'll solve the cpufreq problem where OPP config is set
by two drivers for the same cpu device and will keep the set_opp()
helper that is needed by the Tegra 3d driver, then it all should work
for Tegra. Looking forward to the next update of the OPP patches, thank you.

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

[Viresh Kumar]

On 27-06-22, 01:14, Dmitry Osipenko wrote:
> Looks okay. If you'll solve the cpufreq problem where OPP config is set
> by two drivers for the same cpu device

This is supported, there is some early freeing of resources on the
removal path though, the reasoning for which I already gave in another
email. Though, I am open to sorting that out as well, but nothing
breaks the code for now AFAICT.

> and will keep the set_opp()
> helper that is needed by the Tegra 3d driver, then it all should work
> for Tegra.

I have responded to that as well on another thread.

> Looking forward to the next update of the OPP patches, thank you.

All that I have is already pushed to linux-next, I don't have any more
changes. Yes I still need to send the updated changes to list.

-- 
viresh

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

[Dmitry Osipenko]

27.06.2022 09:45, Viresh Kumar пишет:
>> Looks okay. If you'll solve the cpufreq problem where OPP config is set
>> by two drivers for the same cpu device
> This is supported, there is some early freeing of resources on the
> removal path though, the reasoning for which I already gave in another
> email. Though, I am open to sorting that out as well, but nothing
> breaks the code for now AFAICT.
> 

In case of Tegra, we use tegra-cpufreq driver that sets supported_hw and
registers cpufreq-dt. If cpufreq-dt driver defers the probe, then the
supported_hw will be lost on the re-probe. I haven't checked yet, but I
suppose that cpufreq-dt driver defers on Tegra30 because of the CPU
regulator and that's why we get the "OPP table is missing" error.

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

[Viresh Kumar]

On 27-06-22, 10:14, Dmitry Osipenko wrote:
> 27.06.2022 09:45, Viresh Kumar пишет:
> >> Looks okay. If you'll solve the cpufreq problem where OPP config is set
> >> by two drivers for the same cpu device
> > This is supported, there is some early freeing of resources on the
> > removal path though, the reasoning for which I already gave in another
> > email. Though, I am open to sorting that out as well, but nothing
> > breaks the code for now AFAICT.
> > 
> 
> In case of Tegra, we use tegra-cpufreq driver that sets supported_hw and
> registers cpufreq-dt. If cpufreq-dt driver defers the probe, then the
> supported_hw will be lost on the re-probe. I haven't checked yet, but I
> suppose that cpufreq-dt driver defers on Tegra30 because of the CPU
> regulator and that's why we get the "OPP table is missing" error.

Aha, I get it now. I see, this is a real problem. Will fix it. Give me
some time to think. Thanks.

-- 
viresh

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

[Viresh Kumar]

On 27-06-22, 12:51, Viresh Kumar wrote:
> On 27-06-22, 10:14, Dmitry Osipenko wrote:
> > 27.06.2022 09:45, Viresh Kumar пишет:
> > >> Looks okay. If you'll solve the cpufreq problem where OPP config is set
> > >> by two drivers for the same cpu device
> > > This is supported, there is some early freeing of resources on the
> > > removal path though, the reasoning for which I already gave in another
> > > email. Though, I am open to sorting that out as well, but nothing
> > > breaks the code for now AFAICT.
> > > 
> > 
> > In case of Tegra, we use tegra-cpufreq driver that sets supported_hw and
> > registers cpufreq-dt. If cpufreq-dt driver defers the probe, then the
> > supported_hw will be lost on the re-probe. I haven't checked yet, but I
> > suppose that cpufreq-dt driver defers on Tegra30 because of the CPU
> > regulator and that's why we get the "OPP table is missing" error.
> 
> Aha, I get it now. I see, this is a real problem. Will fix it. Give me
> some time to think. Thanks.

Okay, I fixed this in opp/linux-next, can you or Jon please give it a
go on tegra30 to see if the issue is fixed ?

FWIW, I have fixed this with the IDR API and the OPP core will only
free the resources in clear-config, that the corresponding set-config
has configured. I have tested it with the clk API only though.

Once you confirm, I will resend all the patches and hope no issues are
left here.

Thanks for helping out guys. Really appreciate it.

-- 
viresh

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

[Dmitry Osipenko]

On 6/28/22 10:09, Viresh Kumar wrote:
> On 27-06-22, 12:51, Viresh Kumar wrote:
>> On 27-06-22, 10:14, Dmitry Osipenko wrote:
>>> 27.06.2022 09:45, Viresh Kumar пишет:
>>>>> Looks okay. If you'll solve the cpufreq problem where OPP config is set
>>>>> by two drivers for the same cpu device
>>>> This is supported, there is some early freeing of resources on the
>>>> removal path though, the reasoning for which I already gave in another
>>>> email. Though, I am open to sorting that out as well, but nothing
>>>> breaks the code for now AFAICT.
>>>>
>>>
>>> In case of Tegra, we use tegra-cpufreq driver that sets supported_hw and
>>> registers cpufreq-dt. If cpufreq-dt driver defers the probe, then the
>>> supported_hw will be lost on the re-probe. I haven't checked yet, but I
>>> suppose that cpufreq-dt driver defers on Tegra30 because of the CPU
>>> regulator and that's why we get the "OPP table is missing" error.
>>
>> Aha, I get it now. I see, this is a real problem. Will fix it. Give me
>> some time to think. Thanks.
> 
> Okay, I fixed this in opp/linux-next, can you or Jon please give it a
> go on tegra30 to see if the issue is fixed ?
> 
> FWIW, I have fixed this with the IDR API and the OPP core will only
> free the resources in clear-config, that the corresponding set-config
> has configured. I have tested it with the clk API only though.
> 
> Once you confirm, I will resend all the patches and hope no issues are
> left here.
> 
> Thanks for helping out guys. Really appreciate it.
> 

The opp/linux-next works fine, thank you.

Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

BTW, the idr_alloc() is obsoleted by xa_alloc().

-- 
Best regards,
Dmitry

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

[Viresh Kumar]

On 28-06-22, 13:08, Dmitry Osipenko wrote:
> The opp/linux-next works fine, thank you.
> 
> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Thanks. I should add this to all the core + tegra -patches in that
branch, right ?

> BTW, the idr_alloc() is obsoleted by xa_alloc().

The earlier interface isn't deprecated, right ? I really don't want to
go change it again :)

-- 
viresh

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

[Dmitry Osipenko]

On 6/28/22 13:11, Viresh Kumar wrote:
> On 28-06-22, 13:08, Dmitry Osipenko wrote:
>> The opp/linux-next works fine, thank you.
>>
>> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> 
> Thanks. I should add this to all the core + tegra -patches in that
> branch, right ?

Yes, please.

>> BTW, the idr_alloc() is obsoleted by xa_alloc().
> 
> The earlier interface isn't deprecated, right ? I really don't want to
> go change it again :)
> 

It has been in a process of deprecation for a couple years now. All IDR
instances are slowly converting to XA. You won't need to take mutex with
xa_alloc().

-- 
Best regards,
Dmitry

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

[Viresh Kumar]

On 28-06-22, 13:16, Dmitry Osipenko wrote:
> It has been in a process of deprecation for a couple years now. All IDR
> instances are slowly converting to XA. You won't need to take mutex with
> xa_alloc().

Okay, migrated to it now and pushed :)

-- 
viresh

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

[Jon Hunter]

On 28/06/2022 11:08, Dmitry Osipenko wrote:
> On 6/28/22 10:09, Viresh Kumar wrote:
>> On 27-06-22, 12:51, Viresh Kumar wrote:
>>> On 27-06-22, 10:14, Dmitry Osipenko wrote:
>>>> 27.06.2022 09:45, Viresh Kumar пишет:
>>>>>> Looks okay. If you'll solve the cpufreq problem where OPP config is set
>>>>>> by two drivers for the same cpu device
>>>>> This is supported, there is some early freeing of resources on the
>>>>> removal path though, the reasoning for which I already gave in another
>>>>> email. Though, I am open to sorting that out as well, but nothing
>>>>> breaks the code for now AFAICT.
>>>>>
>>>>
>>>> In case of Tegra, we use tegra-cpufreq driver that sets supported_hw and
>>>> registers cpufreq-dt. If cpufreq-dt driver defers the probe, then the
>>>> supported_hw will be lost on the re-probe. I haven't checked yet, but I
>>>> suppose that cpufreq-dt driver defers on Tegra30 because of the CPU
>>>> regulator and that's why we get the "OPP table is missing" error.
>>>
>>> Aha, I get it now. I see, this is a real problem. Will fix it. Give me
>>> some time to think. Thanks.
>>
>> Okay, I fixed this in opp/linux-next, can you or Jon please give it a
>> go on tegra30 to see if the issue is fixed ?
>>
>> FWIW, I have fixed this with the IDR API and the OPP core will only
>> free the resources in clear-config, that the corresponding set-config
>> has configured. I have tested it with the clk API only though.
>>
>> Once you confirm, I will resend all the patches and hope no issues are
>> left here.
>>
>> Thanks for helping out guys. Really appreciate it.
>>
> 
> The opp/linux-next works fine, thank you.
> 
> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>


Today's -next is also working fine for me too!

Thanks
Jon

-- 
nvpublic

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

[Viresh Kumar]

On 29-06-22, 18:03, Jon Hunter wrote:
> Today's -next is also working fine for me too!

Thanks.

I hope all the trouble with core update was worth it :)

-- 
viresh