OPP: Add support for multiple clocks
Subsystems (5)
cover OPP: Add support for multiple clocks
Hello,
This patchset adds support for device with multiple clocks. None of the clocks is considered primary in this case and all are handled equally.
This is rebased over a lot of other OPP changes and is pushed here:
git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/clk
Krzysztof, can you please test this for your use case. I wasn't able to test he multiple clock support.
--
Viresh
Viresh Kumar (8):
OPP: Use consistent names for OPP table instances
OPP: Remove rate_not_available parameter to _opp_add()
OPP: Reuse _opp_compare_key() in _opp_add_static_v2()
OPP: Make dev_pm_opp_set_opp() independent of frequency
OPP: Allow multiple clocks for a device
OPP: Add key specific assert() method to key finding helpers
OPP: Assert clk_count == 1 for single clk helpers
OPP: Provide a simple implementation to configure multiple clocks
drivers/opp/core.c | 337 +++++++++++++++++++++++++++++++---------- drivers/opp/cpu.c | 12 +- drivers/opp/debugfs.c | 27 +++- drivers/opp/of.c | 91 +++++++---- drivers/opp/opp.h | 22 +-- include/linux/pm_opp.h | 17 ++- 6 files changed, 378 insertions(+), 128 deletions(-)
Signature · 1 line
2.31.1.272.g89b43f80a514
PATCH 1/8 OPP: Use consistent names for OPP table instances
The OPP table is called "opp_table" at most of the places and "table" at few. Make all of them follow the same naming convention, "opp_table".
--- drivers/opp/core.c | 7 ++++--- drivers/opp/cpu.c | 12 ++++++------ drivers/opp/of.c | 12 ++++++------ 3 files changed, 16 insertions(+), 15 deletions(-)
drivers/opp/core.c +4 -3
@@ -1567,15 +1567,16 @@ void dev_pm_opp_remove_all_dynamic(struct device *dev) }EXPORT_SYMBOL_GPL (dev_pm_opp_remove_all_dynamic); -struct dev_pm_opp *_opp_allocate (struct opp_table *table) +struct dev_pm_opp *_opp_allocate (struct opp_table *opp_table) {struct dev_pm_opp *opp;int supply_count, supply_size, icc_size;/* Allocate space for at least one supply */ - supply_count = table->regulator_count >0 ? table->regulator_count :1 ; + supply_count = opp_table->regulator_count >0 ? + opp_table->regulator_count :1 ; supply_size =sizeof (*opp->supplies) * supply_count; - icc_size =sizeof (*opp->bandwidth) * table->path_count; + icc_size =sizeof (*opp->bandwidth) * opp_table->path_count;/* allocate new OPP node and supplies structures */ opp =kzalloc (sizeof (*opp) + supply_size + icc_size,GFP_KERNEL );
drivers/opp/cpu.c +6 -6
@@ -41,7 +41,7 @@* the table if any of the mentioned functions have been invoked in the interim. */ int dev_pm_opp_init_cpufreq_table (struct device *dev, -struct cpufreq_frequency_table **table) +struct cpufreq_frequency_table **opp_table) {struct dev_pm_opp *opp;struct cpufreq_frequency_table *freq_table =NULL ;
@@ -76,7 +76,7 @@ int dev_pm_opp_init_cpufreq_table(struct device *dev, freq_table[i].driver_data = i; freq_table[i].frequency =CPUFREQ_TABLE_END ; - *table = &freq_table[0 ]; + *opp_table = &freq_table[0 ]; out:if (ret)
@@ -94,13 +94,13 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_init_cpufreq_table);* Free up the table allocated by dev_pm_opp_init_cpufreq_table */ void dev_pm_opp_free_cpufreq_table (struct device *dev, -struct cpufreq_frequency_table **table) +struct cpufreq_frequency_table **opp_table) { -if (!table) +if (!opp_table)return ; -kfree (*table); - *table =NULL ; +kfree (*opp_table); + *opp_table =NULL ; }EXPORT_SYMBOL_GPL (dev_pm_opp_free_cpufreq_table);#endif /* CONFIG_CPU_FREQ */
drivers/opp/of.c +6 -6
@@ -767,7 +767,7 @@ void dev_pm_opp_of_remove_table(struct device *dev) }EXPORT_SYMBOL_GPL (dev_pm_opp_of_remove_table); -static int _read_bw (struct dev_pm_opp *new_opp,struct opp_table *table, +static int _read_bw (struct dev_pm_opp *new_opp,struct opp_table *opp_table,struct device_node *np,bool peak) {const char *name = peak ?"opp-peak-kBps" :"opp-avg-kBps" ;
@@ -780,9 +780,9 @@ static int _read_bw(struct dev_pm_opp *new_opp, struct opp_table *table,return -ENODEV ; count = prop->length /sizeof (u32); -if (table->path_count != count) { +if (opp_table->path_count != count) {pr_err ("%s: Mismatch between %s and paths (%d %d)\n" , - __func__, name, count, table->path_count); + __func__, name, count, opp_table->path_count);return -EINVAL ; }
@@ -808,7 +808,7 @@ static int _read_bw(struct dev_pm_opp *new_opp, struct opp_table *table,return ret; } -static int _read_opp_key (struct dev_pm_opp *new_opp,struct opp_table *table, +static int _read_opp_key (struct dev_pm_opp *new_opp,struct opp_table *opp_table,struct device_node *np,bool *rate_not_available) {bool found = false;
@@ -832,10 +832,10 @@ static int _read_opp_key(struct dev_pm_opp *new_opp, struct opp_table *table,* opp-peak-kBps = <path1_value path2_value>; * opp-avg-kBps = <path1_value path2_value>; */ - ret =_read_bw (new_opp, table, np, true); + ret =_read_bw (new_opp, opp_table, np, true);if (!ret) { found = true; - ret =_read_bw (new_opp, table, np, false); + ret =_read_bw (new_opp, opp_table, np, false); }/* The properties were found but we failed to parse them */
Signature · 1 line
2.31.1.272.g89b43f80a514
PATCH 2/8 OPP: Remove rate_not_available parameter to _opp_add()
In reply to Viresh Kumarcommit 32715be4fe95 ("opp: Fix adding OPP entries in a wrong order if rate is unavailable") removed the only user of this field, get rid of rest of it now.
--- drivers/opp/core.c | 4 ++-- drivers/opp/of.c | 10 ++++------ drivers/opp/opp.h | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-)
drivers/opp/core.c +2 -2
@@ -1695,7 +1695,7 @@ void _required_opps_available(struct dev_pm_opp *opp, int count)* should be considered an error by the callers of _opp_add(). */ int _opp_add (struct device *dev,struct dev_pm_opp *new_opp, -struct opp_table *opp_table,bool rate_not_available) +struct opp_table *opp_table) {struct list_head *head;int ret;
@@ -1774,7 +1774,7 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev, new_opp->available = true; new_opp->dynamic = dynamic; - ret =_opp_add (dev, new_opp, opp_table, false); + ret =_opp_add (dev, new_opp, opp_table);if (ret) {/* Don't return error for duplicate OPPs */ if (ret == -EBUSY )
drivers/opp/of.c +4 -6
@@ -808,8 +808,8 @@ static int _read_bw(struct dev_pm_opp *new_opp, struct opp_table *opp_table,return ret; } -static int _read_opp_key (struct dev_pm_opp *new_opp,struct opp_table *opp_table, -struct device_node *np,bool *rate_not_available) +static int _read_opp_key (struct dev_pm_opp *new_opp, +struct opp_table *opp_table,struct device_node *np) {bool found = false;u64 rate;
@@ -825,7 +825,6 @@ static int _read_opp_key(struct dev_pm_opp *new_opp, struct opp_table *opp_table new_opp->rate = (unsigned long )rate; found = true; } - *rate_not_available = !!ret;/* * Bandwidth consists of peak and average (optional) values:
@@ -881,13 +880,12 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,struct dev_pm_opp *new_opp;u32 val;int ret; -bool rate_not_available = false; new_opp =_opp_allocate (opp_table);if (!new_opp)return ERR_PTR (-ENOMEM ); - ret =_read_opp_key (new_opp, opp_table, np, &rate_not_available); + ret =_read_opp_key (new_opp, opp_table, np);if (ret <0 ) {dev_err (dev,"%s: opp key field not found\n" , __func__); goto free_opp;
@@ -920,7 +918,7 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,if (opp_table->is_genpd) new_opp->pstate =pm_genpd_opp_to_performance_state (dev, new_opp); - ret =_opp_add (dev, new_opp, opp_table, rate_not_available); + ret =_opp_add (dev, new_opp, opp_table);if (ret) {/* Don't return error for duplicate OPPs */ if (ret == -EBUSY )
drivers/opp/opp.h +1 -1
@@ -226,7 +226,7 @@ struct opp_device *_add_opp_dev(const struct device *dev, struct opp_table *opp_struct dev_pm_opp *_opp_allocate (struct opp_table *opp_table);void _opp_free (struct dev_pm_opp *opp);int _opp_compare_key (struct dev_pm_opp *opp1,struct dev_pm_opp *opp2); -int _opp_add (struct device *dev,struct dev_pm_opp *new_opp,struct opp_table *opp_table,bool rate_not_available); +int _opp_add (struct device *dev,struct dev_pm_opp *new_opp,struct opp_table *opp_table);int _opp_add_v1 (struct opp_table *opp_table,struct device *dev,unsigned long freq,long u_volt,bool dynamic);void _dev_pm_opp_cpumask_remove_table (const struct cpumask *cpumask,int last_cpu);struct opp_table *_add_opp_table_indexed (struct device *dev,int index,bool getclk);
Signature · 1 line
2.31.1.272.g89b43f80a514
PATCH 3/8 OPP: Reuse _opp_compare_key() in _opp_add_static_v2()
In reply to Viresh KumarReuse _opp_compare_key() in _opp_add_static_v2() instead of just comparing frequency while finding suspend frequency. Also add a comment over _opp_compare_key() explaining its return values.
--- drivers/opp/core.c | 6 ++++++ drivers/opp/of.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-)
drivers/opp/core.c +6 -0
@@ -1618,6 +1618,12 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,return true; } +/* +* Returns +* 0: opp1 == opp2 +* 1: opp1 > opp2 +* -1: opp1 < opp2 +*/ int _opp_compare_key (struct dev_pm_opp *opp1,struct dev_pm_opp *opp2) {if (opp1->rate != opp2->rate)
drivers/opp/of.c +2 -2
@@ -929,8 +929,8 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,/* OPP to select on device suspend */ if (of_property_read_bool (np,"opp-suspend" )) {if (opp_table->suspend_opp) { -/* Pick the OPP with higher rate as suspend OPP */ -if (new_opp->rate > opp_table->suspend_opp->rate) { +/* Pick the OPP with higher rate/bw/level as suspend OPP */ +if (_opp_compare_key (opp_table, new_opp, opp_table->suspend_opp) ==1 ) { opp_table->suspend_opp->suspend = false; new_opp->suspend = true; opp_table->suspend_opp = new_opp;
Signature · 1 line
2.31.1.272.g89b43f80a514
Jon Hunter
maintainerRe: [PATCH 3/8] OPP: Reuse _opp_compare_key() in _opp_add_static_v2()
Quoted 32 lines · Viresh Kumar
Reuse
_opp_compare_key()in_opp_add_static_v2()instead of just comparing frequency while finding suspend frequency. Also add a comment over_opp_compare_key()explaining its return values.Signed-off-by Viresh Kumar <[email protected]>--- drivers/opp/core.c | 6 ++++++ drivers/opp/of.c | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-)drivers/opp/core.c +6
-0@@ -1618,6 +1618,12 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,return true; } +/* +* Returns +* 0: opp1 == opp2 +* 1: opp1 > opp2 +* -1: opp1 < opp2 +*/ int _opp_compare_key (struct dev_pm_opp *opp1,struct dev_pm_opp *opp2) {if (opp1->rate != opp2->rate)drivers/opp/of.c +0
-0
drivers/opp/of.c static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, · line 936
Quoted 10 lines
drivers/opp/of.c +2
-2@@ -929,8 +929,8 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,/* OPP to select on device suspend */ if (of_property_read_bool (np,"opp-suspend" )) {if (opp_table->suspend_opp) { -/* Pick the OPP with higher rate as suspend OPP */ -if (new_opp->rate > opp_table->suspend_opp->rate) { +/* Pick the OPP with higher rate/bw/level as suspend OPP */ +if (_opp_compare_key (opp_table, new_opp, opp_table->suspend_opp) ==1 ) { opp_table->suspend_opp->suspend = false; new_opp->suspend = true; opp_table->suspend_opp = new_opp;
FYI ... if I checkout commit 00d776d33da9 ("OPP: Reuse _opp_compare_key() in _opp_add_static_v2()") from next-20220622 it does not compile ...
drivers/opp/of.c: In function ‘_opp_add_static_v2’:drivers/opp/of.c:933:25: error: passing argument 1 of ‘_opp_compare_key’ from incompatible pointer type [-Werror=incompatible-pointer-types]
if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) {
^~~~~~~~~
In file included from drivers/opp/of.c:22:0:
drivers/opp/opp.h:228:5: note: expected ‘struct dev_pm_opp *’ but argument is of type ‘struct opp_table *’
int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2);
^~~~~~~~~~~~~~~~
drivers/opp/of.c:933:8: error: too many arguments to function ‘_opp_compare_key’
if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) {
^~~~~~~~~~~~~~~~
In file included from drivers/opp/of.c:22:0:
drivers/opp/opp.h:228:5: note: declared here
int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2);
^~~~~~~~~~~~~~~~This breaks bisecting -next and so would be good to fix this.
Cheers
Jon
Signature · 1 line
nvpublic
Re: [PATCH 3/8] OPP: Reuse _opp_compare_key() in _opp_add_static_v2()
Quoted 20 lines · Jon Hunter
FYI ... if I checkout commit
00d776d33da9("OPP: Reuse_opp_compare_key()in_opp_add_static_v2()") from next-20220622 it does not compile ...
drivers/opp/of.c: In function ‘_opp_add_static_v2’:drivers/opp/of.c:933:25: error: passing argument 1 of ‘_opp_compare_key’ from incompatible pointer type [-Werror=incompatible-pointer-types]if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) { ^~~~~~~~~ In file included from drivers/opp/of.c:22:0: drivers/opp/opp.h:228:5: note: expected ‘struct dev_pm_opp *’ but argument is of type ‘struct opp_table *’ int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2); ^~~~~~~~~~~~~~~~ drivers/opp/of.c:933:8: error: too many arguments to function ‘_opp_compare_key’ if (_opp_compare_key(opp_table, new_opp, opp_table->suspend_opp) == 1) { ^~~~~~~~~~~~~~~~ In file included from drivers/opp/of.c:22:0: drivers/opp/opp.h:228:5: note: declared here int _opp_compare_key(struct dev_pm_opp *opp1, struct dev_pm_opp *opp2); ^~~~~~~~~~~~~~~~
Quoted 1 line
This breaks bisecting -next and so would be good to fix this.
Yeah, this was reported yesterday and is already fixed in my branch, along with few more fixes.
git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git opp/linux-next
It hasn't landed into linux-next/master yet though.
Signature · 1 line
viresh
PATCH 4/8 OPP: Make dev_pm_opp_set_opp() independent of frequency
In reply to Viresh Kumardev_pm_opp_set_opp() can be called for any device, it may or may not have a frequency value associated with it.
If a frequency value isn't available, we pass 0 to _set_opp(). Make it optional instead by making _set_opp() accept a pointer instead, as the frequency value is anyway available in the OPP. This makes dev_pm_opp_set_opp() and _set_opp() completely independent of any special key value.
--- drivers/opp/core.c | 52 +++++++++++++++++++++++++++++++++------------- drivers/opp/opp.h | 4 ++-- 2 files changed, 40 insertions(+), 16 deletions(-)
drivers/opp/core.c +38 -14
@@ -766,19 +766,33 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,return ret; } -static inline int _generic_set_opp_clk_only (struct device *dev,struct clk *clk, -unsigned long freq) +static inline int _generic_set_opp_clk_only (struct device *dev, +struct opp_table *opp_table,struct dev_pm_opp *opp,void *data) { +unsigned long *target = data; +unsigned long freq;int ret;/* We may reach here for devices which don't change frequency */ -if (IS_ERR (clk)) +if (IS_ERR (opp_table->clk))return 0 ; - ret =clk_set_rate (clk, freq); +/* One of target and opp must be available */ +if (target) { + freq = *target; + }else if (opp) { + freq = opp->rate; + }else { +WARN_ON (1 ); +return -EINVAL ; + } + + ret =clk_set_rate (opp_table->clk, freq);if (ret) {dev_err (dev,"%s: failed to set clock rate: %d\n" , __func__, ret); + }else { + opp_table->rate_clk_single = freq; }return ret;
@@ -972,7 +986,7 @@ static int _disable_opp_table(struct device *dev, struct opp_table *opp_table) }static int _set_opp (struct device *dev,struct opp_table *opp_table, -struct dev_pm_opp *opp,unsigned long freq) +struct dev_pm_opp *opp,void *clk_data,bool forced) {struct dev_pm_opp *old_opp;int scaling_down, ret;
@@ -987,15 +1001,14 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, old_opp = opp_table->current_opp;/* Return early if nothing to do */ -if (old_opp == opp && opp_table->current_rate == freq && - opp_table->enabled) { +if (!forced && old_opp == opp && opp_table->enabled) {dev_dbg (dev,"%s: OPPs are same, nothing to do\n" , __func__);return 0 ; }dev_dbg (dev,"%s: switching OPP: Freq %lu -> %lu Hz, Level %u -> %u, Bw %u -> %u\n" , - __func__, opp_table->current_rate, freq, old_opp->level, - opp->level, old_opp->bandwidth ? old_opp->bandwidth[0 ].peak :0 , + __func__, old_opp->rate, opp->rate, old_opp->level, opp->level, + old_opp->bandwidth ? old_opp->bandwidth[0 ].peak :0 , opp->bandwidth ? opp->bandwidth[0 ].peak :0 ); scaling_down =_opp_compare_key (old_opp, opp);
@@ -1028,7 +1041,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, } } - ret =_generic_set_opp_clk_only (dev, opp_table->clk, freq); + ret =_generic_set_opp_clk_only (dev, opp_table, opp, clk_data);if (ret)return ret;
@@ -1064,7 +1077,6 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,/* Make sure current_opp doesn't get freed */ dev_pm_opp_get (opp); opp_table->current_opp = opp; - opp_table->current_rate = freq;return ret; }
@@ -1085,6 +1097,7 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)struct opp_table *opp_table;unsigned long freq =0 , temp_freq;struct dev_pm_opp *opp =NULL ; +bool forced = false;int ret; opp_table =_find_opp_table (dev);
@@ -1102,7 +1115,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)* equivalent to a clk_set_rate() */ if (!_get_opp_count (opp_table)) { - ret =_generic_set_opp_clk_only (dev, opp_table->clk, target_freq); + ret =_generic_set_opp_clk_only (dev, opp_table,NULL , + &target_freq); goto put_opp_table; }
@@ -1123,12 +1137,22 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) __func__, freq, ret); goto put_opp_table; } + +/* +* An OPP entry specifies the highest frequency at which other +* properties of the OPP entry apply. Even if the new OPP is +* same as the old one, we may still reach here for a different +* value of the frequency. In such a case, do not abort but +* configure the hardware to the desired frequency forcefully. +*/ + forced = opp_table->rate_clk_single != target_freq; } - ret =_set_opp (dev, opp_table, opp, freq); + ret =_set_opp (dev, opp_table, opp, &target_freq, forced);if (target_freq)dev_pm_opp_put (opp); + put_opp_table:dev_pm_opp_put_opp_table (opp_table);return ret;
@@ -1156,7 +1180,7 @@ int dev_pm_opp_set_opp(struct device *dev, struct dev_pm_opp *opp)return PTR_ERR (opp_table); } - ret =_set_opp (dev, opp_table, opp, opp ? opp->rate :0 ); + ret =_set_opp (dev, opp_table, opp,NULL , false);dev_pm_opp_put_opp_table (opp_table);return ret;
drivers/opp/opp.h +2 -2
@@ -138,7 +138,7 @@ enum opp_table_access {* @clock_latency_ns_max: Max clock latency in nanoseconds. * @parsed_static_opps: Count of devices for which OPPs are initialized from DT. * @shared_opp: OPP is shared between multiple devices. -* @current_rate: Currently configured frequency. +* @rate_clk_single: Currently configured frequency for single clk. * @current_opp: Currently configured OPP for the table. * @suspend_opp: Pointer to OPP to be used during device suspend. * @genpd_virt_dev_lock: Mutex protecting the genpd virtual device pointers.
@@ -187,7 +187,7 @@ struct opp_table {unsigned int parsed_static_opps;enum opp_table_access shared_opp; -unsigned long current_rate; +unsigned long rate_clk_single;struct dev_pm_opp *current_opp;struct dev_pm_opp *suspend_opp;
Signature · 1 line
2.31.1.272.g89b43f80a514
PATCH 5/8 OPP: Allow multiple clocks for a device
In reply to Viresh KumarThis patch adds support to allow multiple clocks for a device.
The design is pretty much similar to how this is done for regulators, and platforms can supply their own version of the config_clks() callback if they have multiple clocks for their device. The core manages the calls via opp_table->config_clks() eventually.
We have kept both "clk" and "clks" fields in the OPP table structure and the reason is provided as a comment in _opp_set_clknames(). The same isn't done for "rates" though and we use rates[0] at most of the places now.
--- drivers/opp/core.c | 165 ++++++++++++++++++++++++++++------------- drivers/opp/debugfs.c | 27 ++++++- drivers/opp/of.c | 67 +++++++++++++---- drivers/opp/opp.h | 16 ++-- include/linux/pm_opp.h | 7 +- 5 files changed, 208 insertions(+), 74 deletions(-)
drivers/opp/core.c +115 -50
@@ -177,7 +177,7 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp)return 0 ; } -return opp->rate; +return opp->rates[0 ]; }EXPORT_SYMBOL_GPL (dev_pm_opp_get_freq);
@@ -426,7 +426,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_opp_count);/* Helpers to read keys */ static unsigned long _read_freq (struct dev_pm_opp *opp,int index) { -return opp->rate; +return opp->rates[0 ]; }static unsigned long _read_level (struct dev_pm_opp *opp,int index)
@@ -766,8 +766,9 @@ static int _set_opp_voltage(struct device *dev, struct regulator *reg,return ret; } -static inline int _generic_set_opp_clk_only (struct device *dev, -struct opp_table *opp_table,struct dev_pm_opp *opp,void *data) +static int +_opp_config_clk_single (struct device *dev,struct opp_table *opp_table, +struct dev_pm_opp *opp,void *data,bool scaling_down) {unsigned long *target = data;unsigned long freq;
@@ -781,7 +782,7 @@ static inline int _generic_set_opp_clk_only(struct device *dev,if (target) { freq = *target; }else if (opp) { - freq = opp->rate; + freq = opp->rates[0 ]; }else {WARN_ON (1 );return -EINVAL ;
@@ -1007,11 +1008,11 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, }dev_dbg (dev,"%s: switching OPP: Freq %lu -> %lu Hz, Level %u -> %u, Bw %u -> %u\n" , - __func__, old_opp->rate, opp->rate, old_opp->level, opp->level, - old_opp->bandwidth ? old_opp->bandwidth[0 ].peak :0 , + __func__, old_opp->rates[0 ], opp->rates[0 ], old_opp->level, + opp->level, old_opp->bandwidth ? old_opp->bandwidth[0 ].peak :0 , opp->bandwidth ? opp->bandwidth[0 ].peak :0 ); - scaling_down =_opp_compare_key (old_opp, opp); + scaling_down =_opp_compare_key (opp_table, old_opp, opp);if (scaling_down ==-1 ) scaling_down =0 ;
@@ -1041,7 +1042,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, } } - ret =_generic_set_opp_clk_only (dev, opp_table, opp, clk_data); + ret = opp_table->config_clks (dev, opp_table, opp, clk_data, scaling_down);if (ret)return ret;
@@ -1115,8 +1116,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)* equivalent to a clk_set_rate() */ if (!_get_opp_count (opp_table)) { - ret =_generic_set_opp_clk_only (dev, opp_table,NULL , - &target_freq); + ret = opp_table->config_clks (dev, opp_table,NULL , + &target_freq, false); goto put_opp_table; }
@@ -1237,6 +1238,8 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)INIT_LIST_HEAD (&opp_table->dev_list);INIT_LIST_HEAD (&opp_table->lazy); + opp_table->clk =ERR_PTR (-ENODEV ); +/* Mark regulator count uninitialized */ opp_table->regulator_count =-1 ;
@@ -1283,18 +1286,22 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,int ret;/* -* Return early if we don't need to get clk or we have already tried it +* Return early if we don't need to get clk or we have already done it * earlier. */ -if (!getclk ||IS_ERR (opp_table) || opp_table->clk) +if (!getclk ||IS_ERR (opp_table) || !IS_ERR (opp_table->clk) || + opp_table->clks)return opp_table;/* Find clk for the device */ opp_table->clk =clk_get (dev,NULL ); ret =PTR_ERR_OR_ZERO (opp_table->clk); -if (!ret) +if (!ret) { + opp_table->config_clks = _opp_config_clk_single; + opp_table->clk_count =1 ;return opp_table; + }if (ret == -ENOENT ) {dev_dbg (dev,"%s: Couldn't find clock: %d\n" , __func__, ret);
@@ -1399,7 +1406,7 @@ static void _opp_table_kref_release(struct kref *kref)_of_clear_opp_table (opp_table); -/* Release clk */ +/* Release automatically acquired single clk */ if (!IS_ERR (opp_table->clk))clk_put (opp_table->clk);
@@ -1487,7 +1494,7 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq)mutex_lock (&opp_table->lock);list_for_each_entry (iter, &opp_table->opp_list, node) { -if (iter->rate == freq) { +if (iter->rates[0 ] == freq) { opp = iter;break ; }
@@ -1594,24 +1601,28 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic);struct dev_pm_opp *_opp_allocate (struct opp_table *opp_table) {struct dev_pm_opp *opp; -int supply_count, supply_size, icc_size; +int supply_count, supply_size, icc_size, clk_size;/* Allocate space for at least one supply */ supply_count = opp_table->regulator_count >0 ? opp_table->regulator_count :1 ; supply_size =sizeof (*opp->supplies) * supply_count; + clk_size =sizeof (*opp->rates) * opp_table->clk_count; icc_size =sizeof (*opp->bandwidth) * opp_table->path_count;/* allocate new OPP node and supplies structures */ opp =kzalloc (sizeof (*opp) + supply_size + icc_size,GFP_KERNEL ); -if (!opp)return NULL ; -/* Put the supplies at the end of the OPP structure as an empty array */ +/* Put the supplies, bw and clock at the end of the OPP structure */ opp->supplies = (struct dev_pm_opp_supply *)(opp +1 ); + + opp->rates = (unsigned long *)(opp->supplies + supply_count); +if (icc_size) - opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + supply_count); + opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->rates + opp_table->clk_count); +INIT_LIST_HEAD (&opp->node);return opp;
@@ -1648,10 +1659,11 @@ static bool _opp_supported_by_regulators(struct dev_pm_opp *opp,* 1: opp1 > opp2 * -1: opp1 < opp2 */ -int _opp_compare_key (struct dev_pm_opp *opp1,struct dev_pm_opp *opp2) +int _opp_compare_key (struct opp_table *opp_table,struct dev_pm_opp *opp1, +struct dev_pm_opp *opp2) { -if (opp1->rate != opp2->rate) -return opp1->rate < opp2->rate ?-1 :1 ; +if (opp_table->clk_count ==1 && opp1->rates[0 ] != opp2->rates[0 ]) +return opp1->rates[0 ] < opp2->rates[0 ] ?-1 :1 ;if (opp1->bandwidth && opp2->bandwidth && opp1->bandwidth[0 ].peak != opp2->bandwidth[0 ].peak)return opp1->bandwidth[0 ].peak < opp2->bandwidth[0 ].peak ?-1 :1 ;
@@ -1676,7 +1688,7 @@ static int _opp_is_duplicate(struct device *dev, struct dev_pm_opp *new_opp,* loop. */ list_for_each_entry (opp, &opp_table->opp_list, node) { - opp_cmp =_opp_compare_key (new_opp, opp); + opp_cmp =_opp_compare_key (opp_table, new_opp, opp);if (opp_cmp >0 ) { *head = &opp->node;continue ;
@@ -1687,8 +1699,8 @@ static int _opp_is_duplicate(struct device *dev, struct dev_pm_opp *new_opp,/* Duplicate OPPs */ dev_warn (dev,"%s: duplicate OPPs detected. Existing: freq: %lu, volt: %lu, enabled: %d. New: freq: %lu, volt: %lu, enabled: %d\n" , - __func__, opp->rate, opp->supplies[0 ].u_volt, - opp->available, new_opp->rate, + __func__, opp->rates[0 ], opp->supplies[0 ].u_volt, + opp->available, new_opp->rates[0 ], new_opp->supplies[0 ].u_volt, new_opp->available);/* Should we compare voltages for all regulators here ? */
@@ -1709,7 +1721,7 @@ void _required_opps_available(struct dev_pm_opp *opp, int count) opp->available = false;pr_warn ("%s: OPP not supported by required OPP %pOF (%lu)\n" , - __func__, opp->required_opps[i]->np, opp->rate); + __func__, opp->required_opps[i]->np, opp->rates[0 ]);return ; } }
@@ -1750,7 +1762,7 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,if (!_opp_supported_by_regulators (new_opp, opp_table)) { new_opp->available = false;dev_warn (dev,"%s: OPP not supported by regulators (%lu)\n" , - __func__, new_opp->rate); + __func__, new_opp->rates[0 ]); }/* required-opps not fully initialized yet */
@@ -1796,7 +1808,7 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev,return -ENOMEM ;/* populate the opp table */ - new_opp->rate = freq; + new_opp->rates[0 ] = freq; tol = u_volt * opp_table->voltage_tolerance_v1 /100 ; new_opp->supplies[0 ].u_volt = u_volt; new_opp->supplies[0 ].u_volt_min = u_volt - tol;
@@ -1991,6 +2003,17 @@ static void _opp_put_regulators(struct opp_table *opp_table) opp_table->regulator_count =-1 ; } +static void _put_clks (struct opp_table *opp_table,int count) +{ +int i; + +for (i = count -1 ; i >=0 ; i--) +clk_put (opp_table->clks[i]); + +kfree (opp_table->clks); + opp_table->clks =NULL ; +} +/** * _opp_set_clknames() - Set clk names for the device * @dev: Device for which clk names is being set.
@@ -2005,30 +2028,66 @@ static void _opp_put_regulators(struct opp_table *opp_table)* This must be called before any OPPs are initialized for the device. */ static int _opp_set_clknames (struct opp_table *opp_table,struct device *dev, -const char *const names[],unsigned int count) +const char *const names[],unsigned int count, +config_clks_t config_clks) { -/* We support only one clock name for now */ -if (count !=1 ) +struct clk *clk; +int ret, i; + +/* Fail early for invalid configurations */ +if (!count || (config_clks && count ==1 ) || (!config_clks && count >1 ))return -EINVAL ;/* Another CPU that shares the OPP table has set the clkname ? */ -if (opp_table->clk_configured) +if (opp_table->clks)return 0 ; -/* clk shouldn't be initialized at this point */ -if (WARN_ON (opp_table->clk)) -return -EBUSY ; + opp_table->clks =kmalloc_array (count,sizeof (*opp_table->clks), +GFP_KERNEL ); +if (!opp_table->clks) +return -ENOMEM ; -/* Find clk for the device */ - opp_table->clk =clk_get (dev, names[0 ]); -if (IS_ERR (opp_table->clk)) { -return dev_err_probe (dev,PTR_ERR (opp_table->clk), -"%s: Couldn't find clock\n" , __func__); +/* Find clks for the device */ +for (i =0 ; i < count; i++) { + clk =clk_get (dev, names[i]); +if (IS_ERR (clk)) { + ret =dev_err_probe (dev,PTR_ERR (clk), +"%s: Couldn't find clock with name: %s\n" , + __func__, names[i]); + goto free_clks; + } + + opp_table->clks[i] = clk; } - opp_table->clk_configured = true; + opp_table->clk_count = count; + +/* Set generic single clk set here */ +if (count ==1 ) { + opp_table->config_clks = _opp_config_clk_single; + +/* +* We could have just dropped the "clk" field and used "clks" +* everywhere. Instead we kept the "clk" field around for +* following reasons: +* +* - avoiding clks[0] everywhere else. +* - not running single clk helpers for multiple clk usecase by +* mistake. +* +* Since this is single-clk case, just update the clk pointer +* too. +*/ + opp_table->clk = opp_table->clks[0 ]; + }else { + opp_table->config_clks = config_clks; + }return 0 ; + +free_clks: +_put_clks (opp_table, i); +return ret; }/**
@@ -2037,11 +2096,13 @@ static int _opp_set_clknames(struct opp_table *opp_table, struct device *dev,*/ static void _opp_put_clknames (struct opp_table *opp_table) { -if (opp_table->clk_configured) { -clk_put (opp_table->clk); - opp_table->clk =ERR_PTR (-EINVAL ); - opp_table->clk_configured = false; - } +if (!opp_table->clks) +return ; + + opp_table->config_clks =NULL ; + opp_table->clk =ERR_PTR (-ENODEV ); + +_put_clks (opp_table, opp_table->clk_count); }/**
@@ -2225,9 +2286,13 @@ struct opp_table *dev_pm_opp_set_config(struct device *dev,/* Configure clocks */ if (config->clk_names) { ret =_opp_set_clknames (opp_table, dev, config->clk_names, - config->clk_count); + config->clk_count, config->config_clks);if (ret) goto err; + }else if (config->config_clks) { +/* Don't allow config callback without clocks */ + ret = -EINVAL ; + goto err; }/* Configure property names */
@@ -2523,7 +2588,7 @@ static int _opp_set_availability(struct device *dev, unsigned long freq,/* Do we have the frequency? */ list_for_each_entry (tmp_opp, &opp_table->opp_list, node) { -if (tmp_opp->rate == freq) { +if (tmp_opp->rates[0 ] == freq) { opp = tmp_opp;break ; }
@@ -2594,7 +2659,7 @@ int dev_pm_opp_adjust_voltage(struct device *dev, unsigned long freq,/* Do we have the frequency? */ list_for_each_entry (tmp_opp, &opp_table->opp_list, node) { -if (tmp_opp->rate == freq) { +if (tmp_opp->rates[0 ] == freq) { opp = tmp_opp;break ; }
drivers/opp/debugfs.c +23 -4
@@ -74,6 +74,24 @@ static void opp_debug_create_bw(struct dev_pm_opp *opp, } } +static void opp_debug_create_clks (struct dev_pm_opp *opp, +struct opp_table *opp_table, +struct dentry *pdentry) +{ +char name[12 ]; +int i; + +if (opp_table->clk_count ==1 ) { +debugfs_create_ulong ("rate_hz" ,S_IRUGO , pdentry, &opp->rates[0 ]); +return ; + } + +for (i =0 ; i < opp_table->clk_count; i++) { +snprintf (name,sizeof (name),"rate_hz_%d" , i); +debugfs_create_ulong (name,S_IRUGO , pdentry, &opp->rates[i]); + } +} +static void opp_debug_create_supplies (struct dev_pm_opp *opp,struct opp_table *opp_table,struct dentry *pdentry)
@@ -117,10 +135,11 @@ void opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)* Get directory name for OPP. * * - Normally rate is unique to each OPP, use it to get unique opp-name. -* - For some devices rate isn't available, use index instead. +* - For some devices rate isn't available or there are multiple, use +* index instead for them. */ -if (likely (opp->rate)) - id = opp->rate; +if (likely (opp_table->clk_count ==1 )) + id = opp->rates[0 ];else id =_get_opp_count (opp_table);
@@ -134,7 +153,6 @@ void opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table)debugfs_create_bool ("turbo" ,S_IRUGO , d, &opp->turbo);debugfs_create_bool ("suspend" ,S_IRUGO , d, &opp->suspend);debugfs_create_u32 ("performance_state" ,S_IRUGO , d, &opp->pstate); -debugfs_create_ulong ("rate_hz" ,S_IRUGO , d, &opp->rate);debugfs_create_u32 ("level" ,S_IRUGO , d, &opp->level);debugfs_create_ulong ("clock_latency_ns" ,S_IRUGO , d, &opp->clock_latency_ns);
@@ -142,6 +160,7 @@ void opp_debug_create_one(struct dev_pm_opp *opp, struct opp_table *opp_table) opp->of_name =of_node_full_name (opp->np);debugfs_create_str ("of_name" ,S_IRUGO , d, (char **)&opp->of_name); +opp_debug_create_clks (opp, opp_table, d);opp_debug_create_supplies (opp, opp_table, d);opp_debug_create_bw (opp, opp_table, d);
drivers/opp/of.c +54 -13
94 lines not on this page · raw message
drivers/opp/opp.h +10 -6
@@ -58,7 +58,7 @@ extern struct list_head opp_tables, lazy_opp_tables;* @suspend: true if suspend OPP * @removed: flag indicating that OPP's reference is dropped by OPP core. * @pstate: Device's power domain's performance state. -* @rate: Frequency in hertz +* @rates: Frequencies in hertz * @level: Performance level * @supplies: Power supplies voltage/current values * @bandwidth: Interconnect bandwidth values
@@ -81,7 +81,7 @@ struct dev_pm_opp {bool suspend;bool removed;unsigned int pstate; -unsigned long rate; +unsigned long *rates;unsigned int level;struct dev_pm_opp_supply *supplies;
@@ -149,8 +149,10 @@ enum opp_table_access {* @supported_hw: Array of version number to support. * @supported_hw_count: Number of elements in supported_hw array. * @prop_name: A name to postfix to many DT properties, while parsing them. -* @clk_configured: Clock name is configured by the platform. -* @clk: Device's clock handle +* @config_clks: Platform specific config_clks() callback. +* @clks: Device's clock handles, for multiple clocks. +* @clk: Device's clock handle, for single clock. +* @clk_count: Number of clocks. * @config_regulators: Platform specific config_regulators() callback. * @regulators: Supply regulators * @regulator_count: Number of power supply regulators. Its value can be -1
@@ -199,8 +201,10 @@ struct opp_table {unsigned int *supported_hw;unsigned int supported_hw_count;const char *prop_name; -bool clk_configured; +config_clks_t config_clks; +struct clk **clks;struct clk *clk; +int clk_count;config_regulators_t config_regulators;struct regulator **regulators;int regulator_count;
@@ -225,7 +229,7 @@ struct opp_table *_find_opp_table(struct device *dev);struct opp_device *_add_opp_dev (const struct device *dev,struct opp_table *opp_table);struct dev_pm_opp *_opp_allocate (struct opp_table *opp_table);void _opp_free (struct dev_pm_opp *opp); -int _opp_compare_key (struct dev_pm_opp *opp1,struct dev_pm_opp *opp2); +int _opp_compare_key (struct opp_table *opp_table,struct dev_pm_opp *opp1,struct dev_pm_opp *opp2);int _opp_add (struct device *dev,struct dev_pm_opp *new_opp,struct opp_table *opp_table);int _opp_add_v1 (struct opp_table *opp_table,struct device *dev,unsigned long freq,long u_volt,bool dynamic);void _dev_pm_opp_cpumask_remove_table (const struct cpumask *cpumask,int last_cpu);
include/linux/pm_opp.h +6 -1
@@ -61,10 +61,14 @@ typedef int (*config_regulators_t)(struct device *dev, struct dev_pm_opp *old_opp, struct dev_pm_opp *new_opp, struct regulator **regulators, unsigned int count); +typedef int (*config_clks_t )(struct device *dev,struct opp_table *opp_table, +struct dev_pm_opp *opp,void *data,bool scaling_down); +/** * struct dev_pm_opp_config - Device OPP configuration values * @clk_names: Clk name. -* @clk_count: Number of clocks, max 1 for now. +* @clk_count: Number of clocks. +* @config_clks: Custom set clk helper. * @prop_name: Name to postfix to properties. * @config_regulators: Custom set regulator helper. * @supported_hw: Array of hierarchy of versions to match.
@@ -80,6 +84,7 @@ typedef int (*config_regulators_t)(struct device *dev,struct dev_pm_opp_config {const char *const *clk_names;unsigned int clk_count; +config_clks_t config_clks;const char *prop_name;config_regulators_t config_regulators;unsigned int *supported_hw;
Signature · 1 line
2.31.1.272.g89b43f80a514
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 6 lines · Viresh Kumar
drivers/opp/core.c +1
-1@@ -1594,24 +1601,28 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic);struct dev_pm_opp *_opp_allocate (struct opp_table *opp_table) {struct dev_pm_opp *opp; -int supply_count, supply_size, icc_size; +int supply_count, supply_size, icc_size, clk_size;
drivers/opp/core.c EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic); · line 1614
Quoted 10 lines
drivers/opp/core.c +1
-0/* Allocate space for at least one supply */ supply_count = opp_table->regulator_count >0 ? opp_table->regulator_count :1 ; supply_size =sizeof (*opp->supplies) * supply_count; + clk_size =sizeof (*opp->rates) * opp_table->clk_count; icc_size =sizeof (*opp->bandwidth) * opp_table->path_count;/* allocate new OPP node and supplies structures */ opp =kzalloc (sizeof (*opp) + supply_size + icc_size,GFP_KERNEL );
The change for this line was lost in rebase I think. I have fixed my branch with this Krzysztof, so testing over it should be fine. Thanks.
Signature · 1 line
viresh
Jon Hunter
maintainerRe: [PATCH 5/8] OPP: Allow multiple clocks for a device
In reply to Viresh KumarQuoted 15 lines · Viresh Kumar
This patch adds support to allow multiple clocks for a device.
The design is pretty much similar to how this is done for regulators, and platforms can supply their own version of the
config_clks()callback if they have multiple clocks for their device. The core manages the calls viaopp_table->config_clks()eventually.We have kept both "clk" and "clks" fields in the OPP table structure and the reason is provided as a comment in
_opp_set_clknames(). The same isn't done for "rates" though and we use rates[0] at most of the places now.Co-developed-by Krzysztof Kozlowski <[email protected]>Signed-off-by Krzysztof Kozlowski <[email protected]>Signed-off-by Viresh Kumar <[email protected]>
Quoted 7 lines
--- drivers/opp/core.c | 165 ++++++++++++++++++++++++++++------------- drivers/opp/debugfs.c | 27 ++++++- drivers/opp/of.c | 67 +++++++++++++---- drivers/opp/opp.h | 16 ++-- include/linux/pm_opp.h | 7 +- 5 files changed, 208 insertions(+), 74 deletions(-)
I am seeing the following panic on -next and bisect is point to this commit ...
[ 2.145604] 8<--- cut here --- [ 2.145615] Unable to handle kernel NULL pointer dereference at virtual address 00000000 [ 2.145625] [00000000] *pgd=00000000 [ 2.145647] Internal error: Oops: 80000005 [#1] PREEMPT SMP ARM [ 2.145660] Modules linked in: [ 2.145671] CPU: 1 PID: 35 Comm: kworker/u8:1 Not tainted 5.19.0-rc3-next-20220622-gf739bc2a47f6 #1 [ 2.145688] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [ 2.145697] Workqueue: events_unbound deferred_probe_work_func [ 2.145740] PC is at 0x0 [ 2.145750] LR is at _set_opp+0x194/0x380 [ 2.145779] pc : [<00000000>] lr : [<c086b578>] psr: 20000013 [ 2.145789] sp : f08f1c80 ip : c152cb40 fp : c264c040 [ 2.145798] r10: 00000000 r9 : c152cb40 r8 : c16f3010 [ 2.145806] r7 : c264c034 r6 : 00000000 r5 : c265d800 r4 : c264c000 [ 2.145815] r3 : 00000000 r2 : c265d800 r1 : c264c000 r0 : c16f3010 [ 2.145825] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 2.145840] Control: 10c5387d Table: 8000404a DAC: 00000051 [ 2.145847] Register r0 information: slab kmalloc-1k start c16f3000 pointer offset 16 size 1024 [ 2.145883] Register r1 information: slab kmalloc-256 start c264c000 pointer offset 0 size 256 [ 2.145914] Register r2 information: slab kmalloc-128 start c265d800 pointer offset 0 size 128 [ 2.145942] Register r3 information: NULL pointer [ 2.145955] Register r4 information: slab kmalloc-256 start c264c000 pointer offset 0 size 256 [ 2.145983] Register r5 information: slab kmalloc-128 start c265d800 pointer offset 0 size 128 [ 2.146012] Register r6 information: NULL pointer [ 2.146023] Register r7 information: slab kmalloc-256 start c264c000 pointer offset 52 size 256 [ 2.146052] Register r8 information: slab kmalloc-1k start c16f3000 pointer offset 16 size 1024 [ 2.146081] Register r9 information: slab task_struct start c152cb40 pointer offset 0 [ 2.146105] Register r10 information: NULL pointer [ 2.146116] Register r11 information: slab kmalloc-256 start c264c000 pointer offset 64 size 256 [ 2.146146] Register r12 information: slab task_struct start c152cb40 pointer offset 0 [ 2.348527] Process kworker/u8:1 (pid: 35, stack limit = 0x(ptrval)) [ 2.354896] Stack: (0xf08f1c80 to 0xf08f2000) [ 2.359273] 1c80: 00000001 c0868db0 00000000 7a13d783 c152cb40 c264c000 c16f3010 c265d800 [ 2.367471] 1ca0: c2661c40 00000001 c2661c40 00000001 c2661c40 c086b8e8 00000000 7a13d783 [ 2.375666] 1cc0: c12ea5a0 c265d800 000f4240 c058cfcc ef7dddec 000f4240 00000000 c2661e88 [ 2.383861] 1ce0: 000f4240 000f4240 c2661e98 c068d238 00000000 c2665e00 000f4240 000f4240 [ 2.392056] 1d00: c2666258 00000000 c2666000 00000001 c2661c40 c068d15c 00000000 c2666000 [ 2.400253] 1d20: c16fd140 00000000 c16fd140 00000000 00000000 c16b78b8 c16b5e00 c068d2d8 [ 2.408450] 1d40: c16b7810 c26661d8 c2666000 c068ed98 c2663d00 c2663d00 00000000 c086914c [ 2.416647] 1d60: c2663d00 c16b7810 c068ebec c16b7894 00000004 c0174868 00000000 c16b78b8 [ 2.424843] 1d80: c16b5e00 c0684630 c16b7810 c068ebec 00000000 00000004 c0174868 c152cb40 [ 2.433041] 1da0: c16b78b8 c0684794 c16b7810 c068ebec 00000000 c068506c 00000a00 c265e040 [ 2.441237] 1dc0: c0f5bdd4 00000004 00000000 7a13d783 00000000 c16b7810 c16b7894 00000004 [ 2.449434] 1de0: 60000013 00000000 c265e10c c265e154 00000000 c06854c4 c265e040 c16b7800 [ 2.457629] 1e00: c16b7810 c152cb40 00000000 c05e42b0 00000001 00000000 ffffffff 00000000 [ 2.465824] 1e20: c16b7810 ff8067a4 01000000 7a13d783 c16b7810 c16b7810 00000000 c12f7700 [ 2.474020] 1e40: 00000000 00000001 c1367c20 c140f00d c1405800 c067a668 c16b7810 00000000 [ 2.482214] 1e60: c12f7700 c0678280 c16b7810 c12f7700 c16b7810 00000017 00000001 c06784e4 [ 2.490411] 1e80: c13b6754 f08f1ee4 c16b7810 c0678574 c12f7700 f08f1ee4 c16b7810 c152cb40 [ 2.498609] 1ea0: 00000001 c06788bc 00000000 f08f1ee4 c0678834 c067675c c1367c20 c14b4e70 [ 2.506804] 1ec0: c1ea60b8 7a13d783 c16b7810 c16b7810 c152cb40 c16b7854 c12fa050 c067810c [ 2.515001] 1ee0: c1400000 c16b7810 00000001 7a13d783 c16b7810 c16b7810 c12fa2f0 c12fa050 [ 2.523197] 1f00: 00000000 c0677410 c16b7810 c12fa038 c12fa038 c06778d0 c12fa06c c176a180 [ 2.531394] 1f20: c1405800 c140f000 00000000 c013dd2c c152cb40 c1405800 c1203d40 c176a180 [ 2.539592] 1f40: c1405800 c140581c c1203d40 c176a198 00000088 c1367177 c1405800 c013e2a4 [ 2.547790] 1f60: c0f07434 00000000 f08f1f7c c176f7c0 c152cb40 c013e064 c176a180 c176f640 [ 2.555984] 1f80: f0831eb4 00000000 00000000 c01459b4 c176f7c0 c01458ac 00000000 00000000 [ 2.564180] 1fa0: 00000000 00000000 00000000 c01001a8 00000000 00000000 00000000 00000000 [ 2.572373] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.580569] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [ 2.588768] _set_opp from dev_pm_opp_set_opp+0x38/0x78 [ 2.594038] dev_pm_opp_set_opp from tegra_pmc_core_pd_set_performance_state+0x44/0xb0 [ 2.602010] tegra_pmc_core_pd_set_performance_state from _genpd_set_performance_state+0x180/0x1d0 [ 2.611018] _genpd_set_performance_state from _genpd_set_performance_state+0xa4/0x1d0 [ 2.618972] _genpd_set_performance_state from genpd_set_performance_state+0x50/0x7c [ 2.626752] genpd_set_performance_state from genpd_runtime_resume+0x1ac/0x25c [ 2.634016] genpd_runtime_resume from __rpm_callback+0x38/0x14c [ 2.640073] __rpm_callback from rpm_callback+0x50/0x54 [ 2.645335] rpm_callback from rpm_resume+0x394/0x7a0 [ 2.650424] rpm_resume from __pm_runtime_resume+0x4c/0x64 [ 2.655947] __pm_runtime_resume from host1x_probe+0x29c/0x654 [ 2.661824] host1x_probe from platform_probe+0x5c/0xb8 [ 2.667080] platform_probe from really_probe+0xc8/0x2a8 [ 2.672433] really_probe from __driver_probe_device+0x84/0xe4 [ 2.678308] __driver_probe_device from driver_probe_device+0x30/0xd0 [ 2.684789] driver_probe_device from __device_attach_driver+0x88/0xb4 [ 2.691350] __device_attach_driver from bus_for_each_drv+0x54/0xb4 [ 2.697647] bus_for_each_drv from __device_attach+0xf0/0x194 [ 2.703430] __device_attach from bus_probe_device+0x84/0x8c [ 2.709129] bus_probe_device from deferred_probe_work_func+0x7c/0xa8 [ 2.715608] deferred_probe_work_func from process_one_work+0x214/0x54c [ 2.722269] process_one_work from worker_thread+0x240/0x508 [ 2.727960] worker_thread from kthread+0x108/0x124 [ 2.732872] kthread from ret_from_fork+0x14/0x2c [ 2.737602] Exception stack(0xf08f1fb0 to 0xf08f1ff8) [ 2.742669] 1fa0: 00000000 00000000 00000000 00000000 [ 2.750865] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.759061] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 2.765690] Code: bad PC value [ 2.768990] ---[ end trace 0000000000000000 ]---
Let me know if you have any thoughts.
Cheers
Jon
Signature · 1 line
nvpublic
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 95 lines · Jon Hunter
I am seeing the following panic on -next and bisect is point to this commit ...
[ 2.145604] 8<--- cut here --- [ 2.145615] Unable to handle kernel NULL pointer dereference at virtual address 00000000 [ 2.145625] [00000000] *pgd=00000000 [ 2.145647] Internal error: Oops: 80000005 [#1] PREEMPT SMP ARM [ 2.145660] Modules linked in: [ 2.145671] CPU: 1 PID: 35 Comm: kworker/u8:1 Not tainted 5.19.0-rc3-next-20220622-gf739bc2a47f6 #1 [ 2.145688] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [ 2.145697] Workqueue: events_unbound deferred_probe_work_func [ 2.145740] PC is at 0x0 [ 2.145750] LR is at _set_opp+0x194/0x380 [ 2.145779] pc : [<00000000>] lr : [<c086b578>] psr: 20000013 [ 2.145789] sp : f08f1c80 ip : c152cb40 fp : c264c040 [ 2.145798] r10: 00000000 r9 : c152cb40 r8 : c16f3010 [ 2.145806] r7 : c264c034 r6 : 00000000 r5 : c265d800 r4 : c264c000 [ 2.145815] r3 : 00000000 r2 : c265d800 r1 : c264c000 r0 : c16f3010 [ 2.145825] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 2.145840] Control: 10c5387d Table: 8000404a DAC: 00000051 [ 2.145847] Register r0 information: slab kmalloc-1k start c16f3000 pointer offset 16 size 1024 [ 2.145883] Register r1 information: slab kmalloc-256 start c264c000 pointer offset 0 size 256 [ 2.145914] Register r2 information: slab kmalloc-128 start c265d800 pointer offset 0 size 128 [ 2.145942] Register r3 information: NULL pointer [ 2.145955] Register r4 information: slab kmalloc-256 start c264c000 pointer offset 0 size 256 [ 2.145983] Register r5 information: slab kmalloc-128 start c265d800 pointer offset 0 size 128 [ 2.146012] Register r6 information: NULL pointer [ 2.146023] Register r7 information: slab kmalloc-256 start c264c000 pointer offset 52 size 256 [ 2.146052] Register r8 information: slab kmalloc-1k start c16f3000 pointer offset 16 size 1024 [ 2.146081] Register r9 information: slab task_struct start c152cb40 pointer offset 0 [ 2.146105] Register r10 information: NULL pointer [ 2.146116] Register r11 information: slab kmalloc-256 start c264c000 pointer offset 64 size 256 [ 2.146146] Register r12 information: slab task_struct start c152cb40 pointer offset 0 [ 2.348527] Process kworker/u8:1 (pid: 35, stack limit = 0x(ptrval)) [ 2.354896] Stack: (0xf08f1c80 to 0xf08f2000) [ 2.359273] 1c80: 00000001 c0868db0 00000000 7a13d783 c152cb40 c264c000 c16f3010 c265d800 [ 2.367471] 1ca0: c2661c40 00000001 c2661c40 00000001 c2661c40 c086b8e8 00000000 7a13d783 [ 2.375666] 1cc0: c12ea5a0 c265d800 000f4240 c058cfcc ef7dddec 000f4240 00000000 c2661e88 [ 2.383861] 1ce0: 000f4240 000f4240 c2661e98 c068d238 00000000 c2665e00 000f4240 000f4240 [ 2.392056] 1d00: c2666258 00000000 c2666000 00000001 c2661c40 c068d15c 00000000 c2666000 [ 2.400253] 1d20: c16fd140 00000000 c16fd140 00000000 00000000 c16b78b8 c16b5e00 c068d2d8 [ 2.408450] 1d40: c16b7810 c26661d8 c2666000 c068ed98 c2663d00 c2663d00 00000000 c086914c [ 2.416647] 1d60: c2663d00 c16b7810 c068ebec c16b7894 00000004 c0174868 00000000 c16b78b8 [ 2.424843] 1d80: c16b5e00 c0684630 c16b7810 c068ebec 00000000 00000004 c0174868 c152cb40 [ 2.433041] 1da0: c16b78b8 c0684794 c16b7810 c068ebec 00000000 c068506c 00000a00 c265e040 [ 2.441237] 1dc0: c0f5bdd4 00000004 00000000 7a13d783 00000000 c16b7810 c16b7894 00000004 [ 2.449434] 1de0: 60000013 00000000 c265e10c c265e154 00000000 c06854c4 c265e040 c16b7800 [ 2.457629] 1e00: c16b7810 c152cb40 00000000 c05e42b0 00000001 00000000 ffffffff 00000000 [ 2.465824] 1e20: c16b7810 ff8067a4 01000000 7a13d783 c16b7810 c16b7810 00000000 c12f7700 [ 2.474020] 1e40: 00000000 00000001 c1367c20 c140f00d c1405800 c067a668 c16b7810 00000000 [ 2.482214] 1e60: c12f7700 c0678280 c16b7810 c12f7700 c16b7810 00000017 00000001 c06784e4 [ 2.490411] 1e80: c13b6754 f08f1ee4 c16b7810 c0678574 c12f7700 f08f1ee4 c16b7810 c152cb40 [ 2.498609] 1ea0: 00000001 c06788bc 00000000 f08f1ee4 c0678834 c067675c c1367c20 c14b4e70 [ 2.506804] 1ec0: c1ea60b8 7a13d783 c16b7810 c16b7810 c152cb40 c16b7854 c12fa050 c067810c [ 2.515001] 1ee0: c1400000 c16b7810 00000001 7a13d783 c16b7810 c16b7810 c12fa2f0 c12fa050 [ 2.523197] 1f00: 00000000 c0677410 c16b7810 c12fa038 c12fa038 c06778d0 c12fa06c c176a180 [ 2.531394] 1f20: c1405800 c140f000 00000000 c013dd2c c152cb40 c1405800 c1203d40 c176a180 [ 2.539592] 1f40: c1405800 c140581c c1203d40 c176a198 00000088 c1367177 c1405800 c013e2a4 [ 2.547790] 1f60: c0f07434 00000000 f08f1f7c c176f7c0 c152cb40 c013e064 c176a180 c176f640 [ 2.555984] 1f80: f0831eb4 00000000 00000000 c01459b4 c176f7c0 c01458ac 00000000 00000000 [ 2.564180] 1fa0: 00000000 00000000 00000000 c01001a8 00000000 00000000 00000000 00000000 [ 2.572373] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.580569] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [ 2.588768] _set_opp from dev_pm_opp_set_opp+0x38/0x78 [ 2.594038] dev_pm_opp_set_opp from tegra_pmc_core_pd_set_performance_state+0x44/0xb0 [ 2.602010] tegra_pmc_core_pd_set_performance_state from _genpd_set_performance_state+0x180/0x1d0 [ 2.611018] _genpd_set_performance_state from _genpd_set_performance_state+0xa4/0x1d0 [ 2.618972] _genpd_set_performance_state from genpd_set_performance_state+0x50/0x7c [ 2.626752] genpd_set_performance_state from genpd_runtime_resume+0x1ac/0x25c [ 2.634016] genpd_runtime_resume from __rpm_callback+0x38/0x14c [ 2.640073] __rpm_callback from rpm_callback+0x50/0x54 [ 2.645335] rpm_callback from rpm_resume+0x394/0x7a0 [ 2.650424] rpm_resume from __pm_runtime_resume+0x4c/0x64 [ 2.655947] __pm_runtime_resume from host1x_probe+0x29c/0x654 [ 2.661824] host1x_probe from platform_probe+0x5c/0xb8 [ 2.667080] platform_probe from really_probe+0xc8/0x2a8 [ 2.672433] really_probe from __driver_probe_device+0x84/0xe4 [ 2.678308] __driver_probe_device from driver_probe_device+0x30/0xd0 [ 2.684789] driver_probe_device from __device_attach_driver+0x88/0xb4 [ 2.691350] __device_attach_driver from bus_for_each_drv+0x54/0xb4 [ 2.697647] bus_for_each_drv from __device_attach+0xf0/0x194 [ 2.703430] __device_attach from bus_probe_device+0x84/0x8c [ 2.709129] bus_probe_device from deferred_probe_work_func+0x7c/0xa8 [ 2.715608] deferred_probe_work_func from process_one_work+0x214/0x54c [ 2.722269] process_one_work from worker_thread+0x240/0x508 [ 2.727960] worker_thread from kthread+0x108/0x124 [ 2.732872] kthread from ret_from_fork+0x14/0x2c [ 2.737602] Exception stack(0xf08f1fb0 to 0xf08f1ff8) [ 2.742669] 1fa0: 00000000 00000000 00000000 00000000 [ 2.750865] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.759061] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 2.765690] Code: bad PC value [ 2.768990] ---[ end trace 0000000000000000 ]---
Quoted 1 line
Let me know if you have any thoughts.
Maybe I understand what's going on here now, Dmitry too reported this yesterday, cc'd. Does below fix it for you ?
drivers/opp/core.c +5 -3
@@ -1120,9 +1120,11 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, } } - ret = opp_table->config_clks (dev, opp_table, opp, clk_data, scaling_down); -if (ret) -return ret; +if (opp_table->config_clks) { + ret = opp_table->config_clks (dev, opp_table, opp, clk_data, scaling_down); +if (ret) +return ret; + }/* Scaling down? Configure required OPPs after frequency */ if (scaling_down) {
Signature · 1 line
viresh
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 111 lines · Viresh Kumar
Quoted 95 lines · Jon Hunter
I am seeing the following panic on -next and bisect is point to this commit ...
[ 2.145604] 8<--- cut here --- [ 2.145615] Unable to handle kernel NULL pointer dereference at virtual address 00000000 [ 2.145625] [00000000] *pgd=00000000 [ 2.145647] Internal error: Oops: 80000005 [#1] PREEMPT SMP ARM [ 2.145660] Modules linked in: [ 2.145671] CPU: 1 PID: 35 Comm: kworker/u8:1 Not tainted 5.19.0-rc3-next-20220622-gf739bc2a47f6 #1 [ 2.145688] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [ 2.145697] Workqueue: events_unbound deferred_probe_work_func [ 2.145740] PC is at 0x0 [ 2.145750] LR is at _set_opp+0x194/0x380 [ 2.145779] pc : [<00000000>] lr : [<c086b578>] psr: 20000013 [ 2.145789] sp : f08f1c80 ip : c152cb40 fp : c264c040 [ 2.145798] r10: 00000000 r9 : c152cb40 r8 : c16f3010 [ 2.145806] r7 : c264c034 r6 : 00000000 r5 : c265d800 r4 : c264c000 [ 2.145815] r3 : 00000000 r2 : c265d800 r1 : c264c000 r0 : c16f3010 [ 2.145825] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 2.145840] Control: 10c5387d Table: 8000404a DAC: 00000051 [ 2.145847] Register r0 information: slab kmalloc-1k start c16f3000 pointer offset 16 size 1024 [ 2.145883] Register r1 information: slab kmalloc-256 start c264c000 pointer offset 0 size 256 [ 2.145914] Register r2 information: slab kmalloc-128 start c265d800 pointer offset 0 size 128 [ 2.145942] Register r3 information: NULL pointer [ 2.145955] Register r4 information: slab kmalloc-256 start c264c000 pointer offset 0 size 256 [ 2.145983] Register r5 information: slab kmalloc-128 start c265d800 pointer offset 0 size 128 [ 2.146012] Register r6 information: NULL pointer [ 2.146023] Register r7 information: slab kmalloc-256 start c264c000 pointer offset 52 size 256 [ 2.146052] Register r8 information: slab kmalloc-1k start c16f3000 pointer offset 16 size 1024 [ 2.146081] Register r9 information: slab task_struct start c152cb40 pointer offset 0 [ 2.146105] Register r10 information: NULL pointer [ 2.146116] Register r11 information: slab kmalloc-256 start c264c000 pointer offset 64 size 256 [ 2.146146] Register r12 information: slab task_struct start c152cb40 pointer offset 0 [ 2.348527] Process kworker/u8:1 (pid: 35, stack limit = 0x(ptrval)) [ 2.354896] Stack: (0xf08f1c80 to 0xf08f2000) [ 2.359273] 1c80: 00000001 c0868db0 00000000 7a13d783 c152cb40 c264c000 c16f3010 c265d800 [ 2.367471] 1ca0: c2661c40 00000001 c2661c40 00000001 c2661c40 c086b8e8 00000000 7a13d783 [ 2.375666] 1cc0: c12ea5a0 c265d800 000f4240 c058cfcc ef7dddec 000f4240 00000000 c2661e88 [ 2.383861] 1ce0: 000f4240 000f4240 c2661e98 c068d238 00000000 c2665e00 000f4240 000f4240 [ 2.392056] 1d00: c2666258 00000000 c2666000 00000001 c2661c40 c068d15c 00000000 c2666000 [ 2.400253] 1d20: c16fd140 00000000 c16fd140 00000000 00000000 c16b78b8 c16b5e00 c068d2d8 [ 2.408450] 1d40: c16b7810 c26661d8 c2666000 c068ed98 c2663d00 c2663d00 00000000 c086914c [ 2.416647] 1d60: c2663d00 c16b7810 c068ebec c16b7894 00000004 c0174868 00000000 c16b78b8 [ 2.424843] 1d80: c16b5e00 c0684630 c16b7810 c068ebec 00000000 00000004 c0174868 c152cb40 [ 2.433041] 1da0: c16b78b8 c0684794 c16b7810 c068ebec 00000000 c068506c 00000a00 c265e040 [ 2.441237] 1dc0: c0f5bdd4 00000004 00000000 7a13d783 00000000 c16b7810 c16b7894 00000004 [ 2.449434] 1de0: 60000013 00000000 c265e10c c265e154 00000000 c06854c4 c265e040 c16b7800 [ 2.457629] 1e00: c16b7810 c152cb40 00000000 c05e42b0 00000001 00000000 ffffffff 00000000 [ 2.465824] 1e20: c16b7810 ff8067a4 01000000 7a13d783 c16b7810 c16b7810 00000000 c12f7700 [ 2.474020] 1e40: 00000000 00000001 c1367c20 c140f00d c1405800 c067a668 c16b7810 00000000 [ 2.482214] 1e60: c12f7700 c0678280 c16b7810 c12f7700 c16b7810 00000017 00000001 c06784e4 [ 2.490411] 1e80: c13b6754 f08f1ee4 c16b7810 c0678574 c12f7700 f08f1ee4 c16b7810 c152cb40 [ 2.498609] 1ea0: 00000001 c06788bc 00000000 f08f1ee4 c0678834 c067675c c1367c20 c14b4e70 [ 2.506804] 1ec0: c1ea60b8 7a13d783 c16b7810 c16b7810 c152cb40 c16b7854 c12fa050 c067810c [ 2.515001] 1ee0: c1400000 c16b7810 00000001 7a13d783 c16b7810 c16b7810 c12fa2f0 c12fa050 [ 2.523197] 1f00: 00000000 c0677410 c16b7810 c12fa038 c12fa038 c06778d0 c12fa06c c176a180 [ 2.531394] 1f20: c1405800 c140f000 00000000 c013dd2c c152cb40 c1405800 c1203d40 c176a180 [ 2.539592] 1f40: c1405800 c140581c c1203d40 c176a198 00000088 c1367177 c1405800 c013e2a4 [ 2.547790] 1f60: c0f07434 00000000 f08f1f7c c176f7c0 c152cb40 c013e064 c176a180 c176f640 [ 2.555984] 1f80: f0831eb4 00000000 00000000 c01459b4 c176f7c0 c01458ac 00000000 00000000 [ 2.564180] 1fa0: 00000000 00000000 00000000 c01001a8 00000000 00000000 00000000 00000000 [ 2.572373] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.580569] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000 [ 2.588768] _set_opp from dev_pm_opp_set_opp+0x38/0x78 [ 2.594038] dev_pm_opp_set_opp from tegra_pmc_core_pd_set_performance_state+0x44/0xb0 [ 2.602010] tegra_pmc_core_pd_set_performance_state from _genpd_set_performance_state+0x180/0x1d0 [ 2.611018] _genpd_set_performance_state from _genpd_set_performance_state+0xa4/0x1d0 [ 2.618972] _genpd_set_performance_state from genpd_set_performance_state+0x50/0x7c [ 2.626752] genpd_set_performance_state from genpd_runtime_resume+0x1ac/0x25c [ 2.634016] genpd_runtime_resume from __rpm_callback+0x38/0x14c [ 2.640073] __rpm_callback from rpm_callback+0x50/0x54 [ 2.645335] rpm_callback from rpm_resume+0x394/0x7a0 [ 2.650424] rpm_resume from __pm_runtime_resume+0x4c/0x64 [ 2.655947] __pm_runtime_resume from host1x_probe+0x29c/0x654 [ 2.661824] host1x_probe from platform_probe+0x5c/0xb8 [ 2.667080] platform_probe from really_probe+0xc8/0x2a8 [ 2.672433] really_probe from __driver_probe_device+0x84/0xe4 [ 2.678308] __driver_probe_device from driver_probe_device+0x30/0xd0 [ 2.684789] driver_probe_device from __device_attach_driver+0x88/0xb4 [ 2.691350] __device_attach_driver from bus_for_each_drv+0x54/0xb4 [ 2.697647] bus_for_each_drv from __device_attach+0xf0/0x194 [ 2.703430] __device_attach from bus_probe_device+0x84/0x8c [ 2.709129] bus_probe_device from deferred_probe_work_func+0x7c/0xa8 [ 2.715608] deferred_probe_work_func from process_one_work+0x214/0x54c [ 2.722269] process_one_work from worker_thread+0x240/0x508 [ 2.727960] worker_thread from kthread+0x108/0x124 [ 2.732872] kthread from ret_from_fork+0x14/0x2c [ 2.737602] Exception stack(0xf08f1fb0 to 0xf08f1ff8) [ 2.742669] 1fa0: 00000000 00000000 00000000 00000000 [ 2.750865] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 2.759061] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 [ 2.765690] Code: bad PC value [ 2.768990] ---[ end trace 0000000000000000 ]---Quoted 1 line
Let me know if you have any thoughts.
Maybe I understand what's going on here now, Dmitry too reported this yesterday, cc'd. Does below fix it for you ?
drivers/opp/core.c +0
-1@@ -1120,9 +1120,11 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table, } } - ret = opp_table->config_clks (dev, opp_table, opp, clk_data, scaling_down);
drivers/opp/core.c static int _set_opp(struct device *dev, struct opp_table *opp_table, · line 1130
Quoted 10 lines
drivers/opp/core.c +5
-2-if (ret) -return ret; +if (opp_table->config_clks) { + ret = opp_table->config_clks (dev, opp_table, opp, clk_data, scaling_down); +if (ret) +return ret; + }/* Scaling down? Configure required OPPs after frequency */ if (scaling_down) {
Yes that fixes it thanks!
Jon
Signature · 1 line
nvpublic
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 178 lines · Jon Hunter
Quoted 175 lines · Viresh Kumar
Quoted 148 lines · Jon Hunter
I am seeing the following panic on -next and bisect is point to this commit ...
[ 2.145604] 8<--- cut here ---
[ 2.145615] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 2.145625] [00000000] *pgd=00000000
[ 2.145647] Internal error: Oops: 80000005 [#1] PREEMPT SMP ARM [ 2.145660] Modules linked in:
[ 2.145671] CPU: 1 PID: 35 Comm: kworker/u8:1 Not tainted 5.19.0-rc3-next-20220622-gf739bc2a47f6 #1
[ 2.145688] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree) [ 2.145697] Workqueue:events_unbounddeferred_probe_work_func[ 2.145740] PC is at0x0
[ 2.145750] LR is at_set_opp+0x194/0x380
[ 2.145779] pc : [<00000000>] lr : [<c086b578>] psr: 20000013 [ 2.145789] sp : f08f1c80 ip : c152cb40 fp :c264c040
[ 2.145798] r10: 00000000 r9 : c152cb40 r8 :c16f3010
[ 2.145806] r7 : c264c034 r6 : 00000000 r5 : c265d800 r4 :c264c000
[ 2.145815] r3 : 00000000 r2 : c265d800 r1 : c264c000 r0 :c16f3010
[ 2.145825] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 2.145840] Control: 10c5387d Table: 8000404a DAC: 00000051 [ 2.145847] Register r0 information: slab kmalloc-1k startc16f3000pointer offset 16 size 1024
[ 2.145883] Register r1 information: slab kmalloc-256 startc264c000pointer offset 0 size 256
[ 2.145914] Register r2 information: slab kmalloc-128 startc265d800pointer offset 0 size 128
[ 2.145942] Register r3 information: NULL pointer
[ 2.145955] Register r4 information: slab kmalloc-256 startc264c000pointer offset 0 size 256
[ 2.145983] Register r5 information: slab kmalloc-128 startc265d800pointer offset 0 size 128
[ 2.146012] Register r6 information: NULL pointer
[ 2.146023] Register r7 information: slab kmalloc-256 startc264c000pointer offset 52 size 256
[ 2.146052] Register r8 information: slab kmalloc-1k startc16f3000pointer offset 16 size 1024
[ 2.146081] Register r9 information: slabtask_structstartc152cb40pointer offset 0
[ 2.146105] Register r10 information: NULL pointer
[ 2.146116] Register r11 information: slab kmalloc-256 startc264c000pointer offset 64 size 256
[ 2.146146] Register r12 information: slabtask_structstartc152cb40pointer offset 0
[ 2.348527] Process kworker/u8:1 (pid: 35, stack limit = 0x(ptrval)) [ 2.354896] Stack: (0xf08f1c80to0xf08f2000)
[ 2.359273] 1c80: 00000001c0868db0000000007a13d783c152cb40c264c000c16f3010c265d800
[ 2.367471] 1ca0:c2661c4000000001c2661c4000000001c2661c40c086b8e8000000007a13d783
[ 2.375666] 1cc0:c12ea5a0c265d800000f4240c058cfccef7dddec000f424000000000c2661e88
[ 2.383861] 1ce0:000f4240000f4240c2661e98c068d23800000000c2665e00000f4240000f4240
[ 2.392056] 1d00:c266625800000000c266600000000001c2661c40c068d15c00000000c2666000
[ 2.400253] 1d20:c16fd14000000000c16fd14000000000 00000000c16b78b8c16b5e00c068d2d8
[ 2.408450] 1d40:c16b7810c26661d8c2666000c068ed98c2663d00c2663d0000000000c086914c
[ 2.416647] 1d60:c2663d00c16b7810c068ebecc16b789400000004c017486800000000c16b78b8
[ 2.424843] 1d80:c16b5e00c0684630c16b7810c068ebec00000000 00000004c0174868c152cb40
[ 2.433041] 1da0:c16b78b8c0684794c16b7810c068ebec00000000c068506c00000a00c265e040
[ 2.441237] 1dc0:c0f5bdd400000004 000000007a13d78300000000c16b7810c16b789400000004
[ 2.449434] 1de0: 60000013 00000000c265e10cc265e15400000000c06854c4c265e040c16b7800
[ 2.457629] 1e00:c16b7810c152cb4000000000c05e42b000000001 00000000 ffffffff 00000000
[ 2.465824] 1e20:c16b7810ff8067a4010000007a13d783c16b7810c16b781000000000c12f7700
[ 2.474020] 1e40: 00000000 00000001c1367c20c140f00dc1405800c067a668c16b781000000000
[ 2.482214] 1e60:c12f7700c0678280c16b7810c12f7700c16b781000000017 00000001c06784e4
[ 2.490411] 1e80:c13b6754f08f1ee4c16b7810c0678574c12f7700f08f1ee4c16b7810c152cb40
[ 2.498609] 1ea0: 00000001c06788bc00000000f08f1ee4c0678834c067675cc1367c20c14b4e70
[ 2.506804] 1ec0:c1ea60b87a13d783c16b7810c16b7810c152cb40c16b7854c12fa050c067810c
[ 2.515001] 1ee0:c1400000c16b7810000000017a13d783c16b7810c16b7810c12fa2f0c12fa050
[ 2.523197] 1f00: 00000000c0677410c16b7810c12fa038c12fa038c06778d0c12fa06cc176a180
[ 2.531394] 1f20:c1405800c140f00000000000c013dd2cc152cb40c1405800c1203d40c176a180
[ 2.539592] 1f40:c1405800c140581cc1203d40c176a19800000088c1367177c1405800c013e2a4
[ 2.547790] 1f60:c0f0743400000000f08f1f7cc176f7c0c152cb40c013e064c176a180c176f640
[ 2.555984] 1f80:f0831eb400000000 00000000c01459b4c176f7c0c01458ac00000000 00000000
[ 2.564180] 1fa0: 00000000 00000000 00000000c01001a800000000 00000000 00000000 00000000
[ 2.572373] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 2.580569] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[ 2.588768]_set_oppfromdev_pm_opp_set_opp+0x38/0x78
[ 2.594038]dev_pm_opp_set_oppfromtegra_pmc_core_pd_set_performance_state+0x44/0xb0
[ 2.602010]tegra_pmc_core_pd_set_performance_statefrom_genpd_set_performance_state+0x180/0x1d0
[ 2.611018]_genpd_set_performance_statefrom_genpd_set_performance_state+0xa4/0x1d0
[ 2.618972]_genpd_set_performance_statefromgenpd_set_performance_state+0x50/0x7c
[ 2.626752]genpd_set_performance_statefromgenpd_runtime_resume+0x1ac/0x25c [ 2.634016] genpd_runtime_resume from __rpm_callback+0x38/0x14c [ 2.640073] __rpm_callback from rpm_callback+0x50/0x54 [ 2.645335] rpm_callback from rpm_resume+0x394/0x7a0 [ 2.650424] rpm_resume from __pm_runtime_resume+0x4c/0x64 [ 2.655947] __pm_runtime_resume from host1x_probe+0x29c/0x654 [ 2.661824] host1x_probe from platform_probe+0x5c/0xb8 [ 2.667080] platform_probe from really_probe+0xc8/0x2a8 [ 2.672433] really_probe from __driver_probe_device+0x84/0xe4 [ 2.678308] __driver_probe_device from driver_probe_device+0x30/0xd0 [ 2.684789] driver_probe_device from __device_attach_driver+0x88/0xb4 [ 2.691350] __device_attach_driver from bus_for_each_drv+0x54/0xb4 [ 2.697647] bus_for_each_drv from __device_attach+0xf0/0x194 [ 2.703430] __device_attach from bus_probe_device+0x84/0x8c [ 2.709129] bus_probe_device from deferred_probe_work_func+0x7c/0xa8 [ 2.715608] deferred_probe_work_func from process_one_work+0x214/0x54c [ 2.722269] process_one_work from worker_thread+0x240/0x508 [ 2.727960] worker_thread from kthread+0x108/0x124 [ 2.732872] kthread from ret_from_fork+0x14/0x2c[ 2.737602] Exception stack(0xf08f1fb0 to
0xf08f1ff8) [ 2.742669] 1fa0: 00000000 00000000 00000000 00000000
[ 2.750865] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 2.759061] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 2.765690] Code: bad PC value
[ 2.768990] ---[ end trace 0000000000000000 ]---Quoted 1 line
Let me know if you have any thoughts.
Maybe I understand what's going on here now, Dmitry too reported this yesterday,
cc'd. Does below fix it for you ?drivers/opp/core.c +5
-3@@ -1120,9 +1120,11 @@ static int _set_opp(struct device *dev, structopp_table *opp_table, } } - ret = opp_table->config_clks(dev, opp_table, opp, clk_data, scaling_down); - if (ret) - return ret; + if (opp_table->config_clks) { + ret = opp_table->config_clks(dev, opp_table, opp, clk_data, scaling_down); + if (ret) + return ret; + }
Quoted 8 lines
Quoted 3 lines
/* Scaling down? Configure required OPPs after frequency */ if (
scaling_down) {Yes that fixes it thanks!
Tested-by Jon Hunter <[email protected]>
Hi Viresh,
Today I noticed that tegra30-devfreq driver now fails to probe because dev_pm_opp_find_freq_ceil() fails with -ERANGE. This patch is guilty for that. Could you please take a look?
Signature · 2 lines
Best regards, Dmitry
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 3 lines · Dmitry Osipenko
Today I noticed that tegra30-devfreq driver now fails to probe because
dev_pm_opp_find_freq_ceil()fails with-ERANGE. This patch is guilty for that. Could you please take a look?
I remember this corner case now [1] and it was easy to miss this. So you want the OPP core to still parse the DT to read opp-hz, but don't want dev_pm_opp_set_opp() to update the clock rate for it.
What was the reason for this again ?
I have a couple of solutions in mind, but one may be other than second and so want to know the real issue at hand first.
Signature · 3 lines
viresh [1] https://lore.kernel.org/lkml/[email protected]/
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 5 lines · Viresh Kumar
Quoted 3 lines · Dmitry Osipenko
Today I noticed that tegra30-devfreq driver now fails to probe because
dev_pm_opp_find_freq_ceil()fails with-ERANGE. This patch is guilty for that. Could you please take a look?
Quoted 9 lines
I remember this corner case now [1] and it was easy to miss this. So you want the OPP core to still parse the DT to read opp-hz, but don't want
dev_pm_opp_set_opp()to update the clock rate for it.What was the reason for this again ?
I have a couple of solutions in mind, but one may be other than second and so want to know the real issue at hand first.
We added memory interconnect support to Tegra and since that time only the memory controller can drive the clock rate. All other drivers, including the devfreq, now issue memory bandwidth requests using ICC.
In case of the devfreq driver, it's the OPP core that makes the bw request using ICC.
But it's the set_freq_table() that fails [2], I see dev_pm_opp_get_opp_count() returns 17, which is correct, and then dev_pm_opp_find_freq_ceil(freq=0) returns freq=1, which shall be freq=12750000.
[2]
https://elixir.bootlin.com/linux/v5.19-rc4/source/drivers/devfreq/devfreq.c#L179
Signature · 2 lines
Best regards, Dmitry
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 10 lines · Dmitry Osipenko
Quoted 4 lines · Viresh Kumar
Quoted 3 lines · Dmitry Osipenko
Today I noticed that tegra30-devfreq driver now fails to probe because
dev_pm_opp_find_freq_ceil()fails with-ERANGE. This patch is guilty for that. Could you please take a look?We added memory interconnect support to Tegra and since that time only the memory controller can drive the clock rate. All other drivers, including the devfreq, now issue memory bandwidth requests using ICC.
Quoted 7 lines
In case of the devfreq driver, it's the OPP core that makes the bw request using ICC.
But it's the
set_freq_table()that fails [2], I seedev_pm_opp_get_opp_count()returns 17, which is correct, and thendev_pm_opp_find_freq_ceil(freq=0)returns freq=1, which shall be freq=12750000.
I am confused, you said earlier that it is failing with -ERANGE, but now it is a bad freq value ?
Which one of these it is ?
The problem I see is here though, because of which I was asking you the question earlier:
- tegra30-devfreq driver calls devm_pm_opp_of_add_table_noclk(), i.e. clk_count == 0.
- _read_rate() (in drivers/opp/of.c) skips reading any opp-hz properties if clk_count is 0.
- And so you can get -ERANGE or some other error.
Can you please see where we are failing. Also I don't see how freq can get set to 1 currently.
Signature · 1 line
viresh
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 30 lines · Viresh Kumar
Quoted 10 lines · Dmitry Osipenko
Quoted 4 lines · Viresh Kumar
Quoted 3 lines · Dmitry Osipenko
Today I noticed that tegra30-devfreq driver now fails to probe because
dev_pm_opp_find_freq_ceil()fails with-ERANGE. This patch is guilty for that. Could you please take a look?We added memory interconnect support to Tegra and since that time only the memory controller can drive the clock rate. All other drivers, including the devfreq, now issue memory bandwidth requests using ICC.
Quoted 7 lines
In case of the devfreq driver, it's the OPP core that makes the bw request using ICC.
But it's the
set_freq_table()that fails [2], I seedev_pm_opp_get_opp_count()returns 17, which is correct, and thendev_pm_opp_find_freq_ceil(freq=0)returns freq=1, which shall be freq=12750000.I am confused, you said earlier that it is failing with
-ERANGE, but now it is a bad freq value ?Which one of these it is ?
The problem I see is here though, because of which I was asking you the question earlier:
- tegra30-devfreq driver calls
devm_pm_opp_of_add_table_noclk(), i.e.clk_count== 0.
Quoted 8 lines
-
_read_rate()(indrivers/opp/of.c) skips reading any opp-hz properties ifclk_countis 0.- And so you can get
-ERANGEor some other error.Can you please see where we are failing. Also I don't see how freq can get set to 1 currently.
The set_freq_table() gets available freqs usingdev_pm_opp_find_freq_ceil() iteration.
The first dev_pm_opp_find_freq_ceil(freq=0) succeeds and returns ceil freq=1.
The second dev_pm_opp_find_freq_ceil(freq=1) fails with -ERANGE.
I haven't looked yet at why freq is set to 1.
Signature · 2 lines
Best regards, Dmitry
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 5 lines · Dmitry Osipenko
The
set_freq_table()gets available freqs usingdev_pm_opp_find_freq_ceil()iteration.The first
dev_pm_opp_find_freq_ceil(freq=0)succeeds and returns ceil freq=1.
I don't see how this can possibly happen. One possibility is that freq is set to 0 and one the next loop you do freq++, which can make it 1.
Quoted 1 line
The second
dev_pm_opp_find_freq_ceil(freq=1)fails with-ERANGE.
Even if we send freq = 1, I don't see how we can get ERANGE if the OPP table is properly initialized.
Quoted 1 line
I haven't looked yet at why freq is set to 1.
Thanks, but I would need some help to get it debugged.
Signature · 1 line
viresh
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 10 lines · Viresh Kumar
Quoted 5 lines · Dmitry Osipenko
The
set_freq_table()gets available freqs usingdev_pm_opp_find_freq_ceil()iteration.The first
dev_pm_opp_find_freq_ceil(freq=0)succeeds and returns ceil freq=1.I don't see how this can possibly happen. One possibility is that freq is set to 0 and one the next loop you do freq++, which can make it 1.
Quoted 8 lines
Quoted 1 line
The second
dev_pm_opp_find_freq_ceil(freq=1)fails with-ERANGE.Even if we send freq = 1, I don't see how we can get ERANGE if the OPP table is properly initialized.
Quoted 1 line
I haven't looked yet at why freq is set to 1.
Thanks, but I would need some help to get it debugged.
Hi Dmitry,
I am looking to send another version of this now and soon merge this in for 5.20-rc1. Can you please help figure out what's going on here ?
Thanks.
Signature · 1 line
viresh
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 11 lines · Viresh Kumar
Quoted 10 lines · Viresh Kumar
Quoted 5 lines · Dmitry Osipenko
The
set_freq_table()gets available freqs usingdev_pm_opp_find_freq_ceil()iteration.The first
dev_pm_opp_find_freq_ceil(freq=0)succeeds and returns ceil freq=1.I don't see how this can possibly happen. One possibility is that freq is set to 0 and one the next loop you do freq++, which can make it 1.
Quoted 6 lines
Quoted 6 lines
Quoted 1 line
The second
dev_pm_opp_find_freq_ceil(freq=1)fails with-ERANGE.Even if we send freq = 1, I don't see how we can get ERANGE if the OPP table is properly initialized.
Quoted 1 line
I haven't looked yet at why freq is set to 1.
Actually the freq was 0 and it was 1 on the next loop like you suggested.
Quoted 6 lines
Quoted 1 line
Thanks, but I would need some help to get it debugged.
Hi Dmitry,
I am looking to send another version of this now and soon merge this in for 5.20-rc1. Can you please help figure out what's going on here ?
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().
Signature · 2 lines
Best regards, Dmitry
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 5 lines · Dmitry Osipenko
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 usesdevm_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.
drivers/devfreq/tegra30-devfreq.c +19 -3
@@ -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;
drivers/opp/core.c +4 -4
@@ -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 ;
Dmitry Osipenko
Tested-byRe: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 22 lines · Viresh Kumar
Quoted 5 lines · Dmitry Osipenko
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 usesdevm_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_clkshelperThere 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.
Quoted 4 lines
The OPP core provides support for the platforms to provide
config_clkshelpers now, lets use them instead ofdevm_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.
Works, thanks you!
Signature · 2 lines
Best regards, Dmitry
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 23 lines · Dmitry Osipenko
Quoted 22 lines · Viresh Kumar
Quoted 5 lines · Dmitry Osipenko
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 usesdevm_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_clkshelperThere 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.
Quoted 8 lines
Quoted 4 lines
The OPP core provides support for the platforms to provide
config_clkshelpers now, lets use them instead ofdevm_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.Works, thanks you!
Tested-by Dmitry Osipenko <[email protected]>
Finally, thanks a lot Dmitry :)
Signature · 1 line
viresh
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
In reply to Viresh KumarQuoted 12 lines · Viresh Kumar
This patch adds support to allow multiple clocks for a device.
The design is pretty much similar to how this is done for regulators, and platforms can supply their own version of the
config_clks()callback if they have multiple clocks for their device. The core manages the calls viaopp_table->config_clks()eventually.We have kept both "clk" and "clks" fields in the OPP table structure and the reason is provided as a comment in
_opp_set_clknames(). The same isn't done for "rates" though and we use rates[0] at most of the places now.
Quoted 7 lines
Co-developed-by Krzysztof Kozlowski <[email protected]>Signed-off-by Krzysztof Kozlowski <[email protected]>Signed-off-by Viresh Kumar <[email protected]>--- drivers/opp/core.c | 165 ++++++++++++++++++++++++++++------------- drivers/opp/debugfs.c | 27 ++++++- drivers/opp/of.c | 67 +++++++++++++----
I don't see bindings change here, but I think you included my pieces of code related to parsing multiple opp-hz. Is that correct? If so, you need to pick up the bindings patch as well.
Best regards,
Krzysztof
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 21 lines · Krzysztof Kozlowski
Quoted 12 lines · Viresh Kumar
This patch adds support to allow multiple clocks for a device.
The design is pretty much similar to how this is done for regulators, and platforms can supply their own version of the
config_clks()callback if they have multiple clocks for their device. The core manages the calls viaopp_table->config_clks()eventually.We have kept both "clk" and "clks" fields in the OPP table structure and the reason is provided as a comment in
_opp_set_clknames(). The same isn't done for "rates" though and we use rates[0] at most of the places now.Quoted 7 lines
Co-developed-by Krzysztof Kozlowski <[email protected]>Signed-off-by Krzysztof Kozlowski <[email protected]>Signed-off-by Viresh Kumar <[email protected]>--- drivers/opp/core.c | 165 ++++++++++++++++++++++++++++------------- drivers/opp/debugfs.c | 27 ++++++- drivers/opp/of.c | 67 +++++++++++++----
Quoted 3 lines
I don't see bindings change here, but I think you included my pieces of code related to parsing multiple opp-hz. Is that correct? If so, you need to pick up the bindings patch as well.
My bad, will pick that.
Signature · 1 line
viresh
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
In reply to Viresh KumarQuoted 15 lines · Viresh Kumar
This patch adds support to allow multiple clocks for a device.
The design is pretty much similar to how this is done for regulators, and platforms can supply their own version of the
config_clks()callback if they have multiple clocks for their device. The core manages the calls viaopp_table->config_clks()eventually.We have kept both "clk" and "clks" fields in the OPP table structure and the reason is provided as a comment in
_opp_set_clknames(). The same isn't done for "rates" though and we use rates[0] at most of the places now.Co-developed-by Krzysztof Kozlowski <[email protected]>Signed-off-by Krzysztof Kozlowski <[email protected]>Signed-off-by Viresh Kumar <[email protected]>
(...)
Quoted 38 lines
drivers/opp/of.c +26
-3+ rates =kmalloc_array (count,sizeof (*rates),GFP_KERNEL ); +if (!rates) +return -ENOMEM ; + + ret =of_property_read_u64_array (np,"opp-hz" , rates, count); +if (ret) { +pr_err ("%s: Error parsing opp-hz: %d\n" , __func__, ret); + }else { +/* +* Rate is defined as an unsigned long in clk API, and so +* casting explicitly to its type. Must be fixed once rate is 64 +* bit guaranteed in clk API. +*/ +for (i =0 ; i < count; i++) { + new_opp->rates[i] = (unsigned long )rates[i]; + +/* This will happen for frequencies > 4.29 GHz */ +WARN_ON (new_opp->rates[i] != rates[i]); + } + } + +kfree (rates); + +return ret; +} +static int _read_bw (struct dev_pm_opp *new_opp,struct opp_table *opp_table,struct device_node *np,bool peak) {@@ -812,19 +859,13 @@ static int _read_opp_key(struct dev_pm_opp *new_opp, struct opp_table *opp_table, struct device_node *np) { bool found = false; -u64 rate;int ret; - ret =of_property_read_u64 (np,"opp-hz" , &rate); -if (!ret) {
drivers/opp/of.c static int _read_opp_key(struct dev_pm_opp *new_opp, · line 867
Quoted 10 lines
drivers/opp/of.c +4
-6-/* -* Rate is defined as an unsigned long in clk API, and so -* casting explicitly to its type. Must be fixed once rate is 64 -* bit guaranteed in clk API. -*/ - new_opp->rate = (unsigned long )rate; + ret =_read_rate (new_opp, opp_table, np); +if (ret) +return ret; +else if (opp_table->clk_count ==1 )
Shouldn't this be >=1? I got several clocks and this one fails.
Best regards,
Krzysztof
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 55 lines · Krzysztof Kozlowski
Quoted 15 lines · Viresh Kumar
This patch adds support to allow multiple clocks for a device.
The design is pretty much similar to how this is done for regulators, and platforms can supply their own version of the
config_clks()callback if they have multiple clocks for their device. The core manages the calls viaopp_table->config_clks()eventually.We have kept both "clk" and "clks" fields in the OPP table structure and the reason is provided as a comment in
_opp_set_clknames(). The same isn't done for "rates" though and we use rates[0] at most of the places now.Co-developed-by Krzysztof Kozlowski <[email protected]>Signed-off-by Krzysztof Kozlowski <[email protected]>Signed-off-by Viresh Kumar <[email protected]>(...)
Quoted 36 lines
drivers/opp/of.c +26
-1+ rates =kmalloc_array (count,sizeof (*rates),GFP_KERNEL ); +if (!rates) +return -ENOMEM ; + + ret =of_property_read_u64_array (np,"opp-hz" , rates, count); +if (ret) { +pr_err ("%s: Error parsing opp-hz: %d\n" , __func__, ret); + }else { +/* +* Rate is defined as an unsigned long in clk API, and so +* casting explicitly to its type. Must be fixed once rate is 64 +* bit guaranteed in clk API. +*/ +for (i =0 ; i < count; i++) { + new_opp->rates[i] = (unsigned long )rates[i]; + +/* This will happen for frequencies > 4.29 GHz */ +WARN_ON (new_opp->rates[i] != rates[i]); + } + } + +kfree (rates); + +return ret; +} +static int _read_bw (struct dev_pm_opp *new_opp,struct opp_table *opp_table,struct device_node *np,bool peak) {@@ -812,19 +859,13 @@ static int _read_opp_key(struct dev_pm_opp *new_opp, struct opp_table *opp_table, struct device_node *np) { bool found = false; -u64 rate;int ret;
Quoted 14 lines
drivers/opp/of.c static int _read_opp_key(struct dev_pm_opp *new_opp, · line 867
Quoted 12 lines
drivers/opp/of.c +4
-8- ret =of_property_read_u64 (np,"opp-hz" , &rate); -if (!ret) { -/* -* Rate is defined as an unsigned long in clk API, and so -* casting explicitly to its type. Must be fixed once rate is 64 -* bit guaranteed in clk API. -*/ - new_opp->rate = (unsigned long )rate; + ret =_read_rate (new_opp, opp_table, np); +if (ret) +return ret; +else if (opp_table->clk_count ==1 )Shouldn't this be >=1? I got several clocks and this one fails.
Actually this might be correct, but you need to update the bindings. Now you require opp-level for case with multiple clocks.
Best regards,
Krzysztof
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 3 lines · Krzysztof Kozlowski
Quoted 1 line
Shouldn't this be >=1? I got several clocks and this one fails.
Actually this might be correct,
Right.
Quoted 2 lines
but you need to update the bindings. Now
you require opp-level for case with multiple clocks.
Will do.
Signature · 1 line
viresh
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
In reply to Krzysztof KozlowskiQuoted 11 lines · Krzysztof Kozlowski
Quoted 7 lines · Krzysztof Kozlowski
drivers/opp/of.c static int _read_opp_key(struct dev_pm_opp *new_opp, · line 867
Quoted 4 lines · Viresh Kumar
drivers/opp/of.c +4
-0+ ret =_read_rate (new_opp, opp_table, np); +if (ret) +return ret; +else if (opp_table->clk_count ==1 )Shouldn't this be >=1? I got several clocks and this one fails.
Actually this might be correct, but you need to update the bindings. Now you require opp-level for case with multiple clocks.
I have thought about this again and adding such "fake" property in DT doesn't look right, specially in binding document. It maybe fine to have a "level" property in your case of UFS, where we want something to represent gears. But others may not want it.
So, in the new version I am sending now, we still consider opp-hz property as the property that uniquely identifies an OPP. Just that we compare all the rates now, and not just the first one. I have updated _opp_compare_keys() for this as well.
The drivers, for multiple clock case, are expected to call dev_pm_opp_set_opp() to set the specific OPP. Though how they find the target OPP is left for the users to handle. For some, we may have another unique OPP property, like level, which can be used to find the OPP. While in case of others, we may want to implement freq-based OPP finder APIs for multiple clock rates. I have decided not to implement them in advance, and add them only someone wants to use them.
Signature · 1 line
viresh
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 10 lines · Viresh Kumar
Quoted 9 lines · Krzysztof Kozlowski
Quoted 7 lines · Krzysztof Kozlowski
drivers/opp/of.c static int _read_opp_key(struct dev_pm_opp *new_opp, · line 867
Quoted 4 lines · Viresh Kumar
drivers/opp/of.c +4
-0+ ret =_read_rate (new_opp, opp_table, np); +if (ret) +return ret; +else if (opp_table->clk_count ==1 )Shouldn't this be >=1? I got several clocks and this one fails.
Quoted 7 lines
Quoted 2 lines
Actually this might be correct, but you need to update the bindings. Now you require opp-level for case with multiple clocks.
I have thought about this again and adding such "fake" property in DT doesn't look right, specially in binding document. It maybe fine to have a "level" property in your case of UFS, where we want something to represent gears. But others may not want it.
I would say it is not different than existing opp-level property. To me it sounded fine, so at least one DT bindings maintainer would accept it. :)
Quoted 6 lines
So, in the new version I am sending now, we still consider opp-hz property as the property that uniquely identifies an OPP. Just that we compare all the rates now, and not just the first one. I have updated
_opp_compare_keys()for this as well.
Quoted 7 lines
The drivers, for multiple clock case, are expected to call
dev_pm_opp_set_opp()to set the specific OPP. Though how they find the target OPP is left for the users to handle. For some, we may have another unique OPP property, like level, which can be used to find the OPP. While in case of others, we may want to implement freq-based OPP finder APIs for multiple clock rates. I have decided not to implement them in advance, and add them only someone wants to use them.
Thanks! Let me take a look at v2.
Best regards,
Krzysztof
Re: [PATCH 5/8] OPP: Allow multiple clocks for a device
Quoted 2 lines · Krzysztof Kozlowski
I would say it is not different than existing opp-level property. To me it sounded fine, so at least one DT bindings maintainer would accept it. :)
:)
No one is stopping a user to use "level" here, just that I didn't wanted to force it for everyone with multiple clocks. From a DT point of view, we should be able to uniquely identify OPP nodes just based on all freq values.
Signature · 1 line
viresh
PATCH 6/8 OPP: Add key specific assert() method to key finding helpers
In reply to Viresh KumarThe helpers for the clock key, at least, would need to assert that the helpers are called only for single clock case. Prepare for that by adding an argument to the key finding helpers.
--- drivers/opp/core.c | 56 +++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 20 deletions(-)
drivers/opp/core.c +36 -20
@@ -477,10 +477,15 @@ static struct dev_pm_opp *_opp_table_find_key(struct opp_table *opp_table, unsigned long *key, int index, bool available, unsigned long (*read)(struct dev_pm_opp *opp, int index), bool (*compare)(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp, - unsigned long opp_key, unsigned long key)) +unsigned long opp_key,unsigned long key), + bool (*assert)(struct opp_table *opp_table)) { struct dev_pm_opp *temp_opp, *opp = ERR_PTR(-ERANGE); +/* Assert that the requirement is met */ +if (assert && !assert (opp_table)) +return ERR_PTR (-EINVAL ); +mutex_lock (&opp_table->lock);list_for_each_entry (temp_opp, &opp_table->opp_list, node) {
@@ -505,7 +510,8 @@ static struct dev_pm_opp * _find_key(struct device *dev, unsigned long *key, int index, bool available, unsigned long (*read)(struct dev_pm_opp *opp, int index), bool (*compare)(struct dev_pm_opp **opp, struct dev_pm_opp *temp_opp, - unsigned long opp_key, unsigned long key)) +unsigned long opp_key,unsigned long key), + bool (*assert)(struct opp_table *opp_table)) { struct opp_table *opp_table;struct dev_pm_opp *opp;
@@ -518,7 +524,7 @@ _find_key(struct device *dev, unsigned long *key, int index, bool available, } opp =_opp_table_find_key (opp_table, key, index, available, read, - compare); + compare, assert);dev_pm_opp_put_opp_table (opp_table);
@@ -527,35 +533,42 @@ _find_key(struct device *dev, unsigned long *key, int index, bool available,static struct dev_pm_opp *_find_key_exact (struct device *dev,unsigned long key,int index,bool available, -unsigned long (*read)(struct dev_pm_opp *opp,int index)) +unsigned long (*read)(struct dev_pm_opp *opp,int index), +bool (*assert)(struct opp_table *opp_table)) {/* * The value of key will be updated here, but will be ignored as the * caller doesn't need it. */ -return _find_key (dev, &key, index, available, read, _compare_exact); +return _find_key (dev, &key, index, available, read, _compare_exact, + assert); }static struct dev_pm_opp *_opp_table_find_key_ceil (struct opp_table *opp_table,unsigned long *key,int index,bool available, -unsigned long (*read)(struct dev_pm_opp *opp,int index)) +unsigned long (*read)(struct dev_pm_opp *opp,int index), +bool (*assert)(struct opp_table *opp_table)) {return _opp_table_find_key (opp_table, key, index, available, read, - _compare_ceil); + _compare_ceil, assert); }static struct dev_pm_opp *_find_key_ceil (struct device *dev,unsigned long *key,int index,bool available, -unsigned long (*read)(struct dev_pm_opp *opp,int index)) +unsigned long (*read)(struct dev_pm_opp *opp,int index), +bool (*assert)(struct opp_table *opp_table)) { -return _find_key (dev, key, index, available, read, _compare_ceil); +return _find_key (dev, key, index, available, read, _compare_ceil, + assert); }static struct dev_pm_opp *_find_key_floor (struct device *dev,unsigned long *key,int index,bool available, -unsigned long (*read)(struct dev_pm_opp *opp,int index)) +unsigned long (*read)(struct dev_pm_opp *opp,int index), +bool (*assert)(struct opp_table *opp_table)) { -return _find_key (dev, key, index, available, read, _compare_floor); +return _find_key (dev, key, index, available, read, _compare_floor, + assert); }/**
@@ -584,14 +597,15 @@ static struct dev_pm_opp *_find_key_floor(struct device *dev,struct dev_pm_opp *dev_pm_opp_find_freq_exact (struct device *dev,unsigned long freq,bool available) { -return _find_key_exact (dev, freq,0 , available, _read_freq); +return _find_key_exact (dev, freq,0 , available, _read_freq,NULL ); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_freq_exact); static noinline struct dev_pm_opp *_find_freq_ceil(struct opp_table *opp_table,unsigned long *freq) { -return _opp_table_find_key_ceil (opp_table, freq,0 , true, _read_freq); +return _opp_table_find_key_ceil (opp_table, freq,0 , true, _read_freq, +NULL ); }/**
@@ -615,7 +629,7 @@ static noinline struct dev_pm_opp *_find_freq_ceil(struct opp_table *opp_table,struct dev_pm_opp *dev_pm_opp_find_freq_ceil (struct device *dev,unsigned long *freq) { -return _find_key_ceil (dev, freq,0 , true, _read_freq); +return _find_key_ceil (dev, freq,0 , true, _read_freq,NULL ); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_freq_ceil);
@@ -640,7 +654,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_ceil);struct dev_pm_opp *dev_pm_opp_find_freq_floor (struct device *dev,unsigned long *freq) { -return _find_key_floor (dev, freq,0 , true, _read_freq); +return _find_key_floor (dev, freq,0 , true, _read_freq,NULL ); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_freq_floor);
@@ -662,7 +676,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_floor);struct dev_pm_opp *dev_pm_opp_find_level_exact (struct device *dev,unsigned int level) { -return _find_key_exact (dev, level,0 , true, _read_level); +return _find_key_exact (dev, level,0 , true, _read_level,NULL ); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_level_exact);
@@ -684,8 +698,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_level_exact);struct dev_pm_opp *dev_pm_opp_find_level_ceil (struct device *dev,unsigned int *level) { -return _find_key_ceil (dev, (unsigned long *)level,0 , true, - _read_level); +return _find_key_ceil (dev, (unsigned long *)level,0 , true, _read_level, +NULL ); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_level_ceil);
@@ -711,7 +725,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_level_ceil);struct dev_pm_opp *dev_pm_opp_find_bw_ceil (struct device *dev,unsigned int *bw,int index) { -return _find_key_ceil (dev, (unsigned long *)bw, index, true, _read_bw); +return _find_key_ceil (dev, (unsigned long *)bw, index, true, _read_bw, +NULL ); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_bw_ceil);
@@ -737,7 +752,8 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_bw_ceil);struct dev_pm_opp *dev_pm_opp_find_bw_floor (struct device *dev,unsigned int *bw,int index) { -return _find_key_floor (dev, (unsigned long *)bw, index, true, _read_bw); +return _find_key_floor (dev, (unsigned long *)bw, index, true, _read_bw, +NULL ); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_bw_floor);
Signature · 1 line
2.31.1.272.g89b43f80a514
PATCH 7/8 OPP: Assert clk_count == 1 for single clk helpers
In reply to Viresh KumarMany helpers can be safely called only for devices that have a single clk associated with them. Assert the same for those routines.
--- drivers/opp/core.c | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-)
drivers/opp/core.c +33 -6
@@ -93,6 +93,12 @@ struct opp_table *_find_opp_table(struct device *dev)return opp_table; } +/* Returns true for single clock, false with WARN otherwise */ +bool assert_single_clk (struct opp_table *opp_table) +{ +return !WARN_ON (opp_table->clk_count !=1 ); +} +/** * dev_pm_opp_get_voltage() - Gets the voltage corresponding to an opp * @opp: opp for which voltage has to be returned for
@@ -177,6 +183,9 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp)return 0 ; } +if (!assert_single_clk (opp->opp_table)) +return 0 ; +return opp->rates[0 ]; }EXPORT_SYMBOL_GPL (dev_pm_opp_get_freq);
@@ -597,7 +606,8 @@ static struct dev_pm_opp *_find_key_floor(struct device *dev,struct dev_pm_opp *dev_pm_opp_find_freq_exact (struct device *dev,unsigned long freq,bool available) { -return _find_key_exact (dev, freq,0 , available, _read_freq,NULL ); +return _find_key_exact (dev, freq,0 , available, _read_freq, + assert_single_clk); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_freq_exact);
@@ -605,7 +615,7 @@ static noinline struct dev_pm_opp *_find_freq_ceil(struct opp_table *opp_table, unsigned long *freq) {return _opp_table_find_key_ceil (opp_table, freq,0 , true, _read_freq, -NULL ); + assert_single_clk); }/**
@@ -629,7 +639,7 @@ static noinline struct dev_pm_opp *_find_freq_ceil(struct opp_table *opp_table,struct dev_pm_opp *dev_pm_opp_find_freq_ceil (struct device *dev,unsigned long *freq) { -return _find_key_ceil (dev, freq,0 , true, _read_freq,NULL ); +return _find_key_ceil (dev, freq,0 , true, _read_freq, assert_single_clk); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_freq_ceil);
@@ -654,7 +664,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_find_freq_ceil);struct dev_pm_opp *dev_pm_opp_find_freq_floor (struct device *dev,unsigned long *freq) { -return _find_key_floor (dev, freq,0 , true, _read_freq,NULL ); +return _find_key_floor (dev, freq,0 , true, _read_freq, assert_single_clk); }EXPORT_SYMBOL_GPL (dev_pm_opp_find_freq_floor);
@@ -1507,6 +1517,9 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq)if (IS_ERR (opp_table))return ; +if (!assert_single_clk (opp_table)) + goto put_table; +mutex_lock (&opp_table->lock);list_for_each_entry (iter, &opp_table->opp_list, node) {
@@ -1528,6 +1541,7 @@ void dev_pm_opp_remove(struct device *dev, unsigned long freq) __func__, freq); } +put_table:/* Drop the reference taken by _find_opp_table() */ dev_pm_opp_put_opp_table (opp_table); }
@@ -1819,6 +1833,9 @@ int _opp_add_v1(struct opp_table *opp_table, struct device *dev,unsigned long tol;int ret; +if (!assert_single_clk (opp_table)) +return -EINVAL ; + new_opp =_opp_allocate (opp_table);if (!new_opp)return -ENOMEM ;
@@ -2600,6 +2617,11 @@ static int _opp_set_availability(struct device *dev, unsigned long freq,return r; } +if (!assert_single_clk (opp_table)) { + r = -EINVAL ; + goto put_table; + } +mutex_lock (&opp_table->lock);/* Do we have the frequency? */
@@ -2671,6 +2693,11 @@ int dev_pm_opp_adjust_voltage(struct device *dev, unsigned long freq,return r; } +if (!assert_single_clk (opp_table)) { + r = -EINVAL ; + goto put_table; + } +mutex_lock (&opp_table->lock);/* Do we have the frequency? */
@@ -2702,11 +2729,11 @@ int dev_pm_opp_adjust_voltage(struct device *dev, unsigned long freq, opp);dev_pm_opp_put (opp); - goto adjust_put_table; + goto put_table; adjust_unlock:mutex_unlock (&opp_table->lock); -adjust_put_table: +put_table:dev_pm_opp_put_opp_table (opp_table);return r; }
Signature · 1 line
2.31.1.272.g89b43f80a514
PATCH 8/8 OPP: Provide a simple implementation to configure multiple clocks
In reply to Viresh KumarThis provides a simple implementation to configure multiple clocks for a device.
--- drivers/opp/core.c | 34 ++++++++++++++++++++++++++++++++++ include/linux/pm_opp.h | 10 ++++++++++ 2 files changed, 44 insertions(+)
drivers/opp/core.c +34 -0
@@ -825,6 +825,40 @@ _opp_config_clk_single(struct device *dev, struct opp_table *opp_table,return ret; } +/* +* Simple implementation for configuring multiple clocks. Configure clocks in +* the order in which they are present in the array while scaling up. +*/ +int dev_pm_opp_config_clks_simple (struct device *dev, +struct opp_table *opp_table,struct dev_pm_opp *opp,void *data, +bool scaling_down) +{ +int ret, i; + +if (scaling_down) { +for (i = opp_table->clk_count -1 ; i >=0 ; i--) { + ret =clk_set_rate (opp_table->clks[i], opp->rates[i]); +if (ret) { +dev_err (dev,"%s: failed to set clock rate: %d\n" , __func__, + ret); +return ret; + } + } + }else { +for (i =0 ; i < opp_table->clk_count; i++) { + ret =clk_set_rate (opp_table->clks[i], opp->rates[i]); +if (ret) { +dev_err (dev,"%s: failed to set clock rate: %d\n" , __func__, + ret); +return ret; + } + } + } + +return ret; +} +EXPORT_SYMBOL_GPL (dev_pm_opp_config_clks_simple); +static int _opp_config_regulator_single (struct device *dev,struct dev_pm_opp *old_opp,struct dev_pm_opp *new_opp,struct regulator **regulators,unsigned int count)
include/linux/pm_opp.h +10 -0
@@ -162,6 +162,9 @@ int dev_pm_opp_unregister_notifier(struct device *dev, struct notifier_block *nbstruct opp_table *dev_pm_opp_set_config (struct device *dev,struct dev_pm_opp_config *config);int devm_pm_opp_set_config (struct device *dev,struct dev_pm_opp_config *config);void dev_pm_opp_clear_config (struct opp_table *opp_table); +int dev_pm_opp_config_clks_simple (struct device *dev, +struct opp_table *opp_table,struct dev_pm_opp *opp,void *data, +bool scaling_down);struct dev_pm_opp *dev_pm_opp_xlate_required_opp (struct opp_table *src_table,struct opp_table *dst_table,struct dev_pm_opp *src_opp);int dev_pm_opp_xlate_performance_state (struct opp_table *src_table,struct opp_table *dst_table,unsigned int pstate);
@@ -345,6 +348,13 @@ static inline int devm_pm_opp_set_config(struct device *dev, struct dev_pm_opp_cstatic inline void dev_pm_opp_clear_config (struct opp_table *opp_table) {} +static inline int dev_pm_opp_config_clks_simple (struct device *dev, +struct opp_table *opp_table,struct dev_pm_opp *opp,void *data, +bool scaling_down) +{ +return -EOPNOTSUPP ; +} +static inline struct dev_pm_opp *dev_pm_opp_xlate_required_opp (struct opp_table *src_table,struct opp_table *dst_table,struct dev_pm_opp *src_opp) {
Signature · 1 line
2.31.1.272.g89b43f80a514