On 04-07-22, 16:17, Dmitry Osipenko wrote:
> Actually the freq was 0 and it was 1 on the next loop like you suggested.
>
> Previously, the _read_opp_key() was always reading the opp-hz. Now it
> skips reading the rates in _read_rate() because opp_table->clk_count=0
> for the tegra30-devfreq driver the uses devm_pm_opp_of_add_table_noclk().
This is exactly what I wrote in an earlier email :)
Anyway, I have pushed two patches on top of my opp/linux-next branch
and they should fix it in a good way now I suppose. Can you please
give that a try.
This is how the diff looks like:
PM / devfreq: tegra30: Register config_clks helper
There is a corner case with Tegra30, where we want to skip clk
configuration via dev_pm_opp_set_opp(), but still want the OPP core to
read the "opp-hz" property so we can find the right OPP via freq finding
helpers.
The OPP core provides support for the platforms to provide config_clks
helpers now, lets use them instead of devm_pm_opp_of_add_table_noclk()
to achieve the same result, as the OPP core won't parse the DT's
"opp-hz" property if the clock isn't provided.
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 65ecf17a36f4..0e0a4058f45c 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -821,6 +821,15 @@ static int devm_tegra_devfreq_init_hw(struct device *dev,
return err;
}
+static int tegra_devfreq_config_clks_nop(struct device *dev,
+ struct opp_table *opp_table,
+ struct dev_pm_opp *opp, void *data,
+ bool scaling_down)
+{
+ /* We want to skip clk configuration via dev_pm_opp_set_opp() */
+ return 0;
+}
+
static int tegra_devfreq_probe(struct platform_device *pdev)
{
u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
@@ -830,6 +839,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
unsigned int i;
long rate;
int err;
+ const char *clk_names[] = { "actmon", NULL };
+ struct dev_pm_opp_config config = {
+ .supported_hw = &hw_version,
+ .supported_hw_count = 1,
+ .clk_names = clk_names,
+ .config_clks = tegra_devfreq_config_clks_nop,
+ };
tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
if (!tegra)
@@ -874,13 +890,13 @@ 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;
}
- err = devm_pm_opp_of_add_table_noclk(&pdev->dev, 0);
+ err = devm_pm_opp_of_add_table_indexed(&pdev->dev, 0);
if (err) {
dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err);
return err;
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 94c19e9b8cbf..03283ada3341 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2142,7 +2142,7 @@ static int _opp_set_clknames(struct opp_table *opp_table, struct device *dev,
count = 1;
/* Fail early for invalid configurations */
- if (!count || (config_clks && count == 1) || (!config_clks && count > 1))
+ if (!count || (!config_clks && count > 1))
return -EINVAL;
/* Another CPU that shares the OPP table has set the clkname ? */
@@ -2168,10 +2168,12 @@ static int _opp_set_clknames(struct opp_table *opp_table, struct device *dev,
}
opp_table->clk_count = count;
+ opp_table->config_clks = config_clks;
/* Set generic single clk set here */
if (count == 1) {
- opp_table->config_clks = _opp_config_clk_single;
+ if (!opp_table->config_clks)
+ opp_table->config_clks = _opp_config_clk_single;
/*
* We could have just dropped the "clk" field and used "clks"
@@ -2186,8 +2188,6 @@ static int _opp_set_clknames(struct opp_table *opp_table, struct device *dev,
* too.
*/
opp_table->clk = opp_table->clks[0];
- } else {
- opp_table->config_clks = config_clks;
}
return 0;
